From 2fef2c7c488fa4db6323683a94849bd36cc4eaf8 Mon Sep 17 00:00:00 2001 From: Kevin Dutra Date: Tue, 31 Oct 2017 14:54:05 -0700 Subject: [PATCH 1/5] #3123 - Global D8 container is not available during container rebuild. --- src/Drupal/DrupalKernel.php | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/Drupal/DrupalKernel.php b/src/Drupal/DrupalKernel.php index 31c195fef6..3911c4c20b 100644 --- a/src/Drupal/DrupalKernel.php +++ b/src/Drupal/DrupalKernel.php @@ -45,6 +45,18 @@ protected function initializeContainer() $container_definition = $this->getCachedContainerDefinition(); if ($this->shouldDrushInvalidateContainer()) { + // Normally when the container is being rebuilt, the existing + // container is still available for use until the newly built one + // replaces it. Certain contrib modules rely on services (like State + // or the config factory) being available for things like defining + // event subscriptions. + if (isset($container_definition)) { + $class = Settings::get('container_base_class', '\Drupal\Core\DependencyInjection\Container'); + $container = new $class($container_definition); + $this->attachSynthetic($container); + \Drupal::setContainer($container); + } + $this->invalidateContainer(); } return parent::initializeContainer(); From 213b9c6a1c00b23dc3f56e9e4c399733ecba7a49 Mon Sep 17 00:00:00 2001 From: Kevin Dutra Date: Tue, 31 Oct 2017 14:58:11 -0700 Subject: [PATCH 2/5] #3123 - Coding standards adjustments. --- src/Drupal/DrupalKernel.php | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/Drupal/DrupalKernel.php b/src/Drupal/DrupalKernel.php index 3911c4c20b..c04a64e341 100644 --- a/src/Drupal/DrupalKernel.php +++ b/src/Drupal/DrupalKernel.php @@ -51,10 +51,10 @@ protected function initializeContainer() // or the config factory) being available for things like defining // event subscriptions. if (isset($container_definition)) { - $class = Settings::get('container_base_class', '\Drupal\Core\DependencyInjection\Container'); - $container = new $class($container_definition); - $this->attachSynthetic($container); - \Drupal::setContainer($container); + $class = Settings::get('container_base_class', '\Drupal\Core\DependencyInjection\Container'); + $container = new $class($container_definition); + $this->attachSynthetic($container); + \Drupal::setContainer($container); } $this->invalidateContainer(); From db810a3a317598792a6dcff6d985533a50006d9b Mon Sep 17 00:00:00 2001 From: Kevin Dutra Date: Tue, 31 Oct 2017 15:17:30 -0700 Subject: [PATCH 3/5] #3123 - Missing include statement. --- src/Drupal/DrupalKernel.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Drupal/DrupalKernel.php b/src/Drupal/DrupalKernel.php index c04a64e341..e113c3b9d0 100644 --- a/src/Drupal/DrupalKernel.php +++ b/src/Drupal/DrupalKernel.php @@ -1,6 +1,7 @@ Date: Thu, 2 Nov 2017 10:51:36 -0700 Subject: [PATCH 4/5] #3123 - Adding a test. --- tests/containerTest.php | 61 +++++++++++++++++++ .../src/EventSubscriber/ConfigSubscriber.php | 40 ++++++++++++ .../modules/d8/woot/woot.services.yml | 5 ++ 3 files changed, 106 insertions(+) create mode 100644 tests/containerTest.php create mode 100644 tests/resources/modules/d8/woot/src/EventSubscriber/ConfigSubscriber.php create mode 100644 tests/resources/modules/d8/woot/woot.services.yml diff --git a/tests/containerTest.php b/tests/containerTest.php new file mode 100644 index 0000000000..e1511259f2 --- /dev/null +++ b/tests/containerTest.php @@ -0,0 +1,61 @@ +setUpDrupal(1, TRUE); + $root = $this->webroot(); + $options = array( + 'root' => $root, + 'uri' => key($sites), + 'yes' => NULL, + ); + + // Copy the 'woot' module over to the Drupal site we just set up. + $this->setupModulesForTests($root); + + // Enable our module. + $this->drush('pm-enable', ['woot'], $options); + + // Set up for a config import with just one small piece. + $this->drush('config-export', array(), $options); + $this->drush('config-set', array('system.site', 'name', 'config_test'), $options); + + // Trigger the container rebuild we need. + $this->drush('cr', [], $options); + $this->drush('cron', [], $options); + + // If the event was registered successfully, then upon a config import, we + // should get the error message. + $this->drush('config-import', [], $options, NULL, NULL, CommandUnishTestCase::EXIT_ERROR); + $this->assertContains("woot config error", $this->getErrorOutput(), 'Event was successfully registered.'); + } + + /** + * Sets up the woot module for the test. + * + * @param string $root + * The web root. + */ + public function setupModulesForTests($root) { + $wootModule = Path::join(__DIR__, '/resources/modules/d8/woot'); + // We install into Unish so that we aren't cleaned up. That causes + // container to go invalid after tearDownAfterClass(). + $targetDir = Path::join($root, 'modules/unish/woot'); + $this->mkdir($targetDir); + $this->recursive_copy($wootModule, $targetDir); + } + +} diff --git a/tests/resources/modules/d8/woot/src/EventSubscriber/ConfigSubscriber.php b/tests/resources/modules/d8/woot/src/EventSubscriber/ConfigSubscriber.php new file mode 100644 index 0000000000..7cb5ff15ff --- /dev/null +++ b/tests/resources/modules/d8/woot/src/EventSubscriber/ConfigSubscriber.php @@ -0,0 +1,40 @@ +getConfigImporter(); + $importer->logError($this->t('woot config error')); + } + +} diff --git a/tests/resources/modules/d8/woot/woot.services.yml b/tests/resources/modules/d8/woot/woot.services.yml new file mode 100644 index 0000000000..37ccc34fef --- /dev/null +++ b/tests/resources/modules/d8/woot/woot.services.yml @@ -0,0 +1,5 @@ +services: + config.config_subscriber: + class: Drupal\woot\EventSubscriber\ConfigSubscriber + tags: + - { name: event_subscriber } From 8db18e81983b8c1f4da490a2ced1a659c84e21a0 Mon Sep 17 00:00:00 2001 From: Kevin Dutra Date: Fri, 3 Nov 2017 07:47:39 -0700 Subject: [PATCH 5/5] #3123 - Requested PR changes. --- src/Drupal/DrupalKernel.php | 1 + tests/containerTest.php | 6 ++++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/Drupal/DrupalKernel.php b/src/Drupal/DrupalKernel.php index e113c3b9d0..3b2548d3f5 100644 --- a/src/Drupal/DrupalKernel.php +++ b/src/Drupal/DrupalKernel.php @@ -51,6 +51,7 @@ protected function initializeContainer() // replaces it. Certain contrib modules rely on services (like State // or the config factory) being available for things like defining // event subscriptions. + // @see https://github.com/drush-ops/drush/issues/3123 if (isset($container_definition)) { $class = Settings::get('container_base_class', '\Drupal\Core\DependencyInjection\Container'); $container = new $class($container_definition); diff --git a/tests/containerTest.php b/tests/containerTest.php index e1511259f2..318ffde7c4 100644 --- a/tests/containerTest.php +++ b/tests/containerTest.php @@ -8,6 +8,8 @@ * Tests the Drush override of DrupalKernel. * * @group base + * + * @see https://github.com/drush-ops/drush/issues/3123 */ class containerTest extends CommandUnishTestCase { @@ -15,11 +17,11 @@ class containerTest extends CommandUnishTestCase { * Tests that the existing container is available while Drush rebuilds it. */ public function testContainer() { - $sites = $this->setUpDrupal(1, TRUE); + $this->setUpDrupal(1, TRUE); $root = $this->webroot(); $options = array( 'root' => $root, - 'uri' => key($sites), + 'uri' => $this->getUri(), 'yes' => NULL, );