diff -u b/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php b/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php --- b/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php +++ b/core/modules/views/lib/Drupal/views/EventSubscriber/RouteSubscriber.php @@ -16,11 +16,16 @@ /** * Builds up the routes of all views. + * + * The general idea done here is to first execute + * + * @see \Drupal\views\Plugin\views\display\PathPluginBase */ class RouteSubscriber implements EventSubscriberInterface { /** - * Stores a list of view,display IDs containing routes. + * Stores a list of view,display IDs containing routes and still has to be + * added. * * @var array */ @@ -67,11 +72,11 @@ $this->viewsDisplayPairs = array(); // @todo Convert this method to some service. - $views = views_get_applicable_views('uses_route'); + $views = $this->getApplicableViews(); foreach ($views as $data) { list($view, $display_id) = $data; $id = $view->storage->id(); - $this->viewsDisplayPairs[$id . ':' . $display_id] = $id . ':' . $display_id; + $this->viewsDisplayPairs[$id . '.' . $display_id] = $id . '.' . $display_id; } } return $this->viewsDisplayPairs; @@ -86,9 +91,8 @@ public function dynamicRoutes(RouteBuildEvent $event) { $collection = $event->getRouteCollection(); - foreach ($this->getViewsDisplayIDsWithRoute() as $pair) { - list($view_id, $display_id) = explode(':', $pair); + list($view_id, $display_id) = explode('.', $pair); $view = $this->viewStorageController->load($view_id); // @todo This should have a executable factory injected. if (($view = $view->getExecutable()) && $view instanceof ViewExecutable) { @@ -110,7 +114,7 @@ */ public function alterRoutes(RouteBuildEvent $event) { foreach ($this->getViewsDisplayIDsWithRoute() as $pair) { - list($view_id, $display_id) = explode(':', $pair); + list($view_id, $display_id) = explode('.', $pair); $view = $this->viewStorageController->load($view_id); // @todo This should have a executable factory injected. if (($view = $view->getExecutable()) && $view instanceof ViewExecutable) { @@ -118,7 +122,10 @@ if ($display instanceof DisplayRouterInterface) { // If the display returns TRUE a route item was found, so it does not // have to be added. - $display->alterRoutes($event->getRouteCollection()); + $result = $display->alterRoutes($event->getRouteCollection()); + foreach ($result as $id_display) { + unset($this->viewsDisplayPairs[$id_display]); + } } } $view->destroy(); @@ -128,2 +135,11 @@ + /** + * Returns all views/display combinations with routes. + * + * @see views_get_applicable_views() + */ + protected function getApplicableViews() { + return views_get_applicable_views('uses_route'); + } + } diff -u b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayRouterInterface.php b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayRouterInterface.php --- b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayRouterInterface.php +++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/DisplayRouterInterface.php @@ -34,8 +34,8 @@ * * @param \Symfony\Component\Routing\RouteCollection $collection * - * @return bool - * Returns TRUE if an existing route was found, otherwise FALSE. + * @return array + * Returns a list of "$view_id.$display_id" elements which got overridden. */ public function alterRoutes(RouteCollection $collection); diff -u b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php --- b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php +++ b/core/modules/views/lib/Drupal/views/Plugin/views/display/PathPluginBase.php @@ -19,6 +19,8 @@ /** * The base display plugin for path/callbacks. This is used for pages and feeds. + * + * @see \Drupal\views\EventSubscriber\RouteSubscriber */ abstract class PathPluginBase extends DisplayPluginBase implements DisplayRouterInterface { @@ -125,7 +127,7 @@ $arg_counter = 0; $this->view->initHandlers(); - $view_arguments = $this->view->argument; + $view_arguments = (array) $this->view->argument; $argument_ids = array_keys($view_arguments); $total_arguments = count($argument_ids); @@ -190,22 +192,30 @@ * {@inheritdoc} */ public function alterRoutes(RouteCollection $collection) { - $found = FALSE; + $altered_views = array(); $view_path = $this->getPath(); foreach ($collection->all() as $name => $route) { // Find all paths which match the path of the current display.. $route_path = RouteCompiler::getPathWithoutDefaults($route); $route_path = RouteCompiler::getPatternOutline($route_path); + // Ensure that we don't override a route which is already controlled by + // views. if (!$route->hasDefault('view_id') && '/' . $view_path == $route_path) { // @todo Figure out whether we need to merge some settings (like // requirements). + // Replace the existing route with a new one based on views. $collection->remove($name); - $found = TRUE; + + $view_id = $this->view->storage->id(); + $display_id = $this->display['id']; + $route = $this->getRoute($view_id, $display_id); + $collection->add($name, $route); + $altered_views[] = $view_id . '.' . $display_id; } } - return $found; + return $altered_views; } /** diff -u b/core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php b/core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php --- b/core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php +++ b/core/modules/views/tests/Drupal/views/Tests/Plugin/display/PathPluginBaseTest.php @@ -7,7 +7,8 @@ namespace Drupal\views\Tests\Plugin\display { -use Drupal\Tests\UnitTestCase; + use Drupal\Core\DependencyInjection\ContainerBuilder; + use Drupal\Tests\UnitTestCase; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -30,7 +31,14 @@ * * @var \Drupal\views\Plugin\views\display\PathPluginBase */ - public $pathPlugin; + protected $pathPlugin; + + /** + * The mocked views access plugin manager. + * + * @var \Drupal\views\Plugin\ViewsPluginManager|\PHPUnit_Framework_MockObject_MockObject + */ + protected $accessPluginManager; public static function getInfo() { return array( @@ -51,6 +59,45 @@ ->setConstructorArgs(array(array(), 'path_base', array(), $this->routeProvider)) ->setMethods(NULL) ->getMock(); + $this->setupAccessPluginManager(); + } + + /** + * Setup access plugin manager in a Drupal class. + */ + public function setupAccessPluginManager() { + $this->accessPluginManager = $this->getMockBuilder('\Drupal\views\Plugin\ViewsPluginManager') + ->disableOriginalConstructor() + ->getMock(); + $container = new ContainerBuilder(); + $container->set('plugin.manager.views.access', $this->accessPluginManager); + \Drupal::setContainer($container); + } + + /** + * Tests the collectRoutes method. + * + * @see \Drupal\views\Plugin\views\display\PathPluginBase::collectRoutes() + */ + public function testCollectRoutes() { + list($view) = $this->setupViewExecutableAccessPlugin(); + + $display = array(); + $display['display_plugin'] = 'page'; + $display['id'] = 'page_1'; + $display['display_options'] = array( + 'path' => 'test_route', + ); + $this->pathPlugin->initDisplay($view, $display); + + $collection = new RouteCollection(); + $this->pathPlugin->collectRoutes($collection); + + $route = $collection->get('view.test_id.page_1'); + $this->assertTrue($route instanceof Route); + $this->assertEquals('test_id', $route->getDefault('view_id')); + $this->assertEquals('page_1', $route->getDefault('display_id')); + } /** @@ -59,13 +106,10 @@ public function testAlterRoute() { $collection = new RouteCollection(); $collection->add('test_route', new Route('test_route', array('_controller' => 'Drupal\Tests\Core\Controller\TestController'))); - $collection->add('test_route_2', new Route('test_route/example', 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); - $view = $this->getMockBuilder('Drupal\views\ViewExecutable') - ->disableOriginalConstructor() - ->getMock(); - // Skip views options caching. - $view->editing = TRUE; + list($view) = $this->setupViewExecutableAccessPlugin(); $display = array(); $display['display_plugin'] = 'page'; @@ -75,10 +119,50 @@ ); $this->pathPlugin->initDisplay($view, $display); - $this->pathPlugin->alterRoutes($collection); + $found_views = $this->pathPlugin->alterRoutes($collection); + $this->assertEquals(array('test_id.page_1'), $found_views); + + // Ensure that the test_route is overridden. + $route = $collection->get('test_route'); + $this->assertTrue($route instanceof Route); + $this->assertEquals('test_id', $route->getDefault('view_id')); + $this->assertEquals('page_1', $route->getDefault('display_id')); + + // Ensure that the test_route_2 is not overridden. + $route = $collection->get('test_route_2'); + $this->assertTrue($route instanceof Route); + $this->assertFalse($route->hasDefault('view_id')); + $this->assertFalse($route->hasDefault('display_id')); + $this->assertSame($collection->get('test_route_2'), $route_2); + } + + /** + * Returns some mocked view entity, view executable, and access plugin. + */ + protected function setupViewExecutableAccessPlugin() { + $view_entity = $this->getMockBuilder('Drupal\views\Entity\View') + ->disableOriginalConstructor() + ->getMock(); + $view_entity->expects($this->any()) + ->method('id') + ->will($this->returnValue('test_id')); + + $view = $this->getMockBuilder('Drupal\views\ViewExecutable') + ->disableOriginalConstructor() + ->getMock(); + $view->storage = $view_entity; + + // Skip views options caching. + $view->editing = TRUE; + + $access_plugin = $this->getMockBuilder('Drupal\views\Plugin\views\access\AccessPluginBase') + ->disableOriginalConstructor() + ->getMockForAbstractClass(); + $this->accessPluginManager->expects($this->any()) + ->method('createInstance') + ->will($this->returnValue($access_plugin)); - $this->assertNull($collection->get('test_route'), 'Route with same path as view got removed.'); - $this->assertInstanceOf('Symfony\Component\Routing\Route', $collection->get('test_route_2')); + return array($view, $view_entity, $access_plugin); } } diff -u b/core/modules/views/views.module b/core/modules/views/views.module --- b/core/modules/views/views.module +++ b/core/modules/views/views.module @@ -689,9 +689,6 @@ $module_handler = \Drupal::moduleHandler(); - // Reset the RouteSubscriber from views. - \Drupal::getContainer()->get('views.route_subscriber')->reset(); - // Set the router to be rebuild. // @todo Figure out why the cache rebuild is trigged but the route table // does not exist yet. @@ -709,6 +706,9 @@ $module_handler = \Drupal::moduleHandler(); + // Reset the RouteSubscriber from views. + \Drupal::getContainer()->get('views.route_subscriber')->reset(); + // Set the router to be rebuild. // @todo Figure out why the cache rebuild is trigged but the route table // does not exist yet. only in patch2: unchanged: --- /dev/null +++ b/core/modules/views/tests/Drupal/views/Tests/EventSubscriber/RouteSubscriberTest.php @@ -0,0 +1,188 @@ + 'Views route subscriber', + 'description' => 'Tests the views route subscriber.', + 'group' => 'Views plugins', + ); + } + + protected function setUp() { + $this->entityManager = $this->getMockBuilder('\Drupal\Core\Entity\EntityManager') + ->disableOriginalConstructor() + ->getMock(); + $this->viewStorageController = $this->getMockBuilder('\Drupal\views\ViewStorageController') + ->disableOriginalConstructor() + ->getMock(); + $this->entityManager->expects($this->any()) + ->method('getStorageController') + ->with('view') + ->will($this->returnValue($this->viewStorageController)); + $this->routeSubscriber = new TestRouteSubscriber($this->entityManager); + } + + /** + * Tests the dynamicRoutes method. + * + * @see \Drupal\views\EventSubscriber\RouteSubscriber::dynamicRoutes() + */ + public function testDynamicRoutes() { + $collection = new RouteCollection(); + $route_event = new RouteBuildEvent($collection, 'views'); + + list($view, $executable, $display_1, $display_2) = $this->setupMocks(); + + $display_1->expects($this->once()) + ->method('collectRoutes'); + $display_2->expects($this->once()) + ->method('collectRoutes'); + + $this->assertNull($this->routeSubscriber->dynamicRoutes($route_event)); + } + + /** + * Tests the alterRoutes method. + * + * @see \Drupal\views\EventSubscriber\RouteSubscriber::alterRoutes() + */ + public function testAlterRoutes() { + $collection = new RouteCollection(); + $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(); + + // The page_1 display overrides an existing route, so the dynamicRoutes + // should only call the second display. + $display_1->expects($this->once()) + ->method('alterRoutes') + ->will($this->returnValue(array('test_id.page_1'))); + $display_1->expects($this->never()) + ->method('collectRoutes'); + + $display_2->expects($this->once()) + ->method('alterRoutes') + ->will($this->returnValue(array())); + $display_2->expects($this->once()) + ->method('collectRoutes'); + + $this->assertNull($this->routeSubscriber->alterRoutes($route_event)); + + // Ensure that after the alterRoutes the collectRoutes method is just called + // once (not for page_1 anymore). + + $this->assertNull($this->routeSubscriber->dynamicRoutes($route_event)); + } + + protected function setupMocks() { + $executable = $this->getMockBuilder('Drupal\views\ViewExecutable') + ->disableOriginalConstructor() + ->getMock(); + $view = $this->getMockBuilder('Drupal\views\Entity\View') + ->disableOriginalConstructor() + ->getMock(); + $this->viewStorageController->expects($this->any()) + ->method('load') + ->will($this->returnValue($view)); + + $view->expects($this->any()) + ->method('getExecutable') + ->will($this->returnValue($executable)); + $view->expects($this->any()) + ->method('id') + ->will($this->returnValue('test_id')); + $executable->storage = $view; + + $executable->expects($this->any()) + ->method('setDisplay') + ->will($this->returnValueMap(array( + array('page_1', TRUE), + array('page_2', TRUE), + array('page_3', FALSE), + ))); + + // Ensure that only the first two displays are actually called. + $display_1 = $this->getMock('Drupal\views\Plugin\views\display\DisplayRouterInterface'); + $display_2 = $this->getMock('Drupal\views\Plugin\views\display\DisplayRouterInterface'); + + $display_bag = $this->getMockBuilder('Drupal\views\DisplayBag') + ->disableOriginalConstructor() + ->getMock(); + $display_bag->expects($this->any()) + ->method('get') + ->will($this->returnValueMap(array( + array('page_1', $display_1), + array('page_2', $display_2), + ))); + $executable->displayHandlers = $display_bag; + + $this->routeSubscriber->applicableViews = array(); + $this->routeSubscriber->applicableViews[] = array($executable, 'page_1'); + $this->routeSubscriber->applicableViews[] = array($executable, 'page_2'); + $this->routeSubscriber->applicableViews[] = array($executable, 'page_3'); + + return array($executable, $view, $display_1, $display_2); + } + +} + +class TestRouteSubscriber extends RouteSubscriber { + + /** + * The applicable views. + */ + public $applicableViews; + + /** + * {@inheritdoc} + */ + protected function getApplicableViews() { + return $this->applicableViews; + } + +}