Skip to content

Commit

Permalink
Make the RouteFactory more generic
Browse files Browse the repository at this point in the history
  • Loading branch information
aschempp committed Jul 9, 2020
1 parent 4692bcc commit 502276a
Show file tree
Hide file tree
Showing 10 changed files with 82 additions and 73 deletions.
13 changes: 7 additions & 6 deletions core-bundle/src/Resources/config/services.yml
Original file line number Diff line number Diff line change
Expand Up @@ -517,10 +517,6 @@ services:

Contao\CoreBundle\Routing\Page\PageRegistry: ~

Contao\CoreBundle\Routing\Page\PageRouteFactory:
arguments:
- '@Contao\CoreBundle\Routing\Page\PageRegistry'

contao.routing.page_router:
class: Symfony\Cmf\Component\Routing\DynamicRouter
arguments:
Expand Down Expand Up @@ -550,10 +546,15 @@ services:
arguments:
- '@contao.security.token_checker'

Contao\CoreBundle\Routing\RouteFactory:
arguments:
- '@Contao\CoreBundle\Routing\Page\PageRegistry'
- !tagged_iterator contao.content_route_provider

contao.routing.route_generator:
class: Contao\CoreBundle\Routing\ContentResolvingGenerator
arguments:
- !tagged_iterator contao.content_route_provider
- '@Contao\CoreBundle\Routing\RouteFactory'
- '@?logger'

contao.routing.route_provider:
Expand All @@ -562,7 +563,7 @@ services:
- '@contao.framework'
- '@database_connection'
- '@contao.routing.candidates'
- '@Contao\CoreBundle\Routing\Page\PageRouteFactory'
- '@Contao\CoreBundle\Routing\RouteFactory'
- '%contao.legacy_routing%'
- '%contao.prepend_locale%'

Expand Down
8 changes: 4 additions & 4 deletions core-bundle/src/Routing/Content/ArticleRouteProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,19 +13,19 @@
namespace Contao\CoreBundle\Routing\Content;

use Contao\ArticleModel;
use Contao\CoreBundle\Routing\Page\PageRouteFactory;
use Contao\CoreBundle\Routing\RouteFactory;
use Contao\PageModel;
use Symfony\Component\Routing\Exception\RouteNotFoundException;
use Symfony\Component\Routing\Route;

class ArticleRouteProvider implements ContentRouteProviderInterface
{
/**
* @var PageRouteFactory
* @var RouteFactory
*/
private $routeFactory;

public function __construct(PageRouteFactory $routeFactory)
public function __construct(RouteFactory $routeFactory)
{
$this->routeFactory = $routeFactory;
}
Expand All @@ -42,7 +42,7 @@ public function getRouteForContent($article): Route
throw new RouteNotFoundException(sprintf('Page ID %s for article ID %s not found', $article->pid, $article->id));
}

return $this->routeFactory->createRoute($page, '/articles/'.($article->alias ?: $article->id), $article);
return $this->routeFactory->createRouteForPage($page, '/articles/'.($article->alias ?: $article->id), $article);
}

public function supportsContent($content): bool
Expand Down
8 changes: 4 additions & 4 deletions core-bundle/src/Routing/Content/PageRouteProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -12,18 +12,18 @@

namespace Contao\CoreBundle\Routing\Content;

use Contao\CoreBundle\Routing\Page\PageRouteFactory;
use Contao\CoreBundle\Routing\RouteFactory;
use Contao\PageModel;
use Symfony\Component\Routing\Route;

class PageRouteProvider implements ContentRouteProviderInterface
{
/**
* @var PageRouteFactory
* @var RouteFactory
*/
private $routeFactory;

public function __construct(PageRouteFactory $routeFactory)
public function __construct(RouteFactory $routeFactory)
{
$this->routeFactory = $routeFactory;
}
Expand All @@ -33,7 +33,7 @@ public function __construct(PageRouteFactory $routeFactory)
*/
public function getRouteForContent($page): Route
{
return $this->routeFactory->createRoute($page);
return $this->routeFactory->createRouteForPage($page);
}

public function supportsContent($content): bool
Expand Down
27 changes: 5 additions & 22 deletions core-bundle/src/Routing/ContentResolvingGenerator.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,26 +13,24 @@
namespace Contao\CoreBundle\Routing;

use Contao\CoreBundle\Exception\ContentRouteNotFoundException;
use Contao\CoreBundle\Routing\Content\ContentRouteProviderInterface;
use Contao\CoreBundle\Routing\Page\PageRoute;
use Psr\Log\LoggerInterface;
use Symfony\Component\Routing\Generator\UrlGenerator as SymfonyUrlGenerator;
use Symfony\Component\Routing\RequestContext;
use Symfony\Component\Routing\Route;
use Symfony\Component\Routing\RouteCollection;

class ContentResolvingGenerator extends SymfonyUrlGenerator
{
/**
* @var array<ContentRouteProviderInterface>
* @var RouteFactory
*/
private $routeProviders;
private $routeFactory;

public function __construct(iterable $routeProviders, LoggerInterface $logger = null)
public function __construct(RouteFactory $routeFactory, LoggerInterface $logger = null)
{
parent::__construct(new RouteCollection(), new RequestContext(), $logger);

$this->routeProviders = $routeProviders;
$this->routeFactory = $routeFactory;
}

/**
Expand All @@ -46,7 +44,7 @@ public function generate($name, $parameters = [], $referenceType = self::ABSOLUT
throw new ContentRouteNotFoundException($name);
}

$route = $this->resolveContent($parameters[PageRoute::CONTENT_PARAMETER]);
$route = $this->routeFactory->createRouteForContent($parameters[PageRoute::CONTENT_PARAMETER]);
unset($parameters[PageRoute::CONTENT_PARAMETER]);

// the Route has a cache of its own and is not recompiled as long as it does not get modified
Expand All @@ -56,19 +54,4 @@ public function generate($name, $parameters = [], $referenceType = self::ABSOLUT

return $this->doGenerate($compiledRoute->getVariables(), $route->getDefaults(), $route->getRequirements(), $compiledRoute->getTokens(), $parameters, $debug_message, $referenceType, $compiledRoute->getHostTokens(), $route->getSchemes());
}

private function resolveContent($content): Route
{
if ($content instanceof Route) {
return $content;
}

foreach ($this->routeProviders as $provider) {
if ($provider->supportsContent($content)) {
return $provider->getRouteForContent($content);
}
}

throw new ContentRouteNotFoundException($content);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,32 @@
* @license LGPL-3.0-or-later
*/

namespace Contao\CoreBundle\Routing\Page;
namespace Contao\CoreBundle\Routing;

use Contao\CoreBundle\Exception\ContentRouteNotFoundException;
use Contao\CoreBundle\Routing\Content\ContentRouteProviderInterface;
use Contao\CoreBundle\Routing\Page\PageRegistry;
use Contao\CoreBundle\Routing\Page\PageRoute;
use Contao\CoreBundle\Routing\Page\RouteConfig;
use Contao\PageModel;
use Symfony\Component\Routing\Route;

class PageRouteFactory
class RouteFactory
{
/**
* @var PageRegistry
*/
private $pageRegistry;

public function __construct(PageRegistry $pageRegistry)
/**
* @var array<ContentRouteProviderInterface>
*/
private $routeProviders;

public function __construct(PageRegistry $pageRegistry, iterable $routeProviders)
{
$this->pageRegistry = $pageRegistry;
$this->routeProviders = $routeProviders;
}

/**
Expand All @@ -38,7 +49,7 @@ public function __construct(PageRegistry $pageRegistry)
*
* A route enhancer might change or replace the route for a specific page.
*/
public function createRoute(PageModel $pageModel, string $defaultParameters = '', $content = null): Route
public function createRouteForPage(PageModel $pageModel, string $defaultParameters = '', $content = null): Route
{
$config = $this->pageRegistry->getRouteConfig($pageModel->type) ?: new RouteConfig();
$pathParameters = $config->getPathParameters();
Expand All @@ -58,4 +69,19 @@ public function createRoute(PageModel $pageModel, string $defaultParameters = ''

return $this->pageRegistry->enhancePageRoute($route);
}

public function createRouteForContent($content): Route
{
if ($content instanceof Route) {
return $content;
}

foreach ($this->routeProviders as $provider) {
if ($provider->supportsContent($content)) {
return $provider->getRouteForContent($content);
}
}

throw new ContentRouteNotFoundException($content);
}
}
9 changes: 4 additions & 5 deletions core-bundle/src/Routing/RouteProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
use Contao\CoreBundle\Exception\NoRootPageFoundException;
use Contao\CoreBundle\Framework\ContaoFramework;
use Contao\CoreBundle\Routing\Page\PageRoute;
use Contao\CoreBundle\Routing\Page\PageRouteFactory;
use Contao\Model;
use Contao\Model\Collection;
use Contao\PageModel;
Expand Down Expand Up @@ -47,7 +46,7 @@ class RouteProvider implements RouteProviderInterface
private $candidates;

/**
* @var PageRouteFactory
* @var RouteFactory
*/
private $routeFactory;

Expand All @@ -64,7 +63,7 @@ class RouteProvider implements RouteProviderInterface
/**
* @internal Do not inherit from this class; decorate the "contao.routing.route_provider" service instead
*/
public function __construct(ContaoFramework $framework, Connection $database, CandidatesInterface $candidates, PageRouteFactory $routeFactory, bool $legacyRouting, bool $prependLocale)
public function __construct(ContaoFramework $framework, Connection $database, CandidatesInterface $candidates, RouteFactory $routeFactory, bool $legacyRouting, bool $prependLocale)
{
$this->framework = $framework;
$this->database = $database;
Expand Down Expand Up @@ -207,7 +206,7 @@ private function addRoutesForPage(PageModel $page, array &$routes, Request $requ
return;
}

$route = $this->routeFactory->createRoute($page);
$route = $this->routeFactory->createRouteForPage($page);
$routes['tl_page.'.$page->id] = $route;

if ($route instanceof PageRoute) {
Expand Down Expand Up @@ -257,7 +256,7 @@ private function addRoutesForRootPage(PageModel $page, array &$routes): void
}

$page->loadDetails();
$route = $this->routeFactory->createRoute($page);
$route = $this->routeFactory->createRouteForPage($page);
$urlPrefix = '';

if ($route instanceof PageRoute) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,8 +112,8 @@
use Contao\CoreBundle\Routing\Matcher\LegacyMatcher;
use Contao\CoreBundle\Routing\Matcher\PublishedFilter;
use Contao\CoreBundle\Routing\Matcher\UrlMatcher;
use Contao\CoreBundle\Routing\Page\PageRouteFactory;
use Contao\CoreBundle\Routing\Route404Provider;
use Contao\CoreBundle\Routing\RouteFactory;
use Contao\CoreBundle\Routing\RouteProvider;
use Contao\CoreBundle\Routing\ScopeMatcher;
use Contao\CoreBundle\Routing\UrlGenerator;
Expand Down Expand Up @@ -2846,7 +2846,7 @@ public function testRegistersTheRoutingRouteProvider(): void
new Reference('contao.framework'),
new Reference('database_connection'),
new Reference('contao.routing.candidates'),
new Reference(PageRouteFactory::class),
new Reference(RouteFactory::class),
new Reference('%contao.legacy_routing%'),
new Reference('%contao.prepend_locale%'),
],
Expand Down
10 changes: 5 additions & 5 deletions core-bundle/tests/Routing/Content/ArticleRouteProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use Contao\ArticleModel;
use Contao\CoreBundle\Routing\Content\ArticleRouteProvider;
use Contao\CoreBundle\Routing\Page\PageRouteFactory;
use Contao\CoreBundle\Routing\RouteFactory;
use Contao\CoreBundle\Tests\TestCase;
use Contao\PageModel;
use PHPUnit\Framework\MockObject\MockObject;
Expand All @@ -24,7 +24,7 @@
class ArticleRouteProviderTest extends TestCase
{
/**
* @var PageRouteFactory|MockObject
* @var RouteFactory|MockObject
*/
private $routeFactory;

Expand All @@ -35,7 +35,7 @@ class ArticleRouteProviderTest extends TestCase

protected function setUp(): void
{
$this->routeFactory = $this->createMock(PageRouteFactory::class);
$this->routeFactory = $this->createMock(RouteFactory::class);
$this->provider = new ArticleRouteProvider($this->routeFactory);
}

Expand All @@ -60,7 +60,7 @@ public function testCreatesParameteredContentRouteForArticle(): void

$this->routeFactory
->expects($this->once())
->method('createRoute')
->method('createRouteForPage')
->with($page, '/articles/foobar', $article)
->willReturn($route)
;
Expand All @@ -83,7 +83,7 @@ public function testCreatesParameteredContentRouteWithIdIfArticleHasNoAlias(): v

$this->routeFactory
->expects($this->once())
->method('createRoute')
->method('createRouteForPage')
->with($page, '/articles/17', $article)
->willReturn($route)
;
Expand Down
8 changes: 4 additions & 4 deletions core-bundle/tests/Routing/Content/PageRouteProviderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

use Contao\ArticleModel;
use Contao\CoreBundle\Routing\Content\PageRouteProvider;
use Contao\CoreBundle\Routing\Page\PageRouteFactory;
use Contao\CoreBundle\Routing\RouteFactory;
use Contao\CoreBundle\Tests\TestCase;
use Contao\PageModel;
use PHPUnit\Framework\MockObject\MockObject;
Expand All @@ -23,7 +23,7 @@
class PageRouteProviderTest extends TestCase
{
/**
* @var PageRouteFactory|MockObject
* @var RouteFactory|MockObject
*/
private $routeFactory;

Expand All @@ -34,7 +34,7 @@ class PageRouteProviderTest extends TestCase

protected function setUp(): void
{
$this->routeFactory = $this->createMock(PageRouteFactory::class);
$this->routeFactory = $this->createMock(RouteFactory::class);
$this->provider = new PageRouteProvider($this->routeFactory);
}

Expand All @@ -51,7 +51,7 @@ public function testCreatesParameteredContentRoute(): void

$this->routeFactory
->expects($this->once())
->method('createRoute')
->method('createRouteForPage')
->with($page)
->willReturn($route)
;
Expand Down
Loading

0 comments on commit 502276a

Please sign in to comment.