diff --git a/page_manager.services.yml b/page_manager.services.yml index c32f69c..068200c 100644 --- a/page_manager.services.yml +++ b/page_manager.services.yml @@ -21,7 +21,7 @@ services: - { name: 'event_subscriber' } page_manager.variant_route_filter: class: Drupal\page_manager\Routing\VariantRouteFilter - arguments: ['@entity.manager', '@router.matcher.final_matcher'] + arguments: ['@entity.manager', '@path.current'] tags: - { name: route_filter } - { name: service_collector, tag: non_lazy_route_enhancer, call: addRouteEnhancer } diff --git a/src/Routing/RouteAttributes.php b/src/Routing/RouteAttributes.php new file mode 100644 index 0000000..91e67e5 --- /dev/null +++ b/src/Routing/RouteAttributes.php @@ -0,0 +1,49 @@ +compile()->getRegex(), $path, $matches); + // See \Symfony\Component\Routing\Matcher\UrlMatcher::mergeDefaults(). + $attributes = $route->getDefaults(); + foreach ($matches as $key => $value) { + if (!is_int($key)) { + $attributes[$key] = $value; + } + } + // See \Symfony\Cmf\Component\Routing\NestedMatcher\UrlMatcher::getAttributes(). + $attributes[RouteObjectInterface::ROUTE_OBJECT] = $route; + $attributes[RouteObjectInterface::ROUTE_NAME] = $name; + return $attributes; + } + +} diff --git a/src/Routing/VariantRouteFilter.php b/src/Routing/VariantRouteFilter.php index a377da8..4eba226 100644 --- a/src/Routing/VariantRouteFilter.php +++ b/src/Routing/VariantRouteFilter.php @@ -9,8 +9,8 @@ use Drupal\Component\Plugin\Exception\ContextException; use Drupal\Core\Entity\EntityManagerInterface; +use Drupal\Core\Path\CurrentPathStack; use Drupal\Core\Routing\RouteFilterInterface; -use Symfony\Cmf\Component\Routing\NestedMatcher\FinalMatcherInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -34,23 +34,23 @@ class VariantRouteFilter implements RouteFilterInterface { protected $pageStorage; /** - * The final routing matcher. + * The current path stack. * - * @var \Symfony\Cmf\Component\Routing\NestedMatcher\FinalMatcherInterface + * @var \Drupal\Core\Path\CurrentPathStack */ - protected $finalMatcher; + protected $currentPath; /** * Constructs a new VariantRouteFilter. * * @param \Drupal\Core\Entity\EntityManagerInterface $entity_manager * The entity manager. - * @param \Symfony\Cmf\Component\Routing\NestedMatcher\FinalMatcherInterface $final_matcher - * The final routing matcher. + * @param \Drupal\Core\Path\CurrentPathStack $current_path + * The current path stack. */ - public function __construct(EntityManagerInterface $entity_manager, FinalMatcherInterface $final_matcher) { + public function __construct(EntityManagerInterface $entity_manager, CurrentPathStack $current_path) { $this->pageStorage = $entity_manager->getStorage('page'); - $this->finalMatcher = $final_matcher; + $this->currentPath = $current_path; } /** @@ -72,18 +72,54 @@ public function filter(RouteCollection $collection, Request $request) { // Store the unaltered request attributes. $original_attributes = $request->attributes->all(); - $this->prepareRequest($collection, $request); $page_manager_route_found = FALSE; foreach ($collection as $name => $route) { - /** @var \Symfony\Component\Routing\Route $route */ + $attributes = $this->getRequestAttributes($route, $name, $request); + + // Add the enhanced attributes to the request. + $request->attributes->add($attributes); + + $this->processRoute($route, $name, $collection, $page_manager_route_found); + + // Restore the original request attributes. + $request->attributes->replace($original_attributes); + } + + return $collection; + } + + /** + * Processes a single route within a collection. + * + * Invalid page manager routes will be removed. Routes not controlled by page + * manager will be moved to the end of the collection. Once a valid page + * manager route has been found, all other page manager routes will also be + * removed. + * + * @param \Symfony\Component\Routing\Route $route + * The route object. + * @param string $name + * The route name. + * @param \Symfony\Component\Routing\RouteCollection $collection + * The route collection being modified. + * @param bool $page_manager_route_found + * A flag indicating whether a valid page manager route has yet been found. + * Passed by reference. + */ + protected function processRoute(Route $route, $name, RouteCollection $collection, &$page_manager_route_found) { $defaults = $route->getDefaults(); - if (isset($defaults['page_manager_page']) && isset($defaults['variant_id'])) { - // If any valid page manager routes have been found, remove all others - // from the collection. + if (!isset($defaults['page_manager_page']) || !isset($defaults['variant_id'])) { + // If this route has no page or variant info, move it to the end of the + // list. + $collection->add($name, $route); + return; + } + + // Once a valid page manager route has been found, remove all others. if ($page_manager_route_found) { $collection->remove($name); - continue; + return; } /** @var \Drupal\page_manager\PageInterface $page */ @@ -93,6 +129,8 @@ public function filter(RouteCollection $collection, Request $request) { try { $access = $variant->access(); } + // Since access checks can throw a context exception, consider that as + // a disallowed variant. catch (ContextException $e) { $access = FALSE; } @@ -103,47 +141,40 @@ public function filter(RouteCollection $collection, Request $request) { $page_manager_route_found = TRUE; } else { + // Remove routes for variants that fail access. $collection->remove($name); } } - else { - // If this route has no page or variant info, move it to the end of the - // list. - $collection->add($name, $route); - } - } - - // Restore the original request attributes. - $request->attributes->replace($original_attributes); - - return $collection; - } /** - * Prepares the request for use by the selection process. + * Prepares the request attributes for use by the selection process. * - * Since route filters run before request attributes are populated, we - * circumvent this by running the final matcher and param converter. + * This is be done because route filters run before request attributes are + * populated. * - * @param \Symfony\Component\Routing\RouteCollection $collection - * The current route collection. + * @param \Symfony\Component\Routing\Route $route + * The route. + * @param string $name + * The route name. * @param \Symfony\Component\HttpFoundation\Request $request * The current request. * - * @return $this + * @return array + * An array of request attributes. */ - protected function prepareRequest(RouteCollection $collection, Request $request) { - // @todo Are we sure that final matcher does not have any side-effects? Do - // we need to create a fake route collection and run this for each route? - $attributes = $this->finalMatcher->finalMatch($collection, $request); + protected function getRequestAttributes(Route $route, $name, Request $request) { + // Extract the raw attributes from the current path. This performs the same + // functionality as \Drupal\Core\Routing\UrlMatcher::finalMatch(). + $path = $this->currentPath->getPath($request); + $attributes = RouteAttributes::extractRawAttributes($route, $name, $path); + // Run the route enhancers on the raw attributes. This performs the same + // functionality as \Symfony\Cmf\Component\Routing\DynamicRouter::match(). foreach ($this->getRouteEnhancers() as $enhancer) { $attributes = $enhancer->enhance($attributes, $request); } - $request->attributes->add($attributes); - - return $this; + return $attributes; } } diff --git a/tests/src/Unit/RouteAttributesTest.php b/tests/src/Unit/RouteAttributesTest.php new file mode 100644 index 0000000..cb00a0b --- /dev/null +++ b/tests/src/Unit/RouteAttributesTest.php @@ -0,0 +1,40 @@ +assertEquals($expected, RouteAttributes::extractRawAttributes($route, $name, $path)); + } + + public function providerTestExtractRawAttributes() { + $data = []; + $data['no-parameters'] = [new Route('/prefix/a'), 'a_route', '/prefix', []]; + $data['no-matching-parameters'] = [new Route('/prefix/{x}'), 'a_route', '/different-prefix/b', []]; + $data['matching-parameters'] = [new Route('/prefix/{x}'), 'a_route', '/prefix/b', ['x' => 'b']]; + $data['with-defaults'] = [new Route('/prefix/{x}', ['foo' => 'bar']), 'a_route', '/different-prefix/b', ['foo' => 'bar']]; + return $data; + } + +} diff --git a/tests/src/Unit/VariantRouteFilterTest.php b/tests/src/Unit/VariantRouteFilterTest.php index 674bd3c..34e2842 100644 --- a/tests/src/Unit/VariantRouteFilterTest.php +++ b/tests/src/Unit/VariantRouteFilterTest.php @@ -12,12 +12,12 @@ use Drupal\Core\Display\ContextAwareVariantInterface; use Drupal\Core\Display\VariantInterface; use Drupal\Core\Entity\EntityManagerInterface; +use Drupal\Core\Path\CurrentPathStack; use Drupal\page_manager\PageExecutableInterface; use Drupal\page_manager\PageInterface; use Drupal\page_manager\Routing\VariantRouteFilter; use Drupal\Tests\UnitTestCase; use Symfony\Cmf\Component\Routing\Enhancer\RouteEnhancerInterface; -use Symfony\Cmf\Component\Routing\NestedMatcher\FinalMatcherInterface; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -43,11 +43,11 @@ class VariantRouteFilterTest extends UnitTestCase { protected $pageStorage; /** - * The mocked final routing matcher. + * The mocked current path stack. * - * @var \Symfony\Cmf\Component\Routing\NestedMatcher\FinalMatcherInterface|\Prophecy\Prophecy\ProphecyInterface + * @var \Drupal\Core\Path\CurrentPathStack|\Prophecy\Prophecy\ProphecyInterface */ - protected $finalMatcher; + protected $currentPath; /** * The route filter under test. @@ -65,9 +65,9 @@ protected function setUp() { $this->entityManager = $this->prophesize(EntityManagerInterface::class); $this->entityManager->getStorage('page') ->willReturn($this->pageStorage); - $this->finalMatcher = $this->prophesize(FinalMatcherInterface::class); + $this->currentPath = $this->prophesize(CurrentPathStack::class); - $this->routeFilter = new VariantRouteFilter($this->entityManager->reveal(), $this->finalMatcher->reveal()); + $this->routeFilter = new VariantRouteFilter($this->entityManager->reveal(), $this->currentPath->reveal()); } /** @@ -95,15 +95,17 @@ public function testFilterEmptyCollection() { $route_collection = new RouteCollection(); $request = new Request(); - $this->finalMatcher->finalMatch($route_collection, $request)->shouldNotBeCalled(); + $this->currentPath->getPath($request)->shouldNotBeCalled(); $result = $this->routeFilter->filter($route_collection, $request); $expected = []; $this->assertSame($expected, $result->all()); + $this->assertSame([], $request->attributes->all()); } /** * @covers ::filter + * @covers ::processRoute */ public function testFilterContextException() { $route_collection = new RouteCollection(); @@ -121,16 +123,18 @@ public function testFilterContextException() { $page = $this->prophesize(PageInterface::class); $page->getExecutable()->willReturn($executable->reveal()); - $this->finalMatcher->finalMatch($route_collection, $request)->willReturn([]); + $this->currentPath->getPath($request)->willReturn(''); $this->pageStorage->load('a_page')->willReturn($page->reveal()); $result = $this->routeFilter->filter($route_collection, $request); $expected = []; $this->assertSame($expected, $result->all()); + $this->assertSame([], $request->attributes->all()); } /** * @covers ::filter + * @covers ::processRoute */ public function testFilterNonMatchingRoute() { $route_collection = new RouteCollection(); @@ -139,15 +143,17 @@ public function testFilterNonMatchingRoute() { $route = new Route('/path/with/{slug}'); $route_collection->add('a_route', $route); - $this->finalMatcher->finalMatch($route_collection, $request)->willReturn([]); + $this->currentPath->getPath($request)->willReturn(''); $result = $this->routeFilter->filter($route_collection, $request); $expected = ['a_route' => $route]; $this->assertSame($expected, $result->all()); + $this->assertSame([], $request->attributes->all()); } /** * @covers ::filter + * @covers ::processRoute */ public function testFilterDeniedAccess() { $route_collection = new RouteCollection(); @@ -165,16 +171,18 @@ public function testFilterDeniedAccess() { $page = $this->prophesize(PageInterface::class); $page->getExecutable()->willReturn($executable->reveal()); - $this->finalMatcher->finalMatch($route_collection, $request)->willReturn([]); + $this->currentPath->getPath($request)->willReturn(''); $this->pageStorage->load('a_page')->willReturn($page->reveal()); $result = $this->routeFilter->filter($route_collection, $request); $expected = []; $this->assertSame($expected, $result->all()); + $this->assertSame([], $request->attributes->all()); } /** * @covers ::filter + * @covers ::processRoute */ public function testFilterAllowedAccess() { $route_collection = new RouteCollection(); @@ -192,16 +200,18 @@ public function testFilterAllowedAccess() { $page = $this->prophesize(PageInterface::class); $page->getExecutable()->willReturn($executable->reveal()); - $this->finalMatcher->finalMatch($route_collection, $request)->willReturn([]); + $this->currentPath->getPath($request)->willReturn(''); $this->pageStorage->load('a_page')->willReturn($page->reveal()); $result = $this->routeFilter->filter($route_collection, $request); $expected = ['a_route' => $route]; $this->assertSame($expected, $result->all()); + $this->assertSame([], $request->attributes->all()); } /** * @covers ::filter + * @covers ::processRoute */ public function testFilterAllowedAccessTwoRoutes() { $route_collection = new RouteCollection(); @@ -222,17 +232,19 @@ public function testFilterAllowedAccessTwoRoutes() { $page = $this->prophesize(PageInterface::class); $page->getExecutable()->willReturn($executable->reveal()); - $this->finalMatcher->finalMatch($route_collection, $request)->willReturn([]); + $this->currentPath->getPath($request)->willReturn(''); $this->pageStorage->load('page_1')->willReturn($page->reveal()); $this->pageStorage->load('page_2')->shouldNotBeCalled(); $result = $this->routeFilter->filter($route_collection, $request); $expected = ['route_1' => $route1]; $this->assertSame($expected, $result->all()); + $this->assertSame([], $request->attributes->all()); } /** * @covers ::filter + * @covers ::processRoute */ public function testFilterAllowedAccessSecondRoute() { $route_collection = new RouteCollection(); @@ -258,17 +270,19 @@ public function testFilterAllowedAccessSecondRoute() { $page2 = $this->prophesize(PageInterface::class); $page2->getExecutable()->willReturn($executable->reveal()); - $this->finalMatcher->finalMatch($route_collection, $request)->willReturn([]); + $this->currentPath->getPath($request)->willReturn(''); $this->pageStorage->load('page_1')->willReturn($page1->reveal())->shouldBeCalled(); $this->pageStorage->load('page_2')->willReturn($page2->reveal())->shouldBeCalled(); $result = $this->routeFilter->filter($route_collection, $request); $expected = ['route_2' => $route2]; $this->assertSame($expected, $result->all()); + $this->assertSame([], $request->attributes->all()); } /** * @covers ::filter + * @covers ::processRoute * * Tests when the first page_manager route is allowed, but other * non-page_manager routes are also present. @@ -298,37 +312,66 @@ public function testFilterAllowedAccessFirstRoute() { $page = $this->prophesize(PageInterface::class); $page->getExecutable()->willReturn($executable->reveal()); - $this->finalMatcher->finalMatch($route_collection, $request)->willReturn([]); + $this->currentPath->getPath($request)->willReturn(''); $this->pageStorage->load('a_page')->willReturn($page->reveal())->shouldBeCalled(); $result = $this->routeFilter->filter($route_collection, $request); $expected = ['route_2' => $route2, 'route_1' => $route1, 'route_4' => $route4]; $this->assertSame($expected, $result->all()); + $this->assertSame([], $request->attributes->all()); } /** - * @covers ::prepareRequest + * @covers ::filter */ - public function testPrepareRequest() { + public function testFilterRequestAttributes() { $route_collection = new RouteCollection(); + $request = new Request([], [], ['foo' => 'bar']); + + $route = new Route('/path/with/{slug}', ['page_manager_page' => 'a_page', 'variant_id' => 'a_variant']); + $route_collection->add('a_route', $route); + + $variant = $this->prophesize(ContextAwareVariantInterface::class); + $variant->access()->willReturn(TRUE); + + $executable = $this->prophesize(PageExecutableInterface::class); + $executable->getRuntimeVariant('a_variant')->willReturn($variant); + + $page = $this->prophesize(PageInterface::class); + $page->getExecutable()->willReturn($executable->reveal()); + + $this->currentPath->getPath($request)->willReturn(''); + $this->pageStorage->load('a_page')->willReturn($page->reveal()); + + $result = $this->routeFilter->filter($route_collection, $request); + $expected = ['a_route' => $route]; + $this->assertSame($expected, $result->all()); + $this->assertSame(['foo' => 'bar'], $request->attributes->all()); + } + + /** + * @covers ::getRequestAttributes + */ + public function testGetRequestAttributes() { $request = new Request(); $route = new Route('/path/with/{slug}'); - $route_collection->add('a_route', $route); + $route_name = 'a_route'; - $this->finalMatcher->finalMatch($route_collection, $request)->willReturn(['slug' => 1]); + $this->currentPath->getPath($request)->willReturn('/path/with/1'); + $expected_attributes = ['slug' => 1, '_route_object' => $route, '_route' => $route_name]; $route_enhancer = $this->prophesize(RouteEnhancerInterface::class); - $route_enhancer->enhance(['slug' => 1], $request)->willReturn(['slug' => 'slug 1']); + $route_enhancer->enhance($expected_attributes, $request)->willReturn(['slug' => 'slug 1']); $this->routeFilter->addRouteEnhancer($route_enhancer->reveal()); $this->assertSame([], $request->attributes->all()); - $method = new \ReflectionMethod($this->routeFilter, 'prepareRequest'); + $method = new \ReflectionMethod($this->routeFilter, 'getRequestAttributes'); $method->setAccessible(TRUE); - $method->invoke($this->routeFilter, $route_collection, $request); + $attributes = $method->invoke($this->routeFilter, $route, $route_name, $request); - $this->assertSame(['slug' => 'slug 1'], $request->attributes->all()); + $this->assertSame(['slug' => 'slug 1'], $attributes); } }