Merge pull request #10544 from annando/router-lock
Lock before recreate / fix cache key misspelling
This commit is contained in:
commit
e34795762d
3 changed files with 42 additions and 13 deletions
|
@ -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;
|
||||
|
|
|
@ -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,11 @@ 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);
|
||||
$lock->shouldReceive('acquire')->andReturn(true);
|
||||
$lock->shouldReceive('isLocked')->andReturn(false);
|
||||
|
||||
$router = (new App\Router([], __DIR__ . '/../../../static/routes.config.php', $l10n, $cache, $lock));
|
||||
|
||||
$module = (new App\Module($name))->determineClass(new App\Arguments('', $command), $router, $config);
|
||||
|
||||
|
|
|
@ -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
|
||||
{
|
||||
|
@ -50,11 +55,15 @@ 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);
|
||||
$this->lock->shouldReceive('acquire')->andReturn(true);
|
||||
$this->lock->shouldReceive('isLocked')->andReturn(false);
|
||||
}
|
||||
|
||||
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 +87,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 +113,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 +122,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 +134,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 +146,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 +158,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 +170,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 +212,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 +229,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'));
|
||||
|
|
Loading…
Reference in a new issue