From edc1c77dd917e232840084dd3917df62c06eb25b Mon Sep 17 00:00:00 2001 From: Roeland Jago Douma Date: Wed, 1 Jul 2020 09:45:46 +0200 Subject: [PATCH] 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 --- .../AppFramework/Routing/RouteConfig.php | 11 +++---- lib/private/Route/Router.php | 32 ++++++++++++------- .../lib/AppFramework/Routing/RoutingTest.php | 7 ++-- 3 files changed, 28 insertions(+), 22 deletions(-) diff --git a/lib/private/AppFramework/Routing/RouteConfig.php b/lib/private/AppFramework/Routing/RouteConfig.php index c10aafd55f..9a74564b61 100644 --- 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 0e43633901..3bb2996145 100644 --- 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); - - $applicationClassName = $appNameSpace . '\\AppInfo\\Application'; - - if (class_exists($applicationClassName)) { - $application = \OC::$server->query($applicationClassName); - } else { - $application = new App($appName); - } - + $application = $this->getApplicationClass($appName); $application->registerRoutes($this, $routes); } } + + private function getApplicationClass(string $appName) { + $appNameSpace = App::buildAppNamespace($appName); + + $applicationClassName = $appNameSpace . '\\AppInfo\\Application'; + + 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 34aaff8231..c078023e65 100644 --- 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) {