Fixes bug when many requests to a page are made and multiple streams request to build the same merged css or js file sometimes results in serving incomplete files to the client if a second request came in and truncated the part of the work from the first request.

Author: Ian410

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

@@ -30,16 +30,23 @@ class Direct implements \Magento\Framework\View\Asset\MergeStrategyInterface
      */
     private $cssUrlResolver;
 
+    /**
+     * @var \Magento\Framework\Math\Random
+     */
+    protected $mathRandom;
+
     /**
      * @param \Magento\Framework\Filesystem $filesystem
      * @param \Magento\Framework\View\Url\CssResolver $cssUrlResolver
      */
     public function __construct(
         \Magento\Framework\Filesystem $filesystem,
-        \Magento\Framework\View\Url\CssResolver $cssUrlResolver
+        \Magento\Framework\View\Url\CssResolver $cssUrlResolver,
+        \Magento\Framework\Math\Random $mathRandom
     ) {
         $this->filesystem = $filesystem;
         $this->cssUrlResolver = $cssUrlResolver;
+        $this->mathRandom = $mathRandom;
     }
 
     /**
@@ -49,17 +56,18 @@ public function merge(array $assetsToMerge, Asset\LocalInterface $resultAsset)
     {
         $mergedContent = $this->composeMergedContent($assetsToMerge, $resultAsset);
         $filePath = $resultAsset->getPath();
+        $tmpFilePath = $filePath . $this->mathRandom->getUniqueHash('_');
         $staticDir = $this->filesystem->getDirectoryWrite(DirectoryList::STATIC_VIEW);
         $tmpDir = $this->filesystem->getDirectoryWrite(DirectoryList::TMP);
-        $tmpDir->writeFile($filePath, $mergedContent);
-        $tmpDir->renameFile($filePath, $filePath, $staticDir);
+        $tmpDir->writeFile($tmpFilePath, $mergedContent);
+        $tmpDir->renameFile($tmpFilePath, $filePath, $staticDir);
     }
 
     /**
      * Merge files together and modify content if needed
      *
      * @param \Magento\Framework\View\Asset\MergeableInterface[] $assetsToMerge
-     * @param \Magento\Framework\View\Asset\LocalInterface $resultAsset
+     * @param \Magento\ Framework\View\Asset\LocalInterface $resultAsset
      * @return string
      * @throws \Magento\Framework\Exception\LocalizedException
      */

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

@@ -12,6 +12,10 @@
 
 class DirectTest extends \PHPUnit\Framework\TestCase
 {
+    /**
+     * @var \Magento\Framework\Math\Random|\PHPUnit_Framework_MockObject_MockObject
+     */
+    protected $mathRandomMock;
     /**
      * @var \Magento\Framework\View\Asset\MergeStrategy\Direct
      */
@@ -50,30 +54,42 @@ protected function setUp()
                 [DirectoryList::TMP, \Magento\Framework\Filesystem\DriverPool::FILE, $this->tmpDir],
             ]);
         $this->resultAsset = $this->createMock(\Magento\Framework\View\Asset\File::class);
-        $this->object = new Direct($filesystem, $this->cssUrlResolver);
+        $this->mathRandomMock = $this->getMockBuilder(\Magento\Framework\Math\Random::class)
+            ->disableOriginalConstructor()
+            ->getMock();
+        $this->object = new Direct($filesystem, $this->cssUrlResolver, $this->mathRandomMock);
     }
 
     public function testMergeNoAssets()
     {
+        $uniqId = '_b3bf82fa6e140594420fa90982a8e877';
         $this->resultAsset->expects($this->once())->method('getPath')->will($this->returnValue('foo/result'));
         $this->staticDir->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->staticDir);
+        $this->mathRandomMock->expects($this->once())
+            ->method('getUniqueHash')
+            ->willReturn($uniqId);
+        $this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result' . $uniqId, '');
+        $this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result' . $uniqId, 'foo/result', $this->staticDir);
         $this->object->merge([], $this->resultAsset);
     }
 
     public function testMergeGeneric()
     {
+        $uniqId = '_be50ccf992fd81818c1a2645d1a29e92';
         $this->resultAsset->expects($this->once())->method('getPath')->will($this->returnValue('foo/result'));
         $assets = $this->prepareAssetsToMerge([' one', 'two']); // note leading space intentionally
         $this->staticDir->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->staticDir);
+        $this->mathRandomMock->expects($this->once())
+            ->method('getUniqueHash')
+            ->willReturn($uniqId);
+        $this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result' . $uniqId, 'onetwo');
+        $this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result' . $uniqId, 'foo/result', $this->staticDir);
         $this->object->merge($assets, $this->resultAsset);
     }
 
     public function testMergeCss()
     {
+        $uniqId = '_f929c374767e00712449660ea673f2f5';
         $this->resultAsset->expects($this->exactly(3))
             ->method('getPath')
             ->will($this->returnValue('foo/result'));
@@ -86,9 +102,12 @@ public function testMergeCss()
             ->method('aggregateImportDirectives')
             ->with('12')
             ->will($this->returnValue('1020'));
+        $this->mathRandomMock->expects($this->once())
+            ->method('getUniqueHash')
+            ->willReturn($uniqId);
         $this->staticDir->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->staticDir);
+        $this->tmpDir->expects($this->once())->method('writeFile')->with('foo/result' . $uniqId, '1020');
+        $this->tmpDir->expects($this->once())->method('renameFile')->with('foo/result' . $uniqId, 'foo/result', $this->staticDir);
         $this->object->merge($assets, $this->resultAsset);
     }