Issue #2381505 by dawehner, pwolanin, aspilicious, rteijeiro: Unserialize...

Issue #2381505 by dawehner, pwolanin, aspilicious, rteijeiro: Unserialize preloaded routes on the fly

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

@@ -7,7 +7,6 @@
 
 namespace Drupal\Core\Routing;
 
-use Symfony\Component\Routing\Route;
 use Symfony\Component\Routing\CompiledRoute as SymfonyCompiledRoute;
 
 /**
@@ -36,13 +35,6 @@ class CompiledRoute extends SymfonyCompiledRoute {
    */
   protected $numParts;
 
-  /**
-   * The Route object of which this object is the compiled version.
-   *
-   * @var \Symfony\Component\Routing\Route
-   */
-  protected $route;
-
   /**
    * Constructs a new compiled route object.
    *
@@ -51,11 +43,9 @@ class CompiledRoute extends SymfonyCompiledRoute {
    * problem. The parent Symfony class does the same, as well, making it
    * difficult to override differently.
    *
-   * @param \Symfony\Component\Routing\Route $route
-   *   A original Route instance.
    * @param int $fit
    *   The fitness of the route.
-   * @param string $fit
+   * @param string $pattern_outline
    *   The pattern outline for this route.
    * @param int $num_parts
    *   The number of parts in the path.
@@ -76,10 +66,9 @@ class CompiledRoute extends SymfonyCompiledRoute {
    * @param array $variables
    *   An array of variables (variables defined in the path and in the host patterns)
    */
-  public function __construct(Route $route, $fit, $pattern_outline, $num_parts, $staticPrefix, $regex, array $tokens, array $pathVariables, $hostRegex = null, array $hostTokens = array(), array $hostVariables = array(), array $variables = array()) {
+  public function __construct($fit, $pattern_outline, $num_parts, $staticPrefix, $regex, array $tokens, array $pathVariables, $hostRegex = null, array $hostTokens = array(), array $hostVariables = array(), array $variables = array()) {
     parent::__construct($staticPrefix, $regex, $tokens, $pathVariables, $hostRegex, $hostTokens, $hostVariables, $variables);
 
-    $this->route = $route;
     $this->fit = $fit;
     $this->patternOutline = $pattern_outline;
     $this->numParts = $num_parts;
@@ -123,26 +112,6 @@ public function getPatternOutline() {
     return $this->patternOutline;
   }
 
-  /**
-   * Returns the Route instance.
-   *
-   * @return Route
-   *   A Route instance.
-   */
-  public function getRoute() {
-    return $this->route;
-  }
-
-  /**
-   * Returns the path.
-   *
-   * @return string
-   *   The path.
-   */
-  public function getPath() {
-    return $this->route->getPath();
-  }
-
   /**
    * Returns the options.
    *
@@ -177,6 +146,8 @@ public function getRequirements() {
    * {@inheritdoc}
    */
   public function serialize() {
+    // Calling the parent method is safer than trying to optimize out the extra
+    // function calls.
     $data = unserialize(parent::serialize());
     $data['fit'] = $this->fit;
     $data['patternOutline'] = $this->patternOutline;
@@ -188,8 +159,7 @@ public function serialize() {
   /**
    * {@inheritdoc}
    */
-  public function unserialize($serialized)
-  {
+  public function unserialize($serialized) {
     parent::unserialize($serialized);
     $data = unserialize($serialized);
 

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

@@ -114,6 +114,7 @@ public function dump(array $options = array()) {
         foreach ($routes as $name => $route) {
           /** @var \Symfony\Component\Routing\Route $route */
           $route->setOption('compiler_class', '\Drupal\Core\Routing\RouteCompiler');
+          /** @var \Drupal\Core\Routing\CompiledRoute $compiled */
           $compiled = $route->compile();
           // The fit value is a binary number which has 1 at every fixed path
           // position and 0 where there is a wildcard. We keep track of all such
@@ -124,7 +125,7 @@ public function dump(array $options = array()) {
           $values = array(
             'name' => $name,
             'fit' => $compiled->getFit(),
-            'path' => $compiled->getPath(),
+            'path' => $route->getPath(),
             'pattern_outline' => $compiled->getPatternOutline(),
             'number_parts' => $compiled->getNumParts(),
             'route' => serialize($route),

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

+<?php
+
+/**
+ * @file
+ * Contains \Drupal\Core\Routing\PreloadableRouteProviderInterface.
+ */
+
+namespace Drupal\Core\Routing;
+
+/**
+ * Extends the router provider interface to pre-load routes.
+ */
+interface PreloadableRouteProviderInterface extends RouteProviderInterface {
+
+  /**
+   * Pre-load routes by their names using the provided list of names.
+   *
+   * This method exists in order to allow performance optimizations. It allows
+   * pre-loading serialized routes that may latter be retrieved using
+   * ::getRoutesByName()
+   *
+   * @param string[] $names
+   *   Array of route names to load.
+   */
+  public function preLoadRoutes($names);
+
+}

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

@@ -46,7 +46,6 @@ public static function compile(Route $route) {
     $num_parts = count(explode('/', trim($pattern_outline, '/')));
 
     return new CompiledRoute(
-      $route,
       $fit,
       $pattern_outline,
       $num_parts,

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

@@ -25,7 +25,7 @@ class RoutePreloader implements EventSubscriberInterface {
   /**
    * The route provider.
    *
-   * @var \Drupal\Core\Routing\RouteProviderInterface
+   * @var \Drupal\Core\Routing\RouteProviderInterface|\Drupal\Core\Routing\PreloadableRouteProviderInterface
    */
   protected $routeProvider;
 
@@ -63,19 +63,13 @@ public function __construct(RouteProviderInterface $route_provider, StateInterfa
    *   The event to process.
    */
   public function onRequest(KernelEvent $event) {
-    // Just preload on normal HTML pages, as they will display menu links.
-    if ($event->getRequest()->getRequestFormat() == 'html') {
-      $this->loadNonAdminRoutes();
+    // Only preload on normal HTML pages, as they will display menu links.
+    if ($this->routeProvider instanceof PreloadableRouteProviderInterface && $event->getRequest()->getRequestFormat() == 'html') {
+      if ($routes = $this->state->get('routing.non_admin_routes', [])) {
+        // Preload all the non-admin routes at once.
+        $this->routeProvider->preLoadRoutes($routes);
       }
     }
-
-  /**
-   * Load all the non-admin routes at once.
-   */
-  protected function loadNonAdminRoutes() {
-    if ($routes = $this->state->get('routing.non_admin_routes', array())) {
-      $this->routeProvider->getRoutesByNames($routes);
-    }
   }
 
   /**

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

@@ -22,7 +22,7 @@
 /**
  * A Route Provider front-end for all Drupal-stored routes.
  */
-class RouteProvider implements RouteProviderInterface, PagedRouteProviderInterface, EventSubscriberInterface {
+class RouteProvider implements PreloadableRouteProviderInterface, PagedRouteProviderInterface, EventSubscriberInterface {
 
   /**
    * The database connection from which to read route information.
@@ -48,10 +48,17 @@ class RouteProvider implements RouteProviderInterface, PagedRouteProviderInterfa
   /**
    * A cache of already-loaded routes, keyed by route name.
    *
-   * @var array
+   * @var \Symfony\Component\Routing\Route[]
    */
   protected $routes = array();
 
+  /**
+   * A cache of already-loaded serialized routes, keyed by route name.
+   *
+   * @var string[]
+   */
+  protected $serializedRoutes = [];
+
   /**
    * The current path.
    *
@@ -131,34 +138,32 @@ public function getRouteByName($name) {
   }
 
   /**
-   * Find many routes by their names using the provided list of names.
-   *
-   * Note that this method may not throw an exception if some of the routes
-   * are not found. It will just return the list of those routes it found.
-   *
-   * This method exists in order to allow performance optimizations. The
-   * simple implementation could be to just repeatedly call
-   * $this->getRouteByName().
-   *
-   * @param array $names
-   *   The list of names to retrieve.
-   *
-   * @return \Symfony\Component\Routing\Route[]
-   *   Iterable thing with the keys the names of the $names argument.
+   * {@inheritdoc}
    */
-  public function getRoutesByNames($names) {
-
+  public function preLoadRoutes($names) {
     if (empty($names)) {
       throw new \InvalidArgumentException('You must specify the route names to load');
     }
 
-    $routes_to_load = array_diff($names, array_keys($this->routes));
+    $routes_to_load = array_diff($names, array_keys($this->routes), array_keys($this->serializedRoutes));
     if ($routes_to_load) {
       $result = $this->connection->query('SELECT name, route FROM {' . $this->connection->escapeTable($this->tableName) . '} WHERE name IN ( :names[] )', array(':names[]' => $routes_to_load));
       $routes = $result->fetchAllKeyed();
+      $this->serializedRoutes += $routes;
+    }
+  }
 
-      foreach ($routes as $name => $route) {
-        $this->routes[$name] = unserialize($route);
+  /**
+   * {@inheritdoc}
+   */
+  public function getRoutesByNames($names) {
+    $this->preLoadRoutes($names);
+
+    foreach ($names as $name) {
+      // The specified route name might not exist or might be serialized.
+      if (!isset($this->routes[$name]) && isset($this->serializedRoutes[$name])) {
+        $this->routes[$name] = unserialize($this->serializedRoutes[$name]);
+        unset($this->serializedRoutes[$name]);
       }
     }
 
@@ -291,6 +296,7 @@ public function getAllRoutes() {
    */
   public function reset() {
     $this->routes  = array();
+    $this->serializedRoutes = array();
   }
 
   /**

core/modules/system/src/Tests/Routing/MockRouteProvider.php

@@ -56,6 +56,13 @@ public function getRouteByName($name) {
     return reset($routes);
   }
 
+  /**
+   * {@inheritdoc}
+   */
+  public function preLoadRoutes($names) {
+    // Nothing to do.
+  }
+
   /**
    * Implements \Symfony\Cmf\Component\Routing\RouteProviderInterface::getRoutesByName().
    */

core/tests/Drupal/Tests/Core/Routing/RouteCompilerTest.php

@@ -62,7 +62,6 @@ public function testCompilation() {
     $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler');
     $compiled = $route->compile();
 
-    $this->assertEquals($route, $compiled->getRoute(), 'Compiled route has the incorrect route object.');
     $this->assertEquals($compiled->getFit(), 5 /* That's 101 binary*/, 'The fit was incorrect.');
     $this->assertEquals($compiled->getPatternOutline(), '/test/%/more', 'The pattern outline was not correct.');
   }
@@ -79,7 +78,6 @@ public function testCompilationDefaultValue() {
     $route->setOption('compiler_class', 'Drupal\Core\Routing\RouteCompiler');
     $compiled = $route->compile();
 
-    $this->assertEquals($route, $compiled->getRoute(), 'Compiled route has an incorrect route object.');
     $this->assertEquals($compiled->getFit(), 5 /* That's 101 binary*/, 'The fit was not correct.');
     $this->assertEquals($compiled->getPatternOutline(), '/test/%/more', 'The pattern outline was not correct.');
   }

core/tests/Drupal/Tests/Core/Routing/RoutePreloaderTest.php

@@ -45,7 +45,7 @@ class RoutePreloaderTest extends UnitTestCase {
    * {@inheritdoc}
    */
   protected function setUp() {
-    $this->routeProvider = $this->getMock('Drupal\Core\Routing\RouteProviderInterface');
+    $this->routeProvider = $this->getMock('Drupal\Core\Routing\PreloadableRouteProviderInterface');
     $this->state = $this->getMock('\Drupal\Core\State\StateInterface');
     $this->preloader = new RoutePreloader($this->routeProvider, $this->state);
   }
@@ -153,8 +153,8 @@ public function testOnRequestOnHtml() {
       ->will($this->returnValue($request));
 
     $this->routeProvider->expects($this->once())
-      ->method('getRoutesByNames')
-      ->with(array('test2'));
+      ->method('preLoadRoutes')
+      ->with(['test2']);
     $this->state->expects($this->once())
       ->method('get')
       ->with('routing.non_admin_routes')