Do not create a RouteActionHandler object for each route

This is not required and doesn't allow us to be properly lazy. On top of
it this doesnt allow us to cache the routes (since closures/objects
can't be cached).

This is the first small step into cleaning up the routing we have

Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>

diff --git a/lib/private/AppFramework/Routing/RouteConfig.php b/lib/private/AppFramework/Routing/RouteConfig.php
index c10aafd55f71c2e530405f6506d012ffba352c54..9a74564b6132c473c4becd1d4a98d108dcf6e4a1 100644 (file)
--- a/lib/private/AppFramework/Routing/RouteConfig.php
+++ b/lib/private/AppFramework/Routing/RouteConfig.php
@@ -140,12 +140,9 @@ class RouteConfig {
 
                $routeName = $routeNamePrefix . $this->appName . '.' . $controller . '.' . $action . $postfix;
 
-               // register the route
-               $handler = new RouteActionHandler($this->container, $controllerName, $actionName);
-
                $router = $this->router->create($routeName, $url)
                        ->method($verb)
-                       ->action($handler);
+                       ->setDefault('caller', [$this->appName, $controllerName, $actionName]);
 
                // optionally register requirements for route. This is used to
                // tell the route parser how url parameters should be matched
@@ -233,9 +230,9 @@ class RouteConfig {
 
                                $routeName = $routeNamePrefix . $this->appName . '.' . strtolower($resource) . '.' . strtolower($method);
 
-                               $this->router->create($routeName, $url)->method($verb)->action(
-                                       new RouteActionHandler($this->container, $controllerName, $actionName)
-                               );
+                               $this->router->create($routeName, $url)
+                                       ->method($verb)
+                                       ->setDefault('caller', [$this->appName, $controllerName, $actionName]);
                        }
                }
        }

diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php
index 0e436339013ff2f63cbedf144c774910a4a99974..3bb29961454cca200ec13043d8a62429845fec16 100644 (file)
--- a/lib/private/Route/Router.php
+++ b/lib/private/Route/Router.php
@@ -288,7 +288,12 @@ class Router implements IRouter {
                }
 
                \OC::$server->getEventLogger()->start('run_route', 'Run route');
-               if (isset($parameters['action'])) {
+               if (isset($parameters['caller'])) {
+                       $caller = $parameters['caller'];
+                       unset($parameters['caller']);
+                       $application = $this->getApplicationClass($caller[0]);
+                       \OC\AppFramework\App::main($caller[1], $caller[2], $application->getContainer(), $parameters);
+               } elseif (isset($parameters['action'])) {
                        $action = $parameters['action'];
                        if (!is_callable($action)) {
                                throw new \Exception('not a callable action');
@@ -394,17 +399,22 @@ class Router implements IRouter {
         */
        private function setupRoutes($routes, $appName) {
                if (is_array($routes)) {
-                       $appNameSpace = App::buildAppNamespace($appName);
+                       $application = $this->getApplicationClass($appName);
+                       $application->registerRoutes($this, $routes);
+               }
+       }
 
-                       $applicationClassName = $appNameSpace . '\\AppInfo\\Application';
+       private function getApplicationClass(string $appName) {
+               $appNameSpace = App::buildAppNamespace($appName);
 
-                       if (class_exists($applicationClassName)) {
-                               $application = \OC::$server->query($applicationClassName);
-                       } else {
-                               $application = new App($appName);
-                       }
+               $applicationClassName = $appNameSpace . '\\AppInfo\\Application';
 
-                       $application->registerRoutes($this, $routes);
+               if (class_exists($applicationClassName)) {
+                       $application = \OC::$server->query($applicationClassName);
+               } else {
+                       $application = new App($appName);
                }
+
+               return $application;
        }
 }

diff --git a/tests/lib/AppFramework/Routing/RoutingTest.php b/tests/lib/AppFramework/Routing/RoutingTest.php
index 34aaff82310d855f54acf63b1c9ae3e4828fe707..c078023e653271bd674e969c0f71e05f3ca17687 100644 (file)
--- a/tests/lib/AppFramework/Routing/RoutingTest.php
+++ b/tests/lib/AppFramework/Routing/RoutingTest.php
@@ -3,7 +3,6 @@
 namespace Test\AppFramework\Routing;
 
 use OC\AppFramework\DependencyInjection\DIContainer;
-use OC\AppFramework\Routing\RouteActionHandler;
 use OC\AppFramework\Routing\RouteConfig;
 use OC\Route\Route;
 use OC\Route\Router;
@@ -420,7 +419,7 @@ class RoutingTest extends \Test\TestCase {
                array $defaults=[]
        ) {
                $route = $this->getMockBuilder(Route::class)
-                       ->onlyMethods(['method', 'action', 'requirements', 'defaults'])
+                       ->onlyMethods(['method', 'setDefault', 'requirements', 'defaults'])
                        ->disableOriginalConstructor()
                        ->getMock();
                $route
@@ -431,8 +430,8 @@ class RoutingTest extends \Test\TestCase {
 
                $route
                        ->expects($this->once())
-                       ->method('action')
-                       ->with($this->equalTo(new RouteActionHandler($container, $controllerName, $actionName)))
+                       ->method('setDefault')
+                       ->with('caller', ['app1', $controllerName, $actionName])
                        ->willReturn($route);
 
                if (count($requirements) > 0) {