Skip to content

Commit 874fb22

Browse files
NadiyaSNadiyaS
committed
MAGETWO-92986: Write Logs for Failed Process of Generating Factories in Extensions
 - revert EntityAbstract class and add error message to class generator
1 parent 09498f2 commit 874fb22

File tree

5 files changed

+72
-114
lines changed

5 files changed

+72
-114
lines changed

‎dev/tests/integration/testsuite/Magento/Framework/Code/GeneratorTest.php

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -173,18 +173,18 @@ public function testGenerateClassExtensionAttributesInterfaceFactoryWithNamespac
173173
}
174174

175175
/**
176-
* It tries to generate a new class file if the generated directory is read-only
176+
* It tries to generate a new class file when the generated directory is read-only
177177
*/
178178
public function testGeneratorClassWithErrorSaveClassFile()
179179
{
180-
$msgPart = 'Error: an object of a generated class may be a dependency for another object, '
181-
. 'but this dependency has not been defined or set correctly in the signature of the related construct '
182-
. 'method';
180+
$factoryClassName = self::CLASS_NAME_WITH_NAMESPACE . 'Factory';
181+
$msgPart = 'Class ' . $factoryClassName . ' generation error: The requested class did not generate properly, '
182+
. 'because the \'generated\' directory permission is read-only.';
183+
$regexpMsgPart = preg_quote($msgPart);
183184
$this->expectException(\RuntimeException::class);
184-
$this->expectExceptionMessageRegExp("/.*$msgPart.*/");
185+
$this->expectExceptionMessageRegExp("/.*$regexpMsgPart.*/");
185186
$this->generatedDirectory->create($this->testRelativePath);
186187
$this->generatedDirectory->changePermissionsRecursively($this->testRelativePath, 0555, 0444);
187-
$factoryClassName = self::CLASS_NAME_WITH_NAMESPACE . 'Factory';
188188
$generatorResult = $this->_generator->generateClass($factoryClassName);
189189
$this->assertFalse($generatorResult);
190190
$pathToSystemLog = $this->logDirectory->getAbsolutePath('system.log');

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,12 @@ public function generateClass($className)
116116
/** @var $logger LoggerInterface */
117117
$logger = $this->getObjectManager()->get(LoggerInterface::class);
118118
$errors = $generator->getErrors();
119-
$message = implode(PHP_EOL, $errors) . ' in [' . $className . ']';
119+
$errors[] = 'Class ' . $className . ' generation error: The requested class did not generate properly, '
120+
. 'because the \'generated\' directory permission is read-only. '
121+
. 'If --- after running the \'bin/magento setup:di:compile\' CLI command when the \'generated\' '
122+
. 'directory permission is set to write --- the requested class did not generate properly, then '
123+
. 'you must add the generated class object to the signature of the related construct method, only.';
124+
$message = implode(PHP_EOL, $errors);
120125
$logger->critical($message);
121126
throw new \RuntimeException($message);
122127
}

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

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,7 @@
55
*/
66
namespace Magento\Framework\Code\Generator;
77

8-
use Magento\Framework\Exception\FileSystemException;
98
use Zend\Code\Generator\ValueGenerator;
10-
use Magento\Framework\Filesystem\Driver\File;
11-
use Magento\Framework\ObjectManagerInterface;
129

1310
abstract class EntityAbstract
1411
{
@@ -44,7 +41,7 @@ abstract class EntityAbstract
4441
/**
4542
* Class generator object
4643
*
47-
* @var CodeGeneratorInterface
44+
* @var \Magento\Framework\Code\Generator\CodeGeneratorInterface
4845
*/
4946
protected $_classGenerator;
5047

@@ -57,20 +54,20 @@ abstract class EntityAbstract
5754
* @param null|string $sourceClassName
5855
* @param null|string $resultClassName
5956
* @param Io $ioObject
60-
* @param CodeGeneratorInterface $classGenerator
57+
* @param \Magento\Framework\Code\Generator\CodeGeneratorInterface $classGenerator
6158
* @param DefinedClasses $definedClasses
6259
*/
6360
public function __construct(
6461
$sourceClassName = null,
6562
$resultClassName = null,
6663
Io $ioObject = null,
67-
CodeGeneratorInterface $classGenerator = null,
64+
\Magento\Framework\Code\Generator\CodeGeneratorInterface $classGenerator = null,
6865
DefinedClasses $definedClasses = null
6966
) {
7067
if ($ioObject) {
7168
$this->_ioObject = $ioObject;
7269
} else {
73-
$this->_ioObject = new Io(new File());
70+
$this->_ioObject = new Io(new \Magento\Framework\Filesystem\Driver\File());
7471
}
7572
if ($classGenerator) {
7673
$this->_classGenerator = $classGenerator;
@@ -109,17 +106,6 @@ public function generate()
109106
$this->_addError('Can\'t generate source code.');
110107
}
111108
}
112-
} catch (FileSystemException $e) {
113-
$message = 'Error: an object of a generated class may be a dependency for another object, but this '
114-
. 'dependency has not been defined or set correctly in the signature of the related construct method. '
115-
. 'Due to the current error, executing the CLI commands `bin/magento setup:di:compile` or `bin/magento '
116-
. 'deploy:mode:set production` does not create the required generated classes. '
117-
. 'Magento cannot write a class file to the "generated" directory that is read-only. Before using the '
118-
. 'read-only file system, the classes to be generated must be created beforehand in the "generated" '
119-
. 'directory. For details, see the "File systems access permissions" topic '
120-
. 'at http://devdocs.magento.com.';
121-
$this->_addError($message);
122-
$this->_addError($e->getMessage());
123109
} catch (\Exception $e) {
124110
$this->_addError($e->getMessage());
125111
}
@@ -203,7 +189,7 @@ protected function _getClassProperties()
203189
'visibility' => 'protected',
204190
'docblock' => [
205191
'shortDescription' => 'Object Manager instance',
206-
'tags' => [['name' => 'var', 'description' => '\\' . ObjectManagerInterface::class]],
192+
'tags' => [['name' => 'var', 'description' => '\\' . \Magento\Framework\ObjectManagerInterface::class]],
207193
],
208194
];
209195

@@ -264,9 +250,9 @@ protected function _validateData()
264250
$this->_addError('Source class ' . $sourceClassName . ' doesn\'t exist.');
265251
return false;
266252
} elseif (/**
267-
* If makeResultFileDirectory only fails because the file is already created,
268-
* a competing process has generated the file, no exception should be thrown.
269-
*/
253+
* If makeResultFileDirectory only fails because the file is already created,
254+
* a competing process has generated the file, no exception should be thrown.
255+
*/
270256
!$this->_ioObject->makeResultFileDirectory($resultClassName)
271257
&& !$this->_ioObject->fileExists($resultDir)
272258
) {
@@ -323,8 +309,9 @@ protected function _getNullDefaultValue()
323309
*
324310
* @return null|string
325311
*/
326-
private function extractParameterType(\ReflectionParameter $parameter): ?string
327-
{
312+
private function extractParameterType(
313+
\ReflectionParameter $parameter
314+
): ?string {
328315
/** @var string|null $typeName */
329316
$typeName = null;
330317
if ($parameter->hasType()) {
@@ -341,7 +328,7 @@ private function extractParameterType(\ReflectionParameter $parameter): ?string
341328
}
342329

343330
if ($parameter->allowsNull()) {
344-
$typeName = '?' . $typeName;
331+
$typeName = '?' .$typeName;
345332
}
346333
}
347334

@@ -353,8 +340,9 @@ private function extractParameterType(\ReflectionParameter $parameter): ?string
353340
*
354341
* @return null|ValueGenerator
355342
*/
356-
private function extractParameterDefaultValue(\ReflectionParameter $parameter): ?ValueGenerator
357-
{
343+
private function extractParameterDefaultValue(
344+
\ReflectionParameter $parameter
345+
): ?ValueGenerator {
358346
/** @var ValueGenerator|null $value */
359347
$value = null;
360348
if ($parameter->isOptional() && $parameter->isDefaultValueAvailable()) {

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

Lines changed: 28 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,7 @@
55
*/
66
namespace Magento\Framework\Code\Test\Unit\Generator;
77

8-
use Magento\Framework\Exception\FileSystemException;
9-
use Magento\Framework\Phrase;
10-
use Magento\Framework\DataObject;
11-
use Magento\Framework\DataObject_MyResult;
12-
use PHPUnit\Framework\TestCase;
13-
use Magento\Framework\Code\Generator\EntityAbstract;
14-
use Magento\Framework\Code\Generator\Io;
15-
use Magento\Framework\Code\Generator\ClassGenerator;
16-
use Magento\Framework\Code\Generator\DefinedClasses;
17-
use \PHPUnit_Framework_MockObject_MockObject as Mock;
18-
19-
class EntityAbstractTest extends TestCase
8+
class EntityAbstractTest extends \PHPUnit\Framework\TestCase
209
{
2110
/**#@+
2211
* Source and result class parameters
@@ -44,7 +33,7 @@ class EntityAbstractTest extends TestCase
4433
/**
4534
* Model under test
4635
*
47-
* @var EntityAbstract|Mock
36+
* @var \Magento\Framework\Code\Generator\EntityAbstract| \PHPUnit_Framework_MockObject_MockObject
4837
*/
4938
protected $_model;
5039

@@ -60,9 +49,9 @@ class EntityAbstractTest extends TestCase
6049

6150
protected function setUp()
6251
{
63-
$this->sourceClass = '\\' . DataObject::class;
64-
$this->resultClass = '\\' . DataObject_MyResult::class;
65-
$this->_model = $this->getMockForAbstractClass(EntityAbstract::class);
52+
$this->sourceClass = '\\' . \Magento\Framework\DataObject::class;
53+
$this->resultClass = '\\' . \Magento\Framework\DataObject_MyResult::class;
54+
$this->_model = $this->getMockForAbstractClass(\Magento\Framework\Code\Generator\EntityAbstract::class);
6655
}
6756

6857
protected function tearDown()
@@ -75,26 +64,37 @@ public function testConstruct()
7564
// without parameters
7665
$this->assertAttributeEmpty('_sourceClassName', $this->_model);
7766
$this->assertAttributeEmpty('_resultClassName', $this->_model);
78-
$this->assertAttributeInstanceOf(Io::class, '_ioObject', $this->_model);
79-
$this->assertAttributeInstanceOf(ClassGenerator::class, '_classGenerator', $this->_model);
80-
$this->assertAttributeInstanceOf(DefinedClasses::class, 'definedClasses', $this->_model);
67+
$this->assertAttributeInstanceOf(\Magento\Framework\Code\Generator\Io::class, '_ioObject', $this->_model);
68+
$this->assertAttributeInstanceOf(
69+
\Magento\Framework\Code\Generator\ClassGenerator::class,
70+
'_classGenerator',
71+
$this->_model
72+
);
73+
$this->assertAttributeInstanceOf(
74+
\Magento\Framework\Code\Generator\DefinedClasses::class,
75+
'definedClasses',
76+
$this->_model
77+
);
8178

8279
// with source class name
83-
$this->_model = $this->getMockForAbstractClass(EntityAbstract::class, [$this->sourceClass]);
80+
$this->_model = $this->getMockForAbstractClass(
81+
\Magento\Framework\Code\Generator\EntityAbstract::class,
82+
[$this->sourceClass]
83+
);
8484
$this->assertAttributeEquals($this->sourceClass, '_sourceClassName', $this->_model);
8585
$this->assertAttributeEquals($this->sourceClass . 'Abstract', '_resultClassName', $this->_model);
8686

8787
// with all arguments
8888
// Configure IoObject mock
89-
$ioObject = $this->getMockBuilder(Io::class)
89+
$ioObject = $this->getMockBuilder(\Magento\Framework\Code\Generator\Io::class)
9090
->disableOriginalConstructor()
9191
->getMock();
92-
$codeGenerator = $this->getMockBuilder(ClassGenerator::class)
92+
$codeGenerator = $this->getMockBuilder(\Magento\Framework\Code\Generator\ClassGenerator::class)
9393
->disableOriginalConstructor()
9494
->getMock();
9595

9696
$this->_model = $this->getMockForAbstractClass(
97-
EntityAbstract::class,
97+
\Magento\Framework\Code\Generator\EntityAbstract::class,
9898
[$this->sourceClass, $this->resultClass, $ioObject, $codeGenerator]
9999
);
100100
$this->assertAttributeEquals($this->resultClass, '_resultClassName', $this->_model);
@@ -186,7 +186,7 @@ public function testGenerate(
186186
}
187187
$abstractGetters = ['_getClassProperties', '_getClassMethods'];
188188
$this->_model = $this->getMockForAbstractClass(
189-
EntityAbstract::class,
189+
\Magento\Framework\Code\Generator\EntityAbstract::class,
190190
$arguments,
191191
'',
192192
true,
@@ -209,51 +209,6 @@ public function testGenerate(
209209
}
210210
}
211211

212-
/**
213-
* @inheritdoc
214-
*/
215-
public function testGenerateFailure()
216-
{
217-
$infoMessage = 'Error: an object of a generated class may be a dependency for another object, but this '
218-
. 'dependency has not been defined or set correctly in the signature of the related construct method. '
219-
. 'Due to the current error, executing the CLI commands `bin/magento setup:di:compile` or `bin/magento '
220-
. 'deploy:mode:set production` does not create the required generated classes. '
221-
. 'Magento cannot write a class file to the "generated" directory that is read-only. Before using the '
222-
. 'read-only file system, the classes to be generated must be created beforehand in the "generated" '
223-
. 'directory. For details, see the "File systems access permissions" topic at http://devdocs.magento.com.';
224-
225-
$exceptionMessage = 'Some description';
226-
227-
$abstractGetters = ['_getClassProperties', '_getClassMethods'];
228-
229-
$arguments = $this->_prepareMocksForGenerateCode(true);
230-
231-
/** @var Io|Mock $ioObjectMock */
232-
$ioObjectMock = $arguments['io_object'];
233-
$ioObjectMock->expects($this->once())
234-
->method('writeResultFile')
235-
->with(self::RESULT_FILE, self::RESULT_CODE)
236-
->willThrowException(new FileSystemException(new Phrase($exceptionMessage)));
237-
238-
$this->_model = $this->getMockForAbstractClass(
239-
EntityAbstract::class,
240-
$arguments,
241-
'',
242-
true,
243-
true,
244-
true,
245-
$abstractGetters
246-
);
247-
// we need to mock abstract methods to set correct return value type
248-
foreach ($abstractGetters as $methodName) {
249-
$this->_model->expects($this->any())->method($methodName)->will($this->returnValue([]));
250-
}
251-
252-
$result = $this->_model->generate();
253-
$this->assertFalse($result);
254-
$this->assertEquals([$infoMessage, $exceptionMessage], $this->_model->getErrors());
255-
}
256-
257212
/**
258213
* Prepares mocks for validation verification
259214
*
@@ -271,7 +226,7 @@ protected function _prepareMocksForValidateData(
271226
$resultFileExists = false
272227
) {
273228
// Configure DefinedClasses mock
274-
$definedClassesMock = $this->createMock(DefinedClasses::class);
229+
$definedClassesMock = $this->createMock(\Magento\Framework\Code\Generator\DefinedClasses::class);
275230
$definedClassesMock->expects($this->once())
276231
->method('isClassLoadable')
277232
->with($this->sourceClass)
@@ -284,7 +239,7 @@ protected function _prepareMocksForValidateData(
284239
}
285240

286241
// Configure IoObject mock
287-
$ioObject = $this->getMockBuilder(Io::class)
242+
$ioObject = $this->getMockBuilder(\Magento\Framework\Code\Generator\Io::class)
288243
->disableOriginalConstructor()
289244
->getMock();
290245

@@ -317,7 +272,7 @@ protected function _prepareMocksForGenerateCode($willWriteCode)
317272
// Configure mocks for the validation step
318273
$mocks = $this->_prepareMocksForValidateData();
319274

320-
$codeGenerator = $this->getMockBuilder(ClassGenerator::class)
275+
$codeGenerator = $this->getMockBuilder(\Magento\Framework\Code\Generator\ClassGenerator::class)
321276
->disableOriginalConstructor()
322277
->getMock();
323278

@@ -334,7 +289,7 @@ protected function _prepareMocksForGenerateCode($willWriteCode)
334289
->will($this->returnValue($willWriteCode ? self::RESULT_CODE : null));
335290

336291
// Add configuration for the generation step
337-
/** @var Io|Mock $ioObject */
292+
/** @var $ioObject \PHPUnit_Framework_MockObject_MockObject */
338293
$ioObject = $mocks['io_object'];
339294
if ($willWriteCode) {
340295
$ioObject->expects($this->once())->method('writeResultFile')->with(self::RESULT_FILE, self::RESULT_CODE);

‎lib/internal/Magento/Framework/Code/Test/Unit/GeneratorTest.php

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -127,15 +127,22 @@ public function testGenerateClassWhenClassIsNotGenerationSuccess()
127127
*/
128128
public function testGenerateClassWithErrors()
129129
{
130-
$this->expectException(\RuntimeException::class);
131-
$this->expectExceptionMessage("Some error message 0\nSome error message 1\nSome error message 2");
132-
$errorMessages = [
133-
'Some error message 0',
134-
'Some error message 1',
135-
'Some error message 2',
136-
];
137130
$expectedEntities = array_values($this->expectedEntities);
138131
$resultClassName = self::SOURCE_CLASS . ucfirst(array_shift($expectedEntities));
132+
$errorMessages = [
133+
'Error message 0',
134+
'Error message 1',
135+
'Error message 2',
136+
];
137+
$mainErrorMessage = 'Class ' . $resultClassName . ' generation error: The requested class did not generate properly, '
138+
. 'because the \'generated\' directory permission is read-only. '
139+
. 'If --- after running the \'bin/magento setup:di:compile\' CLI command when the \'generated\' '
140+
. 'directory permission is set to write --- the requested class did not generate properly, then '
141+
. 'you must add the generated class object to the signature of the related construct method, only.';
142+
$FinalErrorMessage = implode(PHP_EOL, $errorMessages) . "\n" . $mainErrorMessage;
143+
$this->expectException(\RuntimeException::class);
144+
$this->expectExceptionMessage($FinalErrorMessage);
145+
139146
/** @var ObjectManagerInterface|Mock $objectManagerMock */
140147
$objectManagerMock = $this->createMock(ObjectManagerInterface::class);
141148
/** @var EntityAbstract|Mock $entityGeneratorMock */
@@ -163,6 +170,9 @@ public function testGenerateClassWithErrors()
163170
$entityGeneratorMock->expects($this->once())
164171
->method('getErrors')
165172
->willReturn($errorMessages);
173+
$loggerMock->expects($this->once())
174+
->method('critical')
175+
->with($FinalErrorMessage);
166176
$this->model->setObjectManager($objectManagerMock);
167177
$this->model->generateClass($resultClassName);
168178
}

0 commit comments

Comments
 (0)