Skip to content

Commit

Permalink
Rewrite candidates
Browse files Browse the repository at this point in the history
  • Loading branch information
aschempp committed Jul 10, 2020
1 parent 2e507a4 commit a63f337
Show file tree
Hide file tree
Showing 15 changed files with 638 additions and 305 deletions.
2 changes: 1 addition & 1 deletion core-bundle/src/Controller/Page/RootPageController.php
Expand Up @@ -42,7 +42,7 @@ public function __invoke(PageModel $pageModel): Response
throw new \InvalidArgumentException('Invalid page type');
}

return $this->redirectToContent($this->getNextPage((int) $pageModel->id), [], 303);
return $this->redirectToContent($this->getNextPage((int) $pageModel->id));
}

public function enhancePageRoute(PageRoute $route): Route
Expand Down
3 changes: 1 addition & 2 deletions core-bundle/src/Resources/config/legacy_routing.yml
Expand Up @@ -10,9 +10,8 @@ services:
tags:
- routing.loader

contao.routing.legacy_candidates:
contao.routing.candidates:
class: Contao\CoreBundle\Routing\Candidates\LegacyCandidates
decorates: contao.routing.candidates
arguments:
- '%contao.prepend_locale%'
- '%contao.url_suffix%'
Expand Down
10 changes: 9 additions & 1 deletion core-bundle/src/Resources/config/services.yml
Expand Up @@ -461,11 +461,16 @@ services:
- [matchAttribute, [_scope, backend]]

contao.routing.candidates:
class: Contao\CoreBundle\Routing\Candidates\Candidates
class: Contao\CoreBundle\Routing\Candidates\PageCandidates
arguments:
- '@database_connection'
- '@Contao\CoreBundle\Routing\Page\PageRegistry'

contao.routing.locale_candidates:
class: Contao\CoreBundle\Routing\Candidates\LocaleCandidates
arguments:
- '@Contao\CoreBundle\Routing\Page\PageRegistry'

contao.routing.domain_filter:
class: Contao\CoreBundle\Routing\Matcher\DomainFilter

Expand Down Expand Up @@ -571,6 +576,9 @@ services:
class: Contao\CoreBundle\Routing\Route404Provider
arguments:
- '@contao.framework'
- '@database_connection'
- '@contao.routing.locale_candidates'
- '@Contao\CoreBundle\Routing\RouteFactory'

contao.routing.scope_matcher:
class: Contao\CoreBundle\Routing\ScopeMatcher
Expand Down
81 changes: 81 additions & 0 deletions core-bundle/src/Routing/CandidatePagesTrait.php
@@ -0,0 +1,81 @@
<?php

declare(strict_types=1);

/*
* This file is part of Contao.
*
* (c) Leo Feyer
*
* @license LGPL-3.0-or-later
*/

namespace Contao\CoreBundle\Routing;

use Contao\CoreBundle\Framework\ContaoFramework;
use Contao\Model\Collection;
use Contao\PageModel;
use Doctrine\DBAL\Connection;
use Symfony\Cmf\Component\Routing\Candidates\CandidatesInterface;
use Symfony\Component\HttpFoundation\Request;

trait CandidatePagesTrait
{
/**
* @var ContaoFramework
*/
private $framework;

/**
* @var Connection
*/
private $connection;

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

This comment has been minimized.

Copy link
@m-vo

m-vo Jul 11, 2020

Member

I'm not sure about best practice regarding services in traits. I find it a bit strange that I have to 'share' my private variables holding service references with the trait (and I have to initialize it for it). But maybe that's how it works.

I've seen some examples in the Symfony codebase where they added a init() method that gets called from the constructor of the class that is using the trait (but never with a service).

(If this isn't intended to be used outside the core, we should probably add @internal.)

This comment has been minimized.

Copy link
@m-vo

m-vo Jul 11, 2020

Member

Another thought: The variables could also be defined in the class only (and replaced with @property annotations in the trait). But it's coupling this or that way. 🙃


/**
* @return array<string>
*/
private function findCandidatePages(Request $request): array
{
$candidates = $this->candidates->getCandidates($request);

if (empty($candidates)) {
return [];
}

$ids = [];
$aliases = [];

foreach ($candidates as $candidate) {
if (is_numeric($candidate)) {
$ids[] = (int) $candidate;
} else {
$aliases[] = $this->connection->quote($candidate);
}
}

$conditions = [];

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

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

/** @var PageModel $pageModel */
$pageModel = $this->framework->getAdapter(PageModel::class);
$pages = $pageModel->findBy([implode(' OR ', $conditions)], []);

if (!$pages instanceof Collection) {
return [];
}

return $pages->getModels();
}
}
Expand Up @@ -12,13 +12,10 @@

namespace Contao\CoreBundle\Routing\Candidates;

use Contao\CoreBundle\Routing\Page\PageRegistry;
use Doctrine\DBAL\Connection;
use Doctrine\DBAL\FetchMode;
use Symfony\Cmf\Component\Routing\Candidates\CandidatesInterface;
use Symfony\Component\HttpFoundation\Request;

class Candidates implements CandidatesInterface
class AbstractCandidates implements CandidatesInterface
{
/**
* A limit to apply to the number of candidates generated.
Expand All @@ -31,21 +28,6 @@ class Candidates implements CandidatesInterface
*/
private const LIMIT = 20;

/**
* @var Connection
*/
private $connection;

/**
* @var PageRegistry
*/
private $pageRegistry;

/**
* @var bool
*/
private $initialized = false;

/**
* @var array
*/
Expand All @@ -56,10 +38,10 @@ class Candidates implements CandidatesInterface
*/
private $urlSuffixes;

public function __construct(Connection $connection, PageRegistry $pageRegistry)
public function __construct(array $urlPrefixes, array $urlSuffixes)
{
$this->connection = $connection;
$this->pageRegistry = $pageRegistry;
$this->urlPrefixes = $urlPrefixes;
$this->urlSuffixes = $urlSuffixes;
}

public function isCandidate($name): bool
Expand Down Expand Up @@ -100,8 +82,6 @@ public function restrictQuery($queryBuilder): void
*/
public function getCandidates(Request $request): array
{
$this->initialize();

$url = $request->getPathInfo();
$url = rawurldecode(ltrim($url, '/'));
$candidates = [];
Expand Down Expand Up @@ -145,7 +125,17 @@ public function getCandidates(Request $request): array
return array_values(array_unique($candidates));
}

protected function addCandidatesFor(string $url, array &$candidates): void
protected function setUrlPrefixes(array $urlPrefixes): void
{
$this->urlPrefixes = $urlPrefixes;
}

protected function setUrlSuffixes(array $urlSuffixes): void
{
$this->urlSuffixes = $urlSuffixes;
}

private function addCandidatesFor(string $url, array &$candidates): void
{
if ('' === $url) {
$candidates[] = 'index';
Expand All @@ -168,21 +158,4 @@ protected function addCandidatesFor(string $url, array &$candidates): void

$candidates[] = $part;
}

private function initialize(): void
{
if ($this->initialized) {
return;
}

$this->initialized = true;

$urlPrefix = $this->connection
->query("SELECT DISTINCT urlPrefix FROM tl_page WHERE type='root'")
->fetchAll(FetchMode::COLUMN)
;

$this->urlSuffixes = $this->pageRegistry->getUrlSuffixes();
$this->urlPrefixes = $urlPrefix;
}
}
52 changes: 17 additions & 35 deletions core-bundle/src/Routing/Candidates/LegacyCandidates.php
Expand Up @@ -19,69 +19,51 @@
*
* @deprecated
*/
class LegacyCandidates extends Candidates
class LegacyCandidates extends AbstractCandidates
{
/**
* @var bool
*/
private $prependLocale;

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

public function __construct(bool $prependLocale, string $urlSuffix)
{
// Do not call the parent constructor
parent::__construct([], [$urlSuffix]);

$this->prependLocale = $prependLocale;
$this->urlSuffix = $urlSuffix;
}

/**
* Override the URL prefix based on the current request.
*/
public function getCandidates(Request $request): array
{
$url = $request->getPathInfo();
$url = rawurldecode(ltrim($url, '/'));
$candidates = [];
$prefix = $this->getPrefixFromUrl($request);

$url = $this->removeSuffixAndLanguage($url);

if (null === $url) {
if (null === $prefix) {
return [];
}

$this->addCandidatesFor($url, $candidates);
$this->setUrlPrefixes([$prefix]);

return array_values(array_unique($candidates));
return parent::getCandidates($request);
}

private function removeSuffixAndLanguage(string $pathInfo): ?string
private function getPrefixFromUrl(Request $request): ?string
{
if ('' === $pathInfo) {
if (!$this->prependLocale) {
return '';
}

$suffixLength = \strlen($this->urlSuffix);

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

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

if ($this->prependLocale) {
$matches = [];
$url = $request->getPathInfo();
$url = rawurldecode(ltrim($url, '/'));

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

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

return $pathInfo;
return $matches[1];
}
}
57 changes: 57 additions & 0 deletions core-bundle/src/Routing/Candidates/LocaleCandidates.php
@@ -0,0 +1,57 @@
<?php

declare(strict_types=1);

/*
* This file is part of Contao.
*
* (c) Leo Feyer
*
* @license LGPL-3.0-or-later
*/

namespace Contao\CoreBundle\Routing\Candidates;

use Contao\CoreBundle\Routing\Page\PageRegistry;
use Symfony\Component\HttpFoundation\Request;

class LocaleCandidates extends AbstractCandidates
{
/**
* @var PageRegistry
*/
private $pageRegistry;

/**
* @var bool
*/
private $initialized = false;

public function __construct(PageRegistry $pageRegistry)
{
parent::__construct([''], []);

$this->pageRegistry = $pageRegistry;
}

public function getCandidates(Request $request): array
{
$this->initialize();

return parent::getCandidates($request);
}

/**
* Lazy-initialize because we don't want to query the database when creating the service.
*/
private function initialize(): void
{
if ($this->initialized) {
return;
}

$this->initialized = true;

$this->setUrlSuffixes($this->pageRegistry->getUrlSuffixes());
}
}

0 comments on commit a63f337

Please sign in to comment.