Skip to content

Commit 7cf3fec

Browse files
shiftedrealityshiftedreality
committed
Merge branch 'MAGETWO-53223' of https://github.com/magento-tango/magento2ce into MAGETWO-53991
2 parents 440b6c3 + 4d207a5 commit 7cf3fec

File tree

3 files changed

+176
-80
lines changed

3 files changed

+176
-80
lines changed

‎lib/internal/Magento/Framework/View/Asset/Merged.php

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@
55
*/
66
namespace Magento\Framework\View\Asset;
77

8+
use Magento\Framework\App\Filesystem\DirectoryList;
9+
use Magento\Framework\App\ObjectManager;
10+
use Magento\Framework\Filesystem;
11+
812
/**
913
* \Iterator that aggregates one or more assets and provides a single public file with equivalent behavior
1014
*/
@@ -30,6 +34,11 @@ class Merged implements \Iterator
3034
*/
3135
private $assetRepo;
3236

37+
/**
38+
* @var Filesystem
39+
*/
40+
private $filesystem;
41+
3342
/**
3443
* @var MergeableInterface[]
3544
*/
@@ -94,7 +103,14 @@ protected function initialize()
94103
$this->isInitialized = true;
95104
try {
96105
$mergedAsset = $this->createMergedAsset($this->assets);
97-
$this->mergeStrategy->merge($this->assets, $mergedAsset);
106+
$isExists = $this->getFilesystem()
107+
->getDirectoryRead(DirectoryList::STATIC_VIEW)
108+
->isExist($mergedAsset->getRelativeSourceFilePath());
109+
110+
if (!$isExists) {
111+
$this->mergeStrategy->merge($this->assets, $mergedAsset);
112+
}
113+
98114
$this->assets = [$mergedAsset];
99115
} catch (\Exception $e) {
100116
$this->logger->critical($e);
@@ -176,4 +192,17 @@ public static function getRelativeDir()
176192
{
177193
return self::CACHE_VIEW_REL . '/merged';
178194
}
195+
196+
/**
197+
* @return Filesystem
198+
* @deprecated
199+
*/
200+
private function getFilesystem()
201+
{
202+
if (null === $this->filesystem) {
203+
$this->filesystem = ObjectManager::getInstance()->get(Filesystem::class);
204+
}
205+
206+
return $this->filesystem;
207+
}
179208
}

‎lib/internal/Magento/Framework/View/Design/FileResolution/Fallback/Resolver/Minification.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ public function resolve($type, $file, $area = null, ThemeInterface $theme = null
5454
{
5555
$file = $this->minification->addMinifiedSign($file);
5656
$path = $this->fallback->resolve($type, $file, $area, $theme, $locale, $module);
57-
if (!$path && ($newFile = $this->minification->removeMinifiedSign($file))) {
57+
if (!$path && $file != ($newFile = $this->minification->removeMinifiedSign($file))) {
5858
$path = $this->fallback->resolve($type, $newFile, $area, $theme, $locale, $module);
5959
}
6060
return $path;

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

Lines changed: 145 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -3,62 +3,89 @@
33
* Copyright © 2016 Magento. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6-
7-
// @codingStandardsIgnoreFile
8-
96
namespace Magento\Framework\View\Test\Unit\Asset;
107

8+
use Magento\Framework\Filesystem;
9+
use Magento\Framework\TestFramework\Unit\Helper\ObjectManager;
10+
use Magento\Framework\View\Asset\File;
11+
use Magento\Framework\View\Asset\Merged;
12+
use Psr\Log\LoggerInterface;
13+
use Magento\Framework\View\Asset\Repository as AssetRepository;
14+
use Magento\Framework\View\Asset\MergeableInterface;
15+
use Magento\Framework\View\Asset\MergeStrategyInterface;
16+
17+
/**
18+
* Class MergedTest
19+
*/
1120
class MergedTest extends \PHPUnit_Framework_TestCase
1221
{
1322
/**
14-
* @var \PHPUnit_Framework_MockObject_MockObject
23+
* @var LoggerInterface|\PHPUnit_Framework_MockObject_MockObject
1524
*/
16-
protected $_objectManager;
25+
protected $logger;
1726

1827
/**
19-
* @var \PHPUnit_Framework_MockObject_MockObject
28+
* @var MergeStrategyInterface|\PHPUnit_Framework_MockObject_MockObject
2029
*/
21-
protected $_logger;
30+
protected $mergeStrategy;
2231

2332
/**
24-
* @var \PHPUnit_Framework_MockObject_MockObject
33+
* @var MergeableInterface|\PHPUnit_Framework_MockObject_MockObject
2534
*/
26-
protected $_mergeStrategy;
35+
protected $assetJsOne;
2736

2837
/**
29-
* @var \PHPUnit_Framework_MockObject_MockObject
38+
* @var MergeableInterface|\PHPUnit_Framework_MockObject_MockObject
3039
*/
31-
protected $_assetJsOne;
40+
protected $assetJsTwo;
3241

3342
/**
34-
* @var \PHPUnit_Framework_MockObject_MockObject
43+
* @var AssetRepository|\PHPUnit_Framework_MockObject_MockObject
3544
*/
36-
protected $_assetJsTwo;
45+
protected $assetRepo;
3746

3847
/**
39-
* @var \PHPUnit_Framework_MockObject_MockObject
48+
* @var Filesystem|\PHPUnit_Framework_MockObject_MockObject
4049
*/
41-
protected $_assetRepo;
50+
protected $filesystemMock;
51+
52+
/**
53+
* @var Filesystem\Directory\ReadInterface|\PHPUnit_Framework_MockObject_MockObject
54+
*/
55+
protected $directoryReadMock;
4256

4357
protected function setUp()
4458
{
45-
$this->_assetJsOne = $this->getMockForAbstractClass('Magento\Framework\View\Asset\MergeableInterface');
46-
$this->_assetJsOne->expects($this->any())->method('getContentType')->will($this->returnValue('js'));
47-
$this->_assetJsOne->expects($this->any())->method('getPath')
48-
->will($this->returnValue('script_one.js'));
49-
50-
$this->_assetJsTwo = $this->getMockForAbstractClass('Magento\Framework\View\Asset\MergeableInterface');
51-
$this->_assetJsTwo->expects($this->any())->method('getContentType')->will($this->returnValue('js'));
52-
$this->_assetJsTwo->expects($this->any())->method('getPath')
53-
->will($this->returnValue('script_two.js'));
54-
55-
$this->_logger = $this->getMock('Psr\Log\LoggerInterface');
56-
57-
$this->_mergeStrategy = $this->getMock('Magento\Framework\View\Asset\MergeStrategyInterface');
58-
59-
$this->_assetRepo = $this->getMock(
60-
'\Magento\Framework\View\Asset\Repository', [], [], '', false
61-
);
59+
$this->assetJsOne = $this->getMockForAbstractClass(MergeableInterface::class);
60+
$this->assetJsOne->expects($this->any())
61+
->method('getContentType')
62+
->willReturn('js');
63+
$this->assetJsOne->expects($this->any())
64+
->method('getPath')
65+
->willReturn('script_one.js');
66+
67+
$this->assetJsTwo = $this->getMockForAbstractClass(MergeableInterface::class);
68+
$this->assetJsTwo->expects($this->any())
69+
->method('getContentType')
70+
->willReturn('js');
71+
$this->assetJsTwo->expects($this->any())
72+
->method('getPath')
73+
->willReturn('script_two.js');
74+
75+
$this->logger = $this->getMock(LoggerInterface::class);
76+
$this->mergeStrategy = $this->getMock(MergeStrategyInterface::class);
77+
$this->assetRepo = $this->getMockBuilder(AssetRepository::class)
78+
->disableOriginalConstructor()
79+
->getMock();
80+
$this->filesystemMock = $this->getMockBuilder(Filesystem::class)
81+
->disableOriginalConstructor()
82+
->getMock();
83+
$this->directoryReadMock = $this->getMockBuilder(Filesystem\Directory\ReadInterface::class)
84+
->getMockForAbstractClass();
85+
86+
$this->filesystemMock->expects($this->any())
87+
->method('getDirectoryRead')
88+
->willReturn($this->directoryReadMock);
6289
}
6390

6491
/**
@@ -67,7 +94,7 @@ protected function setUp()
6794
*/
6895
public function testConstructorNothingToMerge()
6996
{
70-
new \Magento\Framework\View\Asset\Merged($this->_logger, $this->_mergeStrategy, $this->_assetRepo, []);
97+
new \Magento\Framework\View\Asset\Merged($this->logger, $this->mergeStrategy, $this->assetRepo, []);
7198
}
7299

73100
/**
@@ -77,12 +104,13 @@ public function testConstructorNothingToMerge()
77104
public function testConstructorRequireMergeInterface()
78105
{
79106
$assetUrl = new \Magento\Framework\View\Asset\Remote('http://example.com/style.css', 'css');
80-
new \Magento\Framework\View\Asset\Merged(
81-
$this->_logger,
82-
$this->_mergeStrategy,
83-
$this->_assetRepo,
84-
[$this->_assetJsOne, $assetUrl]
85-
);
107+
108+
(new ObjectManager($this))->getObject(Merged::class, [
109+
'logger' => $this->logger,
110+
'mergeStrategy' => $this->mergeStrategy,
111+
'assetRepo' => $this->assetRepo,
112+
'assets' => [$this->assetJsOne, $assetUrl],
113+
]);
86114
}
87115

88116
/**
@@ -91,59 +119,98 @@ public function testConstructorRequireMergeInterface()
91119
*/
92120
public function testConstructorIncompatibleContentTypes()
93121
{
94-
$assetCss = $this->getMockForAbstractClass('Magento\Framework\View\Asset\MergeableInterface');
95-
$assetCss->expects($this->any())->method('getContentType')->will($this->returnValue('css'));
96-
new \Magento\Framework\View\Asset\Merged(
97-
$this->_logger,
98-
$this->_mergeStrategy,
99-
$this->_assetRepo,
100-
[$this->_assetJsOne, $assetCss]
101-
);
122+
$assetCss = $this->getMockForAbstractClass(MergeableInterface::class);
123+
$assetCss->expects($this->any())
124+
->method('getContentType')
125+
->willReturn('css');
126+
127+
(new ObjectManager($this))->getObject(Merged::class, [
128+
'logger' => $this->logger,
129+
'mergeStrategy' => $this->mergeStrategy,
130+
'assetRepo' => $this->assetRepo,
131+
'assets' => [$this->assetJsOne, $assetCss],
132+
]);
102133
}
103134

104135
public function testIteratorInterfaceMerge()
105136
{
106-
$assets = [$this->_assetJsOne, $this->_assetJsTwo];
107-
$this->_logger->expects($this->never())->method('critical');
108-
$merged = new \Magento\Framework\View\Asset\Merged(
109-
$this->_logger,
110-
$this->_mergeStrategy,
111-
$this->_assetRepo,
112-
$assets
113-
);
137+
$assets = [$this->assetJsOne, $this->assetJsTwo];
138+
139+
$this->logger->expects($this->never())->method('critical');
140+
141+
/** @var Merged $merged */
142+
$merged = (new ObjectManager($this))->getObject(Merged::class, [
143+
'logger' => $this->logger,
144+
'mergeStrategy' => $this->mergeStrategy,
145+
'assetRepo' => $this->assetRepo,
146+
'assets' => $assets,
147+
'filesystem' => $this->filesystemMock,
148+
]);
149+
114150
$mergedAsset = $this->getMock('Magento\Framework\View\Asset\File', [], [], '', false);
115-
$this->_mergeStrategy
151+
$this->mergeStrategy
116152
->expects($this->once())
117153
->method('merge')
118154
->with($assets, $mergedAsset)
119-
->will($this->returnValue(null));
120-
$this->_assetRepo->expects($this->once())->method('createArbitrary')->will($this->returnValue($mergedAsset));
155+
->willReturn(null);
156+
$this->assetRepo->expects($this->once())
157+
->method('createArbitrary')
158+
->willReturn($mergedAsset);
121159
$expectedResult = [$mergedAsset];
122160

123-
$this->_assertIteratorEquals($expectedResult, $merged);
124-
$this->_assertIteratorEquals($expectedResult, $merged); // ensure merging happens only once
161+
$this->assertIteratorEquals($expectedResult, $merged);
162+
$this->assertIteratorEquals($expectedResult, $merged); // ensure merging happens only once
125163
}
126164

127165
public function testIteratorInterfaceMergeFailure()
128166
{
129167
$mergeError = new \Exception('File not found');
130-
$assetBroken = $this->getMockForAbstractClass('Magento\Framework\View\Asset\MergeableInterface');
131-
$assetBroken->expects($this->any())->method('getContentType')->will($this->returnValue('js'));
132-
$assetBroken->expects($this->any())->method('getPath')
133-
->will($this->throwException($mergeError));
134-
135-
$merged = new \Magento\Framework\View\Asset\Merged(
136-
$this->_logger,
137-
$this->_mergeStrategy,
138-
$this->_assetRepo,
139-
[$this->_assetJsOne, $this->_assetJsTwo, $assetBroken]
140-
);
141-
142-
$this->_logger->expects($this->once())->method('critical')->with($this->identicalTo($mergeError));
143-
144-
$expectedResult = [$this->_assetJsOne, $this->_assetJsTwo, $assetBroken];
145-
$this->_assertIteratorEquals($expectedResult, $merged);
146-
$this->_assertIteratorEquals($expectedResult, $merged); // ensure merging attempt happens only once
168+
$assetBroken = $this->getMockForAbstractClass(MergeableInterface::class);
169+
$assetBroken->expects($this->any())
170+
->method('getContentType')
171+
->willReturn('js');
172+
$assetBroken->expects($this->any())
173+
->method('getPath')
174+
->willThrowException($mergeError);
175+
176+
/** @var Merged $merged */
177+
$merged = (new ObjectManager($this))->getObject(Merged::class, [
178+
'logger' => $this->logger,
179+
'mergeStrategy' => $this->mergeStrategy,
180+
'assetRepo' => $this->assetRepo,
181+
'assets' => [$this->assetJsOne, $this->assetJsTwo, $assetBroken],
182+
'filesystem' => $this->filesystemMock,
183+
]);
184+
185+
$this->logger->expects($this->once())->method('critical')->with($this->identicalTo($mergeError));
186+
187+
$expectedResult = [$this->assetJsOne, $this->assetJsTwo, $assetBroken];
188+
$this->assertIteratorEquals($expectedResult, $merged);
189+
$this->assertIteratorEquals($expectedResult, $merged); // ensure merging attempt happens only once
190+
}
191+
192+
public function testMergedFileCheckbefore()
193+
{
194+
/** @var Merged $merged */
195+
$merged = (new ObjectManager($this))->getObject(Merged::class, [
196+
'logger' => $this->logger,
197+
'mergeStrategy' => $this->mergeStrategy,
198+
'assetRepo' => $this->assetRepo,
199+
'assets' => [$this->assetJsOne, $this->assetJsTwo],
200+
'filesystem' => $this->filesystemMock,
201+
]);
202+
203+
$mergedAsset = $this->getMockBuilder(File::class)
204+
->disableOriginalConstructor()
205+
->getMock();
206+
207+
$this->directoryReadMock->expects($this->once())
208+
->method('isExist')
209+
->willReturn(true);
210+
$this->assetRepo->expects($this->once())
211+
->method('createArbitrary')->willReturn($mergedAsset);
212+
213+
$this->assertIteratorEquals([$mergedAsset], $merged);
147214
}
148215

149216
/**
@@ -152,7 +219,7 @@ public function testIteratorInterfaceMergeFailure()
152219
* @param array $expectedItems
153220
* @param \Iterator $actual
154221
*/
155-
protected function _assertIteratorEquals(array $expectedItems, \Iterator $actual)
222+
protected function assertIteratorEquals(array $expectedItems, \Iterator $actual)
156223
{
157224
$actualItems = [];
158225
foreach ($actual as $actualItem) {

0 commit comments

Comments
 (0)