Skip to content

Commit

Permalink
Code review
Browse files Browse the repository at this point in the history
  • Loading branch information
aschempp committed Jul 10, 2020
1 parent e46b79a commit 82fbcc4
Show file tree
Hide file tree
Showing 13 changed files with 56 additions and 45 deletions.
7 changes: 7 additions & 0 deletions core-bundle/src/Resources/config/routing.yml
Expand Up @@ -9,3 +9,10 @@ services:
Contao\CoreBundle\Routing\Content\PageRouteProvider:
arguments:
- '@Contao\CoreBundle\Routing\RouteFactory'

Contao\CoreBundle\Routing\Page\PageRegistry: ~

Contao\CoreBundle\Routing\RouteFactory:
arguments:
- '@Contao\CoreBundle\Routing\Page\PageRegistry'
- !tagged_iterator contao.content_route_provider
7 changes: 0 additions & 7 deletions core-bundle/src/Resources/config/services.yml
Expand Up @@ -520,8 +520,6 @@ services:
- [addRouteFilter, ['@contao.routing.domain_filter']]
- [addRouteFilter, ['@contao.routing.published_filter']]

Contao\CoreBundle\Routing\Page\PageRegistry: ~

contao.routing.page_router:
class: Symfony\Cmf\Component\Routing\DynamicRouter
arguments:
Expand Down Expand Up @@ -551,11 +549,6 @@ 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:
Expand Down
5 changes: 4 additions & 1 deletion core-bundle/src/Resources/contao/classes/Frontend.php
Expand Up @@ -440,7 +440,10 @@ public static function addToUrl($strRequest, $blnIgnoreParams=false, $arrUnset=a
}

// Unset the language parameter
unset($arrGet['language']);
if ($objPage->urlPrefix)
{
unset($arrGet['language']);
}

$strParams = '';
$strConnector = '/';
Expand Down
10 changes: 0 additions & 10 deletions core-bundle/src/Resources/contao/controllers/FrontendIndex.php
Expand Up @@ -180,8 +180,6 @@ public function renderPage($pageModel)

if (preg_match('#^' . $language . $objPage->id . '(' . $suffix . '($|\?)|/)#', Environment::get('relativeRequest')))
{
@trigger_error('Checking for duplicate URLs with ID/alias in FrontendIndex::renderPage() has been deprecated and will no longer work Contao 5.0. Use the Symfony routing instead.', E_USER_DEPRECATED);

throw new PageNotFoundException('Page not found: ' . Environment::get('uri'));
}
}
Expand Down Expand Up @@ -209,8 +207,6 @@ public function renderPage($pageModel)

if (!$hasItem)
{
@trigger_error('Checking for "requireItem" in FrontendIndex::renderPage() has been deprecated and will no longer work Contao 5.0. Use the Symfony routing instead.', E_USER_DEPRECATED);

throw new PageNotFoundException('Page not found: ' . Environment::get('uri'));
}
}
Expand All @@ -221,8 +217,6 @@ public function renderPage($pageModel)
// Trigger the 404 page if the page is not published and the front end preview is not active (see #374)
if (!BE_USER_LOGGED_IN && !$objPage->isPublic)
{
@trigger_error('Checking for published pages in FrontendIndex::renderPage() has been deprecated and will no longer work Contao 5.0. Use the Symfony routing instead.', E_USER_DEPRECATED);

throw new PageNotFoundException('Page not found: ' . Environment::get('uri'));
}

Expand All @@ -248,16 +242,12 @@ public function renderPage($pageModel)
// Do not try to load the 404 page, it can cause an infinite loop!
if (!BE_USER_LOGGED_IN && !$objPage->rootIsPublic)
{
@trigger_error('Checking for published root page in FrontendIndex::renderPage() has been deprecated and will no longer work Contao 5.0. Use the Symfony routing instead.', E_USER_DEPRECATED);

throw new PageNotFoundException('Page not found: ' . Environment::get('uri'));
}

// Check wether the language matches the root page language
if (isset($_GET['language']) && Config::get('addLanguageToUrl') && Input::get('language') != $objPage->rootLanguage)
{
@trigger_error('Checking for page language in FrontendIndex::renderPage() has been deprecated and will no longer work Contao 5.0. Use the Symfony routing instead.', E_USER_DEPRECATED);

throw new PageNotFoundException('Page not found: ' . Environment::get('uri'));
}

Expand Down
2 changes: 1 addition & 1 deletion core-bundle/src/Resources/contao/dca/tl_article.php
Expand Up @@ -773,7 +773,7 @@ public function cutArticle($row, $href, $label, $title, $icon, $attributes)
*/
public function pasteArticle(DataContainer $dc, $row, $table, $cr, $arrClipboard=null)
{
@trigger_error('Deprecated since Contao 4.10, to be removed in Contao 5. Use Contao\CoreBundle\EventListener\DataContainer\ContentCompositionListener::renderArticlePasteButton instead.', E_USER_DEPRECATED);
@trigger_error('tl_article::pasteArticle() is deprecated since Contao 4.10, to be removed in Contao 5. Use ContentCompositionListener::renderArticlePasteButton() instead.', E_USER_DEPRECATED);

return System::getContainer()
->get(ContentCompositionListener::class)
Expand Down
Expand Up @@ -1172,8 +1172,7 @@ protected static function replaceOldBePaths($strContext)
*
* @return string An URL that can be used in the front end
*
* @deprecated Deprecated since Contao 4.2, to be removed in Contao 5.0.
* Use the Symfony router instead.
* @deprecated Deprecated since Contao 4.2, to be removed in Contao 5.0. Use the Symfony router instead.
*/
public static function generateFrontendUrl(array $arrRow, $strParams=null, $strForceLang=null, $blnFixDomain=false)
{
Expand Down
18 changes: 9 additions & 9 deletions core-bundle/src/Resources/contao/library/Contao/Template.php
Expand Up @@ -349,14 +349,14 @@ public function route($strName, $arrParams=array())
/**
* Return a content route relative to the base URL
*
* @param mixed $content The content
* @param array $arrParams The route parameters
* @param mixed $objContent The content
* @param array $arrParams The route parameters
*
* @return string The route
*/
public function routeContent($content, $arrParams=array())
public function contentRoute($objContent, $arrParams=array())
{
$arrParams[PageRoute::CONTENT_PARAMETER] = $content;
$arrParams[PageRoute::CONTENT_PARAMETER] = $objContent;

return $this->route(PageRoute::ROUTE_NAME, $arrParams);
}
Expand Down Expand Up @@ -394,16 +394,16 @@ public function previewRoute($strName, $arrParams=array())
/**
* Return the preview content route
*
* @param mixed $content The content
* @param array $arrParams The route parameters
* @param mixed $objContent The content
* @param array $arrParams The route parameters
*
* @return string The route
*/
public function previewContentRoute($content, $arrParams = array())
public function previewContentRoute($objContent, $arrParams = array())
{
$arrParams[PageRoute::CONTENT_PARAMETER] = $content;
$arrParams[PageRoute::CONTENT_PARAMETER] = $objContent;

return $this->routeContent(PageRoute::ROUTE_NAME, $arrParams);
return $this->contentRoute(PageRoute::ROUTE_NAME, $arrParams);
}

/**
Expand Down
6 changes: 2 additions & 4 deletions core-bundle/src/Resources/contao/pages/PageError404.php
Expand Up @@ -84,8 +84,8 @@ protected function prepare()
// Find the matching root page
$objRootPage = $this->getRootPageFromUrl();

// Forward if the language should be but is not set (see #4028). CMF Routing will take care of this in non-legacy mode.
if (System::getContainer()->getParameter('contao.legacy_routing') && Config::get('addLanguageToUrl'))
// Forward if the language should be but is not set (see #4028).
if ($objRootPage->urlPrefix)
{
// Get the request string without the script name
$strRequest = Environment::get('relativeRequest');
Expand All @@ -100,8 +100,6 @@ protected function prepare()
}
else
{
Input::setGet('language', $objRootPage->language);

if ($strRequest == Environment::get('request'))
{
$strRequest = $objRootPage->language . '/' . $strRequest;
Expand Down
Expand Up @@ -19,14 +19,13 @@
* A content route provider converts content to a URL (represented by a route object).
*
* - Resolve to a Contao\CoreBundle\Routing\Page\PageRoute if the
* content is embedded on a page (e.g. through a "reader" module).
* content is embedded on a page (e.g. through a "reader" module).
*
* - Resolve to a Symfony\Component\Routing\Route if the content's URL
* is not within the CMS (e.g. an absolute URL).
* is not within the CMS (e.g. an absolute URL).
*
* If a provider is responsible for a content object but cannot convert it to a route,
* a Symfony\Component\Routing\Exception\RouteNotFoundException or
* Contao\CoreBundle\Exception\ContentRouteNotFoundException should be thrown
* a Contao\CoreBundle\Exception\ContentRouteNotFoundException should be thrown.
*/
interface ContentRouteProviderInterface
{
Expand Down
2 changes: 0 additions & 2 deletions core-bundle/tests/Fixtures/Functional/Routing/locale.yml

This file was deleted.

2 changes: 0 additions & 2 deletions core-bundle/tests/Fixtures/Functional/Routing/suffix.yml

This file was deleted.

21 changes: 18 additions & 3 deletions core-bundle/tests/Functional/RoutingTest.php
Expand Up @@ -359,9 +359,14 @@ public function getAliases(): \Generator
*/
public function testResolvesAliasesWithLocale(array $fixtures, string $request, int $statusCode, string $pageTitle, array $query, string $host, bool $autoItem): void
{
$fixtures[] = 'locale';
$this->loadFixtureFiles($fixtures);

self::$container
->get('doctrine')
->getConnection()
->exec('UPDATE tl_page SET urlPrefix=language')
;

Config::set('useAutoItem', $autoItem);

$_SERVER['REQUEST_URI'] = $request;
Expand Down Expand Up @@ -666,9 +671,14 @@ public function getAliasesWithLocale(): \Generator
*/
public function testResolvesAliasesWithoutUrlSuffix(array $fixtures, string $request, int $statusCode, string $pageTitle, array $query, string $host, bool $autoItem): void
{
$fixtures[] = 'suffix';
$this->loadFixtureFiles($fixtures);

self::$container
->get('doctrine')
->getConnection()
->exec("UPDATE tl_page SET urlSuffix=''")
;

Config::set('useAutoItem', $autoItem);

$_SERVER['REQUEST_URI'] = $request;
Expand Down Expand Up @@ -1017,9 +1027,14 @@ public function getRootAliases(): \Generator
*/
public function testResolvesTheRootPageWithLocale(array $fixtures, string $request, int $statusCode, string $pageTitle, string $acceptLanguages, string $host): void
{
$fixtures[] = 'locale';
$this->loadFixtureFiles($fixtures);

self::$container
->get('doctrine')
->getConnection()
->exec('UPDATE tl_page SET urlPrefix=language')
;

$_SERVER['REQUEST_URI'] = $request;
$_SERVER['HTTP_HOST'] = $host;
$_SERVER['HTTP_ACCEPT_LANGUAGE'] = $acceptLanguages;
Expand Down
11 changes: 11 additions & 0 deletions core-bundle/tests/Routing/Candidates/CandidatesTest.php
Expand Up @@ -391,6 +391,17 @@ public function getCandidatesProvider(): \Generator
'default' => ['15/foo', '15'],
],
];

yield [
'/foo/bar/baz.html',
['.html'],
['foo/bar'],
[
'default' => ['baz'],
'locale' => ['foo/bar/baz', 'foo/bar', 'foo'],
'legacy' => [],
],
];
}

/**
Expand Down

0 comments on commit 82fbcc4

Please sign in to comment.