Commit 5d94b732 authored by Stefan Bürk's avatar Stefan Bürk
Browse files

[BUGFIX] Verify early for valid auto redirect creation or slug update

Automatically creating redirects on page slug changes and/or
updating children page slugs is bound to a SiteConfiguration.
This means that only pages which has a proper SiteConfiguration
up in the rootline and the corresponding configuration options
set should lead to processing them.

In the past this has been checked and handled late in the service
class `TYPO3\CMS\Redirects\Service\SlugService`. These checks are
still there for proper workflow determination.

With #99188 the `ext:redirects` auto create workflow has been
streamlined as a preparatin for upcoming features and events,
which introduced the `SlugRedirectChangeItem` as a modern data
transport container. This item is created by a corresponding
factory, which als determined and set the corresponding Site
and SiteLanguage object to the change item.

If no site configuration can be found, a `SiteNotFoundException`
is thrown and not properly handled. Some instances may use
`SysFolder` beneath root point (pid === 0) with subpages without
a proper site configuration created on the `SysFolder`. Through
the streamlining in #99188 this is not properly covered and the
`SiteNotFoundException` not properly handled.

This change catches the `SiteNotFoundException` in the factory
service and additionally verify the configuration options early
if a change should be handled or not. If no handling needed or
no SiteConfiguration found, the factory now returns null instead
of a `SlugRedirectChangeItem`.

Resolves: #99675
Related: #99188
Releases: main
Change-Id: Icc147e7c5bad70969956a66252d6e1ac18d51cff
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/77535


Tested-by: core-ci's avatarcore-ci <typo3@b13.com>
Tested-by: Stefan Bürk's avatarStefan Bürk <stefan@buerk.tech>
Tested-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
Reviewed-by: Christian Kuhn's avatarChristian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk's avatarStefan Bürk <stefan@buerk.tech>
Tested-by: Oliver Bartsch's avatarOliver Bartsch <bo@cedev.de>
parent d349baf2
......@@ -59,7 +59,10 @@ class DataHandlerSlugUpdateHook
return;
}
$this->persistedChangedItems[(int)$id] = $this->slugRedirectChangeItemFactory->create((int)$id);
$changeItem = $this->slugRedirectChangeItemFactory->create((int)$id);
if ($changeItem instanceof SlugRedirectChangeItem) {
$this->persistedChangedItems[(int)$id] = $changeItem;
}
}
/**
......
......@@ -18,6 +18,8 @@ declare(strict_types=1);
namespace TYPO3\CMS\Redirects\RedirectUpdate;
use TYPO3\CMS\Backend\Utility\BackendUtility;
use TYPO3\CMS\Core\Exception\SiteNotFoundException;
use TYPO3\CMS\Core\Site\Entity\Site;
use TYPO3\CMS\Core\Site\SiteFinder;
/**
......@@ -39,8 +41,22 @@ class SlugRedirectChangeItemFactory
}
$languageId = (int)$original['sys_language_uid'];
$defaultLanguagePageId = (int)$original['sys_language_uid'] > 0 ? (int)$original['l10n_parent'] : $pageId;
$site = $this->siteFinder->getSiteByPageId($defaultLanguagePageId);
try {
$site = $this->siteFinder->getSiteByPageId($defaultLanguagePageId);
} catch(SiteNotFoundException) {
// Auto redirecs/slug updating is a site configuration. Not finding one means that we should not handle
// the creation of them, thus no need to create a change item.
return null;
}
$siteLanguage = $site->getLanguageById($languageId);
// Verify we should process auto redirect creation or slug updating. If not return early avoiding to create
// a change item which is superflous at all.
$settings = $site->getSettings();
$autoUpdateSlugs = (bool)$settings->get('redirects.autoUpdateSlugs', true);
$autoCreateRedirects = (bool)$settings->get('redirects.autoCreateRedirects', true);
if (!($autoUpdateSlugs || $autoCreateRedirects)) {
return null;
}
// We create a plain slug replacement source, which mirrors the behaviour since redirects implementation. This
// may vanish anytime. Introducing an event here opens up the possibility to add custom source definitions, for
// example doing a real URI building to cover route decorators and enhancers, or creating redirects for more
......
"pages",,,,,,,
,"uid","pid","title","slug","l10n_parent","l10n_source","sys_language_uid","doktype"
,1,0,"SystemFolder Root","/systemfolder-root",0,0,0,254
<?php
declare(strict_types=1);
/*
* This file is part of the TYPO3 CMS project.
*
* It is free software; you can redistribute it and/or modify it under
* the terms of the GNU General Public License, either version 2
* of the License, or any later version.
*
* For the full copyright and license information, please read the
* LICENSE.txt file that was distributed with this source code.
*
* The TYPO3 project - inspiring people to share!
*/
namespace TYPO3\CMS\Redirects\Tests\Functional\RedirectUpdate;
use TYPO3\CMS\Core\Configuration\SiteConfiguration;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Redirects\RedirectUpdate\SlugRedirectChangeItem;
use TYPO3\CMS\Redirects\RedirectUpdate\SlugRedirectChangeItemFactory;
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
class SlugRedirectChangeItemFactoryTest extends FunctionalTestCase
{
protected array $coreExtensionsToLoad = ['redirects'];
/**
* @test
*/
public function returnsNullIfNoSiteConfigurationCanBeFound(): void
{
$this->importCSVDataSet(__DIR__ . '/Fixtures/SysFolderAsRootPage.csv');
/** @var ?SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(1);
self::assertNull($changeItem);
}
/**
* @test
*/
public function returnsNullIfSiteConfigurationFoundButAutoCreateRedirectsAndAutoUpdateSlugsOptionsDisabled(): void
{
$this->importCSVDataSet(__DIR__ . '/Fixtures/SysFolderAsRootPage.csv');
$this->buildBaseSite([
'redirects' => [
'autoUpdateSlugs' => false,
'autoCreateRedirects' => false,
'redirectTTL' => 30,
'httpStatusCode' => 307,
],
]);
/** @var ?SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(1);
self::assertNull($changeItem);
}
/**
* @test
*/
public function returnsChangeItemIfSiteConfigurationFoundAndOnlyAutoCreateRedirectsEnabled(): void
{
$this->importCSVDataSet(__DIR__ . '/Fixtures/SysFolderAsRootPage.csv');
$this->buildBaseSite([
'redirects' => [
'autoUpdateSlugs' => false,
'autoCreateRedirects' => true,
'redirectTTL' => 30,
'httpStatusCode' => 307,
],
]);
/** @var ?SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(1);
self::assertInstanceOf(SlugRedirectChangeItem::class, $changeItem);
self::assertSame(1, $changeItem->getPageId());
self::assertSame(1, $changeItem->getDefaultLanguagePageId());
}
/**
* @test
*/
public function returnsChangeItemIfSiteConfigurationFoundAndOnlyAutoUpdateSlugsEnabled(): void
{
$this->importCSVDataSet(__DIR__ . '/Fixtures/SysFolderAsRootPage.csv');
$this->buildBaseSite([
'redirects' => [
'autoUpdateSlugs' => true,
'autoCreateRedirects' => false,
'redirectTTL' => 30,
'httpStatusCode' => 307,
],
]);
/** @var ?SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(1);
self::assertInstanceOf(SlugRedirectChangeItem::class, $changeItem);
self::assertSame(1, $changeItem->getPageId());
self::assertSame(1, $changeItem->getDefaultLanguagePageId());
}
protected function buildBaseSite(array $settings): void
{
$configuration = [
'rootPageId' => 1,
'base' => '/',
'settings' => $settings,
];
$siteConfiguration = GeneralUtility::makeInstance(SiteConfiguration::class);
$siteConfiguration->write('testing', $configuration);
}
}
......@@ -34,6 +34,11 @@ use TYPO3\CMS\Redirects\Service\RedirectCacheService;
use TYPO3\CMS\Redirects\Service\SlugService;
use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
/**
* @todo Tests in this TestCase simulates what happens in the corresponding `DataHandlerSlugUpdateHook`, mainly which
* is executed in which order. This is somehow clumsy. Either cover proper DataHandler hook execution with
* additional tests avoiding the simulation and testing SlugService in indirect way - or refactor them here.
*/
class SlugServiceTest extends FunctionalTestCase
{
/**
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment