From 108d61a2bba658fcb3c341c7c38e12c31c107942 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Stefan=20B=C3=BCrk?= <stefan@buerk.tech>
Date: Mon, 9 Jan 2023 17:45:27 +0100
Subject: [PATCH] [BUGFIX] Use correct record data in ext:redirect datahandler
hook
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
ext:redirects provides the feature to eventually update subpage
slugs and automatically create redirects for changed pages. To
accomplish that, a combination of two DataHandler hooks are used
along with a centralized service. The service is used before the
page slug change is persistent to the database. Hook execution
for subpage changes are disabled, to contain all changes in one
correlation for eventually reverting.
With #99188 the auto-create chain has been streamlined to prepare
for further changes. However, added functional tests do not reflect
the real procedure properly. Because of that it has been overlooked
that the direct slug change which triggers is handled by the hook
works with old record, thus creating invalid page slug updates and
redirects for the children pages.
This change now properly combine the update data with the current
record data instead of retrieving not-yet updated data from the
database. Corresponding functional tests are adopted to update
the records after calling the hook implementation, which reflects
real procedure order.
Resolves: #99506
Related: #99188
Releases: main
Change-Id: Ib2fac341483b863ee20c228dfb6c8a06ea1978f9
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/77318
Tested-by: core-ci <typo3@b13.com>
Tested-by: Stefan Froemken <froemken@gmail.com>
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Froemken <froemken@gmail.com>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
---
.../Hooks/DataHandlerSlugUpdateHook.php | 3 +-
.../redirects/Classes/Service/SlugService.php | 15 ++++----
.../Functional/Service/SlugServiceTest.php | 36 +++++++++++++++----
3 files changed, 38 insertions(+), 16 deletions(-)
diff --git a/typo3/sysext/redirects/Classes/Hooks/DataHandlerSlugUpdateHook.php b/typo3/sysext/redirects/Classes/Hooks/DataHandlerSlugUpdateHook.php
index 1fe5580ad5bf..0652abe06c56 100644
--- a/typo3/sysext/redirects/Classes/Hooks/DataHandlerSlugUpdateHook.php
+++ b/typo3/sysext/redirects/Classes/Hooks/DataHandlerSlugUpdateHook.php
@@ -82,7 +82,8 @@ class DataHandlerSlugUpdateHook
) {
return;
}
-
+ // We need to merge the current changed to the retrieved record, because they are not persisted yet.
+ $persistedChangedItem = $persistedChangedItem->withChanged(array_merge($persistedChangedItem->getOriginal(), $fieldArray));
$this->slugService->rebuildSlugsForSlugChange($id, $persistedChangedItem, $dataHandler->getCorrelationId());
}
diff --git a/typo3/sysext/redirects/Classes/Service/SlugService.php b/typo3/sysext/redirects/Classes/Service/SlugService.php
index c79f441e18e9..4bc5a4796ee2 100644
--- a/typo3/sysext/redirects/Classes/Service/SlugService.php
+++ b/typo3/sysext/redirects/Classes/Service/SlugService.php
@@ -45,7 +45,7 @@ use TYPO3\CMS\Redirects\RedirectUpdate\SlugRedirectChangeItem;
use TYPO3\CMS\Redirects\RedirectUpdate\SlugRedirectChangeItemFactory;
/**
- * @internal Due to some possible refactorings in TYPO3 v10
+ * @internal Due to some possible refactorings in TYPO3 v10+
*/
class SlugService implements LoggerAwareInterface
{
@@ -103,20 +103,19 @@ class SlugService implements LoggerAwareInterface
public function rebuildSlugsForSlugChange(int $pageId, SlugRedirectChangeItem $changeItem, CorrelationId $correlationId): void
{
- $currentPageRecord = BackendUtility::getRecord('pages', $pageId);
- if ($currentPageRecord === null) {
- return;
- }
- $changeItem = $changeItem->withChanged($currentPageRecord);
$this->initializeSettings($changeItem->getSite());
if ($this->autoUpdateSlugs || $this->autoCreateRedirects) {
$sourceHosts = [];
$this->createCorrelationIds($pageId, $correlationId);
if ($this->autoCreateRedirects) {
- $sourceHosts = $this->createRedirects($changeItem, $changeItem->getDefaultLanguagePageId(), (int)$currentPageRecord['sys_language_uid']);
+ $sourceHosts = $this->createRedirects(
+ $changeItem,
+ $changeItem->getDefaultLanguagePageId(),
+ (int)$changeItem->getChanged()['sys_language_uid']
+ );
}
if ($this->autoUpdateSlugs) {
- $sourceHosts += $this->checkSubPages($currentPageRecord, $changeItem);
+ $sourceHosts += $this->checkSubPages($changeItem->getChanged(), $changeItem);
}
$this->sendNotification();
// rebuild caches only for matched source hosts
diff --git a/typo3/sysext/redirects/Tests/Functional/Service/SlugServiceTest.php b/typo3/sysext/redirects/Tests/Functional/Service/SlugServiceTest.php
index 3dd788c579c4..6a9642975e4a 100644
--- a/typo3/sysext/redirects/Tests/Functional/Service/SlugServiceTest.php
+++ b/typo3/sysext/redirects/Tests/Functional/Service/SlugServiceTest.php
@@ -28,6 +28,7 @@ use TYPO3\CMS\Core\Routing\SiteMatcher;
use TYPO3\CMS\Core\Site\SiteFinder;
use TYPO3\CMS\Core\Utility\GeneralUtility;
use TYPO3\CMS\Core\Utility\StringUtility;
+use TYPO3\CMS\Redirects\RedirectUpdate\SlugRedirectChangeItem;
use TYPO3\CMS\Redirects\RedirectUpdate\SlugRedirectChangeItemFactory;
use TYPO3\CMS\Redirects\Service\RedirectCacheService;
use TYPO3\CMS\Redirects\Service\SlugService;
@@ -113,12 +114,15 @@ class SlugServiceTest extends FunctionalTestCase
*/
public function rebuildSlugsForSlugChangeRenamesSubSlugsAndCreatesRedirects(): void
{
+ $newPageSlug = '/test-new';
$this->buildBaseSite();
$this->createSubject();
$this->importCSVDataSet(__DIR__ . '/Fixtures/SlugServiceTest_pages_test1.csv');
+ /** @var SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(2);
- $this->setPageSlug(2, '/test-new');
+ $changeItem = $changeItem->withChanged(array_merge($changeItem->getOriginal(), ['slug' => $newPageSlug]));
$this->subject->rebuildSlugsForSlugChange(2, $changeItem, $this->correlationId);
+ $this->setPageSlug(2, $newPageSlug);
// These are the slugs after rebuildSlugsForSlugChange() has run
$slugs = [
@@ -154,12 +158,15 @@ class SlugServiceTest extends FunctionalTestCase
*/
public function rebuildSlugsForSlugChangeRenamesSubSlugsAndCreatesRedirectsForRootChange(): void
{
+ $newPageSlug = '/new-home';
$this->buildBaseSite();
$this->createSubject();
$this->importCSVDataSet(__DIR__ . '/Fixtures/SlugServiceTest_pages_test2.csv');
+ /** @var SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(1);
- $this->setPageSlug(1, '/new-home');
+ $changeItem = $changeItem->withChanged(array_merge($changeItem->getOriginal(), ['slug' => $newPageSlug]));
$this->subject->rebuildSlugsForSlugChange(1, $changeItem, $this->correlationId);
+ $this->setPageSlug(1, $newPageSlug);
// These are the slugs after rebuildSlugsForSlugChange() has run
$slugs = [
@@ -201,12 +208,15 @@ class SlugServiceTest extends FunctionalTestCase
*/
public function rebuildSlugsForSlugChangeRenamesSubSlugsAndCreatesRedirectsWithSubFolderBase(): void
{
+ $newPageSlug = '/test-new';
$this->buildBaseSiteInSubfolder();
$this->createSubject();
$this->importCSVDataSet(__DIR__ . '/Fixtures/SlugServiceTest_pages_test1.csv');
+ /** @var SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(2);
- $this->setPageSlug(2, '/test-new');
+ $changeItem = $changeItem->withChanged(array_merge($changeItem->getOriginal(), ['slug' => $newPageSlug]));
$this->subject->rebuildSlugsForSlugChange(2, $changeItem, $this->correlationId);
+ $this->setPageSlug(2, $newPageSlug);
// These are the slugs after rebuildSlugsForSlugChange() has run
$slugs = [
@@ -242,12 +252,15 @@ class SlugServiceTest extends FunctionalTestCase
*/
public function rebuildSlugsForSlugChangeRenamesSubSlugsAndCreatesRedirectsWithLanguages(): void
{
+ $newPageSlug = '/test-new';
$this->buildBaseSiteWithLanguages();
$this->createSubject();
$this->importCSVDataSet(__DIR__ . '/Fixtures/SlugServiceTest_pages_test3.csv');
+ /** @var SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(31);
- $this->setPageSlug(31, '/test-new');
+ $changeItem = $changeItem->withChanged(array_merge($changeItem->getOriginal(), ['slug' => $newPageSlug]));
$this->subject->rebuildSlugsForSlugChange(31, $changeItem, $this->correlationId);
+ $this->setPageSlug(31, $newPageSlug);
// These are the slugs after rebuildSlugsForSlugChange() has run
$slugs = [
@@ -283,12 +296,15 @@ class SlugServiceTest extends FunctionalTestCase
*/
public function rebuildSlugsForSlugChangeRenamesSubSlugsAndCreatesRedirectsWithLanguagesInSubFolder(): void
{
+ $newPageSlug = '/test-new';
$this->buildBaseSiteWithLanguagesInSubFolder();
$this->createSubject();
$this->importCSVDataSet(__DIR__ . '/Fixtures/SlugServiceTest_pages_test3.csv');
+ /** @var SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(31);
- $this->setPageSlug(31, '/test-new');
+ $changeItem = $changeItem->withChanged(array_merge($changeItem->getOriginal(), ['slug' => $newPageSlug]));
$this->subject->rebuildSlugsForSlugChange(31, $changeItem, $this->correlationId);
+ $this->setPageSlug(31, $newPageSlug);
// These are the slugs after rebuildSlugsForSlugChange() has run
$slugs = [
@@ -324,12 +340,15 @@ class SlugServiceTest extends FunctionalTestCase
*/
public function rebuildSlugsForSlugChangeRenamesSubSlugsAndCreatesRedirectsWithDefaultLanguageInSubFolder(): void
{
+ $newPageSlug = '/test-new';
$this->buildBaseSiteWithLanguagesInSubFolder();
$this->createSubject();
$this->importCSVDataSet(__DIR__ . '/Fixtures/SlugServiceTest_pages_test3.csv');
+ /** @var SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(3);
- $this->setPageSlug(3, '/test-new');
+ $changeItem = $changeItem->withChanged(array_merge($changeItem->getOriginal(), ['slug' => $newPageSlug]));
$this->subject->rebuildSlugsForSlugChange(3, $changeItem, $this->correlationId);
+ $this->setPageSlug(3, $newPageSlug);
// These are the slugs after rebuildSlugsForSlugChange() has run
$slugs = [
@@ -366,12 +385,15 @@ class SlugServiceTest extends FunctionalTestCase
*/
public function rebuildSlugsForSlugChangeRenamesSubSlugsAndCreatesRedirectsWithLanguagesForSiteroot(): void
{
+ $newPageSlug = '/test-new';
$this->buildBaseSiteWithLanguages();
$this->createSubject();
$this->importCSVDataSet(__DIR__ . '/Fixtures/SlugServiceTest_pages_test4.csv');
+ /** @var SlugRedirectChangeItem $changeItem */
$changeItem = $this->get(SlugRedirectChangeItemFactory::class)->create(5);
- $this->setPageSlug(5, '/test-new');
+ $changeItem = $changeItem->withChanged(array_merge($changeItem->getOriginal(), ['slug' => $newPageSlug]));
$this->subject->rebuildSlugsForSlugChange(5, $changeItem, $this->correlationId);
+ $this->setPageSlug(5, $newPageSlug);
// These are the slugs after rebuildSlugsForSlugChange() has run
$slugs = [
--
GitLab