From 2aa2c614798cc74aa85d77fdd9ad7a5166303917 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 25 Jul 2021 04:31:48 +0000 Subject: [PATCH 1/5] Lock before recreate / fix cache key misspelling --- src/App/Router.php | 18 ++++++++++++++++-- tests/src/App/RouterTest.php | 26 ++++++++++++++++---------- 2 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/App/Router.php b/src/App/Router.php index 82c493baa..ad4b33d08 100644 --- a/src/App/Router.php +++ b/src/App/Router.php @@ -30,6 +30,7 @@ use Friendica\Core\Cache\Duration; use Friendica\Core\Cache\ICache; use Friendica\Core\Hook; use Friendica\Core\L10n; +use Friendica\Core\Lock\ILock; use Friendica\Network\HTTPException; /** @@ -79,6 +80,9 @@ class Router /** @var ICache */ private $cache; + /** @var ILock */ + private $lock; + /** @var string */ private $baseRoutesFilepath; @@ -89,11 +93,12 @@ class Router * @param ICache $cache * @param RouteCollector|null $routeCollector */ - public function __construct(array $server, string $baseRoutesFilepath, L10n $l10n, ICache $cache, RouteCollector $routeCollector = null) + public function __construct(array $server, string $baseRoutesFilepath, L10n $l10n, ICache $cache, ILock $lock, RouteCollector $routeCollector = null) { $this->baseRoutesFilepath = $baseRoutesFilepath; $this->l10n = $l10n; $this->cache = $cache; + $this->lock = $lock; $httpMethod = $server['REQUEST_METHOD'] ?? self::GET; $this->httpMethod = in_array($httpMethod, self::ALLOWED_METHODS) ? $httpMethod : self::GET; @@ -301,11 +306,20 @@ class Router return $routerDispatchData; } + if (!$this->lock->acquire('getCachedDispatchData', 0)) { + // Immediately return uncached data when we can't aquire a lock + return $this->getDispatchData(); + } + $routerDispatchData = $this->getDispatchData(); $this->cache->set('routerDispatchData', $routerDispatchData, Duration::DAY); if (!empty($routesFileModifiedTime)) { - $this->cache->set('lastRoutesFileMtime', $routesFileModifiedTime, Duration::MONTH); + $this->cache->set('lastRoutesFileModifiedTime', $routesFileModifiedTime, Duration::MONTH); + } + + if ($this->lock->isLocked('getCachedDispatchData')) { + $this->lock->release('getCachedDispatchData'); } return $routerDispatchData; diff --git a/tests/src/App/RouterTest.php b/tests/src/App/RouterTest.php index a5824c455..a3e8b026b 100644 --- a/tests/src/App/RouterTest.php +++ b/tests/src/App/RouterTest.php @@ -24,6 +24,7 @@ namespace Friendica\Test\src\App; use Friendica\App\Router; use Friendica\Core\Cache\ICache; use Friendica\Core\L10n; +use Friendica\Core\Lock\ILock; use Friendica\Module; use Friendica\Network\HTTPException\MethodNotAllowedException; use Friendica\Network\HTTPException\NotFoundException; @@ -39,6 +40,10 @@ class RouterTest extends TestCase * @var ICache */ private $cache; + /** + * @var ILock + */ + private $lock; protected function setUp() : void { @@ -54,7 +59,7 @@ class RouterTest extends TestCase public function testGetModuleClass() { - $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache); + $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache, $this->lock); $routeCollector = $router->getRouteCollector(); $routeCollector->addRoute([Router::GET], '/', 'IndexModuleClassName'); @@ -78,7 +83,7 @@ class RouterTest extends TestCase public function testPostModuleClass() { - $router = new Router(['REQUEST_METHOD' => Router::POST], '', $this->l10n, $this->cache); + $router = new Router(['REQUEST_METHOD' => Router::POST], '', $this->l10n, $this->cache, $this->lock); $routeCollector = $router->getRouteCollector(); $routeCollector->addRoute([Router::POST], '/', 'IndexModuleClassName'); @@ -104,7 +109,7 @@ class RouterTest extends TestCase { $this->expectException(NotFoundException::class); - $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache); + $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache, $this->lock); $router->getModuleClass('/unsupported'); } @@ -113,7 +118,7 @@ class RouterTest extends TestCase { $this->expectException(NotFoundException::class); - $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache); + $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache, $this->lock); $routeCollector = $router->getRouteCollector(); $routeCollector->addRoute([Router::GET], '/test', 'TestModuleClassName'); @@ -125,7 +130,7 @@ class RouterTest extends TestCase { $this->expectException(NotFoundException::class); - $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache); + $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache, $this->lock); $routeCollector = $router->getRouteCollector(); $routeCollector->addRoute([Router::GET], '/optional[/option]', 'OptionalModuleClassName'); @@ -137,7 +142,7 @@ class RouterTest extends TestCase { $this->expectException(NotFoundException::class); - $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache); + $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache, $this->lock); $routeCollector = $router->getRouteCollector(); $routeCollector->addRoute([Router::GET], '/variable/{var}', 'VariableModuleClassName'); @@ -149,7 +154,7 @@ class RouterTest extends TestCase { $this->expectException(MethodNotAllowedException::class); - $router = new Router(['REQUEST_METHOD' => Router::POST], '', $this->l10n, $this->cache); + $router = new Router(['REQUEST_METHOD' => Router::POST], '', $this->l10n, $this->cache, $this->lock); $routeCollector = $router->getRouteCollector(); $routeCollector->addRoute([Router::GET], '/test', 'TestModuleClassName'); @@ -161,7 +166,7 @@ class RouterTest extends TestCase { $this->expectException(MethodNotAllowedException::class); - $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache); + $router = new Router(['REQUEST_METHOD' => Router::GET], '', $this->l10n, $this->cache, $this->lock); $routeCollector = $router->getRouteCollector(); $routeCollector->addRoute([Router::POST], '/test', 'TestModuleClassName'); @@ -203,7 +208,8 @@ class RouterTest extends TestCase ['REQUEST_METHOD' => Router::GET], '', $this->l10n, - $this->cache + $this->cache, + $this->lock ))->loadRoutes($routes); self::assertEquals(Module\Home::class, $router->getModuleClass('/')); @@ -219,7 +225,7 @@ class RouterTest extends TestCase { $router = (new Router([ 'REQUEST_METHOD' => Router::POST - ], '', $this->l10n, $this->cache))->loadRoutes($routes); + ], '', $this->l10n, $this->cache, $this->lock))->loadRoutes($routes); // Don't find GET self::assertEquals(Module\WellKnown\NodeInfo::class, $router->getModuleClass('/post/it')); From 4b5eb055c8b97c2c1b4393186b65bfd7b22554ae Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 25 Jul 2021 04:42:34 +0000 Subject: [PATCH 2/5] Tests --- tests/src/App/ModuleTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/src/App/ModuleTest.php b/tests/src/App/ModuleTest.php index 42d6fb1ef..9b9bdd0cc 100644 --- a/tests/src/App/ModuleTest.php +++ b/tests/src/App/ModuleTest.php @@ -25,6 +25,7 @@ use Friendica\App; use Friendica\Core\Cache\ICache; use Friendica\Core\Config\IConfig; use Friendica\Core\L10n; +use Friendica\Core\Lock\ILock; use Friendica\LegacyModule; use Friendica\Module\HTTPException\PageNotFound; use Friendica\Module\WellKnown\HostMeta; @@ -182,7 +183,9 @@ class ModuleTest extends DatabaseTest $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)); + $lock = Mockery::mock(ILock::class); + + $router = (new App\Router([], __DIR__ . '/../../../static/routes.config.php', $l10n, $cache, $lock)); $module = (new App\Module($name))->determineClass(new App\Arguments('', $command), $router, $config); From 0dd0de1eb369b9a94baf5c39dee7a7ec8d0f51b1 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 25 Jul 2021 04:44:23 +0000 Subject: [PATCH 3/5] Init class --- tests/src/App/RouterTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/App/RouterTest.php b/tests/src/App/RouterTest.php index a3e8b026b..8317702cd 100644 --- a/tests/src/App/RouterTest.php +++ b/tests/src/App/RouterTest.php @@ -55,6 +55,8 @@ class RouterTest extends TestCase $this->cache = Mockery::mock(ICache::class); $this->cache->shouldReceive('get')->andReturn(null); $this->cache->shouldReceive('set')->andReturn(false); + + $this->lock = Mockery::mock(ILock::class); } public function testGetModuleClass() From 12c63c9b971167d206ad494f6908f001641b0d77 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 25 Jul 2021 04:52:17 +0000 Subject: [PATCH 4/5] Tests --- tests/src/App/RouterTest.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/src/App/RouterTest.php b/tests/src/App/RouterTest.php index 8317702cd..987df5ff9 100644 --- a/tests/src/App/RouterTest.php +++ b/tests/src/App/RouterTest.php @@ -57,6 +57,8 @@ class RouterTest extends TestCase $this->cache->shouldReceive('set')->andReturn(false); $this->lock = Mockery::mock(ILock::class); + $this->cache->shouldReceive('acquire')->andReturn(true); + $this->cache->shouldReceive('isLocked')->andReturn(false); } public function testGetModuleClass() From 52378eb298b01655fc5b3f56a561d43fb3720032 Mon Sep 17 00:00:00 2001 From: Michael Date: Sun, 25 Jul 2021 04:56:40 +0000 Subject: [PATCH 5/5] Fighting with the tests --- tests/src/App/ModuleTest.php | 2 ++ tests/src/App/RouterTest.php | 4 ++-- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/src/App/ModuleTest.php b/tests/src/App/ModuleTest.php index 9b9bdd0cc..730d1d886 100644 --- a/tests/src/App/ModuleTest.php +++ b/tests/src/App/ModuleTest.php @@ -184,6 +184,8 @@ class ModuleTest extends DatabaseTest $cache->shouldReceive('set')->withAnyArgs()->andReturn(false)->atMost()->twice(); $lock = Mockery::mock(ILock::class); + $lock->shouldReceive('acquire')->andReturn(true); + $lock->shouldReceive('isLocked')->andReturn(false); $router = (new App\Router([], __DIR__ . '/../../../static/routes.config.php', $l10n, $cache, $lock)); diff --git a/tests/src/App/RouterTest.php b/tests/src/App/RouterTest.php index 987df5ff9..3a3c0469b 100644 --- a/tests/src/App/RouterTest.php +++ b/tests/src/App/RouterTest.php @@ -57,8 +57,8 @@ class RouterTest extends TestCase $this->cache->shouldReceive('set')->andReturn(false); $this->lock = Mockery::mock(ILock::class); - $this->cache->shouldReceive('acquire')->andReturn(true); - $this->cache->shouldReceive('isLocked')->andReturn(false); + $this->lock->shouldReceive('acquire')->andReturn(true); + $this->lock->shouldReceive('isLocked')->andReturn(false); } public function testGetModuleClass()