From 0346f6aa2f2b19c0b916666b211b65a9e627dad1 Mon Sep 17 00:00:00 2001
From: Benni Mack <benni@typo3.org>
Date: Tue, 14 Apr 2020 11:44:55 +0200
Subject: [PATCH] [TASK] Clean up ExtensionService in Extbase

ExtensionService (internal API) is a random place for some methods used
only in one specific parts of Extbase.

Some parts in Extbase internal code regarding request building
and determining if an action is cacheable is moved to the places
where it is needed.

The EnvironmentService class has a method to determine a
HTTP-relevant request, which is only relevent for the RequestBuilder
where it is now moved to, and uses PSR-7 handling if available.

Resolves: #91019
Releases: master
Change-Id: Ie6ca0c751bc7453de1d123665b7fe6ce51b37c62
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64157
Tested-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Benjamin Franzke <bfr@qbus.de>
Reviewed-by: Alexander Schnitzler <git@alexanderschnitzler.de>
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
---
 .../Mvc/Web/FrontendRequestHandler.php        | 35 +++++++------
 .../Classes/Mvc/Web/RequestBuilder.php        | 10 +++-
 .../Classes/Service/EnvironmentService.php    |  8 ---
 .../Classes/Service/ExtensionService.php      | 52 -------------------
 .../extbase/Classes/ServiceProvider.php       |  1 -
 .../Unit/Service/ExtensionServiceTest.php     | 28 ----------
 6 files changed, 27 insertions(+), 107 deletions(-)

diff --git a/typo3/sysext/extbase/Classes/Mvc/Web/FrontendRequestHandler.php b/typo3/sysext/extbase/Classes/Mvc/Web/FrontendRequestHandler.php
index 2e5e58e25115..d70a6ccee982 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Web/FrontendRequestHandler.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Web/FrontendRequestHandler.php
@@ -16,7 +16,6 @@
 namespace TYPO3\CMS\Extbase\Mvc\Web;
 
 use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
-use TYPO3\CMS\Extbase\Service\ExtensionService;
 use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
 
 /**
@@ -26,31 +25,18 @@ use TYPO3\CMS\Frontend\ContentObject\ContentObjectRenderer;
 class FrontendRequestHandler extends AbstractRequestHandler
 {
     /**
-     * @var \TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface
+     * @var ConfigurationManagerInterface
      */
     protected $configurationManager;
 
     /**
-     * @var \TYPO3\CMS\Extbase\Service\ExtensionService
-     */
-    protected $extensionService;
-
-    /**
-     * @param \TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface $configurationManager
+     * @param ConfigurationManagerInterface $configurationManager
      */
     public function injectConfigurationManager(ConfigurationManagerInterface $configurationManager)
     {
         $this->configurationManager = $configurationManager;
     }
 
-    /**
-     * @param \TYPO3\CMS\Extbase\Service\ExtensionService $extensionService
-     */
-    public function injectExtensionService(ExtensionService $extensionService)
-    {
-        $this->extensionService = $extensionService;
-    }
-
     /**
      * Handles the web request. The response will automatically be sent to the client.
      *
@@ -59,7 +45,7 @@ class FrontendRequestHandler extends AbstractRequestHandler
     public function handleRequest()
     {
         $request = $this->requestBuilder->build();
-        if ($this->extensionService->isActionCacheable(null, null, $request->getControllerObjectName(), $request->getControllerActionName())) {
+        if ($this->isActionCacheable($request->getControllerObjectName(), $request->getControllerActionName())) {
             $request->setIsCached(true);
         } else {
             $contentObject = $this->configurationManager->getContentObject();
@@ -90,4 +76,19 @@ class FrontendRequestHandler extends AbstractRequestHandler
     {
         return $this->environmentService->isEnvironmentInFrontendMode();
     }
+
+    protected function isActionCacheable(string $controllerClassName, string $actionName): bool
+    {
+        $frameworkConfiguration = $this->configurationManager->getConfiguration(
+            ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK
+        );
+
+        $nonCacheableActions = $frameworkConfiguration['controllerConfiguration'][$controllerClassName]['nonCacheableActions'] ?? null;
+
+        if (!is_array($nonCacheableActions)) {
+            return true;
+        }
+
+        return !in_array($actionName, $frameworkConfiguration['controllerConfiguration'][$controllerClassName]['nonCacheableActions'], true);
+    }
 }
diff --git a/typo3/sysext/extbase/Classes/Mvc/Web/RequestBuilder.php b/typo3/sysext/extbase/Classes/Mvc/Web/RequestBuilder.php
index 0bbda142a2c0..c12efaba4c00 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Web/RequestBuilder.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Web/RequestBuilder.php
@@ -225,7 +225,7 @@ class RequestBuilder implements SingletonInterface
         // @todo Use Environment
         $request->setRequestUri(GeneralUtility::getIndpEnv('TYPO3_REQUEST_URL'));
         $request->setBaseUri($baseUri);
-        $request->setMethod($this->environmentService->getServerRequestMethod());
+        $request->setMethod($this->getServerRequestMethod($typo3Request));
         if (isset($parameters['format']) && is_string($parameters['format']) && $parameters['format'] !== '') {
             $request->setFormat(filter_var($parameters['format'], FILTER_SANITIZE_STRING));
         } else {
@@ -372,4 +372,12 @@ class RequestBuilder implements SingletonInterface
         }
         return $fieldPaths;
     }
+
+    protected function getServerRequestMethod(?ServerRequestInterface $typo3Request): string
+    {
+        if ($typo3Request instanceof ServerRequestInterface) {
+            return $typo3Request->getMethod();
+        }
+        return isset($_SERVER['REQUEST_METHOD']) && is_string($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : 'GET';
+    }
 }
diff --git a/typo3/sysext/extbase/Classes/Service/EnvironmentService.php b/typo3/sysext/extbase/Classes/Service/EnvironmentService.php
index a22fc116d0ca..d20a0ec314c9 100644
--- a/typo3/sysext/extbase/Classes/Service/EnvironmentService.php
+++ b/typo3/sysext/extbase/Classes/Service/EnvironmentService.php
@@ -44,12 +44,4 @@ class EnvironmentService implements SingletonInterface
     {
         return (defined('TYPO3_MODE') && TYPO3_MODE === 'BE') ?: false;
     }
-
-    /**
-     * @return string
-     */
-    public function getServerRequestMethod(): string
-    {
-        return isset($_SERVER['REQUEST_METHOD']) && is_string($_SERVER['REQUEST_METHOD']) ? $_SERVER['REQUEST_METHOD'] : 'GET';
-    }
 }
diff --git a/typo3/sysext/extbase/Classes/Service/ExtensionService.php b/typo3/sysext/extbase/Classes/Service/ExtensionService.php
index d579811033cb..eb274cc29a67 100644
--- a/typo3/sysext/extbase/Classes/Service/ExtensionService.php
+++ b/typo3/sysext/extbase/Classes/Service/ExtensionService.php
@@ -24,7 +24,6 @@ use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Extbase\Configuration\ConfigurationManagerInterface;
 use TYPO3\CMS\Extbase\Exception;
-use TYPO3\CMS\Extbase\Object\ObjectManagerInterface;
 
 /**
  * Service for determining basic extension params
@@ -32,23 +31,6 @@ use TYPO3\CMS\Extbase\Object\ObjectManagerInterface;
  */
 class ExtensionService implements SingletonInterface
 {
-    /**
-     * todo: Deprecate this constant in favor of the following one:
-     * @see \TYPO3\CMS\Extbase\Utility\ExtensionUtility::PLUGIN_TYPE_PLUGIN
-     */
-    const PLUGIN_TYPE_PLUGIN = 'list_type';
-
-    /**
-     * todo: Deprecate this constant in favor of the following one:
-     * @see \TYPO3\CMS\Extbase\Utility\ExtensionUtility::PLUGIN_TYPE_CONTENT_ELEMENT
-     */
-    const PLUGIN_TYPE_CONTENT_ELEMENT = 'CType';
-
-    /**
-     * @var \TYPO3\CMS\Extbase\Object\ObjectManagerInterface
-     */
-    protected $objectManager;
-
     /**
      * @var ConfigurationManagerInterface
      */
@@ -60,14 +42,6 @@ class ExtensionService implements SingletonInterface
      */
     protected $targetPidPluginCache = [];
 
-    /**
-     * @param \TYPO3\CMS\Extbase\Object\ObjectManagerInterface $objectManager
-     */
-    public function injectObjectManager(ObjectManagerInterface $objectManager): void
-    {
-        $this->objectManager = $objectManager;
-    }
-
     /**
      * @param ConfigurationManagerInterface $configurationManager
      */
@@ -145,32 +119,6 @@ class ExtensionService implements SingletonInterface
         return !empty($pluginNames) ? $pluginNames[0] : null;
     }
 
-    /**
-     * Checks if the given action is cacheable or not.
-     *
-     * @param string|null $extensionName Name of the target extension, without underscores
-     * @param string|null $pluginName Name of the target plugin
-     * @param string $controllerClassName Name of the target controller
-     * @param string $actionName Name of the action to be called
-     * @return bool TRUE if the specified plugin action is cacheable, otherwise FALSE
-     */
-    public function isActionCacheable(?string $extensionName, ?string $pluginName, string $controllerClassName, string $actionName): bool
-    {
-        $frameworkConfiguration = $this->configurationManager->getConfiguration(
-            ConfigurationManagerInterface::CONFIGURATION_TYPE_FRAMEWORK,
-            $extensionName,
-            $pluginName
-        );
-
-        $nonCacheableActions = $frameworkConfiguration['controllerConfiguration'][$controllerClassName]['nonCacheableActions'] ?? null;
-
-        if (!is_array($nonCacheableActions)) {
-            return true;
-        }
-
-        return !in_array($actionName, $frameworkConfiguration['controllerConfiguration'][$controllerClassName]['nonCacheableActions'], true);
-    }
-
     /**
      * Determines the target page of the specified plugin.
      * If plugin.tx_$pluginSignature.view.defaultPid is set, this value is used as target page id
diff --git a/typo3/sysext/extbase/Classes/ServiceProvider.php b/typo3/sysext/extbase/Classes/ServiceProvider.php
index e456f3abcd47..41f5cb1b9ab8 100644
--- a/typo3/sysext/extbase/Classes/ServiceProvider.php
+++ b/typo3/sysext/extbase/Classes/ServiceProvider.php
@@ -83,7 +83,6 @@ class ServiceProvider extends AbstractServiceProvider
     public static function getExtensionService(ContainerInterface $container): Service\ExtensionService
     {
         $extensionService = self::new($container, Service\ExtensionService::class);
-        $extensionService->injectObjectManager($container->get(Object\ObjectManager::class));
         $extensionService->injectConfigurationManager($container->get(Configuration\ConfigurationManager::class));
         return $extensionService;
     }
diff --git a/typo3/sysext/extbase/Tests/Unit/Service/ExtensionServiceTest.php b/typo3/sysext/extbase/Tests/Unit/Service/ExtensionServiceTest.php
index 64be3b983386..f4c8e03d8050 100644
--- a/typo3/sysext/extbase/Tests/Unit/Service/ExtensionServiceTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Service/ExtensionServiceTest.php
@@ -222,34 +222,6 @@ class ExtensionServiceTest extends UnitTestCase
         self::assertEquals($expectedResult, $actualResult);
     }
 
-    /**
-     * @test
-     */
-    public function isActionCacheableReturnsTrueByDefault()
-    {
-        $mockConfiguration = [];
-        $this->mockConfigurationManager->expects(self::any())->method('getConfiguration')->willReturn($mockConfiguration);
-        $actualResult = $this->extensionService->isActionCacheable('SomeExtension', 'SomePlugin', 'SomeController', 'someAction');
-        self::assertTrue($actualResult);
-    }
-
-    /**
-     * @test
-     */
-    public function isActionCacheableReturnsFalseIfActionIsNotCacheable()
-    {
-        $mockConfiguration = [
-            'controllerConfiguration' => [
-                'SomeController' => [
-                    'nonCacheableActions' => ['someAction']
-                ]
-            ]
-        ];
-        $this->mockConfigurationManager->expects(self::any())->method('getConfiguration')->willReturn($mockConfiguration);
-        $actualResult = $this->extensionService->isActionCacheable('SomeExtension', 'SomePlugin', 'SomeController', 'someAction');
-        self::assertFalse($actualResult);
-    }
-
     /**
      * @test
      */
-- 
GitLab