From fd18240cca37c7ba2e1fed1a8caa35cca21f1049 Mon Sep 17 00:00:00 2001 From: Alexander Lukyanov Date: Thu, 10 May 2018 14:19:38 -0400 Subject: [PATCH 1/9] Issue #11354 Merged CSS file name generation --- .../Magento/Framework/View/Asset/Merged.php | 13 +++++++++++++ 1 file changed, 13 insertions(+) mode change 100644 => 100755 lib/internal/Magento/Framework/View/Asset/Merged.php diff --git a/lib/internal/Magento/Framework/View/Asset/Merged.php b/lib/internal/Magento/Framework/View/Asset/Merged.php old mode 100644 new mode 100755 index 5b206b235eb11..943f3eba0632c --- a/lib/internal/Magento/Framework/View/Asset/Merged.php +++ b/lib/internal/Magento/Framework/View/Asset/Merged.php @@ -40,6 +40,11 @@ class Merged implements \Iterator */ protected $contentType; + /** + * @var StorageInterface + */ + private $versionStorage; + /** * @var bool */ @@ -56,11 +61,13 @@ public function __construct( \Psr\Log\LoggerInterface $logger, MergeStrategyInterface $mergeStrategy, \Magento\Framework\View\Asset\Repository $assetRepo, + \Magento\Framework\App\View\Deployment\Version\StorageInterface $versionStorage, array $assets ) { $this->logger = $logger; $this->mergeStrategy = $mergeStrategy; $this->assetRepo = $assetRepo; + $this->versionStorage = $versionStorage; if (!$assets) { throw new \InvalidArgumentException('At least one asset has to be passed for merging.'); @@ -116,6 +123,12 @@ private function createMergedAsset(array $assets) $paths[] = $asset->getPath(); } $paths = array_unique($paths); + + $version=$this->versionStorage->load(); + if ($version) { + $paths[]=$version; + } + $filePath = md5(implode('|', $paths)) . '.' . $this->contentType; return $this->assetRepo->createArbitrary($filePath, self::getRelativeDir()); } From 1336eeadfd5841db144d25a4e5cb1ae2e8479522 Mon Sep 17 00:00:00 2001 From: Alex Lukyanau Date: Thu, 10 May 2018 19:39:01 -0400 Subject: [PATCH 2/9] Regenerated Doc for function --- lib/internal/Magento/Framework/View/Asset/Merged.php | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/lib/internal/Magento/Framework/View/Asset/Merged.php b/lib/internal/Magento/Framework/View/Asset/Merged.php index 943f3eba0632c..ae38bda0f1733 100755 --- a/lib/internal/Magento/Framework/View/Asset/Merged.php +++ b/lib/internal/Magento/Framework/View/Asset/Merged.php @@ -50,10 +50,14 @@ class Merged implements \Iterator */ protected $isInitialized = false; + /** + * Merged constructor. + * * @param \Psr\Log\LoggerInterface $logger * @param MergeStrategyInterface $mergeStrategy * @param \Magento\Framework\View\Asset\Repository $assetRepo + * @param \Magento\Framework\App\View\Deployment\Version\StorageInterface $versionStorage * @param MergeableInterface[] $assets * @throws \InvalidArgumentException */ From 49a099f566243ded66bd5f2cf1244ab591529267 Mon Sep 17 00:00:00 2001 From: Alex Lukyanau Date: Thu, 10 May 2018 19:39:31 -0400 Subject: [PATCH 3/9] Removed whitespace --- lib/internal/Magento/Framework/View/Asset/Merged.php | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/internal/Magento/Framework/View/Asset/Merged.php b/lib/internal/Magento/Framework/View/Asset/Merged.php index ae38bda0f1733..2dddc2e59d9ac 100755 --- a/lib/internal/Magento/Framework/View/Asset/Merged.php +++ b/lib/internal/Magento/Framework/View/Asset/Merged.php @@ -50,7 +50,6 @@ class Merged implements \Iterator */ protected $isInitialized = false; - /** * Merged constructor. * From 6ec1b972f72b9e38e2e4c06f4fdd83db8a0677c4 Mon Sep 17 00:00:00 2001 From: Alex Lukyanau Date: Fri, 11 May 2018 01:07:21 -0400 Subject: [PATCH 4/9] Travic CI Test case errors --- .../Magento/Framework/View/Asset/Merged.php | 2 +- .../Framework/View/Test/Unit/Asset/MergedTest.php | 13 ++++++++++++- 2 files changed, 13 insertions(+), 2 deletions(-) mode change 100644 => 100755 lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php diff --git a/lib/internal/Magento/Framework/View/Asset/Merged.php b/lib/internal/Magento/Framework/View/Asset/Merged.php index 2dddc2e59d9ac..57105ab231e03 100755 --- a/lib/internal/Magento/Framework/View/Asset/Merged.php +++ b/lib/internal/Magento/Framework/View/Asset/Merged.php @@ -41,7 +41,7 @@ class Merged implements \Iterator protected $contentType; /** - * @var StorageInterface + * @var \Magento\Framework\App\View\Deployment\Version\StorageInterface */ private $versionStorage; diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php old mode 100644 new mode 100755 index 164a2ca4d4d1b..34a19c346a64f --- a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php @@ -12,6 +12,7 @@ use Magento\Framework\View\Asset\Repository as AssetRepository; use Magento\Framework\View\Asset\MergeableInterface; use Magento\Framework\View\Asset\MergeStrategyInterface; +use Magento\Framework\App\View\Deployment\Version\StorageInterface; /** * Class MergedTest @@ -43,6 +44,11 @@ class MergedTest extends \PHPUnit\Framework\TestCase */ private $assetRepo; + /** + * @var StorageInterface|\PHPUnit_Framework_MockObject_MockObject + */ + private $versionStorage; + protected function setUp() { $this->assetJsOne = $this->getMockForAbstractClass(MergeableInterface::class); @@ -66,6 +72,7 @@ protected function setUp() $this->assetRepo = $this->getMockBuilder(AssetRepository::class) ->disableOriginalConstructor() ->getMock(); + $this->versionStorage = $this->createMock(StorageInterface::class); } /** @@ -74,7 +81,7 @@ protected function setUp() */ public function testConstructorNothingToMerge() { - new \Magento\Framework\View\Asset\Merged($this->logger, $this->mergeStrategy, $this->assetRepo, []); + new \Magento\Framework\View\Asset\Merged($this->logger, $this->mergeStrategy, $this->assetRepo, $this->versionStorage, []); } /** @@ -89,6 +96,7 @@ public function testConstructorRequireMergeInterface() 'logger' => $this->logger, 'mergeStrategy' => $this->mergeStrategy, 'assetRepo' => $this->assetRepo, + 'versionStorage' => $this->versionStorage, 'assets' => [$this->assetJsOne, $assetUrl], ]); } @@ -108,6 +116,7 @@ public function testConstructorIncompatibleContentTypes() 'logger' => $this->logger, 'mergeStrategy' => $this->mergeStrategy, 'assetRepo' => $this->assetRepo, + 'versionStorage' => $this->versionStorage, 'assets' => [$this->assetJsOne, $assetCss], ]); } @@ -123,6 +132,7 @@ public function testIteratorInterfaceMerge() 'logger' => $this->logger, 'mergeStrategy' => $this->mergeStrategy, 'assetRepo' => $this->assetRepo, + 'versionStorage' => $this->versionStorage, 'assets' => $assets, ]); @@ -157,6 +167,7 @@ public function testIteratorInterfaceMergeFailure() 'logger' => $this->logger, 'mergeStrategy' => $this->mergeStrategy, 'assetRepo' => $this->assetRepo, + 'versionStorage' => $this->versionStorage, 'assets' => [$this->assetJsOne, $this->assetJsTwo, $assetBroken], ]); From 6db98dbc951b4740acf62955274b9a690a8b6aef Mon Sep 17 00:00:00 2001 From: Alex Lukyanau Date: Fri, 11 May 2018 01:29:53 -0400 Subject: [PATCH 5/9] Travic CI - This pull request quality could be better. --- .../Magento/Framework/View/Test/Unit/Asset/MergedTest.php | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php index 34a19c346a64f..2f7742f5ccbd1 100755 --- a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php @@ -81,7 +81,12 @@ protected function setUp() */ public function testConstructorNothingToMerge() { - new \Magento\Framework\View\Asset\Merged($this->logger, $this->mergeStrategy, $this->assetRepo, $this->versionStorage, []); + new \Magento\Framework\View\Asset\Merged( + $this->logger, $this->mergeStrategy, + $this->assetRepo, + $this->versionStorage, + [] + ); } /** From b3bb19bd053e5f2312fb8cbc80190f6ceb7de2ac Mon Sep 17 00:00:00 2001 From: Stanislav Idolov Date: Fri, 11 May 2018 09:11:59 +0300 Subject: [PATCH 6/9] Minor fixes --- .../Magento/Framework/View/Asset/Merged.php | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/lib/internal/Magento/Framework/View/Asset/Merged.php b/lib/internal/Magento/Framework/View/Asset/Merged.php index 57105ab231e03..302eb1226b8ef 100755 --- a/lib/internal/Magento/Framework/View/Asset/Merged.php +++ b/lib/internal/Magento/Framework/View/Asset/Merged.php @@ -5,6 +5,8 @@ */ namespace Magento\Framework\View\Asset; +use Magento\Framework\App\ObjectManager; + /** * \Iterator that aggregates one or more assets and provides a single public file with equivalent behavior */ @@ -56,21 +58,23 @@ class Merged implements \Iterator * @param \Psr\Log\LoggerInterface $logger * @param MergeStrategyInterface $mergeStrategy * @param \Magento\Framework\View\Asset\Repository $assetRepo - * @param \Magento\Framework\App\View\Deployment\Version\StorageInterface $versionStorage * @param MergeableInterface[] $assets + * @param \Magento\Framework\App\View\Deployment\Version\StorageInterface $versionStorage * @throws \InvalidArgumentException */ public function __construct( \Psr\Log\LoggerInterface $logger, MergeStrategyInterface $mergeStrategy, \Magento\Framework\View\Asset\Repository $assetRepo, - \Magento\Framework\App\View\Deployment\Version\StorageInterface $versionStorage, - array $assets + array $assets, + \Magento\Framework\App\View\Deployment\Version\StorageInterface $versionStorage = null ) { $this->logger = $logger; $this->mergeStrategy = $mergeStrategy; $this->assetRepo = $assetRepo; - $this->versionStorage = $versionStorage; + $this->versionStorage = $versionStorage ?: ObjectManager::getInstance()->get( + \Magento\Framework\App\View\Deployment\Version\StorageInterface::class + ); if (!$assets) { throw new \InvalidArgumentException('At least one asset has to be passed for merging.'); @@ -127,9 +131,9 @@ private function createMergedAsset(array $assets) } $paths = array_unique($paths); - $version=$this->versionStorage->load(); + $version = $this->versionStorage->load(); if ($version) { - $paths[]=$version; + $paths[] = $version; } $filePath = md5(implode('|', $paths)) . '.' . $this->contentType; From 77b1179ba3da45a133a08b83b2595cc8a4b48161 Mon Sep 17 00:00:00 2001 From: Stanislav Idolov Date: Fri, 11 May 2018 09:13:44 +0300 Subject: [PATCH 7/9] Fixed unit test --- .../Magento/Framework/View/Test/Unit/Asset/MergedTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php index 2f7742f5ccbd1..0e06b81068b9a 100755 --- a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php @@ -84,8 +84,8 @@ public function testConstructorNothingToMerge() new \Magento\Framework\View\Asset\Merged( $this->logger, $this->mergeStrategy, $this->assetRepo, - $this->versionStorage, - [] + [], + $this->versionStorage ); } From 116ea3641673ddc6b35bcdd2fa9e8c90a68fab05 Mon Sep 17 00:00:00 2001 From: Alex Lukyanau Date: Fri, 11 May 2018 02:24:26 -0400 Subject: [PATCH 8/9] Fixed Unit Tests --- .../Magento/Framework/View/Test/Unit/Asset/MergedTest.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php index 0e06b81068b9a..8f6cb55d562d4 100755 --- a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php @@ -101,8 +101,8 @@ public function testConstructorRequireMergeInterface() 'logger' => $this->logger, 'mergeStrategy' => $this->mergeStrategy, 'assetRepo' => $this->assetRepo, - 'versionStorage' => $this->versionStorage, 'assets' => [$this->assetJsOne, $assetUrl], + 'versionStorage' => $this->versionStorage, ]); } @@ -121,8 +121,8 @@ public function testConstructorIncompatibleContentTypes() 'logger' => $this->logger, 'mergeStrategy' => $this->mergeStrategy, 'assetRepo' => $this->assetRepo, - 'versionStorage' => $this->versionStorage, 'assets' => [$this->assetJsOne, $assetCss], + 'versionStorage' => $this->versionStorage, ]); } @@ -137,8 +137,8 @@ public function testIteratorInterfaceMerge() 'logger' => $this->logger, 'mergeStrategy' => $this->mergeStrategy, 'assetRepo' => $this->assetRepo, - 'versionStorage' => $this->versionStorage, 'assets' => $assets, + 'versionStorage' => $this->versionStorage, ]); $mergedAsset = $this->createMock(\Magento\Framework\View\Asset\File::class); @@ -172,8 +172,8 @@ public function testIteratorInterfaceMergeFailure() 'logger' => $this->logger, 'mergeStrategy' => $this->mergeStrategy, 'assetRepo' => $this->assetRepo, - 'versionStorage' => $this->versionStorage, 'assets' => [$this->assetJsOne, $this->assetJsTwo, $assetBroken], + 'versionStorage' => $this->versionStorage, ]); $this->logger->expects($this->once())->method('critical')->with($this->identicalTo($mergeError)); From c4ba3ff7f745280e8c6f71bc83e18f7e2c7986b6 Mon Sep 17 00:00:00 2001 From: Alex Lukyanau Date: Fri, 11 May 2018 12:29:33 -0400 Subject: [PATCH 9/9] Only one argument is allowed per line in a multi-line function call --- .../Magento/Framework/View/Test/Unit/Asset/MergedTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php index 8f6cb55d562d4..52b45a510e722 100755 --- a/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php +++ b/lib/internal/Magento/Framework/View/Test/Unit/Asset/MergedTest.php @@ -82,7 +82,8 @@ protected function setUp() public function testConstructorNothingToMerge() { new \Magento\Framework\View\Asset\Merged( - $this->logger, $this->mergeStrategy, + $this->logger, + $this->mergeStrategy, $this->assetRepo, [], $this->versionStorage