diff --git a/core/lib/Drupal/Core/Routing/RouteBuilder.php b/core/lib/Drupal/Core/Routing/RouteBuilder.php index d720f07..8a7cb90 100644 --- a/core/lib/Drupal/Core/Routing/RouteBuilder.php +++ b/core/lib/Drupal/Core/Routing/RouteBuilder.php @@ -143,6 +143,7 @@ public function rebuild() { } // Now allow modules to register additional, dynamic routes. + // @todo Either remove this alter or the per-provider alter. $collection = new RouteCollection(); $this->dispatcher->dispatch(RoutingEvents::ALTER, new RouteBuildEvent($collection, 'dynamic_routes')); $this->dumper->addRoutes($collection); diff --git a/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php b/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php index 537b186..335faa5 100644 --- a/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php +++ b/core/modules/config_translation/lib/Drupal/config_translation/Routing/RouteSubscriber.php @@ -37,8 +37,9 @@ public function __construct(ConfigMapperManagerInterface $mapper_manager) { * {@inheritdoc} */ protected function alterRoutes(RouteCollection $collection, $provider) { - // Because ConfigMapperManagerInterface uses the route provider to determine - // the routes to add here this must only run during the dynamic alter stage. + // @todo \Drupal\config_translation\ConfigNamesMapper uses the route + // provider directly, which is unsafe during rebuild. This currently only + // works by coincidence; fix in https://drupal.org/node/2158571. if ($provider != 'dynamic_routes') { return; } diff --git a/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php b/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php index 5768dd3..14da989 100644 --- a/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php +++ b/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php @@ -95,6 +95,32 @@ protected function getViewsDisplayIDsWithRoute() { } /** + * Returns a set of route objects. + * + * @return \Symfony\Component\Routing\RouteCollection + * A route collection. + */ + public function routes() { + $collection = new RouteCollection(); + foreach ($this->getViewsDisplayIDsWithRoute() as $pair) { + list($view_id, $display_id) = explode('.', $pair); + $view = $this->viewStorageController->load($view_id); + // @todo This should have an executable factory injected. + if (($view = $view->getExecutable()) && $view instanceof ViewExecutable) { + if ($view->setDisplay($display_id) && $display = $view->displayHandlers->get($display_id)) { + if ($display instanceof DisplayRouterInterface) { + $this->viewRouteNames += (array) $display->collectRoutes($collection); + } + } + $view->destroy(); + } + } + + $this->state->set('views.view_route_names', $this->viewRouteNames); + return $collection; + } + + /** * {@inheritdoc} */ protected function alterRoutes(RouteCollection $collection, $provider) { @@ -105,10 +131,6 @@ protected function alterRoutes(RouteCollection $collection, $provider) { if (($view = $view->getExecutable()) && $view instanceof ViewExecutable) { if ($view->setDisplay($display_id) && $display = $view->displayHandlers->get($display_id)) { if ($display instanceof DisplayRouterInterface) { - if ($provider == 'dynamic_routes') { - $this->viewRouteNames += (array) $display->collectRoutes($collection); - } - // If the display returns TRUE a route item was found, so it does not // have to be added. $view_route_names = $display->alterRoutes($collection); @@ -121,9 +143,6 @@ protected function alterRoutes(RouteCollection $collection, $provider) { $view->destroy(); } } - if ($provider == 'dynamic_routes') { - $this->state->set('views.view_route_names', $this->viewRouteNames); - } } /** diff --git a/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayPageTest.php b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayPageTest.php index 3e508ea..32c3bfc 100644 --- a/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayPageTest.php +++ b/core/modules/views/lib/Drupal/views/Tests/Plugin/DisplayPageTest.php @@ -97,9 +97,7 @@ public function testPageResponses() { * Checks that the router items are properly registered */ public function testPageRouterItems() { - $subscriber = new RouteSubscriber($this->container->get('entity.manager'), $this->container->get('state')); - $collection = new RouteCollection(); - $subscriber->onAlterRoutes(new RouteBuildEvent($collection, 'dynamic_routes')); + $collection = \Drupal::service('views.route_subscriber')->routes(); // Check the controller defaults. foreach ($collection as $id => $route) { diff --git a/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php b/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php index 64f9c5a..a7759a7 100644 --- a/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php +++ b/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php @@ -73,13 +73,40 @@ protected function setUp() { } /** + * Tests the onDynamicRoutes method. + * + * @see \Drupal\views\EventSubscriber\RouteSubscriber::onDynamicRoutes() + */ + public function testDynamicRoutes() { + list($view, $executable, $display_1, $display_2) = $this->setupMocks(); + + $display_1->expects($this->once()) + ->method('collectRoutes') + ->will($this->returnValue(array('test_id.page_1' => 'views.test_id.page_1'))); + $display_2->expects($this->once()) + ->method('collectRoutes') + ->will($this->returnValue(array('test_id.page_2' => 'views.test_id.page_2'))); + + $this->routeSubscriber->routes(); + + $this->state->expects($this->once()) + ->method('set') + ->with('views.view_route_names', array('test_id.page_1' => 'views.test_id.page_1', 'test_id.page_2' => 'views.test_id.page_2')); + $this->routeSubscriber->destruct(); + } + + /** * Tests the onAlterRoutes method. * * @see \Drupal\views\EventSubscriber\RouteSubscriber::onAlterRoutes() */ public function testOnAlterRoutes() { $collection = new RouteCollection(); - $route_event = new RouteBuildEvent($collection, 'dynamic_routes'); + $collection->add('test_route', new Route('test_route', array('_controller' => 'Drupal\Tests\Core\Controller\TestController'))); + $route_2 = new Route('test_route/example', array('_controller' => 'Drupal\Tests\Core\Controller\TestController')); + $collection->add('test_route_2', $route_2); + + $route_event = new RouteBuildEvent($collection, 'views'); list($view, $executable, $display_1, $display_2) = $this->setupMocks(); @@ -88,9 +115,8 @@ public function testOnAlterRoutes() { $display_1->expects($this->once()) ->method('alterRoutes') ->will($this->returnValue(array('test_id.page_1' => 'test_route'))); - $display_1->expects($this->once()) - ->method('collectRoutes') - ->will($this->returnValue(array('test_id.page_1' => 'views.test_id.page_1'))); + $display_1->expects($this->never()) + ->method('collectRoutes'); $display_2->expects($this->once()) ->method('alterRoutes') @@ -101,6 +127,11 @@ public function testOnAlterRoutes() { $this->assertNull($this->routeSubscriber->onAlterRoutes($route_event)); + // Ensure that after the alterRoutes the collectRoutes method is just called + // once (not for page_1 anymore). + + $this->routeSubscriber->routes(); + $this->state->expects($this->once()) ->method('set') ->with('views.view_route_names', array('test_id.page_1' => 'test_route', 'test_id.page_2' => 'views.test_id.page_2')); diff --git a/core/modules/views/views.routing.yml b/core/modules/views/views.routing.yml index 691640d..3d84341 100644 --- a/core/modules/views/views.routing.yml +++ b/core/modules/views/views.routing.yml @@ -6,3 +6,6 @@ views.ajax: _theme: ajax_base_page requirements: _access: 'TRUE' + +route_callbacks: + - 'views.route_subscriber:routes'