From af696c1dcf8d32c6a5134165bde7fdcf2ff21867 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Mon, 17 Jun 2019 15:26:44 +0200
Subject: [PATCH] [!!!][TASK] Remove 4th parameter of enableFields

Since Versioning is completely handled via
the Context API and set within PageRepository
directly since TYPO3 v9, using the fourth
parameter would result in an invalid scenario.

Instead PageRepository->where_hid_del in
a live scenario now always contains (pid!=-1)
which filters out all non-versioned records.

This is a breaking change to avoid confusion,
however in regular scenarios this does
not affect the system, as PageRepository->versionOL()
filters out these cases in live workspace
anyways.

Resolves: #88574
Releases: master
Change-Id: I538c04997cb67e30c58a4dfa515acd751f868e9c
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61073
Tested-by: Daniel Goerz <daniel.goerz@posteo.de>
Tested-by: Georg Ringer <georg.ringer@gmail.com>
Reviewed-by: Daniel Goerz <daniel.goerz@posteo.de>
Reviewed-by: Georg Ringer <georg.ringer@gmail.com>
---
 ...erOfPageRepository-enableFieldsRemoved.rst | 40 +++++++++++++++++++
 .../frontend/Classes/Page/PageRepository.php  | 11 +++--
 .../Functional/Page/PageRepositoryTest.php    | 34 +---------------
 .../Php/MethodArgumentDroppedMatcher.php      |  6 +++
 4 files changed, 53 insertions(+), 38 deletions(-)
 create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst

diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst
new file mode 100644
index 000000000000..06a8f6d5c09f
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/master/Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst
@@ -0,0 +1,40 @@
+.. include:: ../../Includes.txt
+
+========================================================================
+Breaking: #88574 - 4th parameter of PageRepository->enableFields removed
+========================================================================
+
+See :issue:`88574`
+
+Description
+===========
+
+The fourth parameter of :php:`PageRepository->enableFields()` was meant to filter out versioned records
+which are in Live Workspace (versioning, not workspaces). Although the method has largely been superseded
+with Doctrine DBAL's Restrictions, it is still used in some places.
+
+With the introduction of the Context API, new PageRepository instances can be created to fetch multiple variants
+of certain aspects, instead of modifying existing public properties. Therefore the fourth argument has been removed.
+
+
+Impact
+======
+
+Calling the method above with the fourth parameter set to true has no effect anymore, and will
+trigger a PHP Notice.
+
+
+Affected Installations
+======================
+
+Any TYPO3 installation dealing with non-workspace versioning in Frontend requests with third-party extension
+still relying on non-workspace versioning.
+
+
+Migration
+=========
+
+The fourth parameter on any method call can be removed (if set to "false"), or should be replaced with a
+separate instance of PageRepository with a custom Context.
+
+.. index:: Frontend, PHP-API, FullyScanned
\ No newline at end of file
diff --git a/typo3/sysext/frontend/Classes/Page/PageRepository.php b/typo3/sysext/frontend/Classes/Page/PageRepository.php
index 70bec454c60e..65b763798418 100644
--- a/typo3/sysext/frontend/Classes/Page/PageRepository.php
+++ b/typo3/sysext/frontend/Classes/Page/PageRepository.php
@@ -174,7 +174,7 @@ class PageRepository implements LoggerAwareInterface
             // versioning preview (that means we are online!)
             $this->where_hid_del = ' AND ' . (string)$expressionBuilder->andX(
                 QueryHelper::stripLogicalOperatorPrefix(
-                    $this->enableFields('pages', $show_hidden, ['fe_group' => true], true)
+                    $this->enableFields('pages', $show_hidden, ['fe_group' => true])
                 ),
                 $expressionBuilder->lt('pages.doktype', 200)
             );
@@ -1222,11 +1222,10 @@ class PageRepository implements LoggerAwareInterface
      * @param string $table Table name found in the $GLOBALS['TCA'] array
      * @param int $show_hidden If $show_hidden is set (0/1), any hidden-fields in records are ignored. NOTICE: If you call this function, consider what to do with the show_hidden parameter. Maybe it should be set? See ContentObjectRenderer->enableFields where it's implemented correctly.
      * @param array $ignore_array Array you can pass where keys can be "disabled", "starttime", "endtime", "fe_group" (keys from "enablefields" in TCA) and if set they will make sure that part of the clause is not added. Thus disables the specific part of the clause. For previewing etc.
-     * @param bool $noVersionPreview If set, enableFields will be applied regardless of any versioning preview settings which might otherwise disable enableFields
      * @throws \InvalidArgumentException
      * @return string The clause starting like " AND ...=... AND ...=...
      */
-    public function enableFields($table, $show_hidden = -1, $ignore_array = [], $noVersionPreview = false)
+    public function enableFields($table, $show_hidden = -1, $ignore_array = [])
     {
         if ($show_hidden === -1) {
             // If show_hidden was not set from outside, use the current context
@@ -1244,7 +1243,7 @@ class PageRepository implements LoggerAwareInterface
                 $constraints[] = $expressionBuilder->eq($table . '.' . $ctrl['delete'], 0);
             }
             if ($ctrl['versioningWS'] ?? false) {
-                if (!$this->versioningWorkspaceId > 0) {
+                if ($this->versioningWorkspaceId === 0) {
                     // Filter out placeholder records (new/moved/deleted items)
                     // in case we are NOT in a versioning preview (that means we are online!)
                     $constraints[] = $expressionBuilder->lte(
@@ -1261,7 +1260,7 @@ class PageRepository implements LoggerAwareInterface
                 }
 
                 // Filter out versioned records
-                if (!$noVersionPreview && empty($ignore_array['pid'])) {
+                if (empty($ignore_array['pid'])) {
                     $constraints[] = $expressionBuilder->neq($table . '.pid', -1);
                 }
             }
@@ -1270,7 +1269,7 @@ class PageRepository implements LoggerAwareInterface
             if (is_array($ctrl['enablecolumns'])) {
                 // In case of versioning-preview, enableFields are ignored (checked in
                 // versionOL())
-                if ($this->versioningWorkspaceId <= 0 || !$ctrl['versioningWS'] || $noVersionPreview) {
+                if ($this->versioningWorkspaceId === 0 || !$ctrl['versioningWS']) {
                     if (($ctrl['enablecolumns']['disabled'] ?? false) && !$show_hidden && !($ignore_array['disabled'] ?? false)) {
                         $field = $table . '.' . $ctrl['enablecolumns']['disabled'];
                         $constraints[] = $expressionBuilder->eq($field, 0);
diff --git a/typo3/sysext/frontend/Tests/Functional/Page/PageRepositoryTest.php b/typo3/sysext/frontend/Tests/Functional/Page/PageRepositoryTest.php
index e2326c1c5a85..f0a5485c7678 100644
--- a/typo3/sysext/frontend/Tests/Functional/Page/PageRepositoryTest.php
+++ b/typo3/sysext/frontend/Tests/Functional/Page/PageRepositoryTest.php
@@ -359,9 +359,10 @@ class PageRepositoryTest extends \TYPO3\TestingFramework\Core\Functional\Functio
 
         $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable('pages');
         $expectedSQL = sprintf(
-            ' AND ((%s = 0) AND (%s <= 0) AND (%s = 0) AND (%s <= 123) AND ((%s = 0) OR (%s > 123))) AND (%s < 200)',
+            ' AND ((%s = 0) AND (%s <= 0) AND (%s <> -1) AND (%s = 0) AND (%s <= 123) AND ((%s = 0) OR (%s > 123))) AND (%s < 200)',
             $connection->quoteIdentifier('pages.deleted'),
             $connection->quoteIdentifier('pages.t3ver_state'),
+            $connection->quoteIdentifier('pages.pid'),
             $connection->quoteIdentifier('pages.hidden'),
             $connection->quoteIdentifier('pages.starttime'),
             $connection->quoteIdentifier('pages.endtime'),
@@ -507,37 +508,6 @@ class PageRepositoryTest extends \TYPO3\TestingFramework\Core\Functional\Functio
         );
     }
 
-    /**
-     * @test
-     */
-    public function enableFieldsDoesNotHideVersionedRecordsWhenCheckingVersionOverlays()
-    {
-        $table = $this->getUniqueId('aTable');
-        $GLOBALS['TCA'][$table] = [
-            'ctrl' => [
-                'versioningWS' => true
-            ]
-        ];
-
-        $subject = new PageRepository(new Context([
-            'workspace' => new WorkspaceAspect(23)
-        ]));
-
-        $conditions = $subject->enableFields($table, -1, [], true);
-        $connection = GeneralUtility::makeInstance(ConnectionPool::class)->getConnectionForTable($table);
-
-        $this->assertThat(
-            $conditions,
-            $this->logicalNot($this->stringContains(' AND (' . $connection->quoteIdentifier($table . '.t3ver_state') . ' <= 0)')),
-            'No versioning placeholders'
-        );
-        $this->assertThat(
-            $conditions,
-            $this->logicalNot($this->stringContains(' AND (' . $connection->quoteIdentifier($table . '.pid') . ' <> -1)')),
-            'No records from page -1'
-        );
-    }
-
     protected function assertOverlayRow($row)
     {
         $this->assertIsArray($row);
diff --git a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentDroppedMatcher.php b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentDroppedMatcher.php
index ac72a4824874..ba91f88ae484 100644
--- a/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentDroppedMatcher.php
+++ b/typo3/sysext/install/Configuration/ExtensionScanner/Php/MethodArgumentDroppedMatcher.php
@@ -223,4 +223,10 @@ return [
             'Breaking-87193-DeprecatedFunctionalityRemoved.rst',
         ],
     ],
+    'TYPO3\CMS\Core\Frontend\Page\PageRepository->enableFields' => [
+        'maximumNumberOfArguments' => 3,
+        'restFiles' => [
+            'Breaking-88574-4thParameterOfPageRepository-enableFieldsRemoved.rst',
+        ],
+    ],
 ];
-- 
GitLab