[TASK] Hook up ContainerInterface in backend route dispatcher 46/61246/4
authorBenjamin Franzke <bfr@qbus.de>
Thu, 4 Jul 2019 18:10:52 +0000 (20:10 +0200)
committerBenni Mack <benni@typo3.org>
Sat, 13 Jul 2019 17:34:32 +0000 (19:34 +0200)
Transform InfoModuleController and it's dependency ModuleTemplate
into a symfony manageed services  to retrieve dependencies from
symfony instead of GeneralUtility::makeInstance.
ModuleTemplate is a prototype and is therefore marked shared: false.
It is marked public: true so legacy calls to
GeneralUtility::makeInstance(ModuleTemplate::class) resort to the
container and properly inject dependencies.

Also add a backend.controller symfony tag, to be universally
applied for backend controllers. This tag automatically configures
the controller to be publicly available from $container->get()
which allows the route dispatcher to lazily instantiate
the controller.

Releases: master
Resolves: #88721
Change-Id: I076c306736243e693542f2774dbe1108e28fe731
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/61246
Tested-by: TYPO3com <noreply@typo3.com>
Tested-by: Jörg Bösche <typo3@joergboesche.de>
Tested-by: Steffen Frese <steffenf14@gmail.com>
Tested-by: Benni Mack <benni@typo3.org>
Reviewed-by: Jörg Bösche <typo3@joergboesche.de>
Reviewed-by: Steffen Frese <steffenf14@gmail.com>
Reviewed-by: Benni Mack <benni@typo3.org>
typo3/sysext/backend/Classes/Template/ModuleTemplate.php
typo3/sysext/backend/Configuration/Services.php [new file with mode: 0644]
typo3/sysext/backend/Configuration/Services.yaml
typo3/sysext/backend/Tests/Unit/Http/RouteDispatcherTest.php
typo3/sysext/core/Classes/Http/Dispatcher.php
typo3/sysext/info/Classes/Controller/InfoModuleController.php
typo3/sysext/info/Configuration/Services.yaml

index 3ad0f73..8134550 100644 (file)
@@ -143,6 +143,11 @@ class ModuleTemplate
     protected $iconFactory;
 
     /**
+     * @var FlashMessageService
+     */
+    protected $flashMessageService;
+
+    /**
      * Module ID
      *
      * @var string
@@ -247,18 +252,25 @@ class ModuleTemplate
      * Class constructor
      * Sets up view and property objects
      *
+     * @param PageRenderer $pageRenderer
+     * @param IconFactory $iconFactory
+     * @param FlashMessageService $flashMessageService
      * @throws InvalidTemplateResourceException In case a template is invalid
      */
-    public function __construct()
-    {
+    public function __construct(
+        PageRenderer $pageRenderer,
+        IconFactory $iconFactory,
+        FlashMessageService $flashMessageService
+    ) {
         $this->view = GeneralUtility::makeInstance(StandaloneView::class);
         $this->view->setPartialRootPaths($this->partialRootPaths);
         $this->view->setTemplateRootPaths($this->templateRootPaths);
         $this->view->setLayoutRootPaths($this->layoutRootPaths);
         $this->view->setTemplate($this->templateFile);
-        $this->pageRenderer = GeneralUtility::makeInstance(PageRenderer::class);
+        $this->pageRenderer = $pageRenderer;
+        $this->iconFactory = $iconFactory;
+        $this->flashMessageService = $flashMessageService;
         $this->docHeaderComponent = GeneralUtility::makeInstance(DocHeaderComponent::class);
-        $this->iconFactory = GeneralUtility::makeInstance(IconFactory::class);
     }
 
     /**
@@ -703,9 +715,7 @@ class ModuleTemplate
     protected function getFlashMessageQueue()
     {
         if (!isset($this->flashMessageQueue)) {
-            /** @var FlashMessageService $service */
-            $service = GeneralUtility::makeInstance(FlashMessageService::class);
-            $this->flashMessageQueue = $service->getMessageQueueByIdentifier();
+            $this->flashMessageQueue = $this->flashMessageService->getMessageQueueByIdentifier();
         }
         return $this->flashMessageQueue;
     }
diff --git a/typo3/sysext/backend/Configuration/Services.php b/typo3/sysext/backend/Configuration/Services.php
new file mode 100644 (file)
index 0000000..b61af9a
--- /dev/null
@@ -0,0 +1,11 @@
+<?php
+declare(strict_types = 1);
+namespace TYPO3\CMS\Backend;
+
+use Symfony\Component\DependencyInjection\ContainerBuilder;
+use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator;
+use TYPO3\CMS\Core\DependencyInjection\PublicServicePass;
+
+return function (ContainerConfigurator $container, ContainerBuilder $containerBuilder) {
+    $containerBuilder->addCompilerPass(new PublicServicePass('backend.controller'));
+};
index 1e41e82..5254a59 100644 (file)
@@ -11,3 +11,7 @@ services:
   # @todo: Fix typo3/testing-framework and remove this
   TYPO3\CMS\Backend\View\BackendTemplateView:
     autoconfigure: false
+
+  TYPO3\CMS\Backend\Template\ModuleTemplate:
+    shared: false
+    public: true
index 8280bad..7898bd6 100644 (file)
@@ -15,6 +15,7 @@ namespace TYPO3\CMS\Backend\Tests\Unit\Http;
  */
 
 use Prophecy\Argument;
+use Psr\Container\ContainerInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Backend\Http\RouteDispatcher;
@@ -60,6 +61,8 @@ class RouteDispatcherTest extends UnitTestCase
         $routeProphecy->getOption('module')->willReturn(false);
         $requestProphecy->withAttribute('route', $routeProphecy->reveal())->willReturn($requestProphecy->reveal());
         $requestProphecy->getAttribute('route')->willReturn($routeProphecy->reveal());
+        $containerProphecy = $this->prophesize(ContainerInterface::class);
+        $containerProphecy->has(Argument::any())->willReturn(false);
 
         $target = 42;
         $routeProphecy->getOption('target')->willReturn($target);
@@ -68,7 +71,7 @@ class RouteDispatcherTest extends UnitTestCase
         $this->expectException(\InvalidArgumentException::class);
         $this->expectExceptionCode(1425381442);
 
-        $subject = new RouteDispatcher();
+        $subject = new RouteDispatcher($containerProphecy->reveal());
         $subject->dispatch($requestProphecy->reveal(), $responseProphecy->reveal());
     }
 
@@ -91,6 +94,8 @@ class RouteDispatcherTest extends UnitTestCase
         $routeProphecy->getOption('module')->willReturn(false);
         $requestProphecy->withAttribute('route', $routeProphecy->reveal())->willReturn($requestProphecy->reveal());
         $requestProphecy->getAttribute('route')->willReturn($routeProphecy->reveal());
+        $containerProphecy = $this->prophesize(ContainerInterface::class);
+        $containerProphecy->has(Argument::any())->willReturn(false);
 
         $target = [
             RouteDispatcherClassFixture::class,
@@ -102,7 +107,7 @@ class RouteDispatcherTest extends UnitTestCase
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionCode(1520756142);
 
-        $subject = new RouteDispatcher();
+        $subject = new RouteDispatcher($containerProphecy->reveal());
         $subject->dispatch($requestProphecy->reveal(), $responseProphecy->reveal());
     }
 
@@ -125,6 +130,8 @@ class RouteDispatcherTest extends UnitTestCase
         $routeProphecy->getOption('module')->willReturn(false);
         $requestProphecy->withAttribute('route', $routeProphecy->reveal())->willReturn($requestProphecy->reveal());
         $requestProphecy->getAttribute('route')->willReturn($routeProphecy->reveal());
+        $containerProphecy = $this->prophesize(ContainerInterface::class);
+        $containerProphecy->has(Argument::any())->willReturn(false);
 
         $target = function (ServerRequestInterface $request) {
             throw new \RuntimeException('I have been called. Good!', 1520756466);
@@ -135,7 +142,7 @@ class RouteDispatcherTest extends UnitTestCase
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionCode(1520756466);
 
-        $subject = new RouteDispatcher();
+        $subject = new RouteDispatcher($containerProphecy->reveal());
         $subject->dispatch($requestProphecy->reveal(), $responseProphecy->reveal());
     }
 
@@ -158,6 +165,8 @@ class RouteDispatcherTest extends UnitTestCase
         $routeProphecy->getOption('module')->willReturn(false);
         $requestProphecy->withAttribute('route', $routeProphecy->reveal())->willReturn($requestProphecy->reveal());
         $requestProphecy->getAttribute('route')->willReturn($routeProphecy->reveal());
+        $containerProphecy = $this->prophesize(ContainerInterface::class);
+        $containerProphecy->has(Argument::any())->willReturn(false);
 
         $target = RouteDispatcherClassInvokeFixture::class;
         $routeProphecy->getOption('target')->willReturn($target);
@@ -166,7 +175,41 @@ class RouteDispatcherTest extends UnitTestCase
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionCode(1520756623);
 
-        $subject = new RouteDispatcher();
+        $subject = new RouteDispatcher($containerProphecy->reveal());
+        $subject->dispatch($requestProphecy->reveal(), $responseProphecy->reveal());
+    }
+
+    /**
+     * @test
+     */
+    public function dispatchCallsTargetIfTargetIsInContainer()
+    {
+        $formProtectionProphecy = $this->prophesize(AbstractFormProtection::class);
+        $formProtectionProphecy->validateToken(Argument::cetera())->willReturn(true);
+        FormProtectionFactory::set('default', $formProtectionProphecy->reveal());
+
+        $requestProphecy = $this->prophesize(ServerRequestInterface::class);
+        $responseProphecy = $this->prophesize(ResponseInterface::class);
+        $routerProphecy = $this->prophesize(Router::class);
+        GeneralUtility::setSingletonInstance(Router::class, $routerProphecy->reveal());
+        $routeProphecy = $this->prophesize(Route::class);
+        $routerProphecy->matchRequest($requestProphecy->reveal())->willReturn($routeProphecy->reveal());
+        $routeProphecy->getOption('access')->willReturn('public');
+        $routeProphecy->getOption('module')->willReturn(false);
+        $requestProphecy->withAttribute('route', $routeProphecy->reveal())->willReturn($requestProphecy->reveal());
+        $requestProphecy->getAttribute('route')->willReturn($routeProphecy->reveal());
+
+        $target = 'routedispatcher.classinvokefixture';
+        $routeProphecy->getOption('target')->willReturn($target);
+        $requestProphecy->withAttribute('target', $target)->willReturn($requestProphecy->reveal());
+        $containerProphecy = $this->prophesize(ContainerInterface::class);
+        $containerProphecy->has($target)->willReturn(true);
+        $containerProphecy->get($target)->willReturn(new RouteDispatcherClassInvokeFixture);
+
+        $this->expectException(\RuntimeException::class);
+        $this->expectExceptionCode(1520756623);
+
+        $subject = new RouteDispatcher($containerProphecy->reveal());
         $subject->dispatch($requestProphecy->reveal(), $responseProphecy->reveal());
     }
 
@@ -189,6 +232,8 @@ class RouteDispatcherTest extends UnitTestCase
         $routeProphecy->getOption('module')->willReturn(false);
         $requestProphecy->withAttribute('route', $routeProphecy->reveal())->willReturn($requestProphecy->reveal());
         $requestProphecy->getAttribute('route')->willReturn($routeProphecy->reveal());
+        $containerProphecy = $this->prophesize(ContainerInterface::class);
+        $containerProphecy->has(Argument::any())->willReturn(false);
 
         $target = RouteDispatcherClassWithoutInvokeFixture::class;
         $routeProphecy->getOption('target')->willReturn($target);
@@ -197,7 +242,7 @@ class RouteDispatcherTest extends UnitTestCase
         $this->expectException(\InvalidArgumentException::class);
         $this->expectExceptionCode(1442431631);
 
-        $subject = new RouteDispatcher();
+        $subject = new RouteDispatcher($containerProphecy->reveal());
         $subject->dispatch($requestProphecy->reveal(), $responseProphecy->reveal());
     }
 
@@ -220,6 +265,8 @@ class RouteDispatcherTest extends UnitTestCase
         $routeProphecy->getOption('module')->willReturn(false);
         $requestProphecy->withAttribute('route', $routeProphecy->reveal())->willReturn($requestProphecy->reveal());
         $requestProphecy->getAttribute('route')->willReturn($routeProphecy->reveal());
+        $containerProphecy = $this->prophesize(ContainerInterface::class);
+        $containerProphecy->has(Argument::any())->willReturn(false);
 
         $target = RouteDispatcherClassFixture::class . '::mainAction';
         $routeProphecy->getOption('target')->willReturn($target);
@@ -228,7 +275,7 @@ class RouteDispatcherTest extends UnitTestCase
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionCode(1520756142);
 
-        $subject = new RouteDispatcher();
+        $subject = new RouteDispatcher($containerProphecy->reveal());
         $subject->dispatch($requestProphecy->reveal(), $responseProphecy->reveal());
     }
 
@@ -251,6 +298,8 @@ class RouteDispatcherTest extends UnitTestCase
         $routeProphecy->getOption('module')->willReturn(false);
         $requestProphecy->withAttribute('route', $routeProphecy->reveal())->willReturn($requestProphecy->reveal());
         $requestProphecy->getAttribute('route')->willReturn($routeProphecy->reveal());
+        $containerProphecy = $this->prophesize(ContainerInterface::class);
+        $containerProphecy->has(Argument::any())->willReturn(false);
 
         $target = RouteDispatcherStaticClassFixture::class . '::mainAction';
         $routeProphecy->getOption('target')->willReturn($target);
@@ -259,7 +308,7 @@ class RouteDispatcherTest extends UnitTestCase
         $this->expectException(\RuntimeException::class);
         $this->expectExceptionCode(1520757000);
 
-        $subject = new RouteDispatcher();
+        $subject = new RouteDispatcher($containerProphecy->reveal());
         $subject->dispatch($requestProphecy->reveal(), $responseProphecy->reveal());
     }
 }
index 56a556e..d2998b2 100644 (file)
@@ -14,6 +14,7 @@ namespace TYPO3\CMS\Core\Http;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Psr\Container\ContainerInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
@@ -27,6 +28,16 @@ use TYPO3\CMS\Core\Utility\GeneralUtility;
 class Dispatcher implements DispatcherInterface
 {
     /**
+     * @var ContainerInterface
+     */
+    protected $container;
+
+    public function __construct(ContainerInterface $container)
+    {
+        $this->container = $container;
+    }
+
+    /**
      * Main method that fetches the target from the request and calls the target directly
      *
      * @param ServerRequestInterface $request the current server request
@@ -61,7 +72,7 @@ class Dispatcher implements DispatcherInterface
 
         // Only a class name is given
         if (is_string($target) && strpos($target, ':') === false) {
-            $targetObject = GeneralUtility::makeInstance($target);
+            $targetObject = $this->container->has($target) ? $this->container->get($target) : GeneralUtility::makeInstance($target);
             if (!method_exists($targetObject, '__invoke')) {
                 throw new \InvalidArgumentException('Object "' . $target . '" doesn\'t implement an __invoke() method and cannot be used as target.', 1442431631);
             }
@@ -71,7 +82,7 @@ class Dispatcher implements DispatcherInterface
         // Check if the target is a concatenated string of "className::actionMethod"
         if (is_string($target) && strpos($target, '::') !== false) {
             list($className, $methodName) = explode('::', $target, 2);
-            $targetObject = GeneralUtility::makeInstance($className);
+            $targetObject = $this->container->has($className) ? $this->container->get($className) : GeneralUtility::makeInstance($className);
             return [$targetObject, $methodName];
         }
 
index 9d653aa..3b05e1a 100644 (file)
@@ -14,6 +14,7 @@ namespace TYPO3\CMS\Info\Controller;
  * The TYPO3 project - inspiring people to share!
  */
 
+use Psr\Container\ContainerInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Http\Message\ServerRequestInterface;
 use TYPO3\CMS\Backend\Routing\UriBuilder;
@@ -62,6 +63,21 @@ class InfoModuleController
     protected $view;
 
     /**
+     * @var UriBuilder
+     */
+    protected $uriBuilder;
+
+    /**
+     * @var FlashMessageService
+     */
+    protected $flashMessageService;
+
+    /**
+     * @var ContainerInterface
+     */
+    protected $container;
+
+    /**
      * @var int Value of the GET/POST var 'id'
      */
     protected $id;
@@ -144,9 +160,17 @@ class InfoModuleController
     /**
      * Constructor
      */
-    public function __construct()
-    {
-        $this->moduleTemplate = GeneralUtility::makeInstance(ModuleTemplate::class);
+    public function __construct(
+        ModuleTemplate $moduleTemplate,
+        UriBuilder $uriBuilder,
+        FlashMessageService $flashMessageService,
+        ContainerInterface $container
+    ) {
+        $this->moduleTemplate = $moduleTemplate;
+        $this->uriBuilder = $uriBuilder;
+        $this->flashMessageService = $flashMessageService;
+        $this->container = $container;
+
         $languageService = $this->getLanguageService();
         $languageService->includeLLFile('EXT:info/Resources/Private/Language/locallang_mod_web_info.xlf');
     }
@@ -195,8 +219,7 @@ class InfoModuleController
             $this->moduleTemplate->getPageRenderer()->loadRequireJsModule('TYPO3/CMS/Backend/ContextMenu');
 
             $this->view = $this->getFluidTemplateObject();
-            $uriBuilder = GeneralUtility::makeInstance(UriBuilder::class);
-            $this->view->assign('moduleName', (string)$uriBuilder->buildUriFromRoute($this->moduleName));
+            $this->view->assign('moduleName', (string)$this->uriBuilder->buildUriFromRoute($this->moduleName));
             $this->view->assign('functionMenuModuleContent', $this->getExtObjContent());
             // Setting up the buttons and markers for doc header
             $this->getButtons();
@@ -282,12 +305,11 @@ class InfoModuleController
     {
         $menu = $this->moduleTemplate->getDocHeaderComponent()->getMenuRegistry()->makeMenu();
         $menu->setIdentifier('WebInfoJumpMenu');
-        $uriBuilder = GeneralUtility::makeInstance(UriBuilder::class);
         foreach ($this->MOD_MENU['function'] as $controller => $title) {
             $item = $menu
                 ->makeMenuItem()
                 ->setHref(
-                    (string)$uriBuilder->buildUriFromRoute(
+                    (string)$this->uriBuilder->buildUriFromRoute(
                         $this->moduleName,
                         [
                             'id' => $this->id,
@@ -412,7 +434,11 @@ class InfoModuleController
     protected function checkExtObj()
     {
         if (is_array($this->extClassConf) && $this->extClassConf['name']) {
-            $this->extObj = GeneralUtility::makeInstance($this->extClassConf['name']);
+            if ($this->container->has($this->extClassConf['name'])) {
+                $this->extObj = $this->container->get($this->extClassConf['name']);
+            } else {
+                $this->extObj = GeneralUtility::makeInstance($this->extClassConf['name']);
+            }
             if (is_callable([$this->extObj, 'init'])) {
                 $this->extObj->init($this);
             }
@@ -434,9 +460,8 @@ class InfoModuleController
                 $languageService->getLL('title'),
                 FlashMessage::ERROR
             );
-            $flashMessageService = GeneralUtility::makeInstance(FlashMessageService::class);
             /** @var \TYPO3\CMS\Core\Messaging\FlashMessageQueue $defaultFlashMessageQueue */
-            $defaultFlashMessageQueue = $flashMessageService->getMessageQueueByIdentifier();
+            $defaultFlashMessageQueue = $this->flashMessageService->getMessageQueueByIdentifier();
             $defaultFlashMessageQueue->enqueue($flashMessage);
         } else {
             if (is_callable([$this->extObj, 'main'])) {
index cb3863e..bc5b307 100644 (file)
@@ -6,3 +6,6 @@ services:
 
   TYPO3\CMS\Info\:
     resource: '../Classes/*'
+
+  TYPO3\CMS\Info\Controller\InfoModuleController:
+    tags: ['backend.controller']