From b911da3e1eedff9f7df683889f0d425d47a6fd1c Mon Sep 17 00:00:00 2001 From: Robin Appelman Date: Mon, 13 Feb 2023 14:47:50 +0100 Subject: [PATCH] DI for Router Signed-off-by: Robin Appelman --- lib/private/Route/CachingRouter.php | 27 ++++++---- lib/private/Route/Router.php | 26 +++++++--- lib/private/Server.php | 6 +-- .../lib/AppFramework/Routing/RoutingTest.php | 52 ++++++++++++++++--- tests/lib/Route/RouterTest.php | 12 ++++- 5 files changed, 96 insertions(+), 27 deletions(-) diff --git a/lib/private/Route/CachingRouter.php b/lib/private/Route/CachingRouter.php index f65060e710b88..69fb3c986c961 100644 --- a/lib/private/Route/CachingRouter.php +++ b/lib/private/Route/CachingRouter.php @@ -24,20 +24,27 @@ */ namespace OC\Route; +use OCP\Diagnostics\IEventLogger; +use OCP\ICache; +use OCP\ICacheFactory; +use OCP\IConfig; +use OCP\IRequest; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; class CachingRouter extends Router { - /** - * @var \OCP\ICache - */ - protected $cache; + protected ICache $cache; - /** - * @param \OCP\ICache $cache - */ - public function __construct($cache, LoggerInterface $logger) { - $this->cache = $cache; - parent::__construct($logger); + public function __construct( + ICacheFactory $cacheFactory, + LoggerInterface $logger, + IRequest $request, + IConfig $config, + IEventLogger $eventLogger, + ContainerInterface $container + ) { + $this->cache = $cacheFactory->createLocal('route'); + parent::__construct($logger, $request, $config, $eventLogger, $container); } /** diff --git a/lib/private/Route/Router.php b/lib/private/Route/Router.php index bdd14678fad08..e2a092d861e8a 100644 --- a/lib/private/Route/Router.php +++ b/lib/private/Route/Router.php @@ -35,8 +35,11 @@ use OC\AppFramework\Routing\RouteParser; use OCP\AppFramework\App; use OCP\Diagnostics\IEventLogger; +use OCP\IConfig; +use OCP\IRequest; use OCP\Route\IRouter; use OCP\Util; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Symfony\Component\Routing\Exception\ResourceNotFoundException; use Symfony\Component\Routing\Exception\RouteNotFoundException; @@ -66,11 +69,20 @@ class Router implements IRouter { /** @var RequestContext */ protected $context; private IEventLogger $eventLogger; - - public function __construct(LoggerInterface $logger) { + private IConfig $config; + private ContainerInterface $container; + + public function __construct( + LoggerInterface $logger, + IRequest $request, + IConfig $config, + IEventLogger $eventLogger, + ContainerInterface $container + ) { $this->logger = $logger; + $this->config = $config; $baseUrl = \OC::$WEBROOT; - if (!(\OC::$server->getConfig()->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true')) { + if (!($config->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true')) { $baseUrl .= '/index.php'; } if (!\OC::$CLI && isset($_SERVER['REQUEST_METHOD'])) { @@ -78,13 +90,13 @@ public function __construct(LoggerInterface $logger) { } else { $method = 'GET'; } - $request = \OC::$server->getRequest(); $host = $request->getServerHost(); $schema = $request->getServerProtocol(); $this->context = new RequestContext($baseUrl, $method, $host, $schema); // TODO cache $this->root = $this->getCollection('root'); - $this->eventLogger = \OC::$server->get(IEventLogger::class); + $this->eventLogger = $eventLogger; + $this->container = $container; } /** @@ -253,7 +265,7 @@ public function findMatchingRoute(string $url): array { $this->loadRoutes('settings'); } elseif (substr($url, 0, 6) === '/core/') { \OC::$REQUESTEDAPP = $url; - if (!\OC::$server->getConfig()->getSystemValueBool('maintenance') && !Util::needUpgrade()) { + if (!$this->config->getSystemValueBool('maintenance') && !Util::needUpgrade()) { \OC_App::loadApps(); } $this->loadRoutes('core'); @@ -441,7 +453,7 @@ private function getApplicationClass(string $appName) { $applicationClassName = $appNameSpace . '\\AppInfo\\Application'; if (class_exists($applicationClassName)) { - $application = \OC::$server->query($applicationClassName); + $application = $this->container->get($applicationClassName); } else { $application = new App($appName); } diff --git a/lib/private/Server.php b/lib/private/Server.php index bd33cdf58bd80..f76d45659c6fe 100644 --- a/lib/private/Server.php +++ b/lib/private/Server.php @@ -129,6 +129,7 @@ use OC\Remote\Api\ApiFactory; use OC\Remote\InstanceFactory; use OC\RichObjectStrings\Validator; +use OC\Route\CachingRouter; use OC\Route\Router; use OC\Security\Bruteforce\Throttler; use OC\Security\CertificateManager; @@ -819,11 +820,10 @@ public function __construct($webRoot, \OC\Config $config) { $this->registerService(Router::class, function (Server $c) { $cacheFactory = $c->get(ICacheFactory::class); - $logger = $c->get(LoggerInterface::class); if ($cacheFactory->isLocalCacheAvailable()) { - $router = new \OC\Route\CachingRouter($cacheFactory->createLocal('route'), $logger); + $router = $c->resolve(CachingRouter::class); } else { - $router = new \OC\Route\Router($logger); + $router = $c->resolve(Router::class); } return $router; }); diff --git a/tests/lib/AppFramework/Routing/RoutingTest.php b/tests/lib/AppFramework/Routing/RoutingTest.php index 22037c31d0df6..d7fde02dbcb1d 100644 --- a/tests/lib/AppFramework/Routing/RoutingTest.php +++ b/tests/lib/AppFramework/Routing/RoutingTest.php @@ -6,8 +6,12 @@ use OC\AppFramework\Routing\RouteConfig; use OC\Route\Route; use OC\Route\Router; +use OCP\Diagnostics\IEventLogger; +use OCP\IConfig; +use OCP\IRequest; use OCP\Route\IRouter; use PHPUnit\Framework\MockObject\MockObject; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; class RoutingTest extends \Test\TestCase { @@ -133,7 +137,13 @@ public function testSimpleRouteWithBrokenName() { /** @var IRouter|MockObject $router */ $router = $this->getMockBuilder(Router::class) ->onlyMethods(['create']) - ->setConstructorArgs([$this->createMock(LoggerInterface::class)]) + ->setConstructorArgs([ + $this->createMock(LoggerInterface::class), + $this->createMock(IRequest::class), + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class) + ]) ->getMock(); // load route configuration @@ -154,7 +164,13 @@ public function testSimpleOCSRouteWithBrokenName() { /** @var IRouter|MockObject $router */ $router = $this->getMockBuilder(Router::class) ->onlyMethods(['create']) - ->setConstructorArgs([$this->createMock(LoggerInterface::class)]) + ->setConstructorArgs([ + $this->createMock(LoggerInterface::class), + $this->createMock(IRequest::class), + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class) + ]) ->getMock(); // load route configuration @@ -214,7 +230,13 @@ private function assertSimpleRoute($routes, $name, $verb, $url, $controllerName, /** @var IRouter|MockObject $router */ $router = $this->getMockBuilder(Router::class) ->onlyMethods(['create']) - ->setConstructorArgs([$this->createMock(LoggerInterface::class)]) + ->setConstructorArgs([ + $this->createMock(LoggerInterface::class), + $this->createMock(IRequest::class), + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class) + ]) ->getMock(); // we expect create to be called once: @@ -264,7 +286,13 @@ private function assertSimpleOCSRoute($routes, /** @var IRouter|MockObject $router */ $router = $this->getMockBuilder(Router::class) ->onlyMethods(['create']) - ->setConstructorArgs([$this->createMock(LoggerInterface::class)]) + ->setConstructorArgs([ + $this->createMock(LoggerInterface::class), + $this->createMock(IRequest::class), + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class) + ]) ->getMock(); // we expect create to be called once: @@ -291,7 +319,13 @@ private function assertOCSResource($yaml, $resourceName, $url, $controllerName, /** @var IRouter|MockObject $router */ $router = $this->getMockBuilder(Router::class) ->onlyMethods(['create']) - ->setConstructorArgs([$this->createMock(LoggerInterface::class)]) + ->setConstructorArgs([ + $this->createMock(LoggerInterface::class), + $this->createMock(IRequest::class), + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class) + ]) ->getMock(); // route mocks @@ -338,7 +372,13 @@ private function assertResource($yaml, $resourceName, $url, $controllerName, $pa /** @var IRouter|MockObject $router */ $router = $this->getMockBuilder(Router::class) ->onlyMethods(['create']) - ->setConstructorArgs([$this->createMock(LoggerInterface::class)]) + ->setConstructorArgs([ + $this->createMock(LoggerInterface::class), + $this->createMock(IRequest::class), + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class) + ]) ->getMock(); // route mocks diff --git a/tests/lib/Route/RouterTest.php b/tests/lib/Route/RouterTest.php index 44fef6f1d4fe1..301058f74a625 100644 --- a/tests/lib/Route/RouterTest.php +++ b/tests/lib/Route/RouterTest.php @@ -24,6 +24,10 @@ namespace Test\Route; use OC\Route\Router; +use OCP\Diagnostics\IEventLogger; +use OCP\IConfig; +use OCP\IRequest; +use Psr\Container\ContainerInterface; use Psr\Log\LoggerInterface; use Test\TestCase; @@ -44,7 +48,13 @@ function (string $message, array $data) { $this->fail('Unexpected info log: '.(string)($data['exception'] ?? $message)); } ); - $router = new Router($logger); + $router = new Router( + $logger, + $this->createMock(IRequest::class), + $this->createMock(IConfig::class), + $this->createMock(IEventLogger::class), + $this->createMock(ContainerInterface::class), + ); $this->assertEquals('/index.php/apps/files/', $router->generate('files.view.index'));