Issue #2160811 by sun, damiankloip, dawehner: Router rebuild fails (even via...

Issue #2160811 by sun, damiankloip, dawehner: Router rebuild fails (even via rebuild script), in case the provider of existing routes has changed.

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

@@ -79,50 +79,66 @@ public function dump(array $options = array()) {
     $options += array(
       'provider' => '',
     );
-
-    // Convert all of the routes into database records.
-    $insert = $this->connection->insert($this->tableName)->fields(array(
-      'name',
-      'provider',
-      'fit',
-      'path',
-      'pattern_outline',
-      'number_parts',
-      'route',
-    ));
-
-    foreach ($this->routes as $name => $route) {
-      $route->setOption('compiler_class', '\Drupal\Core\Routing\RouteCompiler');
-      $compiled = $route->compile();
-      $values = array(
-        'name' => $name,
-        'provider' => $options['provider'],
-        'fit' => $compiled->getFit(),
-        'path' => $compiled->getPath(),
-        'pattern_outline' => $compiled->getPatternOutline(),
-        'number_parts' => $compiled->getNumParts(),
-        'route' => serialize($route),
-      );
-      $insert->values($values);
-    }
-
-    // Delete any old records in this provider first, then insert the new ones.
-    // That avoids stale data. The transaction makes it atomic to avoid
-    // unstable router states due to random failures.
-    $transaction = $this->connection->startTransaction();
-    try {
+    // If there are no new routes, just delete any previously existing of this
+    // provider.
+    if (empty($this->routes) || !count($this->routes)) {
       $this->connection->delete($this->tableName)
         ->condition('provider', $options['provider'])
         ->execute();
-      $insert->execute();
-      // We want to reuse the dumper for multiple providers, so on dump, flush
-      // the queued routes.
-      $this->routes = NULL;
-    } catch (\Exception $e) {
-      $transaction->rollback();
-      watchdog_exception('Routing', $e);
-      throw $e;
     }
+    // Convert all of the routes into database records.
+    else {
+      $insert = $this->connection->insert($this->tableName)->fields(array(
+        'name',
+        'provider',
+        'fit',
+        'path',
+        'pattern_outline',
+        'number_parts',
+        'route',
+      ));
+      $names = array();
+      foreach ($this->routes as $name => $route) {
+        $route->setOption('compiler_class', '\Drupal\Core\Routing\RouteCompiler');
+        $compiled = $route->compile();
+        $names[] = $name;
+        $values = array(
+          'name' => $name,
+          'provider' => $options['provider'],
+          'fit' => $compiled->getFit(),
+          'path' => $compiled->getPath(),
+          'pattern_outline' => $compiled->getPatternOutline(),
+          'number_parts' => $compiled->getNumParts(),
+          'route' => serialize($route),
+        );
+        $insert->values($values);
+      }
+
+      // Delete any old records of this provider first, then insert the new ones.
+      // That avoids stale data. The transaction makes it atomic to avoid
+      // unstable router states due to random failures.
+      $transaction = $this->connection->startTransaction();
+      try {
+        // Previously existing routes might have been moved to a new provider,
+        // so ensure that none of the names to insert exists. Also delete any
+        // old records of this provider (which may no longer exist).
+        $delete = $this->connection->delete($this->tableName);
+        $or = $delete->orConditionGroup()
+          ->condition('provider', $options['provider'])
+          ->condition('name', $names);
+        $delete->condition($or);
+        $delete->execute();
+
+        // Insert all new routes.
+        $insert->execute();
+      } catch (\Exception $e) {
+        $transaction->rollback();
+        watchdog_exception('Routing', $e);
+        throw $e;
+      }
+    }
+    // The dumper is reused for multiple providers, so reset the queued routes.
+    $this->routes = NULL;
   }
 
   /**

core/modules/system/lib/Drupal/system/Tests/Routing/MatcherDumperTest.php

@@ -140,6 +140,54 @@ public function testDump() {
     $this->assertEqual($record->pattern_outline, '/test/%/path', 'Dumped route has correct pattern outline.');
     $this->assertEqual($record->fit, 5 /* 101 in binary */, 'Dumped route has correct fit.');
     $this->assertTrue($loaded_route instanceof Route, 'Route object retrieved successfully.');
+  }
+
+  /**
+   * Tests that changing the provider of a route updates the dumped value.
+   */
+  public function testDumpRouteProviderRename() {
+    $connection = Database::getConnection();
+    $dumper = new MatcherDumper($connection, 'test_routes');
+    $this->fixtures->createTables($connection);
+
+    $route = new Route('/test');
+    $collection = new RouteCollection();
+    $collection->add('test', $route);
+
+    $dumper->addRoutes($collection);
+    $dumper->dump(array('provider' => 'module_provider'));
 
+    $record = $connection->query("SELECT * FROM {test_routes} WHERE name = :name", array(':name' => 'test'))->fetchObject();
+    $this->assertEqual($record->provider, 'module_provider');
+
+    // Dump the same route name again with a different provider.
+    $dumper->addRoutes($collection);
+    $dumper->dump(array('provider' => 'module_provider2'));
+
+    // Ensure the route has the new provider.
+    $record = $connection->query("SELECT * FROM {test_routes} WHERE provider = :provider", array(':provider' => 'module_provider'))->fetchObject();
+    $this->assertFalse($record);
+
+    $record = $connection->query("SELECT * FROM {test_routes} WHERE provider = :provider", array(':provider' => 'module_provider2'))->fetchObject();
+    $this->assertEqual($record->path, '/test');
+    $this->assertEqual($record->name, 'test');
+
+    // Test dumping an empty route collection.
+    $dumper->addRoutes(new RouteCollection());
+    $dumper->dump(array('provider' => 'module_provider2'));
+
+    // Ensure the route of the provider no longer exists.
+    $record = $connection->query("SELECT * FROM {test_routes} WHERE provider = :provider", array(':provider' => 'module_provider2'))->fetchObject();
+    $this->assertFalse($record);
+
+    $dumper->addRoutes($collection);
+    $dumper->dump(array('provider' => 'module_provider2'));
+
+    // Test with an unset $routes property.
+    $dumper->dump(array('provider' => 'module_provider2'));
+    // Ensure the route of the provider no longer exists.
+    $record = $connection->query("SELECT * FROM {test_routes} WHERE provider = :provider", array(':provider' => 'module_provider2'))->fetchObject();
+    $this->assertFalse($record);
   }
+
 }