[TASK] Improve performance of UriBuilder by memory cache 27/53827/10
authorClaus Due <claus@namelesscoder.net>
Tue, 29 Aug 2017 17:07:31 +0000 (19:07 +0200)
committerBenni Mack <benni@typo3.org>
Sun, 28 Oct 2018 14:23:24 +0000 (15:23 +0100)
The TYPO3 backend is eager to generate the same URL
with the same parameters multiple times, causing a lot
of cascading calls to URI parsing, token generation etc.

Turning the UriBuilder into a Singleton removes many
hundreds of calls to instantiate the object - and remembering
the URLs that were generated further saves many hundred
calls to UriBuilder->buildUri in for example the page module.

Change-Id: I7444ccf8bb27789c489f7d7c15c65449fa3456a0
Resolves: #82237
Releases: master
Reviewed-on: https://review.typo3.org/53827
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
Reviewed-by: Benni Mack <benni@typo3.org>
Tested-by: Benni Mack <benni@typo3.org>
typo3/sysext/backend/Classes/Routing/UriBuilder.php
typo3/sysext/backend/Tests/Unit/Routing/UriBuilderTest.php
typo3/sysext/fluid/Tests/Unit/ViewHelpers/Be/LinkViewHelperTest.php
typo3/sysext/fluid/Tests/Unit/ViewHelpers/Be/UriViewHelperTest.php

index eca11c0..3cacfce 100644 (file)
@@ -18,6 +18,7 @@ use TYPO3\CMS\Backend\Routing\Exception\RouteNotFoundException;
 use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\FormProtection\FormProtectionFactory;
 use TYPO3\CMS\Core\Http\Uri;
+use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 use TYPO3\CMS\Core\Utility\PathUtility;
 
@@ -29,7 +30,7 @@ use TYPO3\CMS\Core\Utility\PathUtility;
  * Currently only available and useful when called from Router->generate() as the information
  * about possible routes needs to be handed over.
  */
-class UriBuilder
+class UriBuilder implements SingletonInterface
 {
     /**
      * Generates an absolute URL
@@ -42,17 +43,22 @@ class UriBuilder
     const ABSOLUTE_PATH = 'absolute';
 
     /**
-     * @var Route[]
+     * @var Router
      */
-    protected $routes;
+    protected $router;
 
     /**
-     * Fetches the available routes from the Router to be used for generating routes
+     * @var array
      */
-    protected function loadBackendRoutes()
+    protected $generated = [];
+
+    /**
+     * Loads the router to fetch the available routes from the Router to be used for generating routes
+     * @param Router|null $router
+     */
+    public function __construct(Router $router = null)
     {
-        $router = GeneralUtility::makeInstance(Router::class);
-        $this->routes = $router->getRoutes();
+        $this->router = $router ?? GeneralUtility::makeInstance(Router::class);
     }
 
     /**
@@ -88,12 +94,15 @@ class UriBuilder
      */
     public function buildUriFromRoute($name, $parameters = [], $referenceType = self::ABSOLUTE_PATH)
     {
-        $this->loadBackendRoutes();
-        if (!isset($this->routes[$name])) {
+        $cacheIdentifier = 'route' . $name . serialize($parameters) . $referenceType;
+        if (isset($this->generated[$cacheIdentifier])) {
+            return $this->generated[$cacheIdentifier];
+        }
+        if (!isset($this->router->getRoutes()[$name])) {
             throw new RouteNotFoundException('Unable to generate a URL for the named route "' . $name . '" because this route was not found.', 1476050190);
         }
 
-        $route = $this->routes[$name];
+        $route = $this->router->getRoutes()[$name];
         $parameters = array_merge(
             $route->getOptions()['parameters'] ?? [],
             $parameters
@@ -111,7 +120,8 @@ class UriBuilder
             'route' => $route->getPath()
         ] + $parameters;
 
-        return $this->buildUri($parameters, $referenceType);
+        $this->generated[$cacheIdentifier] = $this->buildUri($parameters, $referenceType);
+        return $this->generated[$cacheIdentifier];
     }
 
     /**
@@ -127,11 +137,16 @@ class UriBuilder
     public function buildUriFromModule($moduleName, $parameters = [], $referenceType = self::ABSOLUTE_PATH)
     {
         trigger_error('UriBuilder->buildUriFromModule() will be removed in TYPO3 v10.0, use buildUriFromRoute() instead.', E_USER_DEPRECATED);
+        $cacheIdentifier = 'module' . $moduleName . serialize($parameters) . $referenceType;
+        if (isset($this->generated[$cacheIdentifier])) {
+            return $this->generated[$cacheIdentifier];
+        }
         $parameters = [
             'route' => $moduleName,
             'token' => FormProtectionFactory::get('backend')->generateToken('route', $moduleName)
         ] + $parameters;
-        return $this->buildUri($parameters, $referenceType);
+        $this->generated[$cacheIdentifier] = $this->buildUri($parameters, $referenceType);
+        return $this->generated[$cacheIdentifier];
     }
 
     /**
index 3469534..ccab0a8 100644 (file)
@@ -16,8 +16,8 @@ namespace TYPO3\CMS\Backend\Tests\Unit\Routing;
 
 use TYPO3\CMS\Backend\Routing\Exception\RouteNotFoundException;
 use TYPO3\CMS\Backend\Routing\Route;
+use TYPO3\CMS\Backend\Routing\Router;
 use TYPO3\CMS\Backend\Routing\UriBuilder;
-use TYPO3\TestingFramework\Core\AccessibleObjectInterface;
 use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 
 /**
@@ -26,17 +26,6 @@ use TYPO3\TestingFramework\Core\Unit\UnitTestCase;
 class UriBuilderTest extends UnitTestCase
 {
     /**
-     * @var UriBuilder|\PHPUnit_Framework_MockObject_MockObject|AccessibleObjectInterface
-     */
-    protected $uriBuilder;
-
-    protected function setUp()
-    {
-        parent::setUp();
-        $this->uriBuilder = $this->getAccessibleMock(UriBuilder::class, ['loadBackendRoutes']);
-    }
-
-    /**
      * @return array
      */
     public function validRoutesAreBuiltDataProvider()
@@ -96,9 +85,12 @@ class UriBuilderTest extends UnitTestCase
         array $routeParameters,
         string $expectation
     ) {
-        $this->uriBuilder->_set('routes', $routes);
-
-        $uri = $this->uriBuilder->buildUriFromRoute(
+        $router = new Router();
+        foreach ($routes as $routeName => $route) {
+            $router->addRoute($routeName, $route);
+        }
+        $subject = new UriBuilder($router);
+        $uri = $subject->buildUriFromRoute(
             $routeName,
             $routeParameters
         );
@@ -113,6 +105,7 @@ class UriBuilderTest extends UnitTestCase
     {
         $this->expectException(RouteNotFoundException::class);
         $this->expectExceptionCode(1476050190);
-        $this->uriBuilder->buildUriFromRoute(uniqid('any'));
+        $subject = new UriBuilder(new Router());
+        $subject->buildUriFromRoute(uniqid('any'));
     }
 }
index 1e45076..5425dc3 100644 (file)
@@ -37,6 +37,8 @@ class LinkViewHelperTest extends ViewHelperBaseTestcase
      */
     protected $uriBuilderMock;
 
+    protected $resetSingletonInstances = true;
+
     /**
      * setUp function
      */
@@ -91,7 +93,7 @@ class LinkViewHelperTest extends ViewHelperBaseTestcase
             'referenceType' => 'theReferenceTypeArgument'
         ]);
 
-        GeneralUtility::addInstance(UriBuilder::class, $this->uriBuilderMock);
+        GeneralUtility::setSingletonInstance(UriBuilder::class, $this->uriBuilderMock);
 
         $this->uriBuilderMock->expects($this->once())->method('buildUriFromRoute')
             ->with('theRouteArgument', ['parameter' => 'to pass'], 'theReferenceTypeArgument')->willReturn('theUri');
@@ -116,7 +118,7 @@ class LinkViewHelperTest extends ViewHelperBaseTestcase
                 'referenceType' => 'theReferenceTypeArgument'
             ]
         );
-        GeneralUtility::addInstance(UriBuilder::class, $this->uriBuilderMock);
+        GeneralUtility::setSingletonInstance(UriBuilder::class, $this->uriBuilderMock);
 
         $this->uriBuilderMock->expects($this->once())->method('buildUriFromRoute')
             ->with('theRouteArgument', [], 'theReferenceTypeArgument')->willReturn('theUri');
index dd52c81..7d1e35b 100644 (file)
@@ -36,6 +36,8 @@ class UriViewHelperTest extends ViewHelperBaseTestcase
      */
     protected $uriBuilderMock;
 
+    protected $resetSingletonInstances = true;
+
     /**
      * setUp function
      */
@@ -75,7 +77,7 @@ class UriViewHelperTest extends ViewHelperBaseTestcase
             'referenceType' => 'theReferenceTypeArgument'
         ]);
 
-        GeneralUtility::addInstance(UriBuilder::class, $this->uriBuilderMock);
+        GeneralUtility::setSingletonInstance(UriBuilder::class, $this->uriBuilderMock);
 
         $this->uriBuilderMock->expects($this->once())->method('buildUriFromRoute')
             ->with('theRouteArgument', ['parameter' => 'to pass'], 'theReferenceTypeArgument')->willReturn('theUri');
@@ -95,7 +97,7 @@ class UriViewHelperTest extends ViewHelperBaseTestcase
                 'referenceType' => 'theReferenceTypeArgument'
             ]
         );
-        GeneralUtility::addInstance(UriBuilder::class, $this->uriBuilderMock);
+        GeneralUtility::setSingletonInstance(UriBuilder::class, $this->uriBuilderMock);
 
         $this->uriBuilderMock->expects($this->once())->method('buildUriFromRoute')
             ->with('theRouteArgument', [], 'theReferenceTypeArgument')->willReturn('theUri');