From 621b626ac35b72b51d651d82e9549a8488f93eed Mon Sep 17 00:00:00 2001
From: catch <catch@35733.no-reply.drupal.org>
Date: Tue, 9 Jul 2019 15:28:32 +0100
Subject: [PATCH] Issue #2863986 by bircher, alexpott, pfrenssen,
 claudiu.cristea, Adita, dawehner, gambry, chr.fritsch, catch: Allow updating
 modules with new service dependencies

---
 .../Drupal/Core/Update/UpdateCompilerPass.php | 61 +++++++++++++
 .../Core/Update/UpdateServiceProvider.php     |  6 +-
 .../new_dependency_test.info.yml              |  8 ++
 .../new_dependency_test.install               | 51 +++++++++++
 .../new_dependency_test.services.yml          | 36 ++++++++
 .../src/InjectedService.php                   | 39 ++++++++
 .../new_dependency_test/src/Service.php       | 39 ++++++++
 .../src/ServiceWithDependency.php             | 42 +++++++++
 .../new_dependency_test_with_service.info.yml |  6 ++
 ..._dependency_test_with_service.services.yml |  3 +
 .../src/NewService.php                        | 20 +++++
 .../Update/UpdatePathNewDependencyTest.php    | 90 +++++++++++++++++++
 12 files changed, 400 insertions(+), 1 deletion(-)
 create mode 100644 core/lib/Drupal/Core/Update/UpdateCompilerPass.php
 create mode 100644 core/modules/system/tests/modules/new_dependency_test/new_dependency_test.info.yml
 create mode 100644 core/modules/system/tests/modules/new_dependency_test/new_dependency_test.install
 create mode 100644 core/modules/system/tests/modules/new_dependency_test/new_dependency_test.services.yml
 create mode 100644 core/modules/system/tests/modules/new_dependency_test/src/InjectedService.php
 create mode 100644 core/modules/system/tests/modules/new_dependency_test/src/Service.php
 create mode 100644 core/modules/system/tests/modules/new_dependency_test/src/ServiceWithDependency.php
 create mode 100644 core/modules/system/tests/modules/new_dependency_test_with_service/new_dependency_test_with_service.info.yml
 create mode 100644 core/modules/system/tests/modules/new_dependency_test_with_service/new_dependency_test_with_service.services.yml
 create mode 100644 core/modules/system/tests/modules/new_dependency_test_with_service/src/NewService.php
 create mode 100644 core/modules/system/tests/src/Functional/Update/UpdatePathNewDependencyTest.php

diff --git a/core/lib/Drupal/Core/Update/UpdateCompilerPass.php b/core/lib/Drupal/Core/Update/UpdateCompilerPass.php
new file mode 100644
index 00000000000..6ac05886d04
--- /dev/null
+++ b/core/lib/Drupal/Core/Update/UpdateCompilerPass.php
@@ -0,0 +1,61 @@
+<?php
+
+namespace Drupal\Core\Update;
+
+use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\ContainerInterface;
+use Symfony\Component\DependencyInjection\Reference;
+
+/**
+ * Removes services with unmet dependencies.
+ *
+ * Updates can install new modules that add services that existing services now
+ * depend on. This compiler pass allows the update system to work in such cases.
+ */
+class UpdateCompilerPass implements CompilerPassInterface {
+
+  /**
+   * {@inheritdoc}
+   */
+  public function process(ContainerBuilder $container) {
+    $process_aliases = FALSE;
+    // Loop over the defined services and remove any with unmet dependencies.
+    // The kernel cannot be booted if the container has such services. This
+    // allows modules to run their update hooks to enable newly added
+    // dependencies.
+    do {
+      $has_changed = FALSE;
+      foreach ($container->getDefinitions() as $key => $definition) {
+        foreach ($definition->getArguments() as $argument) {
+          if ($argument instanceof Reference) {
+            $argument_id = (string) $argument;
+            if (!$container->has($argument_id) && $argument->getInvalidBehavior() === ContainerInterface::EXCEPTION_ON_INVALID_REFERENCE) {
+              // If the container does not have the argument and would throw an
+              // exception, remove the service.
+              $container->removeDefinition($key);
+              $container->log($this, sprintf('Removed service "%s"; reason: depends on non-existent service "%s".', $key, $argument_id));
+              $has_changed = TRUE;
+              $process_aliases = TRUE;
+            }
+          }
+        }
+      }
+      // Repeat if services have been removed.
+    } while ($has_changed);
+
+    // Remove aliases to services that have been removed. This does not need to
+    // be part of the loop above because references to aliases have already been
+    // resolved by Symfony's ResolveReferencesToAliasesPass.
+    if ($process_aliases) {
+      foreach ($container->getAliases() as $key => $alias) {
+        $id = (string) $alias;
+        if (!$container->has($id)) {
+          $container->removeAlias($key);
+          $container->log($this, sprintf('Removed alias "%s"; reason: alias to non-existent service "%s".', $key, $id));
+        }
+      }
+    }
+  }
+
+}
diff --git a/core/lib/Drupal/Core/Update/UpdateServiceProvider.php b/core/lib/Drupal/Core/Update/UpdateServiceProvider.php
index 1971ef260ab..22c9131eb38 100644
--- a/core/lib/Drupal/Core/Update/UpdateServiceProvider.php
+++ b/core/lib/Drupal/Core/Update/UpdateServiceProvider.php
@@ -5,11 +5,12 @@ namespace Drupal\Core\Update;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\DependencyInjection\ServiceModifierInterface;
 use Drupal\Core\DependencyInjection\ServiceProviderInterface;
+use Symfony\Component\DependencyInjection\Compiler\PassConfig;
 use Symfony\Component\DependencyInjection\Definition;
 use Symfony\Component\DependencyInjection\Reference;
 
 /**
- * Ensures for some services that they don't cache.
+ * Customises the container for running updates.
  */
 class UpdateServiceProvider implements ServiceProviderInterface, ServiceModifierInterface {
 
@@ -19,12 +20,15 @@ class UpdateServiceProvider implements ServiceProviderInterface, ServiceModifier
   public function register(ContainerBuilder $container) {
     $definition = new Definition('Drupal\Core\Cache\NullBackend', ['null']);
     $container->setDefinition('cache.null', $definition);
+
+    $container->addCompilerPass(new UpdateCompilerPass(), PassConfig::TYPE_REMOVE, 128);
   }
 
   /**
    * {@inheritdoc}
    */
   public function alter(ContainerBuilder $container) {
+    // Ensures for some services that they don't cache.
     $null_cache_service = new Reference('cache.null');
 
     $definition = $container->getDefinition('asset.resolver');
diff --git a/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.info.yml b/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.info.yml
new file mode 100644
index 00000000000..eab08261b53
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.info.yml
@@ -0,0 +1,8 @@
+name: 'New Dependency test'
+type: module
+description: 'Support module for update testing.'
+package: Testing
+version: VERSION
+core: 8.x
+dependencies:
+  - new_dependency_test_with_service
diff --git a/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.install b/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.install
new file mode 100644
index 00000000000..2ab3e6b498c
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.install
@@ -0,0 +1,51 @@
+<?php
+
+/**
+ * @file
+ * Install, update and uninstall functions for the new_dependency_test module.
+ */
+
+/**
+ * Enable the new_dependency_test_with_service module.
+ */
+function new_dependency_test_update_8001() {
+  $services = [
+    'new_dependency_test.hard_dependency',
+    'new_dependency_test.optional_dependency',
+    'new_dependency_test.recursion',
+    'new_dependency_test.alias',
+    'new_dependency_test.alias_dependency',
+    'new_dependency_test.alias2',
+    'new_dependency_test.alias_dependency2',
+  ];
+
+  // Gather the state of the services prior to installing the
+  // new_dependency_test_with_service module.
+  \Drupal::state()->set(
+    'new_dependency_test_update_8001.decorated_service',
+    \Drupal::service('new_dependency_test.another_service')->isDecorated()
+  );
+
+  \Drupal::state()->set(
+    'new_dependency_test_update_8001.decorated_service_custom_inner',
+    \Drupal::service('new_dependency_test.another_service_two')->isDecorated()
+  );
+
+  $map = [];
+  foreach ($services as $id) {
+    $map[$id] = \Drupal::hasService($id);
+  }
+  \Drupal::state()->set('new_dependency_test_update_8001.has_before_install', $map);
+
+  // During the update hooks the container is cleaned up to contain only
+  // services that have their dependencies met. Core services are available.
+  \Drupal::getContainer()->get('module_installer')->install(['new_dependency_test_with_service']);
+
+  // Gather the state of the services after installing the
+  // new_dependency_test_with_service module.
+  $map = [];
+  foreach ($services as $id) {
+    $map[$id] = \Drupal::hasService($id);
+  }
+  \Drupal::state()->set('new_dependency_test_update_8001.has_after_install', $map);
+}
diff --git a/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.services.yml b/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.services.yml
new file mode 100644
index 00000000000..a3283daea8d
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test/new_dependency_test.services.yml
@@ -0,0 +1,36 @@
+services:
+  new_dependency_test.alias2:
+    alias: new_dependency_test.alias_dependency
+  new_dependency_test.alias_dependency2:
+    class: Drupal\new_dependency_test\ServiceWithDependency
+    arguments: ['@new_dependency_test.alias2']
+  new_dependency_test.alias_dependency:
+    class: Drupal\new_dependency_test\ServiceWithDependency
+    arguments: ['@new_dependency_test.alias']
+  new_dependency_test.recursion:
+    class: Drupal\new_dependency_test\ServiceWithDependency
+    arguments: ['@new_dependency_test.hard_dependency']
+  new_dependency_test.alias:
+    alias: new_dependency_test.dependent
+  new_dependency_test.dependent:
+    class: Drupal\new_dependency_test\InjectedService
+    arguments: ['@new_dependency_test_with_service.service']
+  new_dependency_test.hard_dependency:
+    class: Drupal\new_dependency_test\ServiceWithDependency
+    arguments: ['@new_dependency_test.dependent']
+  new_dependency_test.optional_dependency:
+    class: Drupal\new_dependency_test\ServiceWithDependency
+    arguments: ['@?new_dependency_test.dependent']
+  new_dependency_test.another_service:
+    class: Drupal\new_dependency_test\Service
+  new_dependency_test.another_service.decorated:
+    class: Drupal\new_dependency_test\Service
+    decorates: new_dependency_test.another_service
+    arguments: ['@new_dependency_test.another_service.decorated.inner']
+  new_dependency_test.another_service_two:
+    class: Drupal\new_dependency_test\Service
+  new_dependency_test.another_service_two.decorated:
+    class: Drupal\new_dependency_test\Service
+    decorates: new_dependency_test.another_service_two
+    decoration_inner_name: new_dependency_test.foo
+    arguments: ['@new_dependency_test.foo']
diff --git a/core/modules/system/tests/modules/new_dependency_test/src/InjectedService.php b/core/modules/system/tests/modules/new_dependency_test/src/InjectedService.php
new file mode 100644
index 00000000000..7f0b68bfaf4
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test/src/InjectedService.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace Drupal\new_dependency_test;
+
+use Drupal\new_dependency_test_with_service\NewService;
+
+/**
+ * Generic service with a dependency on a service defined in a new module.
+ */
+class InjectedService {
+
+  /**
+   * The injected service.
+   *
+   * @var \Drupal\new_dependency_test_with_service\NewService
+   */
+  protected $service;
+
+  /**
+   * InjectedService constructor.
+   *
+   * @param \Drupal\new_dependency_test_with_service\NewService $service
+   *   The service of the new module.
+   */
+  public function __construct(NewService $service) {
+    $this->service = $service;
+  }
+
+  /**
+   * Get the simple greeting from the service.
+   *
+   * @return string
+   *   The greeting.
+   */
+  public function greet() {
+    return $this->service->greet();
+  }
+
+}
diff --git a/core/modules/system/tests/modules/new_dependency_test/src/Service.php b/core/modules/system/tests/modules/new_dependency_test/src/Service.php
new file mode 100644
index 00000000000..413226ea4e0
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test/src/Service.php
@@ -0,0 +1,39 @@
+<?php
+
+namespace Drupal\new_dependency_test;
+
+/**
+ * A service that can decorated itself.
+ *
+ * @see new_dependency_test.services.yml
+ */
+class Service {
+
+  /**
+   * The decorated service.
+   *
+   * @var \Drupal\new_dependency_test\Service
+   */
+  protected $inner;
+
+  /**
+   * Service constructor.
+   *
+   * @param \Drupal\new_dependency_test\Service|null $inner
+   *   The service to decorate.
+   */
+  public function __construct(Service $inner = NULL) {
+    $this->inner = $inner;
+  }
+
+  /**
+   * Determines if the service is decorated.
+   *
+   * @return bool
+   *   TRUE if the services is decorated, FALSE if not.
+   */
+  public function isDecorated() {
+    return isset($this->inner);
+  }
+
+}
diff --git a/core/modules/system/tests/modules/new_dependency_test/src/ServiceWithDependency.php b/core/modules/system/tests/modules/new_dependency_test/src/ServiceWithDependency.php
new file mode 100644
index 00000000000..c5c57d500e7
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test/src/ServiceWithDependency.php
@@ -0,0 +1,42 @@
+<?php
+
+namespace Drupal\new_dependency_test;
+
+/**
+ * Service that gets the other service of the same module injected.
+ *
+ * This service indirectly depends on a not-yet-defined service.
+ */
+class ServiceWithDependency {
+
+  /**
+   * The injected service.
+   *
+   * @var \Drupal\new_dependency_test\InjectedService
+   */
+  protected $service;
+
+  /**
+   * ServiceWithDependency constructor.
+   *
+   * @param \Drupal\new_dependency_test\InjectedService|null $service
+   *   The service of the same module which has the new dependency.
+   */
+  public function __construct(InjectedService $service = NULL) {
+    $this->service = $service;
+  }
+
+  /**
+   * Gets a greeting from the injected service and adds to it.
+   *
+   * @return string
+   *   The greeting.
+   */
+  public function greet() {
+    if (isset($this->service)) {
+      return $this->service->greet() . ' World';
+    }
+    return 'Sorry, no service.';
+  }
+
+}
diff --git a/core/modules/system/tests/modules/new_dependency_test_with_service/new_dependency_test_with_service.info.yml b/core/modules/system/tests/modules/new_dependency_test_with_service/new_dependency_test_with_service.info.yml
new file mode 100644
index 00000000000..5091a7f79d6
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test_with_service/new_dependency_test_with_service.info.yml
@@ -0,0 +1,6 @@
+name: 'New Dependency test with service'
+type: module
+description: 'Support module for update testing.'
+package: Testing
+version: VERSION
+core: 8.x
diff --git a/core/modules/system/tests/modules/new_dependency_test_with_service/new_dependency_test_with_service.services.yml b/core/modules/system/tests/modules/new_dependency_test_with_service/new_dependency_test_with_service.services.yml
new file mode 100644
index 00000000000..26ba284e29b
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test_with_service/new_dependency_test_with_service.services.yml
@@ -0,0 +1,3 @@
+services:
+  new_dependency_test_with_service.service:
+    class: Drupal\new_dependency_test_with_service\NewService
diff --git a/core/modules/system/tests/modules/new_dependency_test_with_service/src/NewService.php b/core/modules/system/tests/modules/new_dependency_test_with_service/src/NewService.php
new file mode 100644
index 00000000000..6137fc2ecf7
--- /dev/null
+++ b/core/modules/system/tests/modules/new_dependency_test_with_service/src/NewService.php
@@ -0,0 +1,20 @@
+<?php
+
+namespace Drupal\new_dependency_test_with_service;
+
+/**
+ * Generic service returning a greeting.
+ */
+class NewService {
+
+  /**
+   * Get a simple greeting.
+   *
+   * @return string
+   *   The greeting provided by the new service.
+   */
+  public function greet() {
+    return 'Hello';
+  }
+
+}
diff --git a/core/modules/system/tests/src/Functional/Update/UpdatePathNewDependencyTest.php b/core/modules/system/tests/src/Functional/Update/UpdatePathNewDependencyTest.php
new file mode 100644
index 00000000000..57a53278c43
--- /dev/null
+++ b/core/modules/system/tests/src/Functional/Update/UpdatePathNewDependencyTest.php
@@ -0,0 +1,90 @@
+<?php
+
+namespace Drupal\Tests\system\Functional\Update;
+
+use Drupal\FunctionalTests\Update\UpdatePathTestBase;
+use Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException;
+
+/**
+ * Modules can introduce new dependencies and enable them in update hooks.
+ *
+ * @group system
+ * @group legacy
+ */
+class UpdatePathNewDependencyTest extends UpdatePathTestBase {
+
+  /**
+   * {@inheritdoc}
+   */
+  protected function setDatabaseDumpFiles() {
+    $this->databaseDumpFiles = [
+      __DIR__ . '/../../../../tests/fixtures/update/drupal-8.6.0.bare.testing.php.gz',
+    ];
+  }
+
+  /**
+   * Test that a module can add services that depend on new modules.
+   */
+  public function testUpdateNewDependency() {
+    // The new_dependency_test before the update is just an empty info.yml file.
+    // The code of the new_dependency_test module is after the update and
+    // contains the dependency on the new_dependency_test_with_service module.
+    $extension_config = $this->container->get('config.factory')->getEditable('core.extension');
+    $extension_config
+      ->set('module.new_dependency_test', 0)
+      ->set('module', module_config_sort($extension_config->get('module')))
+      ->save(TRUE);
+    drupal_set_installed_schema_version('new_dependency_test', \Drupal::CORE_MINIMUM_SCHEMA_VERSION);
+
+    // Rebuild the container and test that the service with the optional unmet
+    // dependency is still available while the ones that fail are not.
+    try {
+      $this->rebuildContainer();
+      $this->fail('The container has services with unmet dependencies and should have failed to rebuild.');
+    }
+    catch (ServiceNotFoundException $exception) {
+      $this->assertEquals('The service "new_dependency_test.dependent" has a dependency on a non-existent service "new_dependency_test_with_service.service".', $exception->getMessage());
+    }
+
+    // Running the updates enables the dependency.
+    $this->runUpdates();
+
+    $this->assertTrue(array_key_exists('new_dependency_test', $this->container->get('config.factory')->get('core.extension')->get('module')));
+    $this->assertTrue(array_key_exists('new_dependency_test_with_service', $this->container->get('config.factory')->get('core.extension')->get('module')));
+
+    // Tests that the new services are available and working as expected.
+    $this->assertEquals('Hello', $this->container->get('new_dependency_test_with_service.service')->greet());
+    $this->assertEquals('Hello', $this->container->get('new_dependency_test.dependent')->greet());
+    $this->assertEquals('Hello', $this->container->get('new_dependency_test.alias')->greet());
+    $this->assertEquals('Hello World', $this->container->get('new_dependency_test.hard_dependency')->greet());
+    $this->assertEquals('Hello World', $this->container->get('new_dependency_test.optional_dependency')->greet());
+
+    // Tests that existing decorated services work as expected during update.
+    $this->assertTrue(\Drupal::state()->get('new_dependency_test_update_8001.decorated_service'), 'The new_dependency_test.another_service service is decorated');
+    $this->assertTrue(\Drupal::state()->get('new_dependency_test_update_8001.decorated_service_custom_inner'), 'The new_dependency_test.another_service_two service is decorated');
+
+    // Tests that services are available as expected.
+    $before_install = \Drupal::state()->get('new_dependency_test_update_8001.has_before_install', []);
+    $this->assertSame([
+      'new_dependency_test.hard_dependency' => FALSE,
+      'new_dependency_test.optional_dependency' => TRUE,
+      'new_dependency_test.recursion' => FALSE,
+      'new_dependency_test.alias' => FALSE,
+      'new_dependency_test.alias_dependency' => FALSE,
+      'new_dependency_test.alias2' => FALSE,
+      'new_dependency_test.alias_dependency2' => FALSE,
+    ], $before_install);
+
+    $after_install = \Drupal::state()->get('new_dependency_test_update_8001.has_after_install', []);
+    $this->assertSame([
+      'new_dependency_test.hard_dependency' => TRUE,
+      'new_dependency_test.optional_dependency' => TRUE,
+      'new_dependency_test.recursion' => TRUE,
+      'new_dependency_test.alias' => TRUE,
+      'new_dependency_test.alias_dependency' => TRUE,
+      'new_dependency_test.alias2' => TRUE,
+      'new_dependency_test.alias_dependency2' => TRUE,
+    ], $after_install);
+  }
+
+}