Skip to content

Commit 2e58ab6

Browse files
Vinaiamol2jcommerce
authored andcommitted
#20773: Make autoloader PSR-4 compliant
1 parent f69bf0c commit 2e58ab6

File tree

2 files changed

+146
-9
lines changed

2 files changed

+146
-9
lines changed
Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,85 @@
1+
<?php declare(strict_types=1);
2+
/**
3+
* Copyright © Magento, Inc. All rights reserved.
4+
* See COPYING.txt for license details.
5+
*/
6+
7+
namespace Magento\Framework\Code\Generator;
8+
9+
use Magento\Framework\Code\Generator;
10+
use Magento\Framework\Logger\Monolog as MagentoMonologLogger;
11+
use Magento\TestFramework\ObjectManager;
12+
use PHPUnit\Framework\TestCase;
13+
use PHPUnit_Framework_MockObject_MockObject as MockObject;
14+
use Psr\Log\LoggerInterface;
15+
16+
class AutoloaderTest extends TestCase
17+
{
18+
/**
19+
* This method exists to fix the wrong return type hint on \Magento\Framework\App\ObjectManager::getInstance.
20+
* This way the IDE knows it's dealing with an instance of \Magento\TestFramework\ObjectManager and
21+
* not \Magento\Framework\App\ObjectManager. The former has the method addSharedInstance, the latter does not.
22+
*
23+
* @return ObjectManager|\Magento\Framework\App\ObjectManager
24+
* @SuppressWarnings(PHPMD.StaticAccess)
25+
*/
26+
private function getTestFrameworkObjectManager()
27+
{
28+
return ObjectManager::getInstance();
29+
}
30+
31+
/**
32+
* @before
33+
*/
34+
public function setupLoggerTestDouble(): void
35+
{
36+
$loggerTestDouble = $this->createMock(LoggerInterface::class);
37+
$this->getTestFrameworkObjectManager()->addSharedInstance($loggerTestDouble, MagentoMonologLogger::class);
38+
}
39+
40+
/**
41+
* @after
42+
*/
43+
public function removeLoggerTestDouble(): void
44+
{
45+
$this->getTestFrameworkObjectManager()->removeSharedInstance(MagentoMonologLogger::class);
46+
}
47+
48+
/**
49+
* @param \RuntimeException $testException
50+
* @return Generator|MockObject
51+
*/
52+
private function createExceptionThrowingGeneratorTestDouble(\RuntimeException $testException)
53+
{
54+
/** @var Generator|MockObject $generatorStub */
55+
$generatorStub = $this->createMock(Generator::class);
56+
$generatorStub->method('generateClass')->willThrowException($testException);
57+
58+
return $generatorStub;
59+
}
60+
61+
public function testLogsExceptionDuringGeneration(): void
62+
{
63+
$exceptionMessage = 'Test exception thrown during generation';
64+
$testException = new \RuntimeException($exceptionMessage);
65+
66+
$loggerMock = ObjectManager::getInstance()->get(LoggerInterface::class);
67+
$loggerMock->expects($this->once())->method('debug')->with($exceptionMessage, ['exception' => $testException]);
68+
69+
$autoloader = new Autoloader($this->createExceptionThrowingGeneratorTestDouble($testException));
70+
$this->assertNull($autoloader->load(NonExistingClassName::class));
71+
}
72+
73+
public function testFiltersDuplicateExceptionMessages(): void
74+
{
75+
$exceptionMessage = 'Test exception thrown during generation';
76+
$testException = new \RuntimeException($exceptionMessage);
77+
78+
$loggerMock = ObjectManager::getInstance()->get(LoggerInterface::class);
79+
$loggerMock->expects($this->once())->method('debug')->with($exceptionMessage, ['exception' => $testException]);
80+
81+
$autoloader = new Autoloader($this->createExceptionThrowingGeneratorTestDouble($testException));
82+
$autoloader->load(OneNonExistingClassName::class);
83+
$autoloader->load(AnotherNonExistingClassName::class);
84+
}
85+
}

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

Lines changed: 61 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -3,37 +3,89 @@
33
* Copyright © Magento, Inc. All rights reserved.
44
* See COPYING.txt for license details.
55
*/
6+
67
namespace Magento\Framework\Code\Generator;
78

9+
use Magento\Framework\App\ObjectManager;
810
use Magento\Framework\Code\Generator;
11+
use Psr\Log\LoggerInterface;
912

1013
class Autoloader
1114
{
1215
/**
13-
* @var \Magento\Framework\Code\Generator
16+
* @var Generator
1417
*/
1518
protected $_generator;
1619

1720
/**
18-
* @param \Magento\Framework\Code\Generator $generator
21+
* Enables guarding against spamming the debug log with duplicate messages, as
22+
* the generation exception will be thrown multiple times within a single request.
23+
*
24+
* @var string
25+
*/
26+
private $lastGenerationErrorMessage;
27+
28+
/**
29+
* @param Generator $generator
1930
*/
20-
public function __construct(
21-
\Magento\Framework\Code\Generator $generator
22-
) {
31+
public function __construct(Generator $generator)
32+
{
2333
$this->_generator = $generator;
2434
}
2535

2636
/**
2737
* Load specified class name and generate it if necessary
2838
*
39+
* According to PSR-4 section 2.4 an autoloader MUST NOT throw an exception and SHOULD NOT return a value.
40+
*
41+
* @see https://www.php-fig.org/psr/psr-4/
42+
*
2943
* @param string $className
30-
* @return bool True if class was loaded
44+
* @return void
3145
*/
3246
public function load($className)
3347
{
34-
if (!class_exists($className)) {
35-
return Generator::GENERATION_ERROR != $this->_generator->generateClass($className);
48+
if (! class_exists($className)) {
49+
try {
50+
$this->_generator->generateClass($className);
51+
} catch (\Exception $exception) {
52+
$this->tryToLogExceptionMessageIfNotDuplicate($exception);
53+
}
54+
}
55+
}
56+
57+
/**
58+
* @param \Exception $exception
59+
*/
60+
private function tryToLogExceptionMessageIfNotDuplicate(\Exception $exception): void
61+
{
62+
if ($this->lastGenerationErrorMessage !== $exception->getMessage()) {
63+
$this->lastGenerationErrorMessage = $exception->getMessage();
64+
$this->tryToLogException($exception);
65+
}
66+
}
67+
68+
/**
69+
* Try to capture the exception message.
70+
*
71+
* The Autoloader is instantiated before the ObjectManager, so the LoggerInterface can not be injected.
72+
* The Logger is instantiated in the try/catch block because ObjectManager might still not be initialized.
73+
* In that case the exception message can not be captured.
74+
*
75+
* The debug level is used for logging in case class generation fails for a common class, but a custom
76+
* autoloader is used later in the stack. A more severe log level would fill the logs with messages on production.
77+
* The exception message now can be accessed in developer mode if debug logging is enabled.
78+
*
79+
* @param \Exception $exception
80+
* @return void
81+
*/
82+
private function tryToLogException(\Exception $exception): void
83+
{
84+
try {
85+
$logger = ObjectManager::getInstance()->get(LoggerInterface::class);
86+
$logger->debug($exception->getMessage(), ['exception' => $exception]);
87+
} catch (\Exception $ignoreThisException) {
88+
// Do not take an action here, since the original exception might have been caused by logger
3689
}
37-
return true;
3890
}
3991
}

0 commit comments

Comments
 (0)