[TASK] Deprecate second controller action argument 90/58190/5
authorChristian Kuhn <lolli@schwarzbu.ch>
Wed, 5 Sep 2018 12:42:22 +0000 (14:42 +0200)
committerAnja Leichsenring <aleichsenring@ab-softlab.de>
Sat, 8 Sep 2018 12:29:20 +0000 (14:29 +0200)
Core eid and backend dispatching uses only the $request object as
argument, but not the prepared $response object anymore. This was
a misconception in the first place.

The patch deprecates the second argument and logs deprecations
by reflecting the target action at runtime. This can be supressed
with a new feature toggle.

Resolves: #84196
Releases: master
Change-Id: I003aba6010957cd82e6910fb718ef531116296be
Reviewed-on: https://review.typo3.org/58190
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
Tested-by: Anja Leichsenring <aleichsenring@ab-softlab.de>
typo3/sysext/backend/Classes/Http/RequestHandler.php
typo3/sysext/backend/Classes/Http/RouteDispatcher.php
typo3/sysext/core/Classes/Http/Dispatcher.php
typo3/sysext/core/Classes/Http/DispatcherInterface.php
typo3/sysext/core/Configuration/DefaultConfiguration.php
typo3/sysext/core/Configuration/DefaultConfigurationDescription.yaml
typo3/sysext/core/Documentation/Changelog/master/Deprecation-84196-BackendControllerActionsDoNotReceivePreparedResponse.rst [new file with mode: 0644]

index f87743a..6385842 100644 (file)
@@ -58,6 +58,7 @@ class RequestHandler implements RequestHandlerInterface, PsrRequestHandlerInterf
     public function handle(ServerRequestInterface $request): ResponseInterface
     {
         // Use a custom pre-created response for AJAX calls
+        // @deprecated since v9, will be removed in v10: No prepared $response to RouteDispatcher any longer
         if (TYPO3_REQUESTTYPE & TYPO3_REQUESTTYPE_AJAX) {
             $response = new Response('php://temp', 200, [
                 'Content-Type' => 'application/json; charset=utf-8',
index b5044aa..0ff0330 100644 (file)
@@ -20,6 +20,7 @@ use TYPO3\CMS\Backend\Routing\Exception\InvalidRequestTokenException;
 use TYPO3\CMS\Backend\Routing\Route;
 use TYPO3\CMS\Backend\Routing\Router;
 use TYPO3\CMS\Backend\Utility\BackendUtility;
+use TYPO3\CMS\Core\Configuration\Features;
 use TYPO3\CMS\Core\FormProtection\FormProtectionFactory;
 use TYPO3\CMS\Core\Http\Dispatcher;
 use TYPO3\CMS\Core\Type\Bitmask\Permission;
@@ -35,12 +36,12 @@ class RouteDispatcher extends Dispatcher
      * Main method to resolve the route and checks the target of the route, and tries to call it.
      *
      * @param ServerRequestInterface $request the current server request
-     * @param ResponseInterface $response the prepared response
+     * @param ResponseInterface $response the prepared response @deprecated since v9, will be removed in v10
      * @return ResponseInterface the filled response by the callable / controller/action
      * @throws InvalidRequestTokenException if the route was not found
      * @throws \InvalidArgumentException if the defined target for the route is invalid
      */
-    public function dispatch(ServerRequestInterface $request, ResponseInterface $response)
+    public function dispatch(ServerRequestInterface $request, ResponseInterface $response = null): ResponseInterface
     {
         $router = GeneralUtility::makeInstance(Router::class);
         $route = $router->matchRequest($request);
@@ -55,7 +56,35 @@ class RouteDispatcher extends Dispatcher
         }
         $targetIdentifier = $route->getOption('target');
         $target = $this->getCallableFromTarget($targetIdentifier);
-        return call_user_func_array($target, [$request, $response]);
+        $arguments = [$request];
+
+        // @deprecated Test if target accepts one (ok) or two (deprecated) arguments
+        $scanForResponse = !GeneralUtility::makeInstance(Features::class)
+            ->isFeatureEnabled('simplifiedControllerActionDispatching');
+        if ($scanForResponse) {
+            if (is_array($targetIdentifier)) {
+                $controllerActionName = implode('::', $targetIdentifier);
+                $targetReflection = new \ReflectionMethod($controllerActionName);
+            } elseif (is_string($targetIdentifier) && strpos($targetIdentifier, '::') !== false) {
+                $controllerActionName = $targetIdentifier;
+                $targetReflection = new \ReflectionMethod($controllerActionName);
+            } elseif (is_callable($targetIdentifier)) {
+                $controllerActionName = 'closure function';
+                $targetReflection = new \ReflectionFunction($targetIdentifier);
+            } else {
+                $controllerActionName = $targetIdentifier . '::__invoke';
+                $targetReflection = new \ReflectionMethod($controllerActionName);
+            }
+            if ($targetReflection->getNumberOfParameters() >= 2) {
+                trigger_error(
+                    'Handing over second argument $response to controller action ' . $controllerActionName . '() is deprecated and will be removed in v10.',
+                    E_USER_DEPRECATED
+                );
+                $arguments[] = $response;
+            }
+        }
+
+        return call_user_func_array($target, $arguments);
     }
 
     /**
index 133bfe1..9b3a620 100644 (file)
@@ -16,6 +16,7 @@ namespace TYPO3\CMS\Core\Http;
 
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
+use TYPO3\CMS\Core\Configuration\Features;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 /**
@@ -30,15 +31,43 @@ class Dispatcher implements DispatcherInterface
      * Main method that fetches the target from the request and calls the target directly
      *
      * @param ServerRequestInterface $request the current server request
-     * @param ResponseInterface $response the prepared response
-     * @return ResponseInterface the filled response by the callable / controller/action
+     * @param ResponseInterface $response the prepared response @deprecated since v9, will be removed in v10
+     * @return ResponseInterface the filled response by the callable/controller/action
      * @throws \InvalidArgumentException if the defined target is invalid
      */
-    public function dispatch(ServerRequestInterface $request, ResponseInterface $response)
+    public function dispatch(ServerRequestInterface $request, ResponseInterface $response = null): ResponseInterface
     {
         $targetIdentifier = $request->getAttribute('target');
         $target = $this->getCallableFromTarget($targetIdentifier);
-        return call_user_func_array($target, [$request, $response]);
+        $arguments = [$request];
+
+        // @deprecated Test if target accepts one (ok) or two (deprecated) arguments
+        $scanForResponse = !GeneralUtility::makeInstance(Features::class)
+            ->isFeatureEnabled('simplifiedControllerActionDispatching');
+        if ($scanForResponse) {
+            if (is_array($targetIdentifier)) {
+                $controllerActionName = implode('::', $targetIdentifier);
+                $targetReflection = new \ReflectionMethod($controllerActionName);
+            } elseif (is_string($targetIdentifier) && strpos($targetIdentifier, '::') !== false) {
+                $controllerActionName = $targetIdentifier;
+                $targetReflection = new \ReflectionMethod($controllerActionName);
+            } elseif (is_callable($targetIdentifier)) {
+                $controllerActionName = 'closure function';
+                $targetReflection = new \ReflectionFunction($targetIdentifier);
+            } else {
+                $controllerActionName = $targetIdentifier . '::__invoke';
+                $targetReflection = new \ReflectionMethod($controllerActionName);
+            }
+            if ($targetReflection->getNumberOfParameters() >= 2) {
+                trigger_error(
+                    'Handing over second argument $response to controller action ' . $controllerActionName . '() is deprecated and will be removed in v10.',
+                    E_USER_DEPRECATED
+                );
+                $arguments[] = $response;
+            }
+        }
+
+        return call_user_func_array($target, $arguments);
     }
 
     /**
@@ -75,7 +104,7 @@ class Dispatcher implements DispatcherInterface
             return [$targetObject, $methodName];
         }
 
-        // This needs to be checked at last as a string with object::method is recognize as callable
+        // Closures needs to be checked at last as a string with object::method is recognized as callable
         if (is_callable($target)) {
             return $target;
         }
index 07ad9ee..49ed681 100644 (file)
@@ -1,4 +1,5 @@
 <?php
+declare(strict_types = 1);
 namespace TYPO3\CMS\Core\Http;
 
 /*
@@ -13,14 +14,15 @@ namespace TYPO3\CMS\Core\Http;
  *
  * The TYPO3 project - inspiring people to share!
  */
+
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 
 /**
- * An interface for dispatcher that delegate requests/responses to a certain callable, typically a
- * controller / action combination.
+ * An interface for dispatcher that delegate requests to a certain callable, typically a
+ * controller / action combination. Usually called from the RequestHandler.
  *
- * Is usually called from the RequestHandler,
+ * @internal This low level interface is used core internally only
  */
 interface DispatcherInterface
 {
@@ -28,8 +30,7 @@ interface DispatcherInterface
      * Main method to dispatch a request and its response to a callable object
      *
      * @param ServerRequestInterface $request
-     * @param ResponseInterface $response
      * @return ResponseInterface
      */
-    public function dispatch(ServerRequestInterface $request, ResponseInterface $response);
+    public function dispatch(ServerRequestInterface $request): ResponseInterface;
 }
index c5e5bcc..3565068 100644 (file)
@@ -75,6 +75,7 @@ return [
             'redirects.hitCount' => false,
             'unifiedPageTranslationHandling' => false,
             'TypoScript.strictSyntax' => true,
+            'simplifiedControllerActionDispatching' => false,
         ],
         'createGroup' => '',
         'sitename' => 'TYPO3',
index b908f70..e784e06 100644 (file)
@@ -224,6 +224,9 @@ SYS:
               TypoScript.strictSyntax:
                 type: bool
                 description: 'If on, TypoScript is parsed in strict syntax modes. Enabling this feature means old condition syntax (which is deprecated) will trigger deprecation messages.'
+              simplifiedControllerActionDispatching:
+                type: bool
+                description: 'If on, controller actions of backend modules and eID scripts do not receive a deprecated prepared response object as second argument. Enabling this feature slightly improves performance.'
         availablePasswordHashAlgorithms:
             type: array
             description: 'A list of available password hash mechanisms. Extensions may register additional mechanisms here. This is usually not extended in LocalConfiguration.php.'
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Deprecation-84196-BackendControllerActionsDoNotReceivePreparedResponse.rst b/typo3/sysext/core/Documentation/Changelog/master/Deprecation-84196-BackendControllerActionsDoNotReceivePreparedResponse.rst
new file mode 100644 (file)
index 0000000..c19775b
--- /dev/null
@@ -0,0 +1,55 @@
+.. include:: ../../Includes.txt
+
+=================================================================================
+Deprecation: #84196 - Backend controller actions do not receive prepared response
+=================================================================================
+
+See :issue:`84196`
+
+Description
+===========
+
+The second argument to backend and eID controller actions has been deprecated.
+Controllers should create a response object implementing
+:php:`Psr\Http\Message\ResponseInterface` on their own instead of relying
+on a prepared response.
+
+The signature of controller actions should look like::
+
+    public function myAction(ServerRequestInterface $request): ResponseInterface
+
+
+Impact
+======
+
+Controllers should typically instantiate one of the three core response classes
+and return it:
+
+.. code-block:: php
+
+    public function myAction(ServerRequestInterface $request): ResponseInterface
+    {
+        return new HtmlResponse('content');
+        return new JsonResponse($jsonArray);
+        return new RedirectRespons($url);
+    }
+
+
+Affected Installations
+======================
+
+Instances with extensions that register backend controllers (eg. modules) or eID
+may be affected.
+
+The dynamic scanning for not yet adapted controller actions relies on reflection and
+costs some CPU cycles. If all affected extensions have been adapted, the feature toggle
+`simplifiedControllerActionDispatching` should be enabled. This can be managed in
+the backend Settings module.
+
+
+Migration
+=========
+
+See above code examples for typical controller actions return values and signature.
+
+.. index:: Backend, PHP-API, NotScanned