Skip to content

Commit a366578

Browse files
NadiyaSNadiyaS
committed
MAGETWO-93042: [Backport to 2.2 ] Write Logs for Failed Process of Generating Factories in Extensions
1 parent 02dd7e5 commit a366578

File tree

5 files changed

+167
-158
lines changed

5 files changed

+167
-158
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: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,9 @@
99
use Magento\Framework\Code\Generator\EntityAbstract;
1010
use Magento\Framework\Code\Generator\Io;
1111
use Magento\Framework\ObjectManagerInterface;
12-
use Psr\Log\LoggerInterface;
1312
use Magento\Framework\Phrase;
1413
use Magento\Framework\Filesystem\Driver\File;
14+
use Psr\Log\LoggerInterface;
1515

1616
class Generator
1717
{
@@ -41,19 +41,29 @@ class Generator
4141
*/
4242
protected $objectManager;
4343

44+
/**
45+
* Logger instance
46+
*
47+
* @var LoggerInterface
48+
*/
49+
private $logger;
50+
4451
/**
4552
* @param Generator\Io $ioObject
4653
* @param array $generatedEntities
4754
* @param DefinedClasses $definedClasses
55+
* @param LoggerInterface|null $logger
4856
*/
4957
public function __construct(
5058
Io $ioObject = null,
5159
array $generatedEntities = [],
52-
DefinedClasses $definedClasses = null
60+
DefinedClasses $definedClasses = null,
61+
LoggerInterface $logger = null
5362
) {
5463
$this->_ioObject = $ioObject ?: new Io(new File());
5564
$this->definedClasses = $definedClasses ?: new DefinedClasses();
5665
$this->_generatedEntities = $generatedEntities;
66+
$this->logger = $logger;
5767
}
5868

5969
/**
@@ -114,10 +124,14 @@ public function generateClass($className)
114124
$this->tryToLoadSourceClass($className, $generator);
115125
if (!($file = $generator->generate())) {
116126
/** @var $logger LoggerInterface */
117-
$logger = $this->getObjectManager()->get(LoggerInterface::class);
118127
$errors = $generator->getErrors();
119-
$message = implode(PHP_EOL, $errors) . ' in [' . $className . ']';
120-
$logger->critical($message);
128+
$errors[] = 'Class ' . $className . ' generation error: The requested class did not generate properly, '
129+
. 'because the \'generated\' directory permission is read-only. '
130+
. 'If --- after running the \'bin/magento setup:di:compile\' CLI command when the \'generated\' '
131+
. 'directory permission is set to write --- the requested class did not generate properly, then '
132+
. 'you must add the generated class object to the signature of the related construct method, only.';
133+
$message = implode(PHP_EOL, $errors);
134+
$this->getLogger()->critical($message);
121135
throw new \RuntimeException($message);
122136
}
123137
if (!$this->definedClasses->isClassLoadableFromMemory($className)) {
@@ -127,6 +141,19 @@ public function generateClass($className)
127141
}
128142
}
129143

144+
/**
145+
* Retrieve logger
146+
*
147+
* @return LoggerInterface
148+
*/
149+
private function getLogger()
150+
{
151+
if (!$this->logger) {
152+
$this->logger = $this->getObjectManager()->get(LoggerInterface::class);
153+
}
154+
return $this->logger;
155+
}
156+
130157
/**
131158
* Create entity generator
132159
*
@@ -205,7 +232,7 @@ protected function shouldSkipGeneration($resultEntityType, $sourceClassName, $re
205232
{
206233
if (!$resultEntityType || !$sourceClassName) {
207234
return self::GENERATION_ERROR;
208-
} elseif ($this->definedClasses->isClassLoadableFromDisc($resultClass)) {
235+
} elseif ($this->definedClasses->isClassLoadableFromDisk($resultClass)) {
209236
$generatedFileName = $this->_ioObject->generateResultFileName($resultClass);
210237
/**
211238
* Must handle two edge cases: a competing process has generated the class and written it to disc already,

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

Lines changed: 66 additions & 40 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
) {
@@ -318,6 +304,59 @@ protected function _getNullDefaultValue()
318304
return $value;
319305
}
320306

307+
/**
308+
* @param \ReflectionParameter $parameter
309+
*
310+
* @return null|string
311+
*/
312+
private function extractParameterType(
313+
\ReflectionParameter $parameter
314+
): ?string {
315+
/** @var string|null $typeName */
316+
$typeName = null;
317+
if ($parameter->hasType()) {
318+
if ($parameter->isArray()) {
319+
$typeName = 'array';
320+
} elseif ($parameter->getClass()) {
321+
$typeName = $this->_getFullyQualifiedClassName(
322+
$parameter->getClass()->getName()
323+
);
324+
} elseif ($parameter->isCallable()) {
325+
$typeName = 'callable';
326+
} else {
327+
$typeName = $parameter->getType()->getName();
328+
}
329+
330+
if ($parameter->allowsNull()) {
331+
$typeName = '?' .$typeName;
332+
}
333+
}
334+
335+
return $typeName;
336+
}
337+
338+
/**
339+
* @param \ReflectionParameter $parameter
340+
*
341+
* @return null|ValueGenerator
342+
*/
343+
private function extractParameterDefaultValue(
344+
\ReflectionParameter $parameter
345+
): ?ValueGenerator {
346+
/** @var ValueGenerator|null $value */
347+
$value = null;
348+
if ($parameter->isOptional() && $parameter->isDefaultValueAvailable()) {
349+
$valueType = ValueGenerator::TYPE_AUTO;
350+
$defaultValue = $parameter->getDefaultValue();
351+
if ($defaultValue === null) {
352+
$valueType = ValueGenerator::TYPE_NULL;
353+
}
354+
$value = new ValueGenerator($defaultValue, $valueType);
355+
}
356+
357+
return $value;
358+
}
359+
321360
/**
322361
* Retrieve method parameter info
323362
*
@@ -329,26 +368,13 @@ protected function _getMethodParameterInfo(\ReflectionParameter $parameter)
329368
$parameterInfo = [
330369
'name' => $parameter->getName(),
331370
'passedByReference' => $parameter->isPassedByReference(),
332-
'type' => $parameter->getType()
371+
'variadic' => $parameter->isVariadic()
333372
];
334-
335-
if ($parameter->isArray()) {
336-
$parameterInfo['type'] = 'array';
337-
} elseif ($parameter->getClass()) {
338-
$parameterInfo['type'] = $this->_getFullyQualifiedClassName($parameter->getClass()->getName());
339-
} elseif ($parameter->isCallable()) {
340-
$parameterInfo['type'] = 'callable';
373+
if ($type = $this->extractParameterType($parameter)) {
374+
$parameterInfo['type'] = $type;
341375
}
342-
343-
if ($parameter->isOptional() && $parameter->isDefaultValueAvailable()) {
344-
$defaultValue = $parameter->getDefaultValue();
345-
if (is_string($defaultValue)) {
346-
$parameterInfo['defaultValue'] = $parameter->getDefaultValue();
347-
} elseif ($defaultValue === null) {
348-
$parameterInfo['defaultValue'] = $this->_getNullDefaultValue();
349-
} else {
350-
$parameterInfo['defaultValue'] = $defaultValue;
351-
}
376+
if ($default = $this->extractParameterDefaultValue($parameter)) {
377+
$parameterInfo['defaultValue'] = $default;
352378
}
353379

354380
return $parameterInfo;

0 commit comments

Comments
 (0)