From df23690a93ded5991758f01744d38f70aa45f8d3 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 15 Oct 2020 11:45:15 -0400 Subject: [PATCH 1/3] Add routes file recompute on last modification time change --- src/App/Router.php | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/src/App/Router.php b/src/App/Router.php index dfe890fb96..d27a9c1ae9 100644 --- a/src/App/Router.php +++ b/src/App/Router.php @@ -268,20 +268,33 @@ class Router * The cached "routerDispatchData" lasts for a day, and must be cleared manually when there * is any changes in the enabled addons list. * + * Additionally, we check for the base routes file last modification time to automatically + * trigger re-computing the dispatch data. + * * @return array|mixed * @throws HTTPException\InternalServerErrorException */ private function getCachedDispatchData() { $routerDispatchData = $this->cache->get('routerDispatchData'); + $lastRoutesFileModifiedTime = $this->cache->get('lastRoutesFileModifiedTime'); + $forceRecompute = false; - if ($routerDispatchData) { + if ($this->baseRoutesFilepath && file_exists($this->baseRoutesFilepath)) { + $routesFileModifiedTime = filemtime($this->baseRoutesFilepath); + $forceRecompute = $lastRoutesFileModifiedTime != $routesFileModifiedTime; + } + + if (!$forceRecompute && $routerDispatchData) { return $routerDispatchData; } $routerDispatchData = $this->getDispatchData(); $this->cache->set('routerDispatchData', $routerDispatchData, Duration::DAY); + if (!empty($routesFileModifiedTime)) { + $this->cache->set('lastRoutesFileMtime', $routesFileModifiedTime, Duration::MONTH); + } return $routerDispatchData; } From 7d705417a948f3aead707b097970cc31eda38725 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 15 Oct 2020 12:41:49 -0400 Subject: [PATCH 2/3] Adjust testModuleClass() expectations after introducing lastRoutesFileModifiedTime cache key --- tests/src/App/ModuleTest.php | 3 ++- tests/src/App/RouterTest.php | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/src/App/ModuleTest.php b/tests/src/App/ModuleTest.php index 03bb14b605..be49a741aa 100644 --- a/tests/src/App/ModuleTest.php +++ b/tests/src/App/ModuleTest.php @@ -178,7 +178,8 @@ class ModuleTest extends DatabaseTest $cache = \Mockery::mock(ICache::class); $cache->shouldReceive('get')->with('routerDispatchData')->andReturn('')->atMost()->once(); - $cache->shouldReceive('set')->withAnyArgs()->andReturn(false)->atMost()->once(); + $cache->shouldReceive('get')->with('lastRoutesFileModifiedTime')->andReturn('')->atMost()->once(); + $cache->shouldReceive('set')->withAnyArgs()->andReturn(false)->atMost()->twice(); $router = (new App\Router([], __DIR__ . '/../../../static/routes.config.php', $l10n, $cache)); diff --git a/tests/src/App/RouterTest.php b/tests/src/App/RouterTest.php index df1ea5e9ad..b9c23a2b23 100644 --- a/tests/src/App/RouterTest.php +++ b/tests/src/App/RouterTest.php @@ -185,7 +185,7 @@ class RouterTest extends TestCase ], ], '/post' => [ - '/it' => [Module\NodeInfo::class, [Router::POST]], + '/it' => [Module\WellKnown\NodeInfo::class, [Router::POST]], ], '/double' => [Module\Profile\Index::class, [Router::GET, Router::POST]] ], @@ -221,7 +221,7 @@ class RouterTest extends TestCase ], '', $this->l10n, $this->cache))->loadRoutes($routes); // Don't find GET - $this->assertEquals(Module\NodeInfo::class, $router->getModuleClass('/post/it')); + $this->assertEquals(Module\WellKnown\NodeInfo::class, $router->getModuleClass('/post/it')); $this->assertEquals(Module\Profile\Index::class, $router->getModuleClass('/double')); } } From 06e329441561cd91f6f4039243c23bc8bc8def45 Mon Sep 17 00:00:00 2001 From: Hypolite Petovan Date: Thu, 15 Oct 2020 21:45:51 -0400 Subject: [PATCH 3/3] Centralize routes file existence check in App\Router --- src/App/Router.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/App/Router.php b/src/App/Router.php index d27a9c1ae9..e90cb3ace0 100644 --- a/src/App/Router.php +++ b/src/App/Router.php @@ -93,6 +93,10 @@ class Router $this->routeCollector = isset($routeCollector) ? $routeCollector : new RouteCollector(new Std(), new GroupCountBased()); + + if ($this->baseRoutesFilepath && !file_exists($this->baseRoutesFilepath)) { + throw new HTTPException\InternalServerErrorException('Routes file path does\'n exist.'); + } } /** @@ -249,7 +253,7 @@ class Router { $dispatchData = []; - if ($this->baseRoutesFilepath && file_exists($this->baseRoutesFilepath)) { + if ($this->baseRoutesFilepath) { $dispatchData = require $this->baseRoutesFilepath; if (!is_array($dispatchData)) { throw new HTTPException\InternalServerErrorException('Invalid base routes file'); @@ -280,7 +284,7 @@ class Router $lastRoutesFileModifiedTime = $this->cache->get('lastRoutesFileModifiedTime'); $forceRecompute = false; - if ($this->baseRoutesFilepath && file_exists($this->baseRoutesFilepath)) { + if ($this->baseRoutesFilepath) { $routesFileModifiedTime = filemtime($this->baseRoutesFilepath); $forceRecompute = $lastRoutesFileModifiedTime != $routesFileModifiedTime; }