Skip to content

Commit b911da3

Browse files
icewind1991icewind1991
committed
DI for Router
Signed-off-by: Robin Appelman <robin@icewind.nl>
1 parent b68be79 commit b911da3

5 files changed

Lines changed: 96 additions & 27 deletions

File tree

‎lib/private/Route/CachingRouter.php‎

Lines changed: 17 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -24,20 +24,27 @@
2424
*/
2525
namespace OC\Route;
2626

27+
use OCP\Diagnostics\IEventLogger;
28+
use OCP\ICache;
29+
use OCP\ICacheFactory;
30+
use OCP\IConfig;
31+
use OCP\IRequest;
32+
use Psr\Container\ContainerInterface;
2733
use Psr\Log\LoggerInterface;
2834

2935
class CachingRouter extends Router {
30-
/**
31-
* @var \OCP\ICache
32-
*/
33-
protected $cache;
36+
protected ICache $cache;
3437

35-
/**
36-
* @param \OCP\ICache $cache
37-
*/
38-
public function __construct($cache, LoggerInterface $logger) {
39-
$this->cache = $cache;
40-
parent::__construct($logger);
38+
public function __construct(
39+
ICacheFactory $cacheFactory,
40+
LoggerInterface $logger,
41+
IRequest $request,
42+
IConfig $config,
43+
IEventLogger $eventLogger,
44+
ContainerInterface $container
45+
) {
46+
$this->cache = $cacheFactory->createLocal('route');
47+
parent::__construct($logger, $request, $config, $eventLogger, $container);
4148
}
4249

4350
/**

‎lib/private/Route/Router.php‎

Lines changed: 19 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,11 @@
3535
use OC\AppFramework\Routing\RouteParser;
3636
use OCP\AppFramework\App;
3737
use OCP\Diagnostics\IEventLogger;
38+
use OCP\IConfig;
39+
use OCP\IRequest;
3840
use OCP\Route\IRouter;
3941
use OCP\Util;
42+
use Psr\Container\ContainerInterface;
4043
use Psr\Log\LoggerInterface;
4144
use Symfony\Component\Routing\Exception\ResourceNotFoundException;
4245
use Symfony\Component\Routing\Exception\RouteNotFoundException;
@@ -66,25 +69,34 @@ class Router implements IRouter {
6669
/** @var RequestContext */
6770
protected $context;
6871
private IEventLogger $eventLogger;
69-
70-
public function __construct(LoggerInterface $logger) {
72+
private IConfig $config;
73+
private ContainerInterface $container;
74+
75+
public function __construct(
76+
LoggerInterface $logger,
77+
IRequest $request,
78+
IConfig $config,
79+
IEventLogger $eventLogger,
80+
ContainerInterface $container
81+
) {
7182
$this->logger = $logger;
83+
$this->config = $config;
7284
$baseUrl = \OC::$WEBROOT;
73-
if (!(\OC::$server->getConfig()->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true')) {
85+
if (!($config->getSystemValue('htaccess.IgnoreFrontController', false) === true || getenv('front_controller_active') === 'true')) {
7486
$baseUrl .= '/index.php';
7587
}
7688
if (!\OC::$CLI && isset($_SERVER['REQUEST_METHOD'])) {
7789
$method = $_SERVER['REQUEST_METHOD'];
7890
} else {
7991
$method = 'GET';
8092
}
81-
$request = \OC::$server->getRequest();
8293
$host = $request->getServerHost();
8394
$schema = $request->getServerProtocol();
8495
$this->context = new RequestContext($baseUrl, $method, $host, $schema);
8596
// TODO cache
8697
$this->root = $this->getCollection('root');
87-
$this->eventLogger = \OC::$server->get(IEventLogger::class);
98+
$this->eventLogger = $eventLogger;
99+
$this->container = $container;
88100
}
89101

90102
/**
@@ -253,7 +265,7 @@ public function findMatchingRoute(string $url): array {
253265
$this->loadRoutes('settings');
254266
} elseif (substr($url, 0, 6) === '/core/') {
255267
\OC::$REQUESTEDAPP = $url;
256-
if (!\OC::$server->getConfig()->getSystemValueBool('maintenance') && !Util::needUpgrade()) {
268+
if (!$this->config->getSystemValueBool('maintenance') && !Util::needUpgrade()) {
257269
\OC_App::loadApps();
258270
}
259271
$this->loadRoutes('core');
@@ -441,7 +453,7 @@ private function getApplicationClass(string $appName) {
441453
$applicationClassName = $appNameSpace . '\\AppInfo\\Application';
442454

443455
if (class_exists($applicationClassName)) {
444-
$application = \OC::$server->query($applicationClassName);
456+
$application = $this->container->get($applicationClassName);
445457
} else {
446458
$application = new App($appName);
447459
}

‎lib/private/Server.php‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,7 @@
129129
use OC\Remote\Api\ApiFactory;
130130
use OC\Remote\InstanceFactory;
131131
use OC\RichObjectStrings\Validator;
132+
use OC\Route\CachingRouter;
132133
use OC\Route\Router;
133134
use OC\Security\Bruteforce\Throttler;
134135
use OC\Security\CertificateManager;
@@ -819,11 +820,10 @@ public function __construct($webRoot, \OC\Config $config) {
819820

820821
$this->registerService(Router::class, function (Server $c) {
821822
$cacheFactory = $c->get(ICacheFactory::class);
822-
$logger = $c->get(LoggerInterface::class);
823823
if ($cacheFactory->isLocalCacheAvailable()) {
824-
$router = new \OC\Route\CachingRouter($cacheFactory->createLocal('route'), $logger);
824+
$router = $c->resolve(CachingRouter::class);
825825
} else {
826-
$router = new \OC\Route\Router($logger);
826+
$router = $c->resolve(Router::class);
827827
}
828828
return $router;
829829
});

‎tests/lib/AppFramework/Routing/RoutingTest.php‎

Lines changed: 46 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,12 @@
66
use OC\AppFramework\Routing\RouteConfig;
77
use OC\Route\Route;
88
use OC\Route\Router;
9+
use OCP\Diagnostics\IEventLogger;
10+
use OCP\IConfig;
11+
use OCP\IRequest;
912
use OCP\Route\IRouter;
1013
use PHPUnit\Framework\MockObject\MockObject;
14+
use Psr\Container\ContainerInterface;
1115
use Psr\Log\LoggerInterface;
1216

1317
class RoutingTest extends \Test\TestCase {
@@ -133,7 +137,13 @@ public function testSimpleRouteWithBrokenName() {
133137
/** @var IRouter|MockObject $router */
134138
$router = $this->getMockBuilder(Router::class)
135139
->onlyMethods(['create'])
136-
->setConstructorArgs([$this->createMock(LoggerInterface::class)])
140+
->setConstructorArgs([
141+
$this->createMock(LoggerInterface::class),
142+
$this->createMock(IRequest::class),
143+
$this->createMock(IConfig::class),
144+
$this->createMock(IEventLogger::class),
145+
$this->createMock(ContainerInterface::class)
146+
])
137147
->getMock();
138148

139149
// load route configuration
@@ -154,7 +164,13 @@ public function testSimpleOCSRouteWithBrokenName() {
154164
/** @var IRouter|MockObject $router */
155165
$router = $this->getMockBuilder(Router::class)
156166
->onlyMethods(['create'])
157-
->setConstructorArgs([$this->createMock(LoggerInterface::class)])
167+
->setConstructorArgs([
168+
$this->createMock(LoggerInterface::class),
169+
$this->createMock(IRequest::class),
170+
$this->createMock(IConfig::class),
171+
$this->createMock(IEventLogger::class),
172+
$this->createMock(ContainerInterface::class)
173+
])
158174
->getMock();
159175

160176
// load route configuration
@@ -214,7 +230,13 @@ private function assertSimpleRoute($routes, $name, $verb, $url, $controllerName,
214230
/** @var IRouter|MockObject $router */
215231
$router = $this->getMockBuilder(Router::class)
216232
->onlyMethods(['create'])
217-
->setConstructorArgs([$this->createMock(LoggerInterface::class)])
233+
->setConstructorArgs([
234+
$this->createMock(LoggerInterface::class),
235+
$this->createMock(IRequest::class),
236+
$this->createMock(IConfig::class),
237+
$this->createMock(IEventLogger::class),
238+
$this->createMock(ContainerInterface::class)
239+
])
218240
->getMock();
219241

220242
// we expect create to be called once:
@@ -264,7 +286,13 @@ private function assertSimpleOCSRoute($routes,
264286
/** @var IRouter|MockObject $router */
265287
$router = $this->getMockBuilder(Router::class)
266288
->onlyMethods(['create'])
267-
->setConstructorArgs([$this->createMock(LoggerInterface::class)])
289+
->setConstructorArgs([
290+
$this->createMock(LoggerInterface::class),
291+
$this->createMock(IRequest::class),
292+
$this->createMock(IConfig::class),
293+
$this->createMock(IEventLogger::class),
294+
$this->createMock(ContainerInterface::class)
295+
])
268296
->getMock();
269297

270298
// we expect create to be called once:
@@ -291,7 +319,13 @@ private function assertOCSResource($yaml, $resourceName, $url, $controllerName,
291319
/** @var IRouter|MockObject $router */
292320
$router = $this->getMockBuilder(Router::class)
293321
->onlyMethods(['create'])
294-
->setConstructorArgs([$this->createMock(LoggerInterface::class)])
322+
->setConstructorArgs([
323+
$this->createMock(LoggerInterface::class),
324+
$this->createMock(IRequest::class),
325+
$this->createMock(IConfig::class),
326+
$this->createMock(IEventLogger::class),
327+
$this->createMock(ContainerInterface::class)
328+
])
295329
->getMock();
296330

297331
// route mocks
@@ -338,7 +372,13 @@ private function assertResource($yaml, $resourceName, $url, $controllerName, $pa
338372
/** @var IRouter|MockObject $router */
339373
$router = $this->getMockBuilder(Router::class)
340374
->onlyMethods(['create'])
341-
->setConstructorArgs([$this->createMock(LoggerInterface::class)])
375+
->setConstructorArgs([
376+
$this->createMock(LoggerInterface::class),
377+
$this->createMock(IRequest::class),
378+
$this->createMock(IConfig::class),
379+
$this->createMock(IEventLogger::class),
380+
$this->createMock(ContainerInterface::class)
381+
])
342382
->getMock();
343383

344384
// route mocks

‎tests/lib/Route/RouterTest.php‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424
namespace Test\Route;
2525

2626
use OC\Route\Router;
27+
use OCP\Diagnostics\IEventLogger;
28+
use OCP\IConfig;
29+
use OCP\IRequest;
30+
use Psr\Container\ContainerInterface;
2731
use Psr\Log\LoggerInterface;
2832
use Test\TestCase;
2933

@@ -44,7 +48,13 @@ function (string $message, array $data) {
4448
$this->fail('Unexpected info log: '.(string)($data['exception'] ?? $message));
4549
}
4650
);
47-
$router = new Router($logger);
51+
$router = new Router(
52+
$logger,
53+
$this->createMock(IRequest::class),
54+
$this->createMock(IConfig::class),
55+
$this->createMock(IEventLogger::class),
56+
$this->createMock(ContainerInterface::class),
57+
);
4858

4959
$this->assertEquals('/index.php/apps/files/', $router->generate('files.view.index'));
5060

0 commit comments

Comments
 (0)