Skip to content

Commit 1509927

Browse files
OlgaVasyltsunOlgaVasyltsun
committed
MAGETWO-60778: [Backport] - [Magento Cloud] - Static files not being generated correctly on prod(Race condition in merging files) - for 2.0
1 parent 54d4e70 commit 1509927

File tree

4 files changed

+103
-17
lines changed

4 files changed

+103
-17
lines changed

‎lib/internal/Magento/Framework/Filesystem/Directory/Write.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public function renameFile($path, $newPath, WriteInterface $targetDirectory = nu
106106
$targetDirectory->create($this->driver->getParentDirectory($newPath));
107107
}
108108
$absolutePath = $this->driver->getAbsolutePath($this->path, $path);
109-
$absoluteNewPath = $targetDirectory->driver->getAbsolutePath($this->path, $newPath);
109+
$absoluteNewPath = $targetDirectory->getAbsolutePath($newPath);
110110
return $this->driver->rename($absolutePath, $absoluteNewPath, $targetDirectory->driver);
111111
}
112112

‎lib/internal/Magento/Framework/Filesystem/Test/Unit/Directory/WriteTest.php

Lines changed: 70 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,9 @@
77
*/
88
namespace Magento\Framework\Filesystem\Test\Unit\Directory;
99

10+
use Magento\Framework\Filesystem\Directory\WriteInterface;
11+
use Magento\Framework\Filesystem\DriverInterface;
12+
1013
class WriteTest extends \PHPUnit_Framework_TestCase
1114
{
1215
/**
@@ -68,7 +71,7 @@ protected function tearDown()
6871
public function testGetDriver()
6972
{
7073
$this->assertInstanceOf(
71-
\Magento\Framework\Filesystem\DriverInterface::class,
74+
DriverInterface::class,
7275
$this->write->getDriver(),
7376
'getDriver method expected to return instance of Magento\Framework\Filesystem\DriverInterface'
7477
);
@@ -90,7 +93,7 @@ public function testIsWritable()
9093

9194
public function testCreateSymlinkTargetDirectoryExists()
9295
{
93-
$targetDir = $this->getMockBuilder(\Magento\Framework\Filesystem\Directory\WriteInterface::class)
96+
$targetDir = $this->getMockBuilder(WriteInterface::class)
9497
->getMock();
9598
$targetDir->driver = $this->driver;
9699
$sourcePath = 'source/path/file';
@@ -159,4 +162,69 @@ private function getAbsolutePath($path)
159162
{
160163
return $this->path . $path;
161164
}
165+
166+
/**
167+
* @param string $sourcePath
168+
* @param string $targetPath
169+
* @param WriteInterface $targetDir
170+
* @dataProvider getFilePathsDataProvider
171+
*/
172+
public function testRenameFile($sourcePath, $targetPath, $targetDir)
173+
{
174+
if ($targetDir !== null) {
175+
$targetDir->driver = $this->getMockBuilder(DriverInterface::class)->getMockForAbstractClass();
176+
177+
$targetDirPath = 'TARGET_PATH/';
178+
179+
$targetDir->expects($this->once())
180+
->method('getAbsolutePath')
181+
->with($targetPath)
182+
->willReturn($targetDirPath . $targetPath);
183+
$targetDir->expects($this->once())
184+
->method('isExists')
185+
->with(dirname($targetPath))
186+
->willReturn(false);
187+
$targetDir->expects($this->once())
188+
->method('create')
189+
->with(dirname($targetPath));
190+
}
191+
192+
$this->driver->expects($this->any())
193+
->method('getAbsolutePath')
194+
->willReturnMap([
195+
[$this->path, $sourcePath, null, $this->getAbsolutePath($sourcePath)],
196+
[$this->path, $targetPath, null, $this->getAbsolutePath($targetPath)],
197+
]);
198+
$this->driver->expects($this->any())
199+
->method('isFile')
200+
->willReturnMap([
201+
[$this->getAbsolutePath($sourcePath), true],
202+
[$this->getAbsolutePath($targetPath), true],
203+
]);
204+
$this->driver->expects($this->any())
205+
->method('getParentDirectory')
206+
->with($targetPath)
207+
->willReturn(dirname($targetPath));
208+
209+
$this->write->renameFile($sourcePath, $targetPath, $targetDir);
210+
}
211+
212+
/**
213+
* @return array
214+
*/
215+
public function getFilePathsDataProvider()
216+
{
217+
return [
218+
[
219+
'path/to/source.file',
220+
'path/to/target.file',
221+
null,
222+
],
223+
[
224+
'path/to/source.file',
225+
'path/to/target.file',
226+
$this->getMockBuilder(WriteInterface::class)->getMockForAbstractClass(),
227+
],
228+
];
229+
}
162230
}

‎lib/internal/Magento/Framework/View/Asset/MergeStrategy/Direct.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,8 +50,11 @@ public function __construct(
5050
public function merge(array $assetsToMerge, Asset\LocalInterface $resultAsset)
5151
{
5252
$mergedContent = $this->composeMergedContent($assetsToMerge, $resultAsset);
53-
$dir = $this->filesystem->getDirectoryWrite(DirectoryList::STATIC_VIEW);
54-
$dir->writeFile($resultAsset->getPath(), $mergedContent);
53+
$filePath = $resultAsset->getPath();
54+
$staticDir = $this->filesystem->getDirectoryWrite(DirectoryList::STATIC_VIEW);
55+
$tmpDir = $this->filesystem->getDirectoryWrite(DirectoryList::TMP);
56+
$tmpDir->writeFile($filePath, $mergedContent);
57+
$tmpDir->renameFile($filePath, $filePath, $staticDir);
5558
}
5659

5760
/**

‎lib/internal/Magento/Framework/View/Test/Unit/Asset/MergeStrategy/DirectTest.php

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
*/
66
namespace Magento\Framework\View\Test\Unit\Asset\MergeStrategy;
77

8+
use Magento\Framework\Filesystem\Directory\WriteInterface;
89
use \Magento\Framework\View\Asset\MergeStrategy\Direct;
910

1011
use Magento\Framework\App\Filesystem\DirectoryList;
@@ -22,9 +23,14 @@ class DirectTest extends \PHPUnit_Framework_TestCase
2223
protected $cssUrlResolver;
2324

2425
/**
25-
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Filesystem\Directory\WriteInterface
26+
* @var \PHPUnit_Framework_MockObject_MockObject|WriteInterface
2627
*/
27-
protected $writeDir;
28+
protected $staticDir;
29+
30+
/**
31+
* @var \PHPUnit_Framework_MockObject_MockObject|WriteInterface
32+
*/
33+
protected $tmpDir;
2834

2935
/**
3036
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\View\Asset\LocalInterface
@@ -33,29 +39,36 @@ class DirectTest extends \PHPUnit_Framework_TestCase
3339

3440
protected function setUp()
3541
{
36-
$this->cssUrlResolver = $this->getMock('\Magento\Framework\View\Url\CssResolver');
37-
$filesystem = $this->getMock('\Magento\Framework\Filesystem', [], [], '', false);
38-
$this->writeDir = $this->getMockForAbstractClass('\Magento\Framework\Filesystem\Directory\WriteInterface');
42+
$this->cssUrlResolver = $this->getMock(\Magento\Framework\View\Url\CssResolver::class);
43+
$filesystem = $this->getMock(\Magento\Framework\Filesystem::class, [], [], '', false);
44+
$this->staticDir = $this->getMockBuilder(WriteInterface::class)->getMockForAbstractClass();
45+
$this->tmpDir = $this->getMockBuilder(WriteInterface::class)->getMockForAbstractClass();
3946
$filesystem->expects($this->any())
4047
->method('getDirectoryWrite')
41-
->with(DirectoryList::STATIC_VIEW)
42-
->will($this->returnValue($this->writeDir));
43-
$this->resultAsset = $this->getMock('\Magento\Framework\View\Asset\File', [], [], '', false);
48+
->willReturnMap([
49+
[DirectoryList::STATIC_VIEW, \Magento\Framework\Filesystem\DriverPool::FILE, $this->staticDir],
50+
[DirectoryList::TMP, \Magento\Framework\Filesystem\DriverPool::FILE, $this->tmpDir],
51+
]);
52+
$this->resultAsset = $this->getMock(\Magento\Framework\View\Asset\File::class, [], [], '', false);
4453
$this->object = new Direct($filesystem, $this->cssUrlResolver);
4554
}
4655

4756
public function testMergeNoAssets()
4857
{
4958
$this->resultAsset->expects($this->once())->method('getPath')->will($this->returnValue('foo/result'));
50-
$this->writeDir->expects($this->once())->method('writeFile')->with('foo/result', '');
59+
$this->staticDir->expects($this->never())->method('writeFile');
60+
$this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result', '');
61+
$this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result', 'foo/result', $this->staticDir);
5162
$this->object->merge([], $this->resultAsset);
5263
}
5364

5465
public function testMergeGeneric()
5566
{
5667
$this->resultAsset->expects($this->once())->method('getPath')->will($this->returnValue('foo/result'));
5768
$assets = $this->prepareAssetsToMerge([' one', 'two']); // note leading space intentionally
58-
$this->writeDir->expects($this->once())->method('writeFile')->with('foo/result', 'onetwo');
69+
$this->staticDir->expects($this->never())->method('writeFile');
70+
$this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result', 'onetwo');
71+
$this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result', 'foo/result', $this->staticDir);
5972
$this->object->merge($assets, $this->resultAsset);
6073
}
6174

@@ -73,7 +86,9 @@ public function testMergeCss()
7386
->method('aggregateImportDirectives')
7487
->with('12')
7588
->will($this->returnValue('1020'));
76-
$this->writeDir->expects($this->once())->method('writeFile')->with('foo/result', '1020');
89+
$this->staticDir->expects($this->never())->method('writeFile');
90+
$this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result', '1020');
91+
$this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result', 'foo/result', $this->staticDir);
7792
$this->object->merge($assets, $this->resultAsset);
7893
}
7994

@@ -87,7 +102,7 @@ private function prepareAssetsToMerge(array $data)
87102
{
88103
$result = [];
89104
foreach ($data as $content) {
90-
$asset = $this->getMockForAbstractClass('Magento\Framework\View\Asset\LocalInterface');
105+
$asset = $this->getMockForAbstractClass(\Magento\Framework\View\Asset\LocalInterface::class);
91106
$asset->expects($this->once())->method('getContent')->will($this->returnValue($content));
92107
$result[] = $asset;
93108
}

0 commit comments

Comments
 (0)