Issue #2589967 by alexpott, dawehner, damiankloip, valthebald: Rebuild routes...

Issue #2589967 by alexpott, dawehner, damiankloip, valthebald: Rebuild routes immediately when modules are installed

core/core.services.yml

@@ -473,7 +473,7 @@ services:
     class: Drupal\Core\Extension\ModuleInstaller
     tags:
       - { name: service_collector, tag: 'module_install.uninstall_validator', call: addUninstallValidator }
-    arguments: ['@app.root', '@module_handler', '@kernel']
+    arguments: ['@app.root', '@module_handler', '@kernel', '@router.builder']
     lazy: true
   content_uninstall_validator:
     class: Drupal\Core\Entity\ContentUninstallValidator
@@ -763,6 +763,9 @@ services:
     tags:
       - { name: event_subscriber }
       - { name: backend_overridable }
+  router.route_provider.lazy_builder:
+    class: Drupal\Core\Routing\RouteProviderLazyBuilder
+    arguments: ['@router.route_provider', '@router.builder']
   router.route_preloader:
     class: Drupal\Core\Routing\RoutePreloader
     arguments: ['@router.route_provider', '@state', '@cache.bootstrap']

core/lib/Drupal/Core/Extension/ModuleInstaller.php

@@ -279,6 +279,10 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
         // @see https://www.drupal.org/node/2208429
         \Drupal::service('theme_handler')->refreshInfo();
 
+        // In order to make uninstalling transactional if anything uses routes.
+        \Drupal::getContainer()->set('router.route_provider.old', \Drupal::service('router.route_provider'));
+        \Drupal::getContainer()->set('router.route_provider', \Drupal::service('router.route_provider.lazy_builder'));
+
         // Allow the module to perform install tasks.
         $this->moduleHandler->invoke($module, 'install');
 
@@ -289,7 +293,15 @@ public function install(array $module_list, $enable_dependencies = TRUE) {
 
     // If any modules were newly installed, invoke hook_modules_installed().
     if (!empty($modules_installed)) {
-      \Drupal::service('router.builder')->setRebuildNeeded();
+      \Drupal::getContainer()->set('router.route_provider', \Drupal::service('router.route_provider.old'));
+      if (!\Drupal::service('router.route_provider.lazy_builder')->hasRebuilt()) {
+        // Rebuild routes after installing module. This is done here on top of
+        // \Drupal\Core\Routing\RouteBuilder::destruct to not run into errors on
+        // fastCGI which executes ::destruct() after the module installation
+        // page was sent already.
+        \Drupal::service('router.builder')->rebuild();
+      }
+
       $this->moduleHandler->invokeAll('modules_installed', array($modules_installed));
     }
 
@@ -378,6 +390,10 @@ public function uninstall(array $module_list, $uninstall_dependents = TRUE) {
       // Remove all configuration belonging to the module.
       \Drupal::service('config.manager')->uninstall('module', $module);
 
+      // In order to make uninstalling transactional if anything uses routes.
+      \Drupal::getContainer()->set('router.route_provider.old', \Drupal::service('router.route_provider'));
+      \Drupal::getContainer()->set('router.route_provider', \Drupal::service('router.route_provider.lazy_builder'));
+
       // Notify interested components that this module's entity types are being
       // deleted. For example, a SQL-based storage handler can use this as an
       // opportunity to drop the corresponding database tables.
@@ -450,7 +466,11 @@ public function uninstall(array $module_list, $uninstall_dependents = TRUE) {
       $post_update_registry = \Drupal::service('update.post_update_registry');
       $post_update_registry->filterOutInvokedUpdatesByModule($module);
     }
-    \Drupal::service('router.builder')->setRebuildNeeded();
+    // Rebuild routes after installing module. This is done here on top of
+    // \Drupal\Core\Routing\RouteBuilder::destruct to not run into errors on
+    // fastCGI which executes ::destruct() after the Module uninstallation page
+    // was sent already.
+    \Drupal::service('router.builder')->rebuild();
     drupal_get_installed_schema_version(NULL, TRUE);
 
     // Let other modules react.

core/lib/Drupal/Core/Extension/module.api.php

@@ -198,6 +198,12 @@ function hook_modules_installed($modules) {
  * If the module implements hook_schema(), the database tables will
  * be created before this hook is fired.
  *
+ * If the module provides a MODULE.routing.yml or alters routing information
+ * these changes will not be available when this hook is fired. If up-to-date
+ * router information is required, for example to use \Drupal\Core\Url, then
+ * (preferably) use hook_modules_installed() or rebuild the router in the
+ * hook_install() implementation.
+ *
  * Implementations of this hook are by convention declared in the module's
  * .install file. The implementation can rely on the .module file being loaded.
  * The hook will only be called when a module is installed. The module's schema

core/lib/Drupal/Core/Routing/RouteProviderLazyBuilder.php

+<?php
+
+namespace Drupal\Core\Routing;
+
+use Symfony\Cmf\Component\Routing\PagedRouteProviderInterface;
+use Symfony\Component\HttpFoundation\Request;
+
+/**
+ * A Route Provider front-end for all Drupal-stored routes.
+ */
+class RouteProviderLazyBuilder implements PreloadableRouteProviderInterface, PagedRouteProviderInterface {
+
+  /**
+   * The route provider service.
+   *
+   * @var \Drupal\Core\Routing\RouteProviderInterface
+   */
+  protected $routeProvider;
+
+  /**
+   * The route building service.
+   *
+   * @var \Drupal\Core\Routing\RouteBuilderInterface
+   */
+  protected $routeBuilder;
+
+  /**
+   * Flag to determine if the router has been rebuilt.
+   *
+   * @var bool
+   */
+  protected $rebuilt = FALSE;
+
+  /**
+   * RouteProviderLazyBuilder constructor.
+   *
+   * @param \Drupal\Core\Routing\RouteProviderInterface $route_provider
+   *   The route provider service.
+   * @param \Drupal\Core\Routing\RouteBuilderInterface $route_builder
+   *   The route building service.
+   */
+  public function __construct(RouteProviderInterface $route_provider, RouteBuilderInterface $route_builder) {
+    $this->routeProvider = $route_provider;
+    $this->routeBuilder = $route_builder;
+  }
+
+  /**
+   * Gets the real route provider service and rebuilds the router id necessary.
+   *
+   * @return \Drupal\Core\Routing\RouteProviderInterface
+   *   The route provider service.
+   */
+  protected function getRouteProvider() {
+    if (!$this->rebuilt) {
+      $this->routeBuilder->rebuild();
+      $this->rebuilt = TRUE;
+    }
+    return $this->routeProvider;
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getRouteCollectionForRequest(Request $request) {
+    return $this->getRouteProvider()->getRouteCollectionForRequest($request);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getRouteByName($name) {
+    return $this->getRouteProvider()->getRouteByName($name);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function preLoadRoutes($names) {
+    return $this->getRouteProvider()->preLoadRoutes($names);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getRoutesByNames($names) {
+    return $this->getRouteProvider()->getRoutesByNames($names);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getRoutesByPattern($pattern) {
+    return $this->getRouteProvider()->getRoutesByPattern($pattern);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getAllRoutes() {
+    return $this->getRouteProvider()->getAllRoutes();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function reset() {
+    // Don't call getRouteProvider as this is results in recursive rebuilds.
+    return $this->routeProvider->reset();
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getRoutesPaged($offset, $length = NULL) {
+    return $this->getRouteProvider()->getRoutesPaged($offset, $length);
+  }
+
+  /**
+   * {@inheritdoc}
+   */
+  public function getRoutesCount() {
+    return $this->getRouteProvider()->getRoutesCount();
+  }
+
+  /**
+   * Determines if the router has been rebuilt.
+   *
+   * @return bool
+   *   TRUE is the router has been rebuilt, FALSE if not.
+   */
+  public function hasRebuilt() {
+    return $this->rebuilt;
+  }
+
+}

core/modules/content_translation/content_translation.install

@@ -14,15 +14,19 @@ function content_translation_install() {
   // Assign a fairly low weight to ensure our implementation of
   // hook_module_implements_alter() is run among the last ones.
   module_set_weight('content_translation', 10);
+
   // Translation works when at least two languages are added.
   if (count(\Drupal::languageManager()->getLanguages()) < 2) {
-    // @todo: Switch to Url::fromRoute() once https://www.drupal.org/node/2589967 is resolved.
-    $t_args = [':language_url' => Url::fromUri('internal:/admin/config/regional/language')->toString()];
+    $t_args = [
+      ':language_url' => Url::fromRoute('entity.configurable_language.collection')->toString()
+    ];
     $message = t('This site has only a single language enabled. <a href=":language_url">Add at least one more language</a> in order to translate content.', $t_args);
     drupal_set_message($message, 'warning');
   }
   // Point the user to the content translation settings.
-  $t_args = [':settings_url' => Url::fromUri('internal:/admin/config/regional/content-language')->toString()];
+  $t_args = [
+    ':settings_url' => Url::fromRoute('language.content_settings_page')->toString()
+  ];
   $message = t('<a href=":settings_url">Enable translation</a> for <em>content types</em>, <em>taxonomy vocabularies</em>, <em>accounts</em>, or any other element you wish to translate.', $t_args);
   drupal_set_message($message, 'warning');
 }

core/tests/Drupal/KernelTests/Config/DefaultConfigTest.php

@@ -56,6 +56,11 @@ public function testModuleConfig($module) {
     /** @var \Drupal\Core\Config\ConfigManagerInterface $config_manager */
     $config_manager = $this->container->get('config.manager');
 
+    // @todo https://www.drupal.org/node/2308745 Rest has an implicit dependency
+    //   on the Node module remove once solved.
+    if (in_array($module, ['rest', 'hal'])) {
+      $module_installer->install(['node']);
+    }
     $module_installer->install([$module]);
 
     // System and user are required in order to be able to install some of the

core/tests/Drupal/KernelTests/Core/Extension/ModuleInstallerTest.php

+<?php
+
+namespace Drupal\KernelTests\Core\Extension;
+
+use Drupal\Core\Database\Database;
+use Drupal\KernelTests\KernelTestBase;
+use Symfony\Component\Routing\Exception\RouteNotFoundException;
+
+/**
+ * Tests the ModuleInstaller class.
+ *
+ * @coversDefaultClass \Drupal\Core\Extension\ModuleInstaller
+ *
+ * @group Extension
+ */
+class ModuleInstallerTest extends KernelTestBase {
+
+  /**
+   * Modules to install.
+   *
+   * The System module is required because system_rebuild_module_data() is used.
+   *
+   * @var array
+   */
+  public static $modules = ['system'];
+
+  /**
+   * Tests that routes are rebuilt during install and uninstall of modules.
+   *
+   * @covers ::install
+   * @covers ::uninstall
+   */
+  public function testRouteRebuild() {
+    // Remove the routing table manually to ensure it can be created lazily
+    // properly.
+    Database::getConnection()->schema()->dropTable('router');
+
+    $this->container->get('module_installer')->install(['router_test']);
+    $route = $this->container->get('router.route_provider')->getRouteByName('router_test.1');
+    $this->assertEquals('/router_test/test1', $route->getPath());
+
+    $this->container->get('module_installer')->uninstall(['router_test']);
+    $this->setExpectedException(RouteNotFoundException::class);
+    $this->container->get('router.route_provider')->getRouteByName('router_test.1');
+  }
+
+}