From b53f0b1c589c790ae928064447b6a6955fd91492 Mon Sep 17 00:00:00 2001
From: Christian Kuhn <lolli@schwarzbu.ch>
Date: Sun, 22 Aug 2021 17:09:27 +0200
Subject: [PATCH] [TASK] Deprecate ContentObjectRenderer constructor in
 StandaloneView

Handling ContentObjectRenderer instances is messy, especially
within extbase: The ConfigurationManager is a stateful singleton
since it depends on current ContentObjectRenderer being set
within frontend. The extbase Bootstrap takes care of setting the
current cObj.

ConfigurationManager is also misused (ext:form) to "park" the
current cObj for later retrieval. This needs to be tackled with
a dedicated patch.

This is worse with fluid StandaloneView, which accepts cObj as
constructor argument to update ConfigurationManager state.
Funnily, this optional constructor argument is never hand
over in core. Additionally, extbase usually does not use
StandaloneView, but fluid's TemplateView instead.

The patch deprecates the constructor argument of fluid
StandaloneView to avoid a dependency from StandaloneView
to extbase.

Resolves: #94959
Releases: master
Change-Id: I605d4bb1133a30daf418eb418f2d5502d981b4c1
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/70721
Tested-by: core-ci <typo3@b13.com>
Tested-by: Jochen <rothjochen@gmail.com>
Tested-by: Oliver Bartsch <bo@cedev.de>
Tested-by: Christian Kuhn <lolli@schwarzbu.ch>
Reviewed-by: Jochen <rothjochen@gmail.com>
Reviewed-by: Oliver Bartsch <bo@cedev.de>
Reviewed-by: Christian Kuhn <lolli@schwarzbu.ch>
---
 ...ectRendererConstructorInStandaloneView.rst | 43 +++++++++++++++++++
 .../AbstractConfigurationManager.php          | 17 +++++++-
 .../Configuration/ConfigurationManager.php    |  3 +-
 .../AbstractConfigurationManagerTest.php      |  6 ++-
 .../fluid/Classes/View/StandaloneView.php     |  8 ++--
 .../sysext/fluid/Configuration/Services.yaml  |  4 ++
 6 files changed, 72 insertions(+), 9 deletions(-)
 create mode 100644 typo3/sysext/core/Documentation/Changelog/master/Deprecation-94959-ContentObjectRendererConstructorInStandaloneView.rst

diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-94959-ContentObjectRendererConstructorInStandaloneView.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-94959-ContentObjectRendererConstructorInStandaloneView.rst
new file mode 100644
index 000000000000..298fc927ce92
--- /dev/null
+++ b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-94959-ContentObjectRendererConstructorInStandaloneView.rst
@@ -0,0 +1,43 @@
+.. include:: ../../Includes.txt
+
+=========================================================================
+Deprecation: #94959 - ContentObjectRenderer constructor in StandaloneView
+=========================================================================
+
+See :issue:`94959`
+
+Description
+===========
+
+The :php:`ContentObjectRenderer` argument to :php:`TYPO3\CMS\Fluid\View\StandaloneView`
+has been deprecated. The TYPO3 core never used this optional argument and
+it added a hard dependency to extbase classes from StandaloneView, which should
+be avoided.
+
+The ContentObjectRenderer instance within StandaloneView has been used to update
+the extbase :php:`ConfigurationManager` singleton, even though extbase bootstrap
+already sets the current ContentObjectRenderer to ConfigurationManager.
+
+
+Impact
+======
+
+Extensions creating instances of :php:`StandaloneView` and handing over an
+instance of :php:`ContentObjectRenderer` as constructor argument will get
+a deprecation level log message logged.
+
+
+Affected Installations
+======================
+
+Most instances are probably not affected by this change since handing over
+the constructor argument is rather unusual.
+
+
+Migration
+=========
+
+Do not hand over an instance of :php:`ContentObjectRenderer` when creating an
+instance of :php:`StandaloneView`.
+
+.. index:: Fluid, PHP-API, NotScanned, ext:fluid
diff --git a/typo3/sysext/extbase/Classes/Configuration/AbstractConfigurationManager.php b/typo3/sysext/extbase/Classes/Configuration/AbstractConfigurationManager.php
index 4289b8b0c5b2..7f68feb5e004 100644
--- a/typo3/sysext/extbase/Classes/Configuration/AbstractConfigurationManager.php
+++ b/typo3/sysext/extbase/Classes/Configuration/AbstractConfigurationManager.php
@@ -79,6 +79,7 @@ abstract class AbstractConfigurationManager implements SingletonInterface
 
     /**
      * @param ContentObjectRenderer $contentObject
+     * @todo: See note on getContentObject() below.
      */
     public function setContentObject(ContentObjectRenderer $contentObject): void
     {
@@ -86,10 +87,20 @@ abstract class AbstractConfigurationManager implements SingletonInterface
     }
 
     /**
-     * @return ContentObjectRenderer|null
+     * @return ContentObjectRenderer
+     * @todo: This dependency to ContentObjectRenderer on a singleton object is unfortunate:
+     *      The current instance is set through USER cObj and extbase Bootstrap, its null in Backend.
+     *      This getter is misused to retrieve current ContentObjectRenderer state by some extensions (eg. ext:form).
+     *      This dependency should be removed altogether.
+     *      Although the current implementation *always* returns an instance of ContentObjectRenderer, we do not want to
+     *      hard-expect consuming classes on that, since this methods needs to be dropped anyways, so potential null return is kept.
      */
     public function getContentObject(): ?ContentObjectRenderer
     {
+        if ($this->contentObject instanceof ContentObjectRenderer) {
+            return $this->contentObject;
+        }
+        $this->contentObject = GeneralUtility::makeInstance(ContentObjectRenderer::class);
         return $this->contentObject;
     }
 
@@ -155,7 +166,9 @@ abstract class AbstractConfigurationManager implements SingletonInterface
                 $isBackend = ($GLOBALS['TYPO3_REQUEST'] ?? null) instanceof ServerRequestInterface
                     && ApplicationType::fromRequest($GLOBALS['TYPO3_REQUEST'])->isBackend();
                 if ($isBackend) {
-                    FrontendSimulatorUtility::simulateFrontendEnvironment($this->getContentObject());
+                    // @todo: This BE specific switch should be moved to BackendConfigurationManager to drop the dependency to $GLOBALS['TYPO3_REQUEST'] here.
+                    // Use makeInstance here since extbase Bootstrap always setContentObject(null) in Backend, no need to call getContentObject().
+                    FrontendSimulatorUtility::simulateFrontendEnvironment(GeneralUtility::makeInstance(ContentObjectRenderer::class));
                 }
                 $conf = $this->typoScriptService->convertPlainArrayToTypoScriptArray($frameworkConfiguration['persistence']);
                 $frameworkConfiguration['persistence']['storagePid'] = $GLOBALS['TSFE']->cObj->stdWrapValue('storagePid', $conf ?? []);
diff --git a/typo3/sysext/extbase/Classes/Configuration/ConfigurationManager.php b/typo3/sysext/extbase/Classes/Configuration/ConfigurationManager.php
index 15c29554b09e..52571af0afc6 100644
--- a/typo3/sysext/extbase/Classes/Configuration/ConfigurationManager.php
+++ b/typo3/sysext/extbase/Classes/Configuration/ConfigurationManager.php
@@ -24,10 +24,9 @@ use TYPO3\CMS\Extbase\Configuration\Exception\InvalidConfigurationTypeException;
 use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
 
 /**
- * A configuration manager following the strategy pattern (GoF315). It hides the concrete
+ * A configuration manager following the strategy pattern. It hides the concrete
  * implementation of the configuration manager and provides a unified access point.
  *
- * Use the shutdown() method to drop the concrete implementation.
  * @internal only to be used within Extbase, not part of TYPO3 Core API.
  */
 class ConfigurationManager implements ConfigurationManagerInterface
diff --git a/typo3/sysext/extbase/Tests/Unit/Configuration/AbstractConfigurationManagerTest.php b/typo3/sysext/extbase/Tests/Unit/Configuration/AbstractConfigurationManagerTest.php
index bc9a5bce7c8a..4f2b84ab55d3 100644
--- a/typo3/sysext/extbase/Tests/Unit/Configuration/AbstractConfigurationManagerTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Configuration/AbstractConfigurationManagerTest.php
@@ -28,6 +28,8 @@ use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
  */
 class AbstractConfigurationManagerTest extends UnitTestCase
 {
+    protected $resetSingletonInstances = true;
+
     /**
      * @var AbstractConfigurationManager|\PHPUnit\Framework\MockObject\MockObject|AccessibleObjectInterface
      */
@@ -376,9 +378,9 @@ class AbstractConfigurationManagerTest extends UnitTestCase
     /**
      * @test
      */
-    public function getContentObjectReturnsNullIfNoContentObjectHasBeenSet(): void
+    public function getContentObjectReturnsInstanceOfContentObjectRenderer(): void
     {
-        self::assertNull($this->abstractConfigurationManager->getContentObject());
+        self::assertInstanceOf(ContentObjectRenderer::class, $this->abstractConfigurationManager->getContentObject());
     }
 
     /**
diff --git a/typo3/sysext/fluid/Classes/View/StandaloneView.php b/typo3/sysext/fluid/Classes/View/StandaloneView.php
index 205fc978a7f7..d483aa2f3403 100644
--- a/typo3/sysext/fluid/Classes/View/StandaloneView.php
+++ b/typo3/sysext/fluid/Classes/View/StandaloneView.php
@@ -32,15 +32,17 @@ class StandaloneView extends AbstractTemplateView
     /**
      * Constructor
      *
-     * @param ContentObjectRenderer $contentObject The current cObject. If NULL a new instance will be created
+     * @param ContentObjectRenderer|null $contentObject @deprecated The current cObject. If NULL a new instance will be created
      * @throws \InvalidArgumentException
      * @throws \UnexpectedValueException
      */
     public function __construct(ContentObjectRenderer $contentObject = null)
     {
-        // @todo: this needs to be removed in the future
+        // @deprecated since v11, will be removed with v12. Drop $contentObject argument and ConfigurationManager handling.
         $configurationManager = GeneralUtility::getContainer()->get(ConfigurationManager::class);
-        if ($contentObject === null) {
+        if ($contentObject !== null) {
+            trigger_error('Argument $contentObject of class ' . __CLASS__ . ' is deprecated since v11, will be removed with v12.', E_USER_DEPRECATED);
+        } else {
             /** @var ContentObjectRenderer $contentObject */
             $contentObject = GeneralUtility::makeInstance(ContentObjectRenderer::class);
         }
diff --git a/typo3/sysext/fluid/Configuration/Services.yaml b/typo3/sysext/fluid/Configuration/Services.yaml
index 103a3ee81383..fc9b33d1bf8d 100644
--- a/typo3/sysext/fluid/Configuration/Services.yaml
+++ b/typo3/sysext/fluid/Configuration/Services.yaml
@@ -13,6 +13,10 @@ services:
     arguments:
       $context: null
 
+  # @deprecated since v11, will be removed with v12: Obsolete with the removal of ContentObjectRenderer constructor argument
+  TYPO3\CMS\Fluid\View\StandaloneView:
+    autowire: false
+
   cache.fluid_template:
     class: TYPO3\CMS\Core\Cache\Frontend\FrontendInterface
     factory: ['@TYPO3\CMS\Core\Cache\CacheManager', 'getCache']
-- 
GitLab