Skip to content

Commit 2ac90c0

Browse files
author
Dale Sikkema
committed
MAGETWO-31834: Fatal error if trying to navigate to Frontend at first time after install
 - CR comments
1 parent 24e4c07 commit 2ac90c0

File tree

4 files changed

+26
-71
lines changed

4 files changed

+26
-71
lines changed

‎lib/internal/Magento/Framework/Code/Generator/EntityAbstract.php

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -247,7 +247,6 @@ protected function _validateData()
247247
{
248248
$sourceClassName = $this->getSourceClassName();
249249
$resultClassName = $this->_getResultClassName();
250-
$generationDir = $this->_ioObject->getGenerationDirectory();
251250
$resultDir = $this->_ioObject->getResultFileDirectory($resultClassName);
252251

253252
if (!$this->definedClasses->classLoadable($sourceClassName)) {
@@ -257,12 +256,10 @@ protected function _validateData()
257256
$this->_addError('Result class ' . $resultClassName . ' already exists.');
258257
return false;
259258
} elseif (
260-
!$this->_ioObject->makeGenerationDirectory()
261-
&& !$this->_ioObject->fileExists($generationDir)
262-
) {
263-
$this->_addError('Can\'t create directory ' . $generationDir . '.');
264-
return false;
265-
} elseif (
259+
/**
260+
* If makeResultFileDirectory only fails because the file is already created,
261+
* a competing process has generated the file, no exception should be thrown.
262+
*/
266263
!$this->_ioObject->makeResultFileDirectory($resultClassName)
267264
&& !$this->_ioObject->fileExists($resultDir)
268265
) {

‎lib/internal/Magento/Framework/Code/Generator/Io.php

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -91,17 +91,16 @@ public function writeResultFile($fileName, $content)
9191
{
9292
/**
9393
* Rename is atomic on *nix systems, while file_put_contents is not. Writing to a
94-
* temporary file and renaming to the real location helps avoid race conditions on
95-
* some systems. Race condition can occur if the compiler has not been run, when
94+
* temporary file whose name is process-unique and renaming to the real location helps
95+
* avoid race conditions. Race condition can occur if the compiler has not been run, when
9696
* multiple processes are attempting to access the generated file simultaneously.
9797
*/
9898
$content = "<?php\n" . $content;
99-
$rand = rand(0, 1024);
100-
$tmpFile = $fileName . ".$rand";
99+
$tmpFile = $fileName . "." . getmypid();
101100
$this->filesystemDriver->filePutContents($tmpFile, $content);
102101

103102
try {
104-
$result = $this->filesystemDriver->rename($tmpFile, $fileName);
103+
$success = $this->filesystemDriver->rename($tmpFile, $fileName);
105104
} catch (FilesystemException $e) {
106105
if (!file_exists($fileName)) {
107106
throw $e;
@@ -110,11 +109,11 @@ public function writeResultFile($fileName, $content)
110109
* Due to race conditions, file may have already been written, causing rename to fail. As long as
111110
* the file exists, everything is okay.
112111
*/
113-
$result = true;
112+
$success = true;
114113
}
115114
}
116115

117-
return $result;
116+
return $success;
118117
}
119118

120119
/**

‎lib/internal/Magento/Framework/Code/Test/Unit/Generator/EntityAbstractTest.php

Lines changed: 15 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -77,14 +77,13 @@ public function testConstruct()
7777
$this->assertAttributeEquals(self::SOURCE_CLASS . 'Abstract', '_resultClassName', $this->_model);
7878

7979
// with all arguments
80-
$ioObject = $this->getMock('Magento\Framework\Code\Generator\Io', [], [], '', false);
81-
$codeGenerator = $this->getMock(
82-
'Magento\Framework\Code\Generator\ClassGenerator',
83-
[],
84-
[],
85-
'',
86-
false
87-
);
80+
// Configure IoObject mock
81+
$ioObject = $this->getMockBuilder('Magento\Framework\Code\Generator\Io')
82+
->disableOriginalConstructor()
83+
->getMock();
84+
$codeGenerator = $this->getMockBuilder('Magento\Framework\Code\Generator\ClassGenerator')
85+
->disableOriginalConstructor()
86+
->getMock();
8887

8988
$this->_model = $this->getMockForAbstractClass(
9089
'Magento\Framework\Code\Generator\EntityAbstract',
@@ -114,27 +113,18 @@ public function generateDataProvider()
114113
'$sourceClassExists' => true,
115114
'$resultClassExists' => true,
116115
],
117-
'cant_create_generation_directory' => [
118-
'$errors' => ['Can\'t create directory ' . self::GENERATION_DIRECTORY . '.'],
119-
'$validationSuccess' => false,
120-
'$sourceClassExists' => true,
121-
'$resultClassExists' => false,
122-
'$makeGenerationDirSuccess' => false,
123-
],
124116
'cant_create_result_directory' => [
125117
'$errors' => ['Can\'t create directory ' . self::RESULT_DIRECTORY . '.'],
126118
'$validationSuccess' => false,
127119
'$sourceClassExists' => true,
128120
'$resultClassExists' => false,
129-
'$makeGenerationDirSuccess' => true,
130121
'$makeResultDirSuccess' => false,
131122
],
132123
'result_file_exists' => [
133124
'$errors' => [],
134125
'$validationSuccess' => true,
135126
'$sourceClassExists' => true,
136127
'$resultClassExists' => false,
137-
'$makeGenerationDirSuccess' => false,
138128
'$makeResultDirSuccess' => false,
139129
'$resultFileExists' => true,
140130
],
@@ -143,7 +133,6 @@ public function generateDataProvider()
143133
'$validationSuccess' => true,
144134
'$sourceClassExists' => true,
145135
'$resultClassExists' => false,
146-
'$makeGenerationDirSuccess' => true,
147136
'$makeResultDirSuccess' => true,
148137
'$resultFileExists' => true,
149138
'$willWriteCode' => false,
@@ -157,7 +146,6 @@ public function generateDataProvider()
157146
* @param bool $validationSuccess
158147
* @param bool $sourceClassExists
159148
* @param bool $resultClassExists
160-
* @param bool $makeGenerationDirSuccess
161149
* @param bool $makeResultDirSuccess
162150
* @param bool $resultFileExists
163151
* @param bool $willWriteCode
@@ -180,7 +168,6 @@ public function testGenerate(
180168
$validationSuccess = true,
181169
$sourceClassExists = true,
182170
$resultClassExists = false,
183-
$makeGenerationDirSuccess = true,
184171
$makeResultDirSuccess = true,
185172
$resultFileExists = false,
186173
$willWriteCode = true
@@ -191,7 +178,6 @@ public function testGenerate(
191178
$arguments = $this->_prepareMocksForValidateData(
192179
$sourceClassExists,
193180
$resultClassExists,
194-
$makeGenerationDirSuccess,
195181
$makeResultDirSuccess,
196182
$resultFileExists
197183
);
@@ -226,7 +212,6 @@ public function testGenerate(
226212
*
227213
* @param bool $sourceClassExists
228214
* @param bool $resultClassExists
229-
* @param bool $makeGenerationDirSuccess
230215
* @param bool $makeResultDirSuccess
231216
* @param bool $resultFileExists
232217
* @return array
@@ -235,7 +220,6 @@ public function testGenerate(
235220
protected function _prepareMocksForValidateData(
236221
$sourceClassExists = true,
237222
$resultClassExists = false,
238-
$makeGenerationDirSuccess = true,
239223
$makeResultDirSuccess = true,
240224
$resultFileExists = false
241225
) {
@@ -253,31 +237,13 @@ protected function _prepareMocksForValidateData(
253237
}
254238

255239
// Configure IoObject mock
256-
$ioObject = $this->getMock(
257-
'Magento\Framework\Code\Generator\Io',
258-
[
259-
'getResultFileName',
260-
'makeGenerationDirectory',
261-
'makeResultFileDirectory',
262-
'fileExists',
263-
'getGenerationDirectory',
264-
'getResultFileDirectory',
265-
'writeResultFile',
266-
'rename'
267-
],
268-
[],
269-
'',
270-
false
271-
);
240+
$ioObject = $this->getMockBuilder('Magento\Framework\Code\Generator\Io')
241+
->disableOriginalConstructor()
242+
->getMock();
272243

273-
$ioObject->expects($this->any())->method('getGenerationDirectory')->willReturn(self::GENERATION_DIRECTORY);
274244
$ioObject->expects($this->any())->method('getResultFileDirectory')->willReturn(self::RESULT_DIRECTORY);
275-
$makeGenDirInvocations = (!$sourceClassExists || $resultClassExists) ? 0 : 1;
276-
$ioObject->expects($this->exactly($makeGenDirInvocations))
277-
->method('makeGenerationDirectory')
278-
->willReturn($makeGenerationDirSuccess);
279245
$ioObject->expects($this->any())->method('fileExists')->willReturn($resultFileExists);
280-
if ($sourceClassExists && !$resultClassExists && $makeGenerationDirSuccess) {
246+
if ($sourceClassExists && !$resultClassExists) {
281247
$ioObject->expects($this->once())
282248
->method('makeResultFileDirectory')
283249
->with(self::RESULT_CLASS)
@@ -304,13 +270,10 @@ protected function _prepareMocksForGenerateCode($willWriteCode)
304270
// Configure mocks for the validation step
305271
$mocks = $this->_prepareMocksForValidateData();
306272

307-
$codeGenerator = $this->getMock(
308-
'Magento\Framework\Code\Generator\ClassGenerator',
309-
['setName', 'addProperties', 'addMethods', 'setClassDocBlock', 'generate'],
310-
[],
311-
'',
312-
false
313-
);
273+
$codeGenerator = $this->getMockBuilder('Magento\Framework\Code\Generator\ClassGenerator')
274+
->disableOriginalConstructor()
275+
->getMock();
276+
314277
$codeGenerator->expects($this->once())->method('setName')->with(self::RESULT_CLASS)->will($this->returnSelf());
315278
$codeGenerator->expects($this->once())->method('addProperties')->will($this->returnSelf());
316279
$codeGenerator->expects($this->once())->method('addMethods')->will($this->returnSelf());

‎lib/internal/Magento/Framework/Code/Test/Unit/Generator/IoTest.php

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -43,11 +43,7 @@ protected function setUp()
4343
{
4444
$this->_generationDirectory = rtrim(self::GENERATION_DIRECTORY, '/') . '/';
4545

46-
$this->_filesystemDriverMock = $this->getMock(
47-
'Magento\Framework\Filesystem\Driver\File',
48-
['isWritable', 'filePutContents', 'createDirectory', 'isExists', 'rename'],
49-
[]
50-
);
46+
$this->_filesystemDriverMock = $this->getMock('Magento\Framework\Filesystem\Driver\File');
5147

5248
$this->_object = new \Magento\Framework\Code\Generator\Io(
5349
$this->_filesystemDriverMock,

0 commit comments

Comments
 (0)