From 03fb9c7d13f8b18a78637d22a06fe7e26c425e6c Mon Sep 17 00:00:00 2001 From: Fabian Franz Date: Thu, 15 Nov 2012 06:03:51 +0100 Subject: [PATCH 1/3] Issue #1708692 by Fabianx, Berdir, xjm, tim.plunkett, effulgentsia, fubhy: Fixed Simpletest slow and other problems following Bundle services aren't available in the request that enables the module. --- core/lib/Drupal/Core/DrupalKernel.php | 43 +++++++++++++++++++++++++++++--- 1 files changed, 39 insertions(+), 4 deletions(-) diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 072e109..7063be8 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -49,6 +49,15 @@ class DrupalKernel extends Kernel implements DrupalKernelInterface { protected $newModuleList; /** + * Holds the information if the moduleList changed + * + * @var boolean + * Container is rebuild if this var is TRUE during boot() + * and $this->newModuleList is set. + */ + protected $moduleListChanged; + + /** * An array of module data objects. * * The data objects have the same data structure as returned by @@ -203,11 +212,36 @@ public function updateModules(array $module_list, array $module_paths = array()) // If we haven't yet booted, we don't need to do anything: the new module // list will take effect when boot() is called. If we have already booted, // then reboot in order to refresh the bundle list and container. + $this->moduleListChanged = isset($this->newModuleList) && isset($this->moduleList) && $this->getBundleClasses($this->moduleList) !== $this->getBundleClasses($this->newModuleList); + if ($this->booted) { - drupal_container(NULL, TRUE); - $this->booted = FALSE; - $this->boot(); + if ($this->moduleListChanged) { + drupal_container(NULL, TRUE); + $this->booted = FALSE; + $this->boot(); + } + } + } + + /** + * Retrieves the name of bundle classes for a given list of modules. + * + * @param array $module_list + * An associative array whose keys and values are the names of modules. + * + * @return array + * An array whose values are bundle class names. + */ + protected function getBundleClasses($module_list) { + $bundle_classes = array(); + foreach ($module_list as $module) { + $camelized = ContainerBuilder::camelize($module); + $class = "Drupal\\{$module}\\{$camelized}Bundle"; + if (class_exists($class)) { + $bundle_classes[] = $class; + } } + return $bundle_classes; } /** @@ -249,8 +283,9 @@ protected function initializeContainer() { } // First check whether the list of modules changed in this request. if (isset($this->newModuleList)) { - if (isset($this->container) && isset($this->moduleList) && array_keys($this->moduleList) !== array_keys($this->newModuleList)) { + if ($this->moduleListChanged) { unset($this->container); + $this->moduleListChanged = FALSE; } $this->moduleList = $this->newModuleList; unset($this->newModuleList); -- 1.7.4.1 From e9a321b9fc8b029973293aacfa845a4d05e39eff Mon Sep 17 00:00:00 2001 From: Fabian Franz Date: Thu, 15 Nov 2012 18:50:41 +0100 Subject: [PATCH 2/3] Issue #1784312 by katbailey, msonnabaum: Stop doing so much pre-kernel bootstrapping. --- core/includes/bootstrap.inc | 11 ++--- core/lib/Drupal/Core/CoreBundle.php | 44 +++++++++++++++++++++ core/lib/Drupal/Core/DrupalKernel.php | 67 +++++++++++++++++++++++++-------- index.php | 2 +- 4 files changed, 100 insertions(+), 24 deletions(-) diff --git a/core/includes/bootstrap.inc b/core/includes/bootstrap.inc index 1eb63b6..f96d1a4 100644 --- a/core/includes/bootstrap.inc +++ b/core/includes/bootstrap.inc @@ -2443,10 +2443,9 @@ function drupal_container(Container $new_container = NULL, $rebuild = FALSE) { $container = $new_container; } if (!isset($container)) { - // Return a ContainerBuilder instance with the bare essentials needed for any - // full bootstrap regardless of whether there will be a DrupalKernel involved. - // This will get merged with the full Kernel-built Container on normal page - // requests. + // This is only ever used by the installer and by run-tests.sh. + // @todo Remove this entire section once these have been converted to use a + // kernel. $container = new ContainerBuilder(); // Register active configuration storage. @@ -2704,11 +2703,9 @@ function language($type, $reset = FALSE) { // When the language_manager service exists (is both defined and the 'request' // scope is active in the container), use it to get the language. Otherwise // return the default language. - try { + if (drupal_container()->isScopeActive('request')) { $language_manager = drupal_container()->get('language_manager', Container::NULL_ON_INVALID_REFERENCE); } - catch (DependencyInjectionRuntimeException $e) { - } if (isset($language_manager)) { return $language_manager->getLanguage($type); diff --git a/core/lib/Drupal/Core/CoreBundle.php b/core/lib/Drupal/Core/CoreBundle.php index bbc4e2e..fffb126 100644 --- a/core/lib/Drupal/Core/CoreBundle.php +++ b/core/lib/Drupal/Core/CoreBundle.php @@ -32,6 +32,50 @@ class CoreBundle extends Bundle public function build(ContainerBuilder $container) { + // Register active configuration storage. + $container + ->register('config.cachedstorage.storage', 'Drupal\Core\Config\FileStorage') + ->addArgument(config_get_config_directory(CONFIG_ACTIVE_DIRECTORY)); + // @todo Replace this with a cache.factory service plus 'config' argument. + $container + ->register('cache.config', 'Drupal\Core\Cache\CacheBackendInterface') + ->setFactoryClass('Drupal\Core\Cache\CacheFactory') + ->setFactoryMethod('get') + ->addArgument('config'); + + $container + ->register('config.storage', 'Drupal\Core\Config\CachedStorage') + ->addArgument(new Reference('config.cachedstorage.storage')) + ->addArgument(new Reference('cache.config')); + + // Register configuration object factory. + $container->register('config.subscriber.globalconf', 'Drupal\Core\EventSubscriber\ConfigGlobalOverrideSubscriber'); + $container->register('dispatcher', 'Symfony\Component\EventDispatcher\EventDispatcher') + ->addMethodCall('addSubscriber', array(new Reference('config.subscriber.globalconf'))); + $container->register('config.factory', 'Drupal\Core\Config\ConfigFactory') + ->addArgument(new Reference('config.storage')) + ->addArgument(new Reference('dispatcher')); + + // Register staging configuration storage. + $container + ->register('config.storage.staging', 'Drupal\Core\Config\FileStorage') + ->addArgument(config_get_config_directory(CONFIG_STAGING_DIRECTORY)); + + // Register the service for the default database connection. + $container->register('database', 'Drupal\Core\Database\Connection') + ->setFactoryClass('Drupal\Core\Database\Database') + ->setFactoryMethod('getConnection') + ->addArgument('default'); + // Register the KeyValueStore factory. + $container + ->register('keyvalue', 'Drupal\Core\KeyValueStore\KeyValueFactory') + ->addArgument(new Reference('service_container')); + $container + ->register('keyvalue.database', 'Drupal\Core\KeyValueStore\KeyValueDatabaseFactory') + ->addArgument(new Reference('database')); + + // Register the EntityManager. + $container->register('plugin.manager.entity', 'Drupal\Core\Entity\EntityManager'); // The 'request' scope and service enable services to depend on the Request // object and get reconstructed when the request object changes (e.g., // during a subrequest). diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 7063be8..3c2ef33 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -68,6 +68,20 @@ class DrupalKernel extends Kernel implements DrupalKernelInterface { protected $moduleData = array(); /** + * Holds a list of enabled modules and their paths. + * + * This is used to store module data as a container parameter so that it can + * be retrieved for registering namespaces when using a compiled container. + * When not using a compiled container, the namespaces get registered during + * the process of building the container. + * + * @var array + * An associative array whose keys are module names and whose values are + * module paths. + */ + protected $modulePaths = array(); + + /** * PHP code storage object to use for the compiled container. * * @var \Drupal\Component\PhpStorage\PhpStorageInterface @@ -89,6 +103,13 @@ class DrupalKernel extends Kernel implements DrupalKernelInterface { protected $bundleClasses; /** + * Whether the container needs to be dumped once booting is complete. + * + * @var boolean + */ + protected $containerNeedsDumping; + + /** * Constructs a DrupalKernel object. * * @param string $environment @@ -134,6 +155,14 @@ public function boot() { } $this->initializeContainer(); $this->booted = TRUE; + // @todo Remove this once everything in the bootstrap has been converted to + // services in the DIC. + drupal_bootstrap(DRUPAL_BOOTSTRAP_CODE); + // Now that full bootstrap is complete, we can dump the container if it + // was just rebuilt. + if ($this->containerNeedsDumping && !$this->dumpDrupalContainer($this->container, $this->getContainerBaseClass())) { + watchdog('DrupalKernel', 'Container cannot be written to disk'); + } } /** @@ -150,12 +179,20 @@ public function registerBundles() { } $namespaces = $this->classLoader->getNamespaces(); + // We will need to store module locations in a container parameter so that + // we can register all namespaces when using a compiled container. + $this->modulePaths = array(); foreach ($this->moduleList as $module => $weight) { // When installing new modules, the modules in the list passed to // updateModules() do not yet have their namespace registered. $namespace = 'Drupal\\' . $module; if (!isset($namespaces[$namespace]) && $this->moduleData($module)) { - $this->classLoader->registerNamespace($namespace, dirname(DRUPAL_ROOT . '/' . $this->moduleData($module)->uri) . '/lib'); + $path = dirname(DRUPAL_ROOT . '/' . $this->moduleData($module)->uri) . '/lib'; + $this->modulePaths[$module] = $path; + $this->classLoader->registerNamespace($namespace, $path); + } + else { + $this->modulePaths[$module] = $namespaces[$namespace]; } $camelized = ContainerBuilder::camelize($module); $class = "Drupal\\{$module}\\{$camelized}Bundle"; @@ -294,28 +331,31 @@ protected function initializeContainer() { // web frontend or during the installer -- changed the list of enabled // modules. if (isset($this->container)) { + // All namespaces must be registered before we attempt to use any service + // from the container. + $namespaces = $this->classLoader->getNamespaces(); + foreach ($this->container->getParameter('container.modules') as $module => $path) { + $namespace = 'Drupal\\' . $module; + if (!isset($namespaces[$namespace])) { + $this->classLoader->registerNamespace($namespace, $path); + } + } $module_list = $this->moduleList ?: $this->container->get('config.factory')->get('system.module')->load()->get('enabled'); - if (array_keys((array)$module_list) !== $this->container->getParameter('container.modules')) { + if (array_keys((array)$module_list) !== array_keys($this->container->getParameter('container.modules'))) { unset($this->container); } } if (!isset($this->container)) { $this->container = $this->buildContainer(); - if ($this->storage && !$this->dumpDrupalContainer($this->container, $this->getContainerBaseClass())) { - // We want to log this as an error but we cannot call watchdog() until - // the container has been fully built and set in drupal_container(). - $error = 'Container cannot be written to disk'; + if ($this->storage) { + $this->containerNeedsDumping = TRUE; } } $this->container->set('kernel', $this); drupal_container($this->container); - - if (isset($error)) { - watchdog('DrupalKernel', $error); - } } /** @@ -327,12 +367,7 @@ protected function buildContainer() { $this->initializeBundles(); $container = $this->getContainerBuilder(); $container->setParameter('container.bundles', $this->bundleClasses); - $container->setParameter('container.modules', array_keys($this->moduleList)); - - // Merge in the minimal bootstrap container. - if ($bootstrap_container = drupal_container()) { - $container->merge($bootstrap_container); - } + $container->setParameter('container.modules', $this->modulePaths); foreach ($this->bundles as $bundle) { $bundle->build($container); } diff --git a/index.php b/index.php index 4d56290..99f6c1f 100644 --- a/index.php +++ b/index.php @@ -25,7 +25,7 @@ // @see Drupal\Core\EventSubscriber\PathSubscriber; // @see Drupal\Core\EventSubscriber\LegacyRequestSubscriber; require_once DRUPAL_ROOT . '/core/includes/bootstrap.inc'; -drupal_bootstrap(DRUPAL_BOOTSTRAP_CODE); +drupal_bootstrap(DRUPAL_BOOTSTRAP_CONFIGURATION); // @todo Figure out how best to handle the Kernel constructor parameters. $kernel = new DrupalKernel('prod', FALSE, drupal_classloader(), drupal_php_storage('service_container')); -- 1.7.4.1 From b3ee243201902956b5fe0eef64bb440618cfac8f Mon Sep 17 00:00:00 2001 From: Fabian Franz Date: Thu, 15 Nov 2012 19:04:34 +0100 Subject: [PATCH 3/3] Do not modify the container as that is not needed or helpful. --- core/lib/Drupal/Core/DrupalKernel.php | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/core/lib/Drupal/Core/DrupalKernel.php b/core/lib/Drupal/Core/DrupalKernel.php index 3c2ef33..0aabb89 100644 --- a/core/lib/Drupal/Core/DrupalKernel.php +++ b/core/lib/Drupal/Core/DrupalKernel.php @@ -253,7 +253,7 @@ public function updateModules(array $module_list, array $module_paths = array()) if ($this->booted) { if ($this->moduleListChanged) { - drupal_container(NULL, TRUE); + //drupal_container(NULL, TRUE); $this->booted = FALSE; $this->boot(); } -- 1.7.4.1