Skip to content

Commit

Permalink
#20773: Make autoloader PSR-4 compliant
Browse files Browse the repository at this point in the history
  • Loading branch information
Vinai committed Feb 11, 2019
1 parent e47b0d7 commit cab38a2
Show file tree
Hide file tree
Showing 2 changed files with 146 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?php declare(strict_types=1);
/**
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Framework\Code\Generator;

use Magento\Framework\Code\Generator;
use Magento\Framework\Logger\Monolog as MagentoMonologLogger;
use Magento\TestFramework\ObjectManager;
use PHPUnit\Framework\TestCase;
use PHPUnit_Framework_MockObject_MockObject as MockObject;
use Psr\Log\LoggerInterface;

class AutoloaderTest extends TestCase
{
/**
* This method exists to fix the wrong return type hint on \Magento\Framework\App\ObjectManager::getInstance.
* This way the IDE knows it's dealing with an instance of \Magento\TestFramework\ObjectManager and
* not \Magento\Framework\App\ObjectManager. The former has the method addSharedInstance, the latter does not.
*
* @return ObjectManager|\Magento\Framework\App\ObjectManager
* @SuppressWarnings(PHPMD.StaticAccess)
*/
private function getTestFrameworkObjectManager()
{
return ObjectManager::getInstance();
}

/**
* @before
*/
public function setupLoggerTestDouble(): void
{
$loggerTestDouble = $this->createMock(LoggerInterface::class);
$this->getTestFrameworkObjectManager()->addSharedInstance($loggerTestDouble, MagentoMonologLogger::class);
}

/**
* @after
*/
public function removeLoggerTestDouble(): void
{
$this->getTestFrameworkObjectManager()->removeSharedInstance(MagentoMonologLogger::class);
}

/**
* @param \RuntimeException $testException
* @return Generator|MockObject
*/
private function createExceptionThrowingGeneratorTestDouble(\RuntimeException $testException)
{
/** @var Generator|MockObject $generatorStub */
$generatorStub = $this->createMock(Generator::class);
$generatorStub->method('generateClass')->willThrowException($testException);

return $generatorStub;
}

public function testLogsExceptionDuringGeneration(): void
{
$exceptionMessage = 'Test exception thrown during generation';
$testException = new \RuntimeException($exceptionMessage);

$loggerMock = ObjectManager::getInstance()->get(LoggerInterface::class);
$loggerMock->expects($this->once())->method('debug')->with($exceptionMessage, ['exception' => $testException]);

$autoloader = new Autoloader($this->createExceptionThrowingGeneratorTestDouble($testException));
$this->assertNull($autoloader->load(NonExistingClassName::class));
}

public function testFiltersDuplicateExceptionMessages(): void
{
$exceptionMessage = 'Test exception thrown during generation';
$testException = new \RuntimeException($exceptionMessage);

$loggerMock = ObjectManager::getInstance()->get(LoggerInterface::class);
$loggerMock->expects($this->once())->method('debug')->with($exceptionMessage, ['exception' => $testException]);

$autoloader = new Autoloader($this->createExceptionThrowingGeneratorTestDouble($testException));
$autoloader->load(OneNonExistingClassName::class);
$autoloader->load(AnotherNonExistingClassName::class);
}
}
70 changes: 61 additions & 9 deletions lib/internal/Magento/Framework/Code/Generator/Autoloader.php
Original file line number Diff line number Diff line change
Expand Up @@ -3,37 +3,89 @@
* Copyright © Magento, Inc. All rights reserved.
* See COPYING.txt for license details.
*/

namespace Magento\Framework\Code\Generator;

use Magento\Framework\App\ObjectManager;
use Magento\Framework\Code\Generator;
use Psr\Log\LoggerInterface;

class Autoloader
{
/**
* @var \Magento\Framework\Code\Generator
* @var Generator
*/
protected $_generator;

/**
* @param \Magento\Framework\Code\Generator $generator
* Enables guarding against spamming the debug log with duplicate messages, as
* the generation exception will be thrown multiple times within a single request.
*
* @var string
*/
private $lastGenerationErrorMessage;

/**
* @param Generator $generator
*/
public function __construct(
\Magento\Framework\Code\Generator $generator
) {
public function __construct(Generator $generator)
{
$this->_generator = $generator;
}

/**
* Load specified class name and generate it if necessary
*
* According to PSR-4 section 2.4 an autoloader MUST NOT throw an exception and SHOULD NOT return a value.
*
* @see https://www.php-fig.org/psr/psr-4/
*
* @param string $className
* @return bool True if class was loaded
* @return void
*/
public function load($className)
{
if (!class_exists($className)) {
return Generator::GENERATION_ERROR != $this->_generator->generateClass($className);
if (! class_exists($className)) {
try {
$this->_generator->generateClass($className);
} catch (\Exception $exception) {
$this->tryToLogExceptionMessageIfNotDuplicate($exception);
}
}
}

/**
* @param \Exception $exception
*/
private function tryToLogExceptionMessageIfNotDuplicate(\Exception $exception): void
{
if ($this->lastGenerationErrorMessage !== $exception->getMessage()) {
$this->lastGenerationErrorMessage = $exception->getMessage();
$this->tryToLogException($exception);
}
}

/**
* Try to capture the exception message.
*
* The Autoloader is instantiated before the ObjectManager, so the LoggerInterface can not be injected.
* The Logger is instantiated in the try/catch block because ObjectManager might still not be initialized.
* In that case the exception message can not be captured.
*
* The debug level is used for logging in case class generation fails for a common class, but a custom
* autoloader is used later in the stack. A more severe log level would fill the logs with messages on production.
* The exception message now can be accessed in developer mode if debug logging is enabled.
*
* @param \Exception $exception
* @return void
*/
private function tryToLogException(\Exception $exception): void
{
try {
$logger = ObjectManager::getInstance()->get(LoggerInterface::class);
$logger->debug($exception->getMessage(), ['exception' => $exception]);
} catch (\Exception $ignoreThisException) {
// Do not take an action here, since the original exception might have been caused by logger
}
return true;
}
}

0 comments on commit cab38a2

Please sign in to comment.