[BUGFIX] Select proper records in DBAL workspace restrictions 41/56941/7
authorBenni Mack <benni@typo3.org>
Thu, 15 Nov 2018 10:28:11 +0000 (11:28 +0100)
committerBenni Mack <benni@typo3.org>
Fri, 16 Nov 2018 08:51:35 +0000 (09:51 +0100)
A new WorkspaceRestriction is added to solve all issues
once and for all.

For now, this restriction is used to only show
records in pagetree without having duplicated.

Resolves: #84985
Releases: master, 8.7
Change-Id: I22d5f276460107802bef3d390e6781434f1c28d3
Reviewed-on: https://review.typo3.org/56941
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Achim Fritz <af@achimfritz.de>
Tested-by: Achim Fritz <af@achimfritz.de>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
typo3/sysext/backend/Classes/Tree/Repository/PageTreeRepository.php
typo3/sysext/core/Classes/Database/Query/Restriction/WorkspaceRestriction.php [new file with mode: 0644]
typo3/sysext/core/Documentation/Changelog/9.5.x/Important-84985-UnifiedWorkspaceRestrictionForDatabaseQueries.rst [new file with mode: 0644]
typo3/sysext/core/Tests/Unit/Database/Query/Restriction/WorkspaceRestrictionTest.php [new file with mode: 0644]

index 9b48a9c..e352842 100644 (file)
@@ -16,8 +16,8 @@ namespace TYPO3\CMS\Backend\Tree\Repository;
  */
 
 use TYPO3\CMS\Core\Database\ConnectionPool;
-use TYPO3\CMS\Core\Database\Query\Restriction\BackendWorkspaceRestriction;
 use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
+use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction;
 use TYPO3\CMS\Core\DataHandling\PlainDataResolver;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Versioning\VersionState;
@@ -158,12 +158,7 @@ class PageTreeRepository
         $queryBuilder->getRestrictions()
             ->removeAll()
             ->add(GeneralUtility::makeInstance(DeletedRestriction::class))
-            ->add(GeneralUtility::makeInstance(
-                BackendWorkspaceRestriction::class,
-                $this->currentWorkspace,
-                // set this flag to "true" when inside a workspace
-                $this->currentWorkspace !== 0
-            ));
+            ->add(GeneralUtility::makeInstance(WorkspaceRestriction::class, $this->currentWorkspace));
 
         $pageRecords = $queryBuilder
             ->select(...$this->fields)
@@ -181,11 +176,6 @@ class PageTreeRepository
         if ($this->currentWorkspace !== 0 && !empty($pageRecords)) {
             $livePageIds = [];
             foreach ($pageRecords as $pageRecord) {
-                // BackendWorkspaceRestriction includes drafts from ALL workspaces, we need to ensure
-                // that only the live records and the drafts from the current workspace are used
-                if (!in_array((int)$pageRecord['t3ver_wsid'], [0, $this->currentWorkspace], true)) {
-                    continue;
-                }
                 $livePageIds[] = (int)$pageRecord['uid'];
                 $livePagePids[(int)$pageRecord['uid']] = (int)$pageRecord['pid'];
                 if ((int)$pageRecord['t3ver_state'] === VersionState::MOVE_PLACEHOLDER) {
diff --git a/typo3/sysext/core/Classes/Database/Query/Restriction/WorkspaceRestriction.php b/typo3/sysext/core/Classes/Database/Query/Restriction/WorkspaceRestriction.php
new file mode 100644 (file)
index 0000000..6ffe5dd
--- /dev/null
@@ -0,0 +1,86 @@
+<?php
+declare(strict_types = 1);
+namespace TYPO3\CMS\Core\Database\Query\Restriction;
+
+/*
+ * 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!
+ */
+
+use TYPO3\CMS\Core\Database\Query\Expression\CompositeExpression;
+use TYPO3\CMS\Core\Database\Query\Expression\ExpressionBuilder;
+
+/**
+ * Restriction to make queries workspace-aware. This restriction is new compared to the "older"
+ * FrontendWorkspaceRestriction and BackendWorkspaceRestriction in a way that it ALWAYS fetches the live version,
+ * plus in current workspace the workspace records).
+ * It does not care about the state, as this should be done by overlays.
+ *
+ * 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)
+ *
+ * This means, that all records which are fetched need to run through either
+ * - BackendUtility::getRecordWSOL() (when having one or a few records)
+ * - PageRepository->versionOL()
+ * - PlainDataResolver (when having lots of records)
+ */
+class WorkspaceRestriction implements QueryRestrictionInterface
+{
+    /**
+     * @var int
+     */
+    protected $workspaceId;
+
+    /**
+     * @param int $workspaceId
+     */
+    public function __construct(int $workspaceId = 0)
+    {
+        $this->workspaceId = (int)$workspaceId;
+    }
+
+    /**
+     * Main method to build expressions for given tables
+     *
+     * @param array $queriedTables Array of tables, where array key is table alias and value is a table name
+     * @param ExpressionBuilder $expressionBuilder Expression builder instance to add restrictions with
+     * @return CompositeExpression The result of query builder expression(s)
+     */
+    public function buildExpression(array $queriedTables, ExpressionBuilder $expressionBuilder): CompositeExpression
+    {
+        $constraints = [];
+        foreach ($queriedTables as $tableAlias => $tableName) {
+            if (empty($GLOBALS['TCA'][$tableName]['ctrl']['versioningWS'] ?? false)) {
+                continue;
+            }
+            if ($this->workspaceId === 0) {
+                // Only include ws_id=0
+                $workspaceIdExpression = $expressionBuilder->eq($tableAlias . '.t3ver_wsid', 0);
+            } else {
+                // Include live records PLUS records from the given workspace
+                $workspaceIdExpression = $expressionBuilder->in(
+                    $tableAlias . '.t3ver_wsid',
+                    [0, $this->workspaceId]
+                );
+            }
+            // Always filter out "pid=-1" records
+            $constraints[] = $expressionBuilder->andX(
+                $workspaceIdExpression,
+                $expressionBuilder->neq(
+                    $tableAlias . '.pid',
+                    -1
+                )
+            );
+        }
+        return $expressionBuilder->andX(...$constraints);
+    }
+}
diff --git a/typo3/sysext/core/Documentation/Changelog/9.5.x/Important-84985-UnifiedWorkspaceRestrictionForDatabaseQueries.rst b/typo3/sysext/core/Documentation/Changelog/9.5.x/Important-84985-UnifiedWorkspaceRestrictionForDatabaseQueries.rst
new file mode 100644 (file)
index 0000000..d09e6fe
--- /dev/null
@@ -0,0 +1,24 @@
+.. include:: ../../Includes.txt
+
+======================================================================
+Important: #84985 - Unified Workspace Restriction for Database Queries
+======================================================================
+
+See :issue:`84985`
+
+Description
+===========
+
+A new `WorkspaceRestriction` is added to overcome certain downsides of the existing
+`FrontendWorkspaceRestriction` and `BackendWorkspaceRestriction`. The new workspace restriction
+limits a SQL query to only select records which are "online" (pid != -1) and in live or current
+workspace.
+
+As an important note and limitation of any workspace-related restrictions, fetching the exact
+records need to be handled after the SQL results are fetched, by overlaying the records with
+`BackendUtility::getRecordWSOL()`, `PageRepository->versionOL()` or `PlainDataResolver`.
+
+For now, the WorkspaceRestriction must be used explicitly in various contexts and is not applied
+automatically.
+
+.. index:: Database, PHP-API, NotScanned
diff --git a/typo3/sysext/core/Tests/Unit/Database/Query/Restriction/WorkspaceRestrictionTest.php b/typo3/sysext/core/Tests/Unit/Database/Query/Restriction/WorkspaceRestrictionTest.php
new file mode 100644 (file)
index 0000000..f7573b3
--- /dev/null
@@ -0,0 +1,73 @@
+<?php
+declare(strict_types = 1);
+namespace TYPO3\CMS\Core\Tests\Unit\Database\Query\Restriction;
+
+/*
+ * 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!
+ */
+
+use TYPO3\CMS\Core\Database\Query\Restriction\WorkspaceRestriction;
+
+class WorkspaceRestrictionTest extends AbstractRestrictionTestCase
+{
+    /**
+     * @test
+     */
+    public function buildExpressionAddsLiveWorkspaceWhereClause()
+    {
+        $GLOBALS['TCA']['aTable']['ctrl'] = [
+            'versioningWS' => true,
+        ];
+        $subject = new WorkspaceRestriction(0);
+        $expression = $subject->buildExpression(['aTable' => 'aTable'], $this->expressionBuilder);
+        $this->assertSame('("aTable"."t3ver_wsid" = 0) AND ("aTable"."pid" <> -1)', (string)$expression);
+    }
+
+    /**
+     * @test
+     */
+    public function buildExpressionAddsNonLiveWorkspaceWhereClause()
+    {
+        $GLOBALS['TCA']['aTable']['ctrl'] = [
+            'versioningWS' => true,
+        ];
+        $subject = new WorkspaceRestriction(42);
+        $expression = $subject->buildExpression(['aTable' => 'aTable'], $this->expressionBuilder);
+        $this->assertSame('("aTable"."t3ver_wsid" IN (0, 42)) AND ("aTable"."pid" <> -1)', (string)$expression);
+    }
+
+    /**
+     * @test
+     */
+    public function buildExpressionAddsLiveWorkspaceLimitedWhereClause()
+    {
+        $GLOBALS['TCA']['aTable']['ctrl'] = [
+            'versioningWS' => false,
+        ];
+        $subject = new WorkspaceRestriction(0);
+        $expression = $subject->buildExpression(['aTable' => 'aTable'], $this->expressionBuilder);
+        $this->assertSame('', (string)$expression);
+    }
+
+    /**
+     * @test
+     */
+    public function buildExpressionAddsNonLiveWorkspaceLimitedWhereClause()
+    {
+        $GLOBALS['TCA']['aTable']['ctrl'] = [
+            'versioningWS' => false,
+        ];
+        $subject = new WorkspaceRestriction(42);
+        $expression = $subject->buildExpression(['aTable' => 'aTable'], $this->expressionBuilder);
+        $this->assertSame('', (string)$expression);
+    }
+}