From a79a780e79a343905c524521e1a6aa66fd8fc8d6 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Mon, 31 Oct 2022 17:57:19 +0100
Subject: [PATCH] [BUGFIX] Resolve modified slugs in versioned pages of preview
This change now also fetches versioned records when
previewing a workspace, allowing to also query the
versioned records if their slug has been changed.
This extends WorkspaceRestriction to have a similar
behaviour as previously known from PageRepository (pre v8 times),
to fetch versioned records.
Resolves: #97940
Releases: main, 11.5
Change-Id: I7fe9c081d5a087bf5276510e1a915fc92b72956d
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/76355
Tested-by: core-ci <typo3@b13.com>
Tested-by: Nikita Hovratov <nikita.h@live.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Nikita Hovratov <nikita.h@live.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Benni Mack <benni@typo3.org>
---
.../Restriction/WorkspaceRestriction.php | 54 +++++++++------
.../Routing/PageSlugCandidateProvider.php | 10 +--
.../Restriction/WorkspaceRestrictionTest.php | 25 +++++++
.../SiteHandling/SlugSiteRequestTest.php | 66 +++++++++++++++++++
4 files changed, 132 insertions(+), 23 deletions(-)
diff --git a/typo3/sysext/core/Classes/Database/Query/Restriction/WorkspaceRestriction.php b/typo3/sysext/core/Classes/Database/Query/Restriction/WorkspaceRestriction.php
index 74a04c78f3e2..d2d01c898889 100644
--- a/typo3/sysext/core/Classes/Database/Query/Restriction/WorkspaceRestriction.php
+++ b/typo3/sysext/core/Classes/Database/Query/Restriction/WorkspaceRestriction.php
@@ -29,7 +29,7 @@ use TYPO3\CMS\Core\Versioning\VersionState;
*
* As workspaces cannot be fully overlaid within ONE query, this query does the following:
* - In live context, only fetch published records
- * - In a workspace, fetch all LIVE records and all workspace records which do not have "-1" (= all new placeholders get fetched as well)
+ * - In a workspace, fetch all LIVE records and all workspace records which do not have "1" (= all new placeholders get fetched as well)
*
* This means, that all records which are fetched need to run through either
* - BackendUtility::getRecordWSOL() (when having one or a few records)
@@ -38,17 +38,21 @@ use TYPO3\CMS\Core\Versioning\VersionState;
*/
class WorkspaceRestriction implements QueryRestrictionInterface
{
- /**
- * @var int
- */
- protected $workspaceId;
+ protected int $workspaceId;
/**
- * @param int $workspaceId
+ * Used to also query records within a workspace, which is useful for DB queries
+ * that check for a specific field (e.g. "slug") which might have changed within a workspace.
+ * Please note that some duplicates might be shown and the "reduce" logic needs to be
+ * added after querying. Setting this flag might also be a problem when using the DB query
+ * with limit and offset settings.
*/
- public function __construct(int $workspaceId = 0)
+ protected bool $includeAllVersionedRecords;
+
+ public function __construct(int $workspaceId = 0, bool $includeAllVersionedRecords = false)
{
$this->workspaceId = $workspaceId;
+ $this->includeAllVersionedRecords = $includeAllVersionedRecords;
}
/**
@@ -77,19 +81,31 @@ class WorkspaceRestriction implements QueryRestrictionInterface
}
// Always filter out versioned records that have an "offline" record
// But include moved records AND newly created records (t3ver_oid=0)
- $constraints[] = $expressionBuilder->and(
- $workspaceIdExpression,
- $expressionBuilder->or(
- $expressionBuilder->eq(
- $tableAlias . '.t3ver_oid',
- 0
- ),
- $expressionBuilder->eq(
- $tableAlias . '.t3ver_state',
- VersionState::MOVE_POINTER
+ if ($this->includeAllVersionedRecords === false) {
+ $constraints[] = $expressionBuilder->and(
+ $workspaceIdExpression,
+ $expressionBuilder->or(
+ $expressionBuilder->eq(
+ $tableAlias . '.t3ver_oid',
+ 0
+ ),
+ $expressionBuilder->eq(
+ $tableAlias . '.t3ver_state',
+ VersionState::MOVE_POINTER
+ )
+ )
+ );
+ } else {
+ // Include live records plus records from the given workspace
+ // but never include versioned records marked as deleted
+ $constraints[] = $expressionBuilder->and(
+ $workspaceIdExpression,
+ $expressionBuilder->neq(
+ 't3ver_state',
+ VersionState::DELETE_PLACEHOLDER
)
- )
- );
+ );
+ }
}
return $expressionBuilder->and(...$constraints);
}
diff --git a/typo3/sysext/core/Classes/Routing/PageSlugCandidateProvider.php b/typo3/sysext/core/Classes/Routing/PageSlugCandidateProvider.php
index 29a1ca61847d..727aaf59a9f6 100644
--- a/typo3/sysext/core/Classes/Routing/PageSlugCandidateProvider.php
+++ b/typo3/sysext/core/Classes/Routing/PageSlugCandidateProvider.php
@@ -22,7 +22,6 @@ use TYPO3\CMS\Core\Context\LanguageAspect;
use TYPO3\CMS\Core\Database\Connection;
use TYPO3\CMS\Core\Database\ConnectionPool;
use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
-use TYPO3\CMS\Core\Database\Query\Restriction\FrontendWorkspaceRestriction;
use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction;
use TYPO3\CMS\Core\Domain\Repository\PageRepository;
use TYPO3\CMS\Core\Exception\SiteNotFoundException;
@@ -115,13 +114,14 @@ class PageSlugCandidateProvider
*/
public function getRealPageIdForPageIdAsPossibleCandidate(int $pageId): ?int
{
+ $workspaceId = (int)$this->context->getPropertyFromAspect('workspace', 'id');
$queryBuilder = GeneralUtility::makeInstance(ConnectionPool::class)
->getQueryBuilderForTable('pages');
$queryBuilder
->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class))
- ->add(GeneralUtility::makeInstance(FrontendWorkspaceRestriction::class));
+ ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $workspaceId));
$statement = $queryBuilder
->select('uid', 'l10n_parent')
@@ -201,7 +201,7 @@ class PageSlugCandidateProvider
->getRestrictions()
->removeAll()
->add(GeneralUtility::makeInstance(DeletedRestriction::class))
- ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $workspaceId));
+ ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $workspaceId, true));
$statement = $queryBuilder
->select('uid', 'sys_language_uid', 'l10n_parent', 'l18n_cfg', 'pid', 'slug', 'mount_pid', 'mount_pid_ol', 't3ver_state', 'doktype', 't3ver_wsid', 't3ver_oid')
@@ -221,6 +221,8 @@ class PageSlugCandidateProvider
)
// Exact match will be first, that's important
->orderBy('slug', 'desc')
+ // versioned records should be rendered before the live records
+ ->addOrderBy('t3ver_wsid', 'desc')
// Sort pages that are not MountPoint pages before mount points
->addOrderBy('mount_pid_ol', 'asc')
->addOrderBy('mount_pid', 'asc')
@@ -233,7 +235,7 @@ class PageSlugCandidateProvider
while ($row = $statement->fetchAssociative()) {
$mountPageInformation = null;
- $pageIdInDefaultLanguage = (int)($languageId > 0 ? $row['l10n_parent'] : $row['uid']);
+ $pageIdInDefaultLanguage = (int)($languageId > 0 ? $row['l10n_parent'] : ($row['t3ver_oid'] ?: $row['uid']));
// When this page was added before via recursion, this page should be skipped
if (in_array($pageIdInDefaultLanguage, $excludeUids, true)) {
continue;
diff --git a/typo3/sysext/core/Tests/Unit/Database/Query/Restriction/WorkspaceRestrictionTest.php b/typo3/sysext/core/Tests/Unit/Database/Query/Restriction/WorkspaceRestrictionTest.php
index 54ca51f7edb7..2b4c27b09867 100644
--- a/typo3/sysext/core/Tests/Unit/Database/Query/Restriction/WorkspaceRestrictionTest.php
+++ b/typo3/sysext/core/Tests/Unit/Database/Query/Restriction/WorkspaceRestrictionTest.php
@@ -72,4 +72,29 @@ class WorkspaceRestrictionTest extends AbstractRestrictionTestCase
$expression = $subject->buildExpression(['aTable' => 'aTable'], $this->expressionBuilder);
self::assertSame('', (string)$expression);
}
+
+ /**
+ * @test
+ */
+ public function buildExpressionQueriesAllVersionedRecordsWithinAWorkspaceAsWell(): void
+ {
+ $GLOBALS['TCA']['aTable']['ctrl'] = [
+ 'versioningWS' => true,
+ ];
+ $subject = new WorkspaceRestriction(42, true);
+ $expression = $subject->buildExpression(['aTable' => 'aTable'], $this->expressionBuilder);
+ self::assertSame('("aTable"."t3ver_wsid" IN (0, 42)) AND ("t3ver_state" <> 2)', (string)$expression);
+ }
+ /**
+ * @test
+ */
+ public function buildExpressionQueriesAllVersionedRecordsWithinLiveStillRemovesDeletedState(): void
+ {
+ $GLOBALS['TCA']['aTable']['ctrl'] = [
+ 'versioningWS' => true,
+ ];
+ $subject = new WorkspaceRestriction(0, true);
+ $expression = $subject->buildExpression(['aTable' => 'aTable'], $this->expressionBuilder);
+ self::assertSame('("aTable"."t3ver_wsid" = 0) AND ("t3ver_state" <> 2)', (string)$expression);
+ }
}
diff --git a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
index b08432bf5ae8..4147284ed836 100644
--- a/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/SiteHandling/SlugSiteRequestTest.php
@@ -1187,4 +1187,70 @@ class SlugSiteRequestTest extends AbstractTestCase
self::assertSame($expectedStatusCode, $response->getStatusCode());
self::assertSame($expectedHeaders, $response->getHeaders());
}
+
+ public function pageIsRenderedForVersionedPageDataProvider(): \Generator
+ {
+ yield 'Live page with logged-in user' => [
+ 'url' => 'https://website.local/en-en/welcome',
+ 'pageTitle' => 'EN: Welcome',
+ 'Online Page ID' => 1100,
+ 'Workspace ID' => 0,
+ 'Backend User ID' => 1,
+ 'statusCode' => 200,
+ ];
+ yield 'Live page with logged-in user accessed even though versioned page slug was changed' => [
+ 'url' => 'https://website.local/en-en/welcome',
+ 'pageTitle' => 'EN: Welcome to ACME Inc',
+ 'Online Page ID' => 1100,
+ 'Workspace ID' => 1,
+ 'Backend User ID' => 1,
+ 'statusCode' => 200,
+ ];
+ yield 'Versioned page with logged-in user and modified slug' => [
+ 'url' => 'https://website.local/en-en/welcome-modified',
+ 'pageTitle' => 'EN: Welcome to ACME Inc',
+ 'Online Page ID' => 1100,
+ 'Workspace ID' => 1,
+ 'Backend User ID' => 1,
+ 'statusCode' => 200,
+ ];
+ yield 'Versioned page without logged-in user renders 404' => [
+ 'url' => 'https://website.local/en-en/welcome-modified',
+ 'pageTitle' => null,
+ 'Online Page ID' => null,
+ 'Workspace ID' => 1,
+ 'Backend User ID' => 0,
+ 'statusCode' => 404,
+ ];
+ }
+
+ /**
+ * @test
+ * @dataProvider pageIsRenderedForVersionedPageDataProvider
+ */
+ public function pageIsRenderedForVersionedPage(string $url, ?string $expectedPageTitle, ?int $expectedPageId, int $workspaceId, int $backendUserId, int $expectedStatusCode): void
+ {
+ $this->writeSiteConfiguration(
+ 'website-local',
+ $this->buildSiteConfiguration(1000, 'https://website.local/'),
+ [
+ $this->buildDefaultLanguageConfiguration('EN', '/en-en/'),
+ $this->buildLanguageConfiguration('FR', '/fr-fr/', ['EN']),
+ $this->buildLanguageConfiguration('FR-CA', '/fr-ca/', ['FR', 'EN']),
+ ]
+ );
+ $response = $this->executeFrontendSubRequest(
+ (new InternalRequest($url)),
+ (new InternalRequestContext())
+ ->withWorkspaceId($backendUserId !== 0 ? $workspaceId : 0)
+ ->withBackendUserId($backendUserId)
+ );
+ $responseStructure = ResponseContent::fromString(
+ (string)$response->getBody()
+ );
+
+ self::assertSame($expectedStatusCode, $response->getStatusCode());
+ self::assertSame($expectedPageId, $responseStructure->getScopePath('page/uid'));
+ self::assertSame($expectedPageTitle, $responseStructure->getScopePath('page/title'));
+ }
}
--
GitLab