[TASK] Make TreeBuilder a stateless service

We can avoid ugly setters in TypoScript TreeBuilder
class by handing over arguments to the main function,
effectively making this service stateless.

Especially cache and tokenizer should not be injected
since they are runtime dependencies and should be given
as arguments from controllers.

$forceProcessExtensionStatics is an extbase specific
thing kept for now, but it will be solved differently
later, when extbase BackendConfigurationManager switches
to new TypoScript parser.

Change-Id: I5854c5a6d44737bc26071459fc746f52f4387e39
Resolves: #98586
Related: #97816
Releases: main
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/76099
Tested-by: Stefan Bürk <stefan@buerk.tech>
Tested-by: core-ci <typo3@b13.com>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Stefan Bürk <stefan@buerk.tech>
Reviewed-by: Benni Mack <benni@typo3.org>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>

typo3/sysext/core/Classes/TypoScript/IncludeTree/TreeBuilder.php

@@ -98,54 +98,39 @@ final class TreeBuilder
     private array $includedSysTemplateUids = [];
 
     /**
-     * Either 'constants' or 'setup'
+     * @param 'constants'|'setup' $type
      */
     private string $type;
 
+    private TokenizerInterface $tokenizer;
+    private ?PhpFrontend $cache = null;
+
     public function __construct(
         private readonly ConnectionPool $connectionPool,
         private readonly PackageManager $packageManager,
         private readonly Context $context,
         private readonly TreeFromLineStreamBuilder $treeFromTokenStreamBuilder,
-        private TokenizerInterface $tokenizer,
-        private ?PhpFrontend $cache = null,
     ) {
     }
 
     /**
-     * Setting a different Tokenizer than the default injected LossyTokenizer.
-     * Disables caching to ensure backend TypoScript IncludeTrees are never cached!
-     * Used in backend Template module only.
-     */
-    public function setTokenizer(TokenizerInterface $tokenizer): void
-    {
-        $this->tokenizer = $tokenizer;
-        $this->disableCache();
-    }
-
-    /**
-     * Used in FE in no_cache = true context.
-     * @todo: Maybe change this and simply hand over a cache to getTreeBySysTemplateRowsAndSite() when needed?
-     */
-    public function disableCache(): void
-    {
-        $this->cache = null;
-    }
-
-    /**
-     * This is a special case for extbase BE modules and should not be used otherwise. See property comment.
+     * @param 'constants'|'setup' $type
+     * @param bool $forceProcessExtensionStatics @todo: Needed for extbase, we may be able to solve this differently?
      */
-    public function forceProcessExtensionStatics(): void
-    {
-        $this->forceProcessExtensionStatics = true;
-    }
-
-    public function getTreeBySysTemplateRowsAndSite(string $type, array $sysTemplateRows, ?SiteInterface $site = null): RootInclude
-    {
+    public function getTreeBySysTemplateRowsAndSite(
+        string $type,
+        array $sysTemplateRows,
+        TokenizerInterface $tokenizer,
+        ?SiteInterface $site = null,
+        PhpFrontend $cache = null,
+        bool $forceProcessExtensionStatics = false
+    ): RootInclude {
         if (!in_array($type, ['constants', 'setup'])) {
             throw new \RuntimeException('type must be either constants or setup', 1653737656);
         }
-
+        $this->tokenizer = $tokenizer;
+        $this->cache = $cache;
+        $this->forceProcessExtensionStatics = $forceProcessExtensionStatics;
         $this->type = $type;
         $this->includedSysTemplateUids = [];
         $this->extensionStaticsProcessed = false;

typo3/sysext/core/Configuration/Services.yaml

@@ -308,10 +308,6 @@ services:
 
   TYPO3\CMS\Core\TypoScript\IncludeTree\TreeBuilder:
     public: true
-    arguments:
-      $cache: '@cache.typoscript'
-    # TreeBuilder has state and should not be re-used
-    shared: false
 
   TYPO3\CMS\Core\TypoScript\IncludeTree\Visitor\IncludeTreeAstBuilderVisitor:
     public: true

typo3/sysext/core/Tests/Functional/TypoScript/IncludeTree/TreeBuilderTest.php

@@ -26,6 +26,7 @@
 use TYPO3\CMS\Core\TypoScript\IncludeTree\Traverser\IncludeTreeTraverser;
 use TYPO3\CMS\Core\TypoScript\IncludeTree\TreeBuilder;
 use TYPO3\CMS\Core\TypoScript\IncludeTree\Visitor\IncludeTreeAstBuilderVisitor;
+use TYPO3\CMS\Core\TypoScript\Tokenizer\LossyTokenizer;
 use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
 
 class TreeBuilderTest extends FunctionalTestCase
@@ -78,7 +79,7 @@ public function singleRootTemplate(): void
         $sysTemplateRepository = $this->get(SysTemplateRepository::class);
         /** @var TreeBuilder $treeBuilder */
         $treeBuilder = $this->get(TreeBuilder::class);
-        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline));
+        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
         $ast = $this->getAst($includeTree);
         self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
     }
@@ -101,7 +102,7 @@ public function singleRootTemplateLoadsFromGlobals(): void
         $sysTemplateRepository = $this->get(SysTemplateRepository::class);
         /** @var TreeBuilder $treeBuilder */
         $treeBuilder = $this->get(TreeBuilder::class);
-        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline));
+        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
         $ast = $this->getAst($includeTree);
         self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
         self::assertSame('barValue', $ast->getChildByName('bar')->getValue());
@@ -129,6 +130,7 @@ public function singleRootTemplateLoadConstantFromSite(): void
         $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite(
             'constants',
             $sysTemplateRepository->getSysTemplateRowsByRootline($rootline),
+            new LossyTokenizer(),
             $siteFinder->getSiteByPageId(1)
         );
         $ast = $this->getAst($includeTree);
@@ -157,7 +159,7 @@ public function twoPagesTwoTemplates(): void
         $sysTemplateRepository = $this->get(SysTemplateRepository::class);
         /** @var TreeBuilder $treeBuilder */
         $treeBuilder = $this->get(TreeBuilder::class);
-        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline));
+        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
         $ast = $this->getAst($includeTree);
         self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
         self::assertSame('barValue', $ast->getChildByName('bar')->getValue());
@@ -185,7 +187,7 @@ public function twoPagesTwoTemplatesBothClear(): void
         $sysTemplateRepository = $this->get(SysTemplateRepository::class);
         /** @var TreeBuilder $treeBuilder */
         $treeBuilder = $this->get(TreeBuilder::class);
-        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline));
+        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
         $ast = $this->getAst($includeTree);
         self::assertNull($ast->getChildByName('foo'));
         self::assertSame('barValue', $ast->getChildByName('bar')->getValue());
@@ -208,7 +210,7 @@ public function twoTemplatesOnPagePrefersTheOneWithLowerSorting(): void
         $sysTemplateRepository = $this->get(SysTemplateRepository::class);
         /** @var TreeBuilder $treeBuilder */
         $treeBuilder = $this->get(TreeBuilder::class);
-        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline));
+        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
         $ast = $this->getAst($includeTree);
         self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
         self::assertNull($ast->getChildByName('bar'));
@@ -231,7 +233,7 @@ public function basedOnSimple(): void
         $sysTemplateRepository = $this->get(SysTemplateRepository::class);
         /** @var TreeBuilder $treeBuilder */
         $treeBuilder = $this->get(TreeBuilder::class);
-        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline));
+        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
         $ast = $this->getAst($includeTree);
         self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
         self::assertSame('loadedByBasedOn', $ast->getChildByName('bar')->getValue());
@@ -254,7 +256,7 @@ public function basedOnAfterIncludeStatic(): void
         $sysTemplateRepository = $this->get(SysTemplateRepository::class);
         /** @var TreeBuilder $treeBuilder */
         $treeBuilder = $this->get(TreeBuilder::class);
-        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline));
+        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
         $ast = $this->getAst($includeTree);
         self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
         self::assertSame('loadedByBasedOn', $ast->getChildByName('bar')->getValue());
@@ -277,7 +279,7 @@ public function basedOnBeforeIncludeStatic(): void
         $sysTemplateRepository = $this->get(SysTemplateRepository::class);
         /** @var TreeBuilder $treeBuilder */
         $treeBuilder = $this->get(TreeBuilder::class);
-        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline));
+        $includeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRepository->getSysTemplateRowsByRootline($rootline), new LossyTokenizer());
         $ast = $this->getAst($includeTree);
         self::assertSame('fooValue', $ast->getChildByName('foo')->getValue());
         self::assertSame('includeStaticTarget', $ast->getChildByName('bar')->getValue());

typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php

@@ -24,6 +24,7 @@
 use TYPO3\CMS\Backend\FrontendBackendUserAuthentication;
 use TYPO3\CMS\Core\Cache\CacheManager;
 use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface;
+use TYPO3\CMS\Core\Cache\Frontend\PhpFrontend;
 use TYPO3\CMS\Core\Compatibility\PublicPropertyDeprecationTrait;
 use TYPO3\CMS\Core\Configuration\PageTsConfig;
 use TYPO3\CMS\Core\Context\Context;
@@ -65,6 +66,8 @@
 use TYPO3\CMS\Core\TypoScript\IncludeTree\Visitor\IncludeTreeConditionMatcherVisitor;
 use TYPO3\CMS\Core\TypoScript\IncludeTree\Visitor\IncludeTreeSetupConditionConstantSubstitutionVisitor;
 use TYPO3\CMS\Core\TypoScript\TemplateService;
+use TYPO3\CMS\Core\TypoScript\Tokenizer\LossyTokenizer;
+use TYPO3\CMS\Core\TypoScript\Tokenizer\TokenizerInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\HttpUtility;
 use TYPO3\CMS\Core\Utility\MathUtility;
@@ -1191,13 +1194,19 @@ public function getFromCache(ServerRequestInterface $request): void
         // @deprecated: b/w compat. Remove when TemplateService is dropped.
         $this->tmpl->rootLine = $localRootline;
 
+        $tokenizer = GeneralUtility::makeInstance(LossyTokenizer::class);
+
         if ($this->no_cache) {
             // $this->no_cache = true might have been set by earlier TypoScriptFrontendInitialization middleware.
             // This means we don't do any fancy cache stuff, calculate full TypoScript and ignore page cache.
-            $this->prepareUncachedRendering($request, $sysTemplateRows, $localRootline);
+            $this->prepareUncachedRendering($request, $sysTemplateRows, $localRootline, $tokenizer);
             return;
         }
 
+        $cacheManager = GeneralUtility::makeInstance(CacheManager::class);
+        /** @var PhpFrontend $typoscriptCache */
+        $typoscriptCache = $cacheManager->getCache('typoscript');
+
         // We *always* need the TypoScript constants, one way or the other: Setup conditions can use constants,
         // so we need the constants to substitute their values within setup conditions.
         // @todo: This is currently a rather naive approach - we simply always create the full constant AST plus
@@ -1215,7 +1224,7 @@ public function getFromCache(ServerRequestInterface $request): void
         //        beneficial since the code below is *always* executed even in full cached context.
         $treeBuilder = GeneralUtility::makeInstance(TreeBuilder::class);
         $includeTreeTraverser = GeneralUtility::makeInstance(ConditionVerdictAwareIncludeTreeTraverser::class);
-        $constantIncludeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRows, $this->getSite());
+        $constantIncludeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRows, $tokenizer, $this->getSite(), $typoscriptCache);
         $includeTreeTraverser->resetVisitors();
         $conditionMatcherVisitor = GeneralUtility::makeInstance(IncludeTreeConditionMatcherVisitor::class);
         $conditionMatcherVisitor->setConditionMatcher(GeneralUtility::makeInstance(FrontendConditionMatcher::class, $this->context, $this->id, $this->rootLine));
@@ -1240,7 +1249,7 @@ public function getFromCache(ServerRequestInterface $request): void
         //        at all, but just require the final AST in many cases directly. Both above strategies would be caches shared and
         //        re-usable between multiple different pages when constants and conditions resolve identical, which will safe a ton
         //        of time in both not-yet-cached-page and user-int scenarios since AST parsing is gone.
-        $setupIncludeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('setup', $sysTemplateRows, $this->getSite());
+        $setupIncludeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('setup', $sysTemplateRows, $tokenizer, $this->getSite(), $typoscriptCache);
         $includeTreeTraverser->resetVisitors();
         $setupConditionConstantSubstitutionVisitor = GeneralUtility::makeInstance(IncludeTreeSetupConditionConstantSubstitutionVisitor::class);
         $setupConditionConstantSubstitutionVisitor->setFlattenedConstants($flatConstants);
@@ -1400,13 +1409,12 @@ public function getFromCache(ServerRequestInterface $request): void
      *        This should be cleaned up and merged with getFromCache() in a more efficient way, for
      *        now it is a simple solution to keep the scenarios apart.
      */
-    private function prepareUncachedRendering(ServerRequestInterface $request, array $sysTemplateRows, array $localRootline): void
+    private function prepareUncachedRendering(ServerRequestInterface $request, array $sysTemplateRows, array $localRootline, TokenizerInterface $tokenizer): void
     {
         $includeTreeTraverser = GeneralUtility::makeInstance(ConditionVerdictAwareIncludeTreeTraverser::class);
         $treeBuilder = GeneralUtility::makeInstance(TreeBuilder::class);
-        $treeBuilder->disableCache();
 
-        $constantIncludeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRows, $this->getSite());
+        $constantIncludeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRows, $tokenizer, $this->getSite());
         $includeTreeTraverser->resetVisitors();
         $conditionMatcherVisitor = GeneralUtility::makeInstance(IncludeTreeConditionMatcherVisitor::class);
         $conditionMatcherVisitor->setConditionMatcher(GeneralUtility::makeInstance(FrontendConditionMatcher::class, $this->context, $this->id, $this->rootLine));
@@ -1418,7 +1426,7 @@ private function prepareUncachedRendering(ServerRequestInterface $request, array
         $constantAst = $constantAstBuilderVisitor->getAst();
         $flatConstants = $constantAst->flatten();
 
-        $setupIncludeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('setup', $sysTemplateRows, $this->getSite());
+        $setupIncludeTree = $treeBuilder->getTreeBySysTemplateRowsAndSite('setup', $sysTemplateRows, $tokenizer, $this->getSite());
         $includeTreeTraverser->resetVisitors();
         $setupConditionConstantSubstitutionVisitor = GeneralUtility::makeInstance(IncludeTreeSetupConditionConstantSubstitutionVisitor::class);
         $setupConditionConstantSubstitutionVisitor->setFlattenedConstants($flatConstants);

typo3/sysext/tstemplate/Classes/Controller/ConstantEditorController.php

@@ -54,7 +54,6 @@ public function __construct(
         private readonly AstBuilderInterface $astBuilder,
         private readonly LosslessTokenizer $losslessTokenizer,
     ) {
-        $this->treeBuilder->setTokenizer($losslessTokenizer);
     }
 
     public function handleRequest(ServerRequestInterface $request): ResponseInterface
@@ -132,7 +131,7 @@ private function showAction(ServerRequestInterface $request): ResponseInterface
         $sysTemplateRows = $this->sysTemplateRepository->getSysTemplateRowsByRootlineWithUidOverride($rootLine, $request, $selectedTemplateUid);
         /** @var SiteInterface|null $site */
         $site = $request->getAttribute('site');
-        $constantIncludeTree = $this->treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRows, $site);
+        $constantIncludeTree = $this->treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRows, $this->losslessTokenizer, $site);
         $constantAstBuilderVisitor = GeneralUtility::makeInstance(IncludeTreeCommentAwareAstBuilderVisitor::class);
         $this->treeTraverser->resetVisitors();
         $this->treeTraverser->addVisitor($constantAstBuilderVisitor);
@@ -231,7 +230,7 @@ private function saveAction(ServerRequestInterface $request): ResponseInterface
         /** @var SiteInterface|null $site */
         $site = $request->getAttribute('site');
         $sysTemplateRows = $this->sysTemplateRepository->getSysTemplateRowsByRootlineWithUidOverride($rootLine, $request, $selectedTemplateUid);
-        $constantIncludeTree = $this->treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRows, $site);
+        $constantIncludeTree = $this->treeBuilder->getTreeBySysTemplateRowsAndSite('constants', $sysTemplateRows, $this->losslessTokenizer, $site);
         $constantAstBuilderVisitor = GeneralUtility::makeInstance(IncludeTreeCommentAwareAstBuilderVisitor::class);
         $this->treeTraverser->resetVisitors();
         $this->treeTraverser->addVisitor($constantAstBuilderVisitor);