Commit 82304ed9 authored by catch's avatar catch

Issue #2776235 by mikeryan, alexpott: Cached autoloader misses cause failures...

Issue #2776235 by mikeryan, alexpott: Cached autoloader misses cause failures when missed class becomes available
parent 3d7c5304
<?php
namespace Drupal\Component\ClassFinder;
use Doctrine\Common\Reflection\ClassFinderInterface;
/**
* A Utility class that uses active autoloaders to find a file for a class.
*/
class ClassFinder implements ClassFinderInterface {
/**
* {@inheritdoc}
*/
public function findFile($class) {
$loaders = spl_autoload_functions();
foreach ($loaders as $loader) {
if (is_array($loader) && isset($loader[0]) && is_object($loader[0]) && method_exists($loader[0], 'findFile')) {
$file = call_user_func_array([$loader[0], 'findFile'], [$class]);
// Different implementations return different empty values. For example,
// \Composer\Autoload\ClassLoader::findFile() returns FALSE whilst
// \Doctrine\Common\Reflection\ClassFinderInterface::findFile()
// documents that a NULL should be returned.
if (!empty($file)) {
return $file;
}
}
}
return NULL;
}
}
This diff is collapsed.
The Drupal ClassFinder Component
Thanks for using this Drupal component.
You can participate in its development on Drupal.org, through our issue system:
https://www.drupal.org/project/issues/drupal
You can get the full Drupal repo here:
https://www.drupal.org/project/drupal/git-instructions
You can browse the full Drupal repo here:
http://cgit.drupalcode.org/drupal
HOW-TO: Test this Drupal component
In order to test this component, you'll need to get the entire Drupal repo and
run the tests there.
You'll find the tests under core/tests/Drupal/Tests/Component.
You can get the full Drupal repo here:
https://www.drupal.org/project/drupal/git-instructions
You can find more information about running PHPUnit tests with Drupal here:
https://www.drupal.org/node/2116263
Each component in the Drupal\Component namespace has its own annotated test
group. You can use this group to run only the tests for this component. Like
this:
$ ./vendor/bin/phpunit -c core --group ClassFinder
{
"name": "drupal/core-class-finder",
"description": "This class provides a class finding utility.",
"keywords": ["drupal"],
"homepage": "https://www.drupal.org/project/drupal",
"license": "GPL-2.0+",
"require": {
"php": ">=5.5.9",
"doctrine/common": "2.5.*"
},
"autoload": {
"psr-4": {
"Drupal\\Component\\ClassFinder\\": ""
}
}
}
......@@ -2,6 +2,7 @@
namespace Drupal\Core;
use Composer\Autoload\ClassLoader;
use Drupal\Component\Assertion\Handle;
use Drupal\Component\FileCache\FileCacheFactory;
use Drupal\Component\Utility\Unicode;
......@@ -761,6 +762,10 @@ protected function moduleData($module) {
* needed.
*/
public function updateModules(array $module_list, array $module_filenames = array()) {
$pre_existing_module_namespaces = [];
if ($this->booted && is_array($this->moduleList)) {
$pre_existing_module_namespaces = $this->getModuleNamespacesPsr4($this->getModuleFileNames());
}
$this->moduleList = $module_list;
foreach ($module_filenames as $name => $extension) {
$this->moduleData[$name] = $extension;
......@@ -773,6 +778,20 @@ public function updateModules(array $module_list, array $module_filenames = arra
// container in order to refresh the serviceProvider list and container.
$this->containerNeedsRebuild = TRUE;
if ($this->booted) {
// We need to register any new namespaces to a new class loader because
// the current class loader might have stored a negative result for a
// class that is now available.
// @see \Composer\Autoload\ClassLoader::findFile()
$new_namespaces = array_diff_key(
$this->getModuleNamespacesPsr4($this->getModuleFileNames()),
$pre_existing_module_namespaces
);
if (!empty($new_namespaces)) {
$additional_class_loader = new ClassLoader();
$this->classLoaderAddMultiplePsr4($new_namespaces, $additional_class_loader);
$additional_class_loader->register();
}
$this->initializeContainer();
}
}
......@@ -1387,8 +1406,15 @@ protected function getModuleNamespacesPsr4($module_file_names) {
* Array where each key is a namespace like 'Drupal\system', and each value
* is either a PSR-4 base directory, or an array of PSR-4 base directories
* associated with this namespace.
* @param object $class_loader
* The class loader. Normally \Composer\Autoload\ClassLoader, as included by
* the front controller, but may also be decorated; e.g.,
* \Symfony\Component\ClassLoader\ApcClassLoader.
*/
protected function classLoaderAddMultiplePsr4(array $namespaces = array()) {
protected function classLoaderAddMultiplePsr4(array $namespaces = array(), $class_loader = NULL) {
if ($class_loader === NULL) {
$class_loader = $this->classLoader;
}
foreach ($namespaces as $prefix => $paths) {
if (is_array($paths)) {
foreach ($paths as $key => $value) {
......@@ -1398,7 +1424,7 @@ protected function classLoaderAddMultiplePsr4(array $namespaces = array()) {
elseif (is_string($paths)) {
$paths = $this->root . '/' . $paths;
}
$this->classLoader->addPsr4($prefix . '\\', $paths);
$class_loader->addPsr4($prefix . '\\', $paths);
}
}
......
......@@ -7,7 +7,7 @@ services:
arguments: [migrate]
plugin.manager.migrate.source:
class: Drupal\migrate\Plugin\MigrateSourcePluginManager
arguments: [source, '@container.namespaces', '@cache.discovery', '@module_handler', '@class_loader']
arguments: [source, '@container.namespaces', '@cache.discovery', '@module_handler']
plugin.manager.migrate.process:
class: Drupal\migrate\Plugin\MigratePluginManager
arguments: [process, '@container.namespaces', '@cache.discovery', '@module_handler', 'Drupal\migrate\Annotation\MigrateProcessPlugin']
......
......@@ -6,6 +6,7 @@
use Doctrine\Common\Reflection\StaticReflectionParser as BaseStaticReflectionParser;
use Drupal\Component\Annotation\AnnotationInterface;
use Drupal\Component\Annotation\Reflection\MockFileFinder;
use Drupal\Component\ClassFinder\ClassFinder;
use Drupal\Core\Plugin\Discovery\AnnotatedClassDiscovery;
use Drupal\migrate\Annotation\MultipleProviderAnnotationInterface;
......@@ -20,9 +21,9 @@
class AnnotatedClassDiscoveryAutomatedProviders extends AnnotatedClassDiscovery {
/**
* Any class loader with a findFile() method.
* A utility object that can use active autoloaders to find files for classes.
*
* @var \Composer\Autoload\ClassLoader
* @var \Doctrine\Common\Reflection\ClassFinderInterface
*/
protected $finder;
......@@ -41,17 +42,10 @@ class AnnotatedClassDiscoveryAutomatedProviders extends AnnotatedClassDiscovery
* Defaults to 'Drupal\Component\Annotation\Plugin'.
* @param string[] $annotation_namespaces
* Additional namespaces to scan for annotation definitions.
* @param $class_loader
* The class loader already knows where to find the classes so it is reused
* as the class finder.
*/
public function __construct($subdir, \Traversable $root_namespaces, $plugin_definition_annotation_name = 'Drupal\Component\Annotation\Plugin', array $annotation_namespaces = [], $class_loader) {
public function __construct($subdir, \Traversable $root_namespaces, $plugin_definition_annotation_name = 'Drupal\Component\Annotation\Plugin', array $annotation_namespaces = []) {
parent::__construct($subdir, $root_namespaces, $plugin_definition_annotation_name, $annotation_namespaces);
if (!method_exists($class_loader, 'findFile')) {
throw new \LogicException(sprintf('Class loader (%s) must implement findFile() method', get_class($class_loader)));
}
$this->finder = $class_loader;
$this->finder = new ClassFinder();
}
......
......@@ -40,12 +40,9 @@ class MigrateSourcePluginManager extends MigratePluginManager {
* Cache backend instance to use.
* @param \Drupal\Core\Extension\ModuleHandlerInterface $module_handler
* The module handler to invoke the alter hook with.
* @param object $class_loader
* The class loader.
*/
public function __construct($type, \Traversable $namespaces, CacheBackendInterface $cache_backend, ModuleHandlerInterface $module_handler, $class_loader) {
public function __construct($type, \Traversable $namespaces, CacheBackendInterface $cache_backend, ModuleHandlerInterface $module_handler) {
parent::__construct($type, $namespaces, $cache_backend, $module_handler, 'Drupal\migrate\Annotation\MigrateSource');
$this->classLoader = $class_loader;
}
/**
......@@ -53,7 +50,7 @@ public function __construct($type, \Traversable $namespaces, CacheBackendInterfa
*/
protected function getDiscovery() {
if (!$this->discovery) {
$discovery = new AnnotatedClassDiscoveryAutomatedProviders($this->subdir, $this->namespaces, $this->pluginDefinitionAnnotationName, $this->additionalAnnotationNamespaces, $this->classLoader);
$discovery = new AnnotatedClassDiscoveryAutomatedProviders($this->subdir, $this->namespaces, $this->pluginDefinitionAnnotationName, $this->additionalAnnotationNamespaces);
$this->discovery = new ContainerDerivativeDiscoveryDecorator($discovery);
}
return $this->discovery;
......
......@@ -70,4 +70,18 @@ function testClassLoadingDisabledModules() {
}
}
/**
* Ensures the negative caches in the class loader don't result in crashes.
*/
public function testMultipleModules() {
$this->drupalLogin($this->rootUser);
$edit = [
"modules[Testing][module_install_class_loader_test1][enable]" => TRUE,
"modules[Testing][module_install_class_loader_test2][enable]" => TRUE,
];
$this->drupalPostForm('admin/modules', $edit, t('Install'));
$this->rebuildContainer();
$this->assertTrue(\Drupal::moduleHandler()->moduleExists('module_install_class_loader_test2'), 'The module_install_class_loader_test2 module has been installed.');
}
}
name: 'Module install class loader test1'
description: 'Support module for tests that the class loader behaves as expected during module install.'
type: module
package: Testing
core: 8.x
version: VERSION
services:
module_install_class_loader_test1.event_subscriber:
class: Drupal\module_install_class_loader_test1\EventSubscriber
tags:
- { name: event_subscriber }
<?php
namespace Drupal\module_install_class_loader_test1;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
/**
* An event subscriber that does different things depending on whether classes
* exist.
*/
class EventSubscriber implements EventSubscriberInterface {
/**
* {@inheritdoc}
*/
public static function getSubscribedEvents() {
$events = [];
// If the autoloader is not fixed during module install when the modules
// module_install_class_loader_test1 and module_install_class_loader_test2
// are enabled in the same request the class_exists() will cause a crash.
// This is because \Composer\Autoload\ClassLoader maintains a negative
// cache.
if (class_exists('\Drupal\module_install_class_loader_test2\EventSubscriber')) {
$events = [];
}
return $events;
}
}
name: 'Module install class loader test2'
description: 'Support module for tests that the class loader behaves as expected during module install.'
type: module
package: Testing
core: 8.x
version: VERSION
dependencies:
- module_install_class_loader_test1
services:
module_install_class_loader_test1.event_subscriber:
class: Drupal\module_install_class_loader_test2\EventSubscriber
tags:
- { name: event_subscriber }
<?php
namespace Drupal\module_install_class_loader_test2;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;
/**
* An event subscriber that does nothing.
*/
class EventSubscriber implements EventSubscriberInterface {
/**
* {@inheritdoc}
*/
public static function getSubscribedEvents() {
return [];
}
}
<?php
namespace Drupal\Tests\Component\ClassFinder;
use Composer\Autoload\ClassLoader;
use Drupal\Component\ClassFinder\ClassFinder;
use Drupal\Tests\UnitTestCase;
/**
* @coversDefaultClass \Drupal\Component\ClassFinder\ClassFinder
* @group ClassFinder
*/
class ClassFinderTest extends UnitTestCase {
/**
* @covers ::findFile
*/
public function testFindFile() {
$finder = new ClassFinder();
// The full path is returned therefore only tests with
// assertStringEndsWith() so the test is portable.
$this->assertStringEndsWith('core/tests/Drupal/Tests/UnitTestCase.php', $finder->findFile(UnitTestCase::class));
$class = 'Not\\A\\Class';
$this->assertNull($finder->findFile($class));
// Register an autoloader that can find this class.
$loader = new ClassLoader();
$loader->addClassMap([$class => __FILE__]);
$loader->register();
$this->assertEquals(__FILE__, $finder->findFile($class));
// This shouldn't prevent us from finding the original file.
$this->assertStringEndsWith('core/tests/Drupal/Tests/UnitTestCase.php', $finder->findFile(UnitTestCase::class));
// Clean up the additional autoloader after the test.
$loader->unregister();
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment