From 475c4b4f74829b12ce44cf6a84115f0ed044d80d Mon Sep 17 00:00:00 2001
From: Maxim Medinskiy <mmedinskiy@magento.com>
Date: Mon, 21 Nov 2016 17:58:22 +0200
Subject: [PATCH] MAGETWO-60611: [Magento Cloud] - Static files not being
generated correctly on prod(Race condition in merging files)
---
.../Framework/Filesystem/Directory/Write.php | 2 +-
.../Test/Unit/Directory/WriteTest.php | 70 ++++++++++++++++++-
.../View/Asset/MergeStrategy/Direct.php | 5 +-
.../Unit/Asset/MergeStrategy/DirectTest.php | 29 ++++++--
4 files changed, 94 insertions(+), 12 deletions(-)
diff --git a/lib/internal/Magento/Framework/Filesystem/Directory/Write.php b/lib/internal/Magento/Framework/Filesystem/Directory/Write.php
index b7752ddccf8..24f8ff2507c 100644
--- a/lib/internal/Magento/Framework/Filesystem/Directory/Write.php
+++ b/lib/internal/Magento/Framework/Filesystem/Directory/Write.php
@@ -106,7 +106,7 @@ class Write extends Read implements WriteInterface
$targetDirectory->create($this->driver->getParentDirectory($newPath));
}
$absolutePath = $this->driver->getAbsolutePath($this->path, $path);
- $absoluteNewPath = $targetDirectory->driver->getAbsolutePath($this->path, $newPath);
+ $absoluteNewPath = $targetDirectory->getAbsolutePath($newPath);
return $this->driver->rename($absolutePath, $absoluteNewPath, $targetDirectory->driver);
}
diff --git a/lib/internal/Magento/Framework/Filesystem/Test/Unit/Directory/WriteTest.php b/lib/internal/Magento/Framework/Filesystem/Test/Unit/Directory/WriteTest.php
index 1a84fb6a69a..ce3511d671d 100644
--- a/lib/internal/Magento/Framework/Filesystem/Test/Unit/Directory/WriteTest.php
+++ b/lib/internal/Magento/Framework/Filesystem/Test/Unit/Directory/WriteTest.php
@@ -7,6 +7,9 @@
*/
namespace Magento\Framework\Filesystem\Test\Unit\Directory;
+use Magento\Framework\Filesystem\Directory\WriteInterface;
+use Magento\Framework\Filesystem\DriverInterface;
+
class WriteTest extends \PHPUnit_Framework_TestCase
{
/**
@@ -68,7 +71,7 @@ class WriteTest extends \PHPUnit_Framework_TestCase
public function testGetDriver()
{
$this->assertInstanceOf(
- \Magento\Framework\Filesystem\DriverInterface::class,
+ DriverInterface::class,
$this->write->getDriver(),
'getDriver method expected to return instance of Magento\Framework\Filesystem\DriverInterface'
);
@@ -90,8 +93,7 @@ class WriteTest extends \PHPUnit_Framework_TestCase
public function testCreateSymlinkTargetDirectoryExists()
{
- $targetDir = $this->getMockBuilder(\Magento\Framework\Filesystem\Directory\WriteInterface::class)
- ->getMock();
+ $targetDir = $this->getMockBuilder(WriteInterface::class)->getMock();
$targetDir->driver = $this->driver;
$sourcePath = 'source/path/file';
$destinationDirectory = 'destination/path';
@@ -159,4 +161,66 @@ class WriteTest extends \PHPUnit_Framework_TestCase
{
return $this->path . $path;
}
+
+ /**
+ * @param string $sourcePath
+ * @param string $targetPath
+ * @param WriteInterface $targetDir
+ * @dataProvider getFilePathsDataProvider
+ */
+ public function testRenameFile($sourcePath, $targetPath, $targetDir)
+ {
+ if ($targetDir !== null) {
+ $targetDir->driver = $this->getMockBuilder(DriverInterface::class)->getMockForAbstractClass();
+ $targetDirPath = 'TARGET_PATH/';
+ $targetDir->expects($this->once())
+ ->method('getAbsolutePath')
+ ->with($targetPath)
+ ->willReturn($targetDirPath . $targetPath);
+ $targetDir->expects($this->once())
+ ->method('isExists')
+ ->with(dirname($targetPath))
+ ->willReturn(false);
+ $targetDir->expects($this->once())
+ ->method('create')
+ ->with(dirname($targetPath));
+ }
+
+ $this->driver->expects($this->any())
+ ->method('getAbsolutePath')
+ ->willReturnMap([
+ [$this->path, $sourcePath, null, $this->getAbsolutePath($sourcePath)],
+ [$this->path, $targetPath, null, $this->getAbsolutePath($targetPath)],
+ ]);
+ $this->driver->expects($this->any())
+ ->method('isFile')
+ ->willReturnMap([
+ [$this->getAbsolutePath($sourcePath), true],
+ [$this->getAbsolutePath($targetPath), true],
+ ]);
+ $this->driver->expects($this->any())
+ ->method('getParentDirectory')
+ ->with($targetPath)
+ ->willReturn(dirname($targetPath));
+ $this->write->renameFile($sourcePath, $targetPath, $targetDir);
+ }
+
+ /**
+ * @return array
+ */
+ public function getFilePathsDataProvider()
+ {
+ return [
+ [
+ 'path/to/source.file',
+ 'path/to/target.file',
+ null,
+ ],
+ [
+ 'path/to/source.file',
+ 'path/to/target.file',
+ $this->getMockBuilder(WriteInterface::class)->getMockForAbstractClass(),
+ ],
+ ];
+ }
}
diff --git a/lib/internal/Magento/Framework/View/Asset/MergeStrategy/Direct.php b/lib/internal/Magento/Framework/View/Asset/MergeStrategy/Direct.php
index 68cc706ad9b..039c27d3e18 100644
--- a/lib/internal/Magento/Framework/View/Asset/MergeStrategy/Direct.php
+++ b/lib/internal/Magento/Framework/View/Asset/MergeStrategy/Direct.php
@@ -50,8 +50,11 @@ class Direct implements \Magento\Framework\View\Asset\MergeStrategyInterface
public function merge(array $assetsToMerge, Asset\LocalInterface $resultAsset)
{
$mergedContent = $this->composeMergedContent($assetsToMerge, $resultAsset);
+ $filePath = $resultAsset->getPath();
$dir = $this->filesystem->getDirectoryWrite(DirectoryList::STATIC_VIEW);
- $dir->writeFile($resultAsset->getPath(), $mergedContent);
+ $tmpDir = $this->filesystem->getDirectoryWrite(DirectoryList::TMP);
+ $tmpDir->writeFile($filePath, $mergedContent);
+ $tmpDir->renameFile($filePath, $filePath, $dir);
}
/**
diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergeStrategy/DirectTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergeStrategy/DirectTest.php
index 23041feb2b8..23e4d42d849 100644
--- a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergeStrategy/DirectTest.php
+++ b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergeStrategy/DirectTest.php
@@ -5,6 +5,7 @@
*/
namespace Magento\Framework\View\Test\Unit\Asset\MergeStrategy;
+use Magento\Framework\Filesystem\Directory\WriteInterface;
use \Magento\Framework\View\Asset\MergeStrategy\Direct;
use Magento\Framework\App\Filesystem\DirectoryList;
@@ -22,10 +23,15 @@ class DirectTest extends \PHPUnit_Framework_TestCase
protected $cssUrlResolver;
/**
- * @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\Filesystem\Directory\WriteInterface
+ * @var \PHPUnit_Framework_MockObject_MockObject|WriteInterface
*/
protected $writeDir;
+ /**
+ * @var \PHPUnit_Framework_MockObject_MockObject|WriteInterface
+ */
+ protected $tmpDir;
+
/**
* @var \PHPUnit_Framework_MockObject_MockObject|\Magento\Framework\View\Asset\LocalInterface
*/
@@ -35,11 +41,14 @@ class DirectTest extends \PHPUnit_Framework_TestCase
{
$this->cssUrlResolver = $this->getMock(\Magento\Framework\View\Url\CssResolver::class);
$filesystem = $this->getMock(\Magento\Framework\Filesystem::class, [], [], '', false);
- $this->writeDir = $this->getMockForAbstractClass(\Magento\Framework\Filesystem\Directory\WriteInterface::class);
+ $this->writeDir = $this->getMockBuilder(WriteInterface::class)->getMockForAbstractClass();
+ $this->tmpDir = $this->getMockBuilder(WriteInterface::class)->getMockForAbstractClass();
$filesystem->expects($this->any())
->method('getDirectoryWrite')
- ->with(DirectoryList::STATIC_VIEW)
- ->will($this->returnValue($this->writeDir));
+ ->willReturnMap([
+ [DirectoryList::STATIC_VIEW, \Magento\Framework\Filesystem\DriverPool::FILE, $this->writeDir],
+ [DirectoryList::TMP, \Magento\Framework\Filesystem\DriverPool::FILE, $this->tmpDir],
+ ]);
$this->resultAsset = $this->getMock(\Magento\Framework\View\Asset\File::class, [], [], '', false);
$this->object = new Direct($filesystem, $this->cssUrlResolver);
}
@@ -47,7 +56,9 @@ class DirectTest extends \PHPUnit_Framework_TestCase
public function testMergeNoAssets()
{
$this->resultAsset->expects($this->once())->method('getPath')->will($this->returnValue('foo/result'));
- $this->writeDir->expects($this->once())->method('writeFile')->with('foo/result', '');
+ $this->writeDir->expects($this->never())->method('writeFile');
+ $this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result', '');
+ $this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result', 'foo/result', $this->writeDir);
$this->object->merge([], $this->resultAsset);
}
@@ -55,7 +66,9 @@ class DirectTest extends \PHPUnit_Framework_TestCase
{
$this->resultAsset->expects($this->once())->method('getPath')->will($this->returnValue('foo/result'));
$assets = $this->prepareAssetsToMerge([' one', 'two']); // note leading space intentionally
- $this->writeDir->expects($this->once())->method('writeFile')->with('foo/result', 'onetwo');
+ $this->writeDir->expects($this->never())->method('writeFile');
+ $this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result', 'onetwo');
+ $this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result', 'foo/result', $this->writeDir);
$this->object->merge($assets, $this->resultAsset);
}
@@ -73,7 +86,9 @@ class DirectTest extends \PHPUnit_Framework_TestCase
->method('aggregateImportDirectives')
->with('12')
->will($this->returnValue('1020'));
- $this->writeDir->expects($this->once())->method('writeFile')->with('foo/result', '1020');
+ $this->writeDir->expects($this->never())->method('writeFile');
+ $this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result', '1020');
+ $this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result', 'foo/result', $this->writeDir);
$this->object->merge($assets, $this->resultAsset);
}
--
GitLab