Skip to content

Commit 95ddc08

Browse files
committed
MAGETWO-35298: Deploy script modifies LESS files with "@urls-resolved: true" line(-s)
- fixed code defects - fixed file generator test - fixed code review bugs - fixed fileassembler application
1 parent 97d48ee commit 95ddc08

File tree

7 files changed

+124
-98
lines changed

7 files changed

+124
-98
lines changed

app/code/Magento/Developer/Model/Less/FileGenerator/PublicationDecorator.php

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,21 +8,32 @@
88
use Magento\Framework\Less\FileGenerator\RelatedGenerator;
99
use Magento\Framework\View\Asset\LocalInterface;
1010

11+
/**
12+
* Class PublicationDecorator
13+
* Decorates generator of related assets and publishes them
14+
*
15+
* @package Magento\Developer\Model\Less\FileGenerator
16+
*/
1117
class PublicationDecorator extends RelatedGenerator
1218
{
1319
/**
1420
* @var \Magento\Framework\App\View\Asset\Publisher
1521
*/
1622
private $publisher;
1723

24+
/**
25+
* @param \Magento\Framework\Filesystem $filesystem
26+
* @param \Magento\Framework\View\Asset\Repository $assetRepo
27+
* @param \Magento\Framework\Less\File\Temporary $temporaryFile
28+
* @param \Magento\Framework\App\View\Asset\Publisher $publisher
29+
*/
1830
public function __construct(
1931
\Magento\Framework\Filesystem $filesystem,
2032
\Magento\Framework\View\Asset\Repository $assetRepo,
21-
\Magento\Framework\Less\PreProcessor\Instruction\Import $importProcessor,
2233
\Magento\Framework\Less\File\Temporary $temporaryFile,
2334
\Magento\Framework\App\View\Asset\Publisher $publisher
2435
) {
25-
parent::__construct($filesystem, $assetRepo, $importProcessor, $temporaryFile);
36+
parent::__construct($filesystem, $assetRepo, $temporaryFile);
2637
$this->publisher = $publisher;
2738
}
2839

dev/tools/Magento/Tools/Webdev/file_assembler.php

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,15 +35,15 @@
3535
$logger = new Log($params->getVerbose());
3636

3737
} catch (\Zend_Console_Getopt_Exception $e) {
38-
echo $e->getUsageMessage();
39-
echo 'Please, use quotes(") for wrapping strings.' . "\n";
38+
echo $e->getMessage() . PHP_EOL;
39+
echo 'Please, use quotes(") for wrapping strings.' . PHP_EOL;
4040
exit(1);
4141
}
4242

4343
$bootstrap = \Magento\Framework\App\Bootstrap::create(BP, $_SERVER);
4444
/** @var \Magento\Framework\App\Http $app */
4545
$app = $bootstrap->createApplication(
46-
'Magento\Tools\WebDev\App\FileAssembler',
46+
'Magento\Tools\Webdev\App\FileAssembler',
4747
['params' => $params, 'logger' => $logger]
4848
);
4949
$bootstrap->run($app);

lib/internal/Magento/Framework/Less/Config.php

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,12 @@ class Config
1414
*/
1515
const TMP_LESS_DIR = 'less';
1616

17-
public function getLessDirectory()
17+
/**
18+
* Returns relative path to less materialization directory
19+
*
20+
* @return string
21+
*/
22+
public function getLessMaterializationRelativePath()
1823
{
1924
return DirectoryList::TMP_MATERIALIZATION_DIR . '/' . self::TMP_LESS_DIR;
2025
}

lib/internal/Magento/Framework/Less/File/Temporary.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ public function __construct(
4242
*/
4343
public function createFile($relativePath, $contents)
4444
{
45-
$filePath = $this->config->getLessDirectory() . '/' . $relativePath;
45+
$filePath = $this->config->getLessMaterializationRelativePath() . '/' . $relativePath;
4646

4747
if (!$this->tmpDirectory->isExist($filePath)) {
4848
$this->tmpDirectory->writeFile($filePath, $contents);

lib/internal/Magento/Framework/Less/FileGenerator.php

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ class FileGenerator implements SourceFileGeneratorInterface
7171
* @param FileGenerator\RelatedGenerator $relatedGenerator
7272
* @param Config $config
7373
* @param File\Temporary $temporaryFile
74+
*
75+
* @SuppressWarnings(PHPMD.CouplingBetweenObjects)
7476
*/
7577
public function __construct(
7678
\Magento\Framework\Filesystem $filesystem,
@@ -83,7 +85,6 @@ public function __construct(
8385
File\Temporary $temporaryFile
8486
) {
8587
$this->tmpDirectory = $filesystem->getDirectoryWrite(DirectoryList::VAR_DIR);
86-
$this->pubDirectory = $filesystem->getDirectoryWrite(DirectoryList::PUB);
8788
$this->assetRepo = $assetRepo;
8889
$this->assetSource = $assetSource;
8990

@@ -113,12 +114,12 @@ public function generateFileTree(Chain $chain)
113114
while ($this->isProcessLocked()) {
114115
sleep(1);
115116
}
116-
$lockFilePath = $this->config->getLessDirectory() . '/' . self::LOCK_FILE;
117+
$lockFilePath = $this->config->getLessMaterializationRelativePath() . '/' . self::LOCK_FILE;
117118
$this->tmpDirectory->writeFile($lockFilePath, time());
118119

119120
$this->magentoImportProcessor->process($chain);
120121
$this->importProcessor->process($chain);
121-
$this->relatedGenerator->generate();
122+
$this->relatedGenerator->generate($this->importProcessor);
122123
$lessRelativePath = preg_replace('#\.css$#', '.less', $chain->getAsset()->getPath());
123124
$tmpFilePath = $this->temporaryFile->createFile($lessRelativePath, $chain->getContent());
124125

@@ -133,7 +134,7 @@ public function generateFileTree(Chain $chain)
133134
*/
134135
protected function isProcessLocked()
135136
{
136-
$lockFilePath = $this->config->getLessDirectory() . '/' . self::LOCK_FILE;
137+
$lockFilePath = $this->config->getLessMaterializationRelativePath() . '/' . self::LOCK_FILE;
137138
if ($this->tmpDirectory->isExist($lockFilePath)) {
138139
$lockTime = time() - (int)$this->tmpDirectory->readFile($lockFilePath);
139140
if ($lockTime >= self::MAX_LOCK_TIME) {

lib/internal/Magento/Framework/Less/FileGenerator/RelatedGenerator.php

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
namespace Magento\Framework\Less\FileGenerator;
77

88
use Magento\Framework\App\Filesystem\DirectoryList;
9+
use Magento\Framework\Less\PreProcessor\Instruction\Import;
910
use Magento\Framework\View\Asset\LocalInterface;
1011

1112
class RelatedGenerator
@@ -20,11 +21,6 @@ class RelatedGenerator
2021
*/
2122
private $assetRepo;
2223

23-
/**
24-
* @var \Magento\Framework\Less\PreProcessor\Instruction\Import
25-
*/
26-
private $importProcessor;
27-
2824
/**
2925
* @var \Magento\Framework\Less\File\Temporary
3026
*/
@@ -33,32 +29,31 @@ class RelatedGenerator
3329
/**
3430
* @param \Magento\Framework\Filesystem $filesystem
3531
* @param \Magento\Framework\View\Asset\Repository $assetRepo
36-
* @param \Magento\Framework\Less\PreProcessor\Instruction\Import $importProcessor
3732
* @param \Magento\Framework\Less\File\Temporary $temporaryFile
3833
*/
3934
public function __construct(
4035
\Magento\Framework\Filesystem $filesystem,
4136
\Magento\Framework\View\Asset\Repository $assetRepo,
42-
\Magento\Framework\Less\PreProcessor\Instruction\Import $importProcessor,
4337
\Magento\Framework\Less\File\Temporary $temporaryFile
4438
) {
4539
$this->tmpDirectory = $filesystem->getDirectoryWrite(DirectoryList::VAR_DIR);
4640
$this->assetRepo = $assetRepo;
4741

48-
$this->importProcessor = $importProcessor;
4942
$this->temporaryFile = $temporaryFile;
5043
}
5144

5245
/**
5346
* Create all asset files, referenced from already processed ones
5447
*
48+
* @param Import $importGenerator
49+
*
5550
* @return void
5651
*/
57-
public function generate()
52+
public function generate(Import $importGenerator)
5853
{
5954
do {
60-
$relatedFiles = $this->importProcessor->getRelatedFiles();
61-
$this->importProcessor->resetRelatedFiles();
55+
$relatedFiles = $importGenerator->getRelatedFiles();
56+
$importGenerator->resetRelatedFiles();
6257
foreach ($relatedFiles as $relatedFileInfo) {
6358
list($relatedFileId, $asset) = $relatedFileInfo;
6459

lib/internal/Magento/Framework/Less/Test/Unit/FileGeneratorTest.php

Lines changed: 90 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,6 @@
44
* See COPYING.txt for license details.
55
*/
66

7-
// @codingStandardsIgnoreFile
8-
97
namespace Magento\Framework\Less\Test\Unit;
108

119
class FileGeneratorTest extends \PHPUnit_Framework_TestCase
@@ -41,118 +39,134 @@ class FileGeneratorTest extends \PHPUnit_Framework_TestCase
4139
private $object;
4240

4341
/**
44-
* @var \Magento\Framework\App\View\Asset\Publisher|\PHPUnit_Framework_MockObject_MockObject
42+
* @var \Magento\Framework\Less\FileGenerator\RelatedGenerator|\PHPUnit_Framework_MockObject_MockObject
43+
*/
44+
private $relatedGenerator;
45+
46+
/**
47+
* @var \Magento\Framework\Less\Config|\PHPUnit_Framework_MockObject_MockObject
4548
*/
46-
private $publisher;
49+
private $config;
50+
51+
/**
52+
* @var \Magento\Framework\Less\File\Temporary|\PHPUnit_Framework_MockObject_MockObject
53+
*/
54+
private $temporaryFile;
4755

4856
protected function setUp()
4957
{
50-
$this->tmpDirectory = $this->getMockForAbstractClass('\Magento\Framework\Filesystem\Directory\WriteInterface');
51-
$this->rootDirectory = $this->getMockForAbstractClass('\Magento\Framework\Filesystem\Directory\ReadInterface');
58+
$this->tmpDirectory = $this->getMockForAbstractClass('Magento\Framework\Filesystem\Directory\WriteInterface');
59+
$this->rootDirectory = $this->getMockForAbstractClass('Magento\Framework\Filesystem\Directory\ReadInterface');
5260
$this->rootDirectory->expects($this->any())
5361
->method('getRelativePath')
5462
->will($this->returnArgument(0));
5563
$this->rootDirectory->expects($this->any())
5664
->method('readFile')
57-
->will($this->returnCallback(function ($file) {
58-
return "content of '$file'";
59-
}));
65+
->will(
66+
$this->returnCallback(
67+
function ($file) {
68+
return "content of '$file'";
69+
}
70+
)
71+
);
6072
$filesystem = $this->getMock('\Magento\Framework\Filesystem', [], [], '', false);
61-
$filesystem->expects($this->exactly(2))
73+
$filesystem->expects($this->once())
6274
->method('getDirectoryWrite')
63-
//->with(DirectoryList::VAR_DIR)
6475
->will($this->returnValue($this->tmpDirectory));
6576
$this->assetRepo = $this->getMock('\Magento\Framework\View\Asset\Repository', [], [], '', false);
6677
$this->magentoImport = $this->getMock(
67-
'\Magento\Framework\Less\PreProcessor\Instruction\MagentoImport', [], [], '', false
78+
'Magento\Framework\Less\PreProcessor\Instruction\MagentoImport',
79+
[],
80+
[],
81+
'',
82+
false
6883
);
6984
$this->import = $this->getMock(
70-
'\Magento\Framework\Less\PreProcessor\Instruction\Import', [], [], '', false
85+
'Magento\Framework\Less\PreProcessor\Instruction\Import',
86+
[],
87+
[],
88+
'',
89+
false
7190
);
7291

7392
$assetSource = $this->getMock(
74-
'Magento\Framework\View\Asset\Source', [], [], '', false
93+
'Magento\Framework\View\Asset\Source',
94+
[],
95+
[],
96+
'',
97+
false
7598
);
7699

77-
$this->publisher = $this->getMock('Magento\Framework\App\View\Asset\Publisher', [], [], '', false);
78-
100+
$this->relatedGenerator = $this->getMockBuilder('Magento\Framework\Less\FileGenerator\RelatedGenerator')
101+
->disableOriginalConstructor()
102+
->setMethods([])
103+
->getMock();
104+
$this->config = $this->getMockBuilder('Magento\Framework\Less\Config')
105+
->disableOriginalConstructor()
106+
->setMethods([])
107+
->getMock();
108+
$this->temporaryFile = $this->getMockBuilder('Magento\Framework\Less\File\Temporary')
109+
->disableOriginalConstructor()
110+
->setMethods([])
111+
->getMock();
79112
$this->object = new \Magento\Framework\Less\FileGenerator(
80-
$filesystem, $this->assetRepo, $this->magentoImport, $this->import, $assetSource, $this->publisher
113+
$filesystem,
114+
$this->assetRepo,
115+
$this->magentoImport,
116+
$this->import,
117+
$assetSource,
118+
$this->relatedGenerator,
119+
$this->config,
120+
$this->temporaryFile
81121
);
82122
}
83123

84124
public function testGenerateLessFileTree()
85125
{
86-
$originalContent = 'original content';
126+
$lessDirectory = 'path/to/less';
87127
$expectedContent = 'updated content';
88-
$expectedRelativePath = 'view_preprocessed/less/some/file.less';
89-
$expectedPath = '/var/view_preprocessed/less/some/file.less';
128+
$expectedRelativePath = 'some/file.less';
129+
$expectedPath = $lessDirectory . '/some/file.less';
90130

91-
$asset = $this->getMock('\Magento\Framework\View\Asset\File', [], [], '', false);
92-
$asset->expects($this->exactly(2))
93-
->method('getPath')
94-
->will($this->returnValue('some/file.css'));
95-
$chain = new \Magento\Framework\View\Asset\PreProcessor\Chain($asset, $originalContent, 'less');
131+
132+
$asset = $this->getMock('Magento\Framework\View\Asset\File', [], [], '', false);
133+
$chain = $this->getMock('Magento\Framework\View\Asset\PreProcessor\Chain', [], [], '', false);
134+
135+
$this->config->expects($this->any())
136+
->method('getLessDirectory')
137+
->willReturn($lessDirectory);
138+
$this->tmpDirectory->expects($this->once())
139+
->method('isExist')
140+
->willReturn(true);
96141

97142
$this->magentoImport->expects($this->once())
98143
->method('process')
99144
->with($chain);
100145
$this->import->expects($this->once())
101146
->method('process')
102147
->with($chain);
148+
$this->relatedGenerator->expects($this->once())
149+
->method('generate')
150+
->with($this->import);
103151

104-
$relatedAssetOne = $this->getMock('\Magento\Framework\View\Asset\File', [], [], '', false);
105-
$relatedAssetOne->expects($this->any())
152+
$asset->expects($this->once())
106153
->method('getPath')
107-
->will($this->returnValue('related/file_one.css'));
108-
$relatedAssetOne->expects($this->any())
109-
->method('getContent')
110-
->will($this->returnValue("content of 'related/file_one.css'"));
111-
$relatedAssetTwo = $this->getMock('\Magento\Framework\View\Asset\File', [], [], '', false);
112-
$relatedAssetTwo->expects($this->any())
113-
->method('getPath')
114-
->will($this->returnValue('related/file_two.css'));
115-
$relatedAssetTwo->expects($this->any())
154+
->will($this->returnValue('some/file.css'));
155+
$chain->expects($this->once())
116156
->method('getContent')
117-
->will($this->returnValue("content of 'related/file_two.css'"));
118-
$assetsMap = [
119-
['related/file_one.css', $asset, $relatedAssetOne],
120-
['related/file_two.css', $asset, $relatedAssetTwo],
121-
];
122-
$this->assetRepo->expects($this->any())
123-
->method('createRelated')
124-
->will($this->returnValueMap($assetsMap));
125-
126-
$relatedFilesOne = [['related/file_one.css', $asset]];
127-
$this->import->expects($this->at(1))
128-
->method('getRelatedFiles')
129-
->will($this->returnValue($relatedFilesOne));
130-
$relatedFilesTwo = [['related/file_two.css', $asset]];
131-
$this->import->expects($this->at(3))
132-
->method('getRelatedFiles')
133-
->will($this->returnValue($relatedFilesTwo));
134-
$this->import->expects($this->at(5))
135-
->method('getRelatedFiles')
136-
->will($this->returnValue([]));
137-
138-
$writeMap = [
139-
[$expectedRelativePath, $expectedContent],
140-
['related/file_one.css', "content of 'related/file_one.css'"],
141-
['related/file_two.css', "content of 'related/file_two.css'"],
142-
];
143-
$pathsMap = [
144-
[$expectedRelativePath, $expectedPath],
145-
['related/file_one.css', '/var/view_preprocessed/less/related/file_one.css'],
146-
['related/file_two.css', '/var/view_preprocessed/less/related/file_two.css'],
147-
];
148-
$this->tmpDirectory->expects($this->any())
149-
->method('writeFile')
150-
->will($this->returnValueMap($writeMap));
151-
$this->tmpDirectory->expects($this->any())
152-
->method('getAbsolutePath')
153-
->will($this->returnValueMap($pathsMap));
154-
155-
$actual = $this->object->generateFileTree($chain);
156-
$this->assertSame($expectedPath, $actual);
157+
->willReturn($expectedContent);
158+
$chain->expects($this->once())
159+
->method('getAsset')
160+
->willReturn($asset);
161+
162+
$this->temporaryFile->expects($this->once())
163+
->method('createFile')
164+
->with(
165+
$expectedRelativePath,
166+
$expectedContent
167+
)
168+
->willReturn($expectedPath);
169+
170+
$this->assertSame($expectedPath, $this->object->generateFileTree($chain));
157171
}
158172
}

0 commit comments

Comments
 (0)