From c7c6a7fec85a594bf3ee337def09b19acba3caed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 14 Mar 2020 19:52:35 +0100 Subject: [PATCH] [2.3.0] Cached tenant lookup (#316) * Begin work on cached lookup * Apply fixes from StyleCI * wip * wip cache invalidation * Apply fixes from StyleCI * Finish cache invalidation * Apply fixes from StyleCI * Remove config from TestCase * Enable cache in the single test file * Separate data & domains logic * Apply fixes from StyleCI * wip * Apply fixes from StyleCI --- assets/config.php | 2 + .../Database/CachedTenantResolver.php | 68 ++++++++ .../Database/DatabaseStorageDriver.php | 85 +++++++-- .../Database/DomainRepository.php | 3 +- test | 8 +- tests/CachedResolverTest.php | 164 ++++++++++++++++++ 6 files changed, 313 insertions(+), 17 deletions(-) create mode 100644 src/StorageDrivers/Database/CachedTenantResolver.php create mode 100644 tests/CachedResolverTest.php diff --git a/assets/config.php b/assets/config.php index 9155dae4..96cf05db 100644 --- a/assets/config.php +++ b/assets/config.php @@ -16,6 +16,8 @@ return [ 'tenants' => 'tenants', 'domains' => 'domains', ], + 'cache_store' => false, // What store should be used to cache tenant resolution. Set to false to disable cache, null to use default store, or a string with a specific cache store name. + 'cache_ttl' => 3600, // seconds ], 'redis' => [ 'driver' => Stancl\Tenancy\StorageDrivers\RedisStorageDriver::class, diff --git a/src/StorageDrivers/Database/CachedTenantResolver.php b/src/StorageDrivers/Database/CachedTenantResolver.php new file mode 100644 index 00000000..9a4a345c --- /dev/null +++ b/src/StorageDrivers/Database/CachedTenantResolver.php @@ -0,0 +1,68 @@ +cache = $cacheManager->store($config->get('tenancy.storage_drivers.db.cache_store')); + $this->config = $config; + } + + protected function ttl(): int + { + return $this->config->get('tenancy.storage_drivers.db.cache_ttl'); + } + + public function getTenantIdByDomain(string $domain, Closure $query): string + { + return $this->cache->remember('_tenancy_domain_to_id:' . $domain, $this->ttl(), $query); + } + + public function getDataById(string $id, Closure $dataQuery): ?array + { + return $this->cache->remember('_tenancy_id_to_data:' . $id, $this->ttl(), $dataQuery); + } + + public function getDomainsById(string $id, Closure $domainsQuery): ?array + { + return $this->cache->remember('_tenancy_id_to_domains:' . $id, $this->ttl(), $domainsQuery); + } + + public function invalidateTenant(string $id): void + { + $this->invalidateTenantData($id); + $this->invalidateTenantDomains($id); + } + + public function invalidateTenantData(string $id): void + { + $this->cache->forget('_tenancy_id_to_data:' . $id); + } + + public function invalidateTenantDomains(string $id): void + { + $this->cache->forget('_tenancy_id_to_domains:' . $id); + } + + public function invalidateDomainToIdMapping(array $domains): void + { + foreach ($domains as $domain) { + $this->cache->forget('_tenancy_domain_to_id:' . $domain); + } + } +} diff --git a/src/StorageDrivers/Database/DatabaseStorageDriver.php b/src/StorageDrivers/Database/DatabaseStorageDriver.php index 383cb890..42ce00b4 100644 --- a/src/StorageDrivers/Database/DatabaseStorageDriver.php +++ b/src/StorageDrivers/Database/DatabaseStorageDriver.php @@ -32,12 +32,16 @@ class DatabaseStorageDriver implements StorageDriver, CanDeleteKeys, CanFindByAn /** @var DomainRepository */ protected $domains; + /** @var CachedTenantResolver */ + protected $cache; + /** @var Tenant The default tenant. */ protected $tenant; - public function __construct(Application $app, ConfigRepository $config) + public function __construct(Application $app, ConfigRepository $config, CachedTenantResolver $cache) { $this->app = $app; + $this->cache = $cache; $this->centralDatabase = $this->getCentralConnection(); $this->tenants = new TenantRepository($config); $this->domains = new DomainRepository($config); @@ -60,7 +64,16 @@ class DatabaseStorageDriver implements StorageDriver, CanDeleteKeys, CanFindByAn public function findByDomain(string $domain): Tenant { - $id = $this->domains->getTenantIdByDomain($domain); + $query = function () use ($domain) { + return $this->domains->getTenantIdByDomain($domain); + }; + + if ($this->usesCache()) { + $id = $this->cache->getTenantIdByDomain($domain, $query); + } else { + $id = $query(); + } + if (! $id) { throw new TenantCouldNotBeIdentifiedException($domain); } @@ -70,14 +83,29 @@ class DatabaseStorageDriver implements StorageDriver, CanDeleteKeys, CanFindByAn public function findById(string $id): Tenant { - $tenant = $this->tenants->find($id); + $dataQuery = function () use ($id) { + $data = $this->tenants->find($id); - if (! $tenant) { + return $data ? $this->tenants->decodeData($data) : null; + }; + $domainsQuery = function () use ($id) { + return $this->domains->getTenantDomains($id); + }; + + if ($this->usesCache()) { + $data = $this->cache->getDataById($id, $dataQuery); + $domains = $this->cache->getDomainsById($id, $domainsQuery); + } else { + $data = $dataQuery(); + $domains = $domainsQuery(); + } + + if (! $data) { throw new TenantDoesNotExistException($id); } - return Tenant::fromStorage($this->tenants->decodeData($tenant)) - ->withDomains($this->domains->getTenantDomains($id)); + return Tenant::fromStorage($data) + ->withDomains($domains); } /** @@ -128,19 +156,33 @@ class DatabaseStorageDriver implements StorageDriver, CanDeleteKeys, CanFindByAn public function updateTenant(Tenant $tenant): void { - $this->centralDatabase->transaction(function () use ($tenant) { + $originalDomains = $this->domains->getTenantDomains($tenant); + + $this->centralDatabase->transaction(function () use ($tenant, $originalDomains) { $this->tenants->updateTenant($tenant); - $this->domains->updateTenantDomains($tenant); + $this->domains->updateTenantDomains($tenant, $originalDomains); }); + + if ($this->usesCache()) { + $this->cache->invalidateTenant($tenant->id); + $this->cache->invalidateDomainToIdMapping($originalDomains); + } } public function deleteTenant(Tenant $tenant): void { + $originalDomains = $this->domains->getTenantDomains($tenant); + $this->centralDatabase->transaction(function () use ($tenant) { $this->tenants->where('id', $tenant->id)->delete(); $this->domains->where('tenant_id', $tenant->id)->delete(); }); + + if ($this->usesCache()) { + $this->cache->invalidateTenant($tenant->id); + $this->cache->invalidateDomainToIdMapping($originalDomains); + } } /** @@ -179,16 +221,37 @@ class DatabaseStorageDriver implements StorageDriver, CanDeleteKeys, CanFindByAn public function put(string $key, $value, Tenant $tenant = null): void { - $this->tenants->put($key, $value, $tenant ?? $this->currentTenant()); + $tenant = $tenant ?? $this->currentTenant(); + $this->tenants->put($key, $value, $tenant); + + if ($this->usesCache()) { + $this->cache->invalidateTenantData($tenant->id); + } } public function putMany(array $kvPairs, Tenant $tenant = null): void { - $this->tenants->putMany($kvPairs, $tenant ?? $this->currentTenant()); + $tenant = $tenant ?? $this->currentTenant(); + $this->tenants->putMany($kvPairs, $tenant); + + if ($this->usesCache()) { + $this->cache->invalidateTenantData($tenant->id); + } } public function deleteMany(array $keys, Tenant $tenant = null): void { - $this->tenants->deleteMany($keys, $tenant ?? $this->currentTenant()); + $tenant = $tenant ?? $this->currentTenant(); + $this->tenants->deleteMany($keys, $tenant); + + if ($this->usesCache()) { + $this->cache->invalidateTenantData($tenant->id); + } + } + + public function usesCache(): bool + { + // null is also truthy here + return $this->app['config']['tenancy.storage_drivers.db.cache_store'] !== false; } } diff --git a/src/StorageDrivers/Database/DomainRepository.php b/src/StorageDrivers/Database/DomainRepository.php index e8ac2f13..4e21b9ad 100644 --- a/src/StorageDrivers/Database/DomainRepository.php +++ b/src/StorageDrivers/Database/DomainRepository.php @@ -33,9 +33,8 @@ class DomainRepository extends Repository }, $tenant->domains)); } - public function updateTenantDomains(Tenant $tenant) + public function updateTenantDomains(Tenant $tenant, array $originalDomains) { - $originalDomains = $this->getTenantDomains($tenant); $deletedDomains = array_diff($originalDomains, $tenant->domains); $newDomains = array_diff($tenant->domains, $originalDomains); diff --git a/test b/test index 3f8244b3..1d492f02 100755 --- a/test +++ b/test @@ -1,7 +1,7 @@ #!/bin/bash set -e -printf "Variant 1\n\n" -docker-compose exec test env TENANCY_TEST_STORAGE_DRIVER=db vendor/bin/phpunit --coverage-php coverage/2.cov "$@" -printf "Variant 2\n\n" -docker-compose exec test env TENANCY_TEST_STORAGE_DRIVER=redis vendor/bin/phpunit --coverage-php coverage/1.cov "$@" +printf "Variant 1 (DB)\n\n" +docker-compose exec test env TENANCY_TEST_STORAGE_DRIVER=db vendor/bin/phpunit --coverage-php coverage/1.cov "$@" +printf "Variant 2 (Redis)\n\n" +docker-compose exec test env TENANCY_TEST_STORAGE_DRIVER=redis vendor/bin/phpunit --coverage-php coverage/2.cov "$@" diff --git a/tests/CachedResolverTest.php b/tests/CachedResolverTest.php new file mode 100644 index 00000000..903539fd --- /dev/null +++ b/tests/CachedResolverTest.php @@ -0,0 +1,164 @@ +markTestSkipped('This test is only relevant for the DB storage driver.'); + } + + config(['tenancy.storage_drivers.db.cache_store' => null]); // default driver + } + + /** @test */ + public function a_query_is_not_made_for_tenant_id_once_domain_is_cached() + { + $tenant = Tenant::new() + ->withData(['foo' => 'bar']) + ->withDomains(['foo.localhost']) + ->save(); + + // query is made + $queried = tenancy()->findByDomain('foo.localhost'); + $this->assertEquals($tenant->data, $queried->data); + $this->assertSame($tenant->domains, $queried->domains); + + // cache is set + $this->assertEquals($tenant->id, Cache::get('_tenancy_domain_to_id:foo.localhost')); + $this->assertEquals($tenant->data, Cache::get('_tenancy_id_to_data:' . $tenant->id)); + $this->assertSame($tenant->domains, Cache::get('_tenancy_id_to_domains:' . $tenant->id)); + + // query is not made + DatabaseStorageDriver::getCentralConnection()->enableQueryLog(); + $cached = tenancy()->findByDomain('foo.localhost'); + $this->assertEquals($tenant->data, $cached->data); + $this->assertSame($tenant->domains, $cached->domains); + $this->assertSame([], DatabaseStorageDriver::getCentralConnection()->getQueryLog()); + } + + /** @test */ + public function a_query_is_not_made_for_tenant_once_id_is_cached() + { + $tenant = Tenant::new() + ->withData(['foo' => 'bar']) + ->withDomains(['foo.localhost']) + ->save(); + + // query is made + $queried = tenancy()->find($tenant->id); + $this->assertEquals($tenant->data, $queried->data); + $this->assertSame($tenant->domains, $queried->domains); + + // cache is set + $this->assertEquals($tenant->data, Cache::get('_tenancy_id_to_data:' . $tenant->id)); + $this->assertSame($tenant->domains, Cache::get('_tenancy_id_to_domains:' . $tenant->id)); + + // query is not made + DatabaseStorageDriver::getCentralConnection()->enableQueryLog(); + $cached = tenancy()->find($tenant->id); + $this->assertEquals($tenant->data, $cached->data); + $this->assertSame($tenant->domains, $cached->domains); + $this->assertSame([], DatabaseStorageDriver::getCentralConnection()->getQueryLog()); + } + + /** @test */ + public function modifying_tenant_domains_invalidates_the_cached_domain_to_id_mapping() + { + $tenant = Tenant::new() + ->withDomains(['foo.localhost', 'bar.localhost']) + ->save(); + + // queried + $this->assertSame($tenant->id, tenancy()->findByDomain('foo.localhost')->id); + $this->assertSame($tenant->id, tenancy()->findByDomain('bar.localhost')->id); + + // assert cache set + $this->assertSame($tenant->id, Cache::get('_tenancy_domain_to_id:foo.localhost')); + $this->assertSame($tenant->id, Cache::get('_tenancy_domain_to_id:bar.localhost')); + + $tenant + ->removeDomains(['foo.localhost', 'bar.localhost']) + ->addDomains(['xyz.localhost']) + ->save(); + + // assert neither domain is cached + $this->assertSame(null, Cache::get('_tenancy_domain_to_id:foo.localhost')); + $this->assertSame(null, Cache::get('_tenancy_domain_to_id:bar.localhost')); + $this->assertSame(null, Cache::get('_tenancy_domain_to_id:xyz.localhost')); + } + + /** @test */ + public function modifying_tenants_data_invalidates_tenant_data_cache() + { + $tenant = Tenant::new()->withData(['foo' => 'bar'])->save(); + + // cache record is set + $this->assertSame('bar', tenancy()->find($tenant->id)->get('foo')); + $this->assertSame('bar', Cache::get('_tenancy_id_to_data:' . $tenant->id)['foo']); + + // cache record is invalidated + $tenant->set('foo', 'xyz'); + $this->assertSame(null, Cache::get('_tenancy_id_to_data:' . $tenant->id)); + + // cache record is set + $this->assertSame('xyz', tenancy()->find($tenant->id)->get('foo')); + $this->assertSame('xyz', Cache::get('_tenancy_id_to_data:' . $tenant->id)['foo']); + + // cache record is invalidated + $tenant->foo = 'abc'; + $tenant->save(); + $this->assertSame(null, Cache::get('_tenancy_id_to_data:' . $tenant->id)); + } + + /** @test */ + public function modifying_tenants_domains_invalidates_tenant_domain_cache() + { + $tenant = Tenant::new() + ->withData(['foo' => 'bar']) + ->withDomains(['foo.localhost']) + ->save(); + + // cache record is set + $this->assertSame(['foo.localhost'], tenancy()->find($tenant->id)->domains); + $this->assertSame(['foo.localhost'], Cache::get('_tenancy_id_to_domains:' . $tenant->id)); + + // cache record is invalidated + $tenant->addDomains(['bar.localhost'])->save(); + $this->assertEquals(null, Cache::get('_tenancy_id_to_domains:' . $tenant->id)); + + $this->assertEquals(['foo.localhost', 'bar.localhost'], tenancy()->find($tenant->id)->domains); + } + + /** @test */ + public function deleting_a_tenant_invalidates_all_caches() + { + $tenant = Tenant::new() + ->withData(['foo' => 'bar']) + ->withDomains(['foo.localhost']) + ->save(); + + tenancy()->findByDomain('foo.localhost'); + $this->assertEquals($tenant->id, Cache::get('_tenancy_domain_to_id:foo.localhost')); + $this->assertEquals($tenant->data, Cache::get('_tenancy_id_to_data:' . $tenant->id)); + $this->assertEquals(['foo.localhost'], Cache::get('_tenancy_id_to_domains:' . $tenant->id)); + + $tenant->delete(); + $this->assertEquals(null, Cache::get('_tenancy_domain_to_id:foo.localhost')); + $this->assertEquals(null, Cache::get('_tenancy_id_to_data:' . $tenant->id)); + $this->assertEquals(null, Cache::get('_tenancy_id_to_domains:' . $tenant->id)); + } +}