Skip to content

Commit

Permalink
Update RouteProvider to use candidates based on root page
Browse files Browse the repository at this point in the history
  • Loading branch information
@aschempp
aschempp committed Apr 8, 2020
1 parent 29bd111 commit 510da35
Show file tree
Hide file tree
Showing 5 changed files with 87 additions and 173 deletions.
2 changes: 1 addition & 1 deletion core-bundle/src/Resources/config/services.yml
View file
Expand Up @@ -579,7 +579,7 @@ services:
arguments:
- '@contao.framework'
- '@database_connection'
- '%contao.url_suffix%'
- '@contao.routing.candidates'
- '%contao.prepend_locale%'

contao.routing.route_404_provider:
Expand Down
70 changes: 9 additions & 61 deletions core-bundle/src/Routing/RouteProvider.php
View file
Expand Up @@ -20,6 +20,7 @@
use Contao\PageModel;
use Contao\System;
use Doctrine\DBAL\Connection;
use Symfony\Cmf\Component\Routing\Candidates\CandidatesInterface;
use Symfony\Cmf\Component\Routing\RouteProviderInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Exception\RouteNotFoundException;
Expand All @@ -39,9 +40,9 @@ class RouteProvider implements RouteProviderInterface
private $database;

/**
* @var string
* @var CandidatesInterface
*/
private $urlSuffix;
private $candidates;

/**
* @var bool
Expand All @@ -51,11 +52,11 @@ 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, string $urlSuffix, bool $prependLocale)
public function __construct(ContaoFramework $framework, Connection $database, CandidatesInterface $candidates, bool $prependLocale)
{
$this->framework = $framework;
$this->database = $database;
$this->urlSuffix = $urlSuffix;
$this->candidates = $candidates;
$this->prependLocale = $prependLocale;
}

Expand All @@ -72,19 +73,18 @@ public function getRouteCollectionForRequest(Request $request): RouteCollection

$routes = [];

if ('/' === $pathInfo || ($this->prependLocale && preg_match('@^/([a-z]{2}(-[A-Z]{2})?)/$@', $pathInfo))) {
if ('/' === $pathInfo || ($this->framework->isLegacyRouting() && $this->prependLocale && preg_match('@^/([a-z]{2}(-[A-Z]{2})?)/$@', $pathInfo))) {
$this->addRoutesForRootPages($this->findRootPages($request->getHttpHost()), $routes);

return $this->createCollectionForRoutes($routes, $request->getLanguages());
}

$pathInfo = $this->removeSuffixAndLanguage($pathInfo);
$candidates = $this->candidates->getCandidates($request);

if (null === $pathInfo) {
if (0 === \count($candidates)) {
return new RouteCollection();
}

$candidates = $this->getAliasCandidates($pathInfo);
$pages = $this->findPages($candidates);

$this->addRoutesForPages($pages, $routes);
Expand Down Expand Up @@ -148,58 +148,6 @@ public function getRoutesByNames($names): array
return $routes;
}

private function removeSuffixAndLanguage(string $pathInfo): ?string
{
$suffixLength = \strlen($this->urlSuffix);

if (0 !== $suffixLength) {
if (substr($pathInfo, -$suffixLength) !== $this->urlSuffix) {
return null;
}

$pathInfo = substr($pathInfo, 0, -$suffixLength);
}

if (0 === strncmp($pathInfo, '/', 1)) {
$pathInfo = substr($pathInfo, 1);
}

if ($this->prependLocale) {
$matches = [];

if (!preg_match('@^([a-z]{2}(-[A-Z]{2})?)/(.+)$@', $pathInfo, $matches)) {
return null;
}

$pathInfo = $matches[3];
}

return $pathInfo;
}

/**
* Compiles all possible aliases by applying dirname() to the request (e.g. news/archive/item, news/archive, news).
*
* @return array<string>
*/
private function getAliasCandidates(string $pathInfo): array
{
$pos = strpos($pathInfo, '/');

if (false === $pos) {
return [$pathInfo];
}

$candidates = [$pathInfo];

while ('/' !== $pathInfo && false !== strpos($pathInfo, '/')) {
$pathInfo = \dirname($pathInfo);
$candidates[] = $pathInfo;
}

return $candidates;
}

/**
* @param iterable<PageModel> $pages
*/
Expand Down Expand Up @@ -249,7 +197,7 @@ private function addRoutesForPage(PageModel $page, array &$routes): void
$defaults['parameters'] = '';

$requirements = ['parameters' => '(/.+)?'];
$path = sprintf('/%s{parameters}%s', $page->alias ?: $page->id, $this->urlSuffix);
$path = sprintf('/%s{parameters}%s', $page->alias ?: $page->id, $page->urlSuffix);

if ($this->prependLocale) {
$path = '/{_locale}'.$path;
Expand Down
22 changes: 22 additions & 0 deletions core-bundle/tests/Fixtures/Functional/Routing/page-without-alias.yml
View file
@@ -0,0 +1,22 @@
tl_page:
- id: 1
pid: 0
sorting: 128
tstamp: 1539679767
title: Page without alias
alias: page-without-alias
type: root
language: en
fallback: '1'
includeLayout: '1'
layout: 1
published: '1'

- id: 15
pid: 1
sorting: 128
tstamp: 1539698035
title: Home
alias: ''
type: regular
published: '1'
40 changes: 40 additions & 0 deletions core-bundle/tests/Functional/RoutingTest.php
View file
Expand Up @@ -293,6 +293,26 @@ public function getAliases(): \Generator
'root-with-folder-urls.local',
true,
];

yield 'Renders the page if the URL contains a page ID and the page has no alias' => [
['theme', 'page-without-alias'],
'/15.html',
200,
'Home - Page without alias',
['language' => 'en'],
'localhost',
true,
];

yield 'Renders the 404 page if the URL contains a page ID but the page has an alias' => [
['theme', 'root-with-home'],
'/2.html',
404,
'(404 Not Found)',
[],
'root-with-home.local',
true,
];
}

/**
Expand Down Expand Up @@ -543,6 +563,26 @@ public function getAliasesWithLocale(): \Generator
'root-with-folder-urls.local',
true,
];

yield 'Renders the page if the URL contains a page ID and the page has no alias' => [
['theme', 'page-without-alias'],
'/en/15.html',
200,
'Home - Page without alias',
['language' => 'en'],
'localhost',
true,
];

yield 'Renders the 404 page if the URL contains a page ID but the page has an alias' => [
['theme', 'root-with-home'],
'/en/2.html',
404,
'(404 Not Found)',
[],
'root-with-home.local',
true,
];
}

/**
Expand Down
126 changes: 15 additions & 111 deletions core-bundle/tests/Routing/RouteProviderTest.php
View file
Expand Up @@ -22,6 +22,7 @@
use Contao\PageModel;
use Doctrine\DBAL\Connection;
use PHPUnit\Framework\MockObject\MockObject;
use Symfony\Cmf\Component\Routing\Candidates\CandidatesInterface;
use Symfony\Component\HttpFoundation\Request;
use Symfony\Component\Routing\Exception\RouteNotFoundException;
use Symfony\Component\Routing\Route;
Expand Down Expand Up @@ -181,124 +182,19 @@ public function testReturnsAnEmptyCollectionIfThePathContainsAutoItem(): void
public function testReturnsAnEmptyCollectionIfTheUrlSuffixDoesNotMatch(): void
{
$request = $this->mockRequestWithPath('/foo.php');
$provider = $this->getRouteProvider($this->mockFramework($this->mockAdapter(['findBy'])));

$this->assertEmpty($this->getRouteProvider()->getRouteCollectionForRequest($request));
$this->assertEmpty($provider->getRouteCollectionForRequest($request));
}

public function testReturnsAnEmptyCollectionIfTheLanguageIsNotGiven(): void
{
$request = $this->mockRequestWithPath('/foo.html');
$provider = $this->getRouteProvider(null, '.html', true);
$provider = $this->getRouteProvider($this->mockFramework($this->mockAdapter(['findBy'])), '.html', true);

$this->assertEmpty($provider->getRouteCollectionForRequest($request));
}

/**
* @dataProvider getAliasCandidates
*/
public function testFindsPagesByAliasCandidates(string $path, string $urlSuffix, bool $prependLocale, array $aliases, array $ids = []): void
{
$conditions = [];

if (!empty($ids)) {
$conditions[] = 'tl_page.id IN ('.implode(',', $ids).')';
}

if (!empty($aliases)) {
$conditions[] = 'tl_page.alias IN ('.implode(',', $aliases).')';
}

$pageAdapter = $this->mockAdapter(['findBy']);
$pageAdapter
->expects($this->once())
->method('findBy')
->with([implode(' OR ', $conditions)], [])
->willReturn(null)
;

$framework = $this->mockFramework($pageAdapter);
$request = $this->mockRequestWithPath($path);

$provider = $this->getRouteProvider($framework, $urlSuffix, $prependLocale);
$provider->getRouteCollectionForRequest($request);
}

public function getAliasCandidates(): \Generator
{
yield [
'/foo.html',
'.html',
false,
['foo'],
];

yield [
'/bar.php',
'.php',
false,
['bar'],
];

yield [
'/de/foo.html',
'.html',
true,
['foo'],
];

yield [
'/de/foo/bar.html',
'.html',
true,
['foo/bar', 'foo'],
];

yield [
'/foo/bar.html',
'.html',
false,
['foo/bar', 'foo'],
];

yield [
'/foo/bar/baz/some/more.html',
'.html',
false,
['foo/bar/baz/some/more', 'foo/bar/baz/some', 'foo/bar/baz', 'foo/bar', 'foo'],
];

yield [
'/de/foo/bar.html',
'.html',
true,
['foo/bar', 'foo'],
];

yield [
'/15.html',
'.html',
false,
[],
[15],
];

yield [
'/de/15.html',
'.html',
true,
[],
[15],
];

yield [
'/15/foo.html',
'.html',
false,
['15/foo'],
[15],
];
}

/**
* @dataProvider getRoutes
*/
Expand Down Expand Up @@ -478,7 +374,7 @@ public function getRoutes(): \Generator
*/
public function testAddsRoutesForAPage(string $alias, string $language, string $domain, string $urlSuffix, bool $prependLocale, ?string $scheme): void
{
$page = $this->createPage($language, $alias, true, $domain, $scheme);
$page = $this->createPage($language, $alias, true, $domain, $scheme, $urlSuffix);
$page
->expects($this->once())
->method('loadDetails')
Expand Down Expand Up @@ -627,7 +523,7 @@ private function mockFramework(Adapter $pageAdapter = null, Adapter $configAdapt
/**
* @return PageModel&MockObject
*/
private function createPage(string $language, string $alias, bool $fallback = true, string $domain = '', string $scheme = null): PageModel
private function createPage(string $language, string $alias, bool $fallback = true, string $domain = '', string $scheme = null, string $urlSuffix = '.html'): PageModel
{
/** @var PageModel&MockObject $page */
$page = $this->mockClassWithProperties(PageModel::class);
Expand All @@ -636,6 +532,7 @@ private function createPage(string $language, string $alias, bool $fallback = tr
$page->type = 'regular';
$page->alias = $alias;
$page->domain = $domain;
$page->urlSuffix = $urlSuffix;
$page->rootLanguage = $language;
$page->rootIsFallback = $fallback;
$page->rootUseSSL = 'https' === $scheme;
Expand All @@ -659,6 +556,13 @@ private function getRouteProvider(ContaoFramework $framework = null, string $url
->willReturnArgument(0)
;

return new RouteProvider($framework, $connection, $urlSuffix, $prependLocale);
$candidates = $this->createMock(CandidatesInterface::class);
$candidates
->expects($this->any())
->method('getCandidates')
->willReturn(['foo'])
;

return new RouteProvider($framework, $connection, $candidates, $prependLocale);
}
}

0 comments on commit 510da35

Please sign in to comment.