[!!!][TASK] Stop guessing pid in extbase BackendConfigurationManager

lolli42 · lolli42 · commit c07f24517de5 · 2024-12-01T02:34:44.000+01:00
Stop guessing a page in extbase BackendConfigurationmanager with extbase based backend modules not in page context. Resolves: #105728 Related: #105719 Related: #105602 Related: #84861 Releases: main Change-Id: Ib5e5584bae4faa6e922810d6cce50689b6a1f36d Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/87288 Tested-by: Christian Kuhn <lolli@schwarzbu.ch> Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch> Tested-by: core-ci <typo3@b13.com> Reviewed-by: Stefan Bürk <stefan@buerk.tech> Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de> Reviewed-by: Markus Klein <markus.klein@typo3.org> Tested-by: Stefan Bürk <stefan@buerk.tech> Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
diff --git a/typo3/sysext/core/Documentation/Changelog/14.0/Breaking-105728-ExtbaseBackendModulesDoNotGuessPageIdForTypoScriptAnymore.rst b/typo3/sysext/core/Documentation/Changelog/14.0/Breaking-105728-ExtbaseBackendModulesDoNotGuessPageIdForTypoScriptAnymore.rst
@@ -0,0 +1,83 @@
+..  include:: /Includes.rst.txt
+
+..  _breaking-105728-1732882067:
+
+==============================================================================================
+Breaking: #105728 - Extbase backend modules not in page context rely on global TypoScript only
+==============================================================================================
+
+See :issue:`105728`
+
+Description
+===========
+
+Configuration of extbase based backend modules can be done using frontend TypoScript.
+
+The standard prefix in TypoScript to do this is :typoscript:`module.tx_myextension`, extbase backend
+module controllers can typically retrieve their configuration using a call like
+:php:`$configuration = $this->configurationManager->getConfiguration(ConfigurationManagerInterface::CONFIGURATION_TYPE_SETTINGS, 'myextension');`.
+
+TypoScript itself is always bound to a page: In the frontend, there must be either some rootline page with
+a sys_template record, or a page that has a site set. The frontend rendering will otherwise bail out with
+an error message.
+
+Extbase based backend modules are sometimes bound to pages as well: They can have a rendered page tree
+configured by their module configuration, and then receive the selected page uid within the request as GET parameter :php:`id`.
+
+Other extbase based backend modules however are not within page scope and do not render the page tree. Examples of such
+modules within the core are the backend modules delivered by the `form` and `beuser` extension.
+
+Such extbase based backend without page tree modules had a hard time to calculate their relevant frontend TypoScript
+based configuration: Since TypoScript is bound to pages, they looked for "the first" valid page in the page
+tree, and the first valid sys_template record to calculate their TypoScript configuration. This dependency
+and guesswork made final configuration of extbase backend module configuration not in page context brittle,
+intransparent and clumsy.
+
+TYPO3 v14 puts an end to this: Extbase backend modules without page context compile their TypoScript configuration
+from "global" TypoScript only and stop calculating TypoScript from guessing "the first valid" page.
+
+The key call to register such "global" TypoScript is :php:`ExtensionManagementUtility::addTypoScriptSetup()` in
+:file:`ext_localconf.php` files.
+
+
+Impact
+======
+
+Configuration of extbase based backend modules may change if their configuration is defined by the first
+given valid page in the page tree. Configuration of such backend modules can no longer be changed by including
+TypoScript on the "first valid" page.
+
+
+Affected installations
+======================
+
+Instances with extbase based backend modules without page tree may be affected.
+
+
+Migration
+=========
+
+Configuration of extbase based backend modules without page tree must be supplied programmatically
+and "made global" by extending :php:`$GLOBALS['TYPO3_CONF_VARS']['FE']['defaultTypoScript_setup']`
+using :php:`ExtensionManagementUtility::addTypoScriptSetup()` within extensions
+:file:`ext_localconf.php` files. The backend module of the `form` extension is a good example.
+Additional locations of extensions that deliver form `yaml` definitions are defined like this:
+
+..  code-block:: php
+    :caption: EXT:my_extension/ext_localconf.php
+
+    ExtensionManagementUtility::addTypoScriptSetup('
+        module.tx_form {
+            settings {
+                yamlConfigurations {
+                    1732884807 = EXT:my_extension/Configuration/Yaml/FormSetup.yaml
+                }
+            }
+        }
+    ');
+
+Note it is also possible to use :php:`ExtensionManagementUtility::addTypoScriptConstants()`
+to declare "global" TypoScrip constants, and to use them in above TypoScript.
+
+
+..  index:: Backend, PHP-API, TypoScript, NotScanned, ext:extbase
diff --git a/typo3/sysext/extbase/Classes/Configuration/BackendConfigurationManager.php b/typo3/sysext/extbase/Classes/Configuration/BackendConfigurationManager.php
@@ -26,7 +26,6 @@
 use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Database\Query\QueryHelper;
 use TYPO3\CMS\Core\Database\Query\Restriction\DeletedRestriction;
-use TYPO3\CMS\Core\Database\Query\Restriction\HiddenRestriction;
 use TYPO3\CMS\Core\Exception\SiteNotFoundException;
 use TYPO3\CMS\Core\Site\Entity\NullSite;
 use TYPO3\CMS\Core\Site\Entity\Site;
@@ -54,17 +53,6 @@
  * pages in the first place - they may not have a page tree und thus no page id at all, like
  * for instance the ext:beuser module.
  *
- * Unfortunately, extbase *still* has to calculate *some* TypoScript in any case, even if there
- * is no page id at all: The default configuration of extbase Backend modules is the "module."
- * TypoScript setup top-level key. The base config of this is delivered by extbase extensions that
- * have Backend Modules using ext_typoscript_setup.typoscript, and/or via TYPO3_CONF_VARS TypoScript
- * setup defaults. Those have to be loaded in any case, even if there is no page at all in the page
- * tree.
- *
- * The code thus has to hop through quite some loops to "find" some relevant page id it can guess
- * if none is incoming from the request. It even fakes a default sys_template row to trigger
- * TypoScript loading of globals and ext_typoscript_setup.typoscript if it couldn't find anything.
- *
  * @internal only to be used within Extbase, not part of TYPO3 Core API.
  */
 final readonly class BackendConfigurationManager
@@ -99,9 +87,14 @@ public function getConfiguration(ServerRequestInterface $request, array $configu
         $pluginNameFromConfig = $configuration['pluginName'] ?? null;
         $configuration = $this->typoScriptService->convertTypoScriptArrayToPlainArray($configuration);
 
-        $frameworkConfiguration = $this->getExtbaseConfiguration($request);
+        $typoscriptSetup = $this->getTypoScriptSetup($request);
+        $frameworkConfiguration = [];
+        if (isset($typoscriptSetup['config.']['tx_extbase.'])) {
+            $frameworkConfiguration = $this->typoScriptService->convertTypoScriptArrayToPlainArray($typoscriptSetup['config.']['tx_extbase.']);
+        }
+
         if (!isset($frameworkConfiguration['persistence']['storagePid'])) {
-            $currentPageId = $this->getCachedCurrentPageId($request);
+            $currentPageId = $this->getCurrentPageId($request);
             $frameworkConfiguration['persistence']['storagePid'] = $currentPageId;
         }
         // only merge $configuration and override controller configuration when retrieving configuration of the current plugin
@@ -145,7 +138,7 @@ public function getConfiguration(ServerRequestInterface $request, array $configu
      */
     public function getTypoScriptSetup(ServerRequestInterface $request): array
     {
-        $currentPageId = $this->getCachedCurrentPageId($request);
+        $currentPageId = $this->getCurrentPageId($request);
 
         $cacheIdentifier = 'extbase-backend-typoscript-pageId-' . $currentPageId;
         $setupArray = $this->runtimeCache->get($cacheIdentifier);
@@ -174,34 +167,33 @@ public function getTypoScriptSetup(ServerRequestInterface $request): array
 
         $rootLine = [];
         $sysTemplateRows = [];
-        $sysTemplateFakeRow = [
-            'uid' => 0,
-            'pid' => 0,
-            'title' => 'Fake sys_template row to force extension statics loading',
-            'root' => 1,
-            'clear' => 3,
-            'include_static_file' => '',
-            'basedOn' => '',
-            'includeStaticAfterBasedOn' => 0,
-            'static_file_mode' => false,
-            'constants' => '',
-            'config' => '',
-            'deleted' => 0,
-            'hidden' => 0,
-            'starttime' => 0,
-            'endtime' => 0,
-            'sorting' => 0,
-        ];
         if ($currentPageId > 0) {
             $rootLine = GeneralUtility::makeInstance(RootlineUtility::class, $currentPageId)->get();
             $sysTemplateRows = $this->sysTemplateRepository->getSysTemplateRowsByRootline($rootLine, $request);
             ksort($rootLine);
         }
-
         $sets = $site instanceof Site ? $this->setRegistry->getSets(...$site->getSets()) : [];
         if (empty($sysTemplateRows) && $sets === []) {
-            // If there is no page (pid 0 only), or if the first 'is_siteroot' site has no sys_template record or assigned site sets,
-            // then we "fake" a sys_template row: This triggers inclusion of 'global' and 'extension static' TypoScript.
+            // If no page with sys_template rows or site sets could be derived, we
+            // "fake" a row to trigger inclusion of 'global' TypoScript only.
+            $sysTemplateFakeRow = [
+                'uid' => 0,
+                'pid' => 0,
+                'title' => 'Fake sys_template row to force global TypoScript loading',
+                'root' => 1,
+                'clear' => 3,
+                'include_static_file' => '',
+                'basedOn' => '',
+                'includeStaticAfterBasedOn' => 0,
+                'static_file_mode' => false,
+                'constants' => '',
+                'config' => '',
+                'deleted' => 0,
+                'hidden' => 0,
+                'starttime' => 0,
+                'endtime' => 0,
+                'sorting' => 0,
+            ];
             $sysTemplateRows[] = $sysTemplateFakeRow;
         }
 
@@ -220,19 +212,6 @@ public function getTypoScriptSetup(ServerRequestInterface $request): array
         return $setupArray;
     }
 
-    /**
-     * Returns the TypoScript configuration found in config.tx_extbase
-     */
-    private function getExtbaseConfiguration(ServerRequestInterface $request): array
-    {
-        $setup = $this->getTypoScriptSetup($request);
-        $extbaseConfiguration = [];
-        if (isset($setup['config.']['tx_extbase.'])) {
-            $extbaseConfiguration = $this->typoScriptService->convertTypoScriptArrayToPlainArray($setup['config.']['tx_extbase.']);
-        }
-        return $extbaseConfiguration;
-    }
-
     /**
      * Returns the TypoScript configuration found in module.tx_yourextension_yourmodule
      * merged with the global configuration of your extension from module.tx_yourextension
@@ -256,39 +235,10 @@ private function getPluginConfiguration(ServerRequestInterface $request, string
         return $pluginConfiguration;
     }
 
-    private function getCachedCurrentPageId(ServerRequestInterface $request): int
-    {
-        $currentPageId = $this->runtimeCache->get('extbase-backend-typoscript-currentPageId');
-        if (!is_int($currentPageId)) {
-            $currentPageId = $this->getCurrentPageId($request);
-            $this->runtimeCache->set('extbase-backend-typoscript-currentPageId', $currentPageId);
-        }
-        return $currentPageId;
-    }
-
     /**
-     * The full madness to guess a page id:
-     * - First try to get one from the request, accessing POST / GET 'id'
-     * - else, fetch the first page in page tree that has 'is_siteroot' set
-     * - else, fetch the first sys_template record that has 'root' flag set, and use its pid
-     * - else, 0, indicating "there are no 'is_siteroot' pages and no sys_template 'root' records"
-     *
-     * @return int current page id. If no page is selected current root page id is returned
+     * Get page id from the request, accessing POST / GET 'id'
      */
     private function getCurrentPageId(ServerRequestInterface $request): int
-    {
-        $currentPageId = $this->getCurrentPageIdFromRequest($request);
-        $currentPageId = $currentPageId ?: $this->getCurrentPageIdFromCurrentSiteRoot();
-        $currentPageId = $currentPageId ?: $this->getCurrentPageIdFromRootTemplate();
-        return $currentPageId ?: 0;
-    }
-
-    /**
-     * Gets the current page ID from the GET/POST data.
-     *
-     * @return int the page UID, will be 0 if none has been set
-     */
-    private function getCurrentPageIdFromRequest(ServerRequestInterface $request): int
     {
         // @todo: This misuses 'id' as a broken convention for pages-uid. The filelist module for instance
         //        uses 'id' as "storage-uid:path", which is only mitigated here by testing the argument
@@ -302,64 +252,6 @@ private function getCurrentPageIdFromRequest(ServerRequestInterface $request): i
         return $id;
     }
 
-    /**
-     * Gets the current page ID from the first site root in tree.
-     *
-     * @return int the page UID, will be 0 if none has been set
-     */
-    private function getCurrentPageIdFromCurrentSiteRoot(): int
-    {
-        $queryBuilder = $this->connectionPool->getQueryBuilderForTable('pages');
-        $queryBuilder->getRestrictions()->removeAll()
-            ->add(GeneralUtility::makeInstance(DeletedRestriction::class))
-            ->add(GeneralUtility::makeInstance(HiddenRestriction::class));
-        $rootPage = $queryBuilder
-            ->select('uid')
-            ->from('pages')
-            ->where(
-                $queryBuilder->expr()->eq('is_siteroot', $queryBuilder->createNamedParameter(1, Connection::PARAM_INT)),
-                $queryBuilder->expr()->eq('sys_language_uid', $queryBuilder->createNamedParameter(0, Connection::PARAM_INT)),
-                // Only consider live root page IDs, never return a versioned root page ID
-                $queryBuilder->expr()->eq('t3ver_oid', $queryBuilder->createNamedParameter(0, Connection::PARAM_INT)),
-                $queryBuilder->expr()->eq('t3ver_wsid', $queryBuilder->createNamedParameter(0, Connection::PARAM_INT))
-            )
-            ->orderBy('sorting')
-            ->setMaxResults(1)
-            ->executeQuery()
-            ->fetchAssociative();
-        if (empty($rootPage)) {
-            return 0;
-        }
-        return (int)$rootPage['uid'];
-    }
-
-    /**
-     * Gets the current page ID from the first created root template.
-     *
-     * @return int the page UID, will be 0 if none has been set
-     */
-    private function getCurrentPageIdFromRootTemplate(): int
-    {
-        $queryBuilder = $this->connectionPool->getQueryBuilderForTable('sys_template');
-        $queryBuilder->getRestrictions()->removeAll()
-            ->add(GeneralUtility::makeInstance(DeletedRestriction::class))
-            ->add(GeneralUtility::makeInstance(HiddenRestriction::class));
-        $rootTemplate = $queryBuilder
-            ->select('pid')
-            ->from('sys_template')
-            ->where(
-                $queryBuilder->expr()->eq('root', $queryBuilder->createNamedParameter(1, Connection::PARAM_INT))
-            )
-            ->orderBy('crdate')
-            ->setMaxResults(1)
-            ->executeQuery()
-            ->fetchAssociative();
-        if (empty($rootTemplate)) {
-            return 0;
-        }
-        return (int)$rootTemplate['pid'];
-    }
-
     /**
      * Returns an array of storagePIDs that are below a list of storage pids.
      *
@@ -390,7 +282,6 @@ private function getRecursiveStoragePids(array $storagePids, int $recursionDepth
     /**
      * Recursively fetch all children of a given page
      *
-     * @param int $pid uid of the page
      * @return int[] List of child row $uid's
      */
     private function getPageChildrenRecursive(int $pid, int $depth, int $begin, string $permsClause): array
diff --git a/typo3/sysext/extbase/Tests/Functional/Configuration/BackendConfigurationManagerTest.php b/typo3/sysext/extbase/Tests/Functional/Configuration/BackendConfigurationManagerTest.php
@@ -19,7 +19,6 @@
 
 use PHPUnit\Framework\Attributes\Test;
 use TYPO3\CMS\Core\Authentication\BackendUserAuthentication;
-use TYPO3\CMS\Core\Database\ConnectionPool;
 use TYPO3\CMS\Core\Http\ServerRequest;
 use TYPO3\CMS\Extbase\Configuration\BackendConfigurationManager;
 use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
@@ -54,7 +53,8 @@ public function getConfigurationRecursivelyMergesCurrentExtensionConfigurationWi
                 'throwPageNotFoundExceptionIfActionCantBeResolved' => '0',
             ],
         ];
-        self::assertEquals($expectedResult, $subject->getConfiguration(new ServerRequest(), [], 'CurrentExtensionName'));
+        $request = (new ServerRequest())->withQueryParams(['id' => 1]);
+        self::assertEquals($expectedResult, $subject->getConfiguration($request, [], 'CurrentExtensionName'));
     }
 
     #[Test]
@@ -85,7 +85,8 @@ public function getConfigurationRecursivelyMergesCurrentPluginConfigurationWithF
                 'throwPageNotFoundExceptionIfActionCantBeResolved' => '0',
             ],
         ];
-        self::assertEquals($expectedResult, $subject->getConfiguration(new ServerRequest(), [], 'CurrentExtensionName', 'CurrentPluginName'));
+        $request = (new ServerRequest())->withQueryParams(['id' => 1]);
+        self::assertEquals($expectedResult, $subject->getConfiguration($request, [], 'CurrentExtensionName', 'CurrentPluginName'));
     }
 
     #[Test]
@@ -108,41 +109,6 @@ public function getCurrentPageIdReturnsPageIdFromPost(): void
         self::assertEquals(321, $actualResult);
     }
 
-    #[Test]
-    public function getCurrentPageIdReturnsPidFromFirstRootTemplateIfIdIsNotSetAndNoRootPageWasFound(): void
-    {
-        (new ConnectionPool())->getConnectionForTable('sys_template')->insert(
-            'sys_template',
-            [
-                'pid' => 123,
-                'deleted' => 0,
-                'hidden' => 0,
-                'root' => 1,
-            ]
-        );
-        $subject = $this->get(BackendConfigurationManager::class);
-        $getCurrentPageIdReflectionMethod = (new \ReflectionMethod($subject, 'getCurrentPageId'));
-        $actualResult = $getCurrentPageIdReflectionMethod->invoke($subject, new ServerRequest());
-        self::assertEquals(123, $actualResult);
-    }
-
-    #[Test]
-    public function getCurrentPageIdReturnsUidFromFirstRootPageIfIdIsNotSet(): void
-    {
-        (new ConnectionPool())->getConnectionForTable('pages')->insert(
-            'pages',
-            [
-                'deleted' => 0,
-                'hidden' => 0,
-                'is_siteroot' => 1,
-            ]
-        );
-        $subject = $this->get(BackendConfigurationManager::class);
-        $getCurrentPageIdReflectionMethod = (new \ReflectionMethod($subject, 'getCurrentPageId'));
-        $actualResult = $getCurrentPageIdReflectionMethod->invoke($subject, new ServerRequest());
-        self::assertEquals(1, $actualResult);
-    }
-
     #[Test]
     public function getCurrentPageIdReturnsDefaultStoragePidIfIdIsNotSetNoRootTemplateAndRootPageWasFound(): void
     {
