From c52d12cb993a9b027339fd3d17e103e3bbafb678 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 14 Mar 2020 00:16:57 +0100 Subject: [PATCH] Begin work on cached lookup --- assets/config.php | 2 + .../Database/CachedTenantResolver.php | 45 ++++++++++++++ .../Database/DatabaseStorageDriver.php | 42 +++++++++++-- test | 8 +-- tests/CachedResolverTest.php | 61 +++++++++++++++++++ tests/TestCase.php | 1 + 6 files changed, 149 insertions(+), 10 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..3cf1a532 --- /dev/null +++ b/src/StorageDrivers/Database/CachedTenantResolver.php @@ -0,0 +1,45 @@ +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 findById(string $id, Closure $dataQuery, Closure $domainsQuery): Tenant + { + $data = $this->cache->remember('_tenancy_id_to_data:' . $id, $this->ttl(), $dataQuery); + $domains = $this->cache->remember('_tenancy_id_to_domains:' . $id, $this->ttl(), $domainsQuery); + + return Tenant::fromStorage($data)->withDomains($domains); + } + + // todo update cache on writes to data & domains +} diff --git a/src/StorageDrivers/Database/DatabaseStorageDriver.php b/src/StorageDrivers/Database/DatabaseStorageDriver.php index 383cb890..457b8373 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,25 @@ class DatabaseStorageDriver implements StorageDriver, CanDeleteKeys, CanFindByAn public function findById(string $id): Tenant { - $tenant = $this->tenants->find($id); + $dataQuery = function () use ($id) { + return $this->tenants->decodeData($this->tenants->find($id)); + }; + $domainsQuery = function () use ($id) { + return $this->domains->getTenantDomains($id); + }; - if (! $tenant) { + if ($this->usesCache()) { + return $this->cache->findById($id, $dataQuery, $domainsQuery); + } else { + $data = $dataQuery(); + } + + if (! $data) { throw new TenantDoesNotExistException($id); } - return Tenant::fromStorage($this->tenants->decodeData($tenant)) - ->withDomains($this->domains->getTenantDomains($id)); + return Tenant::fromStorage($data) + ->withDomains($domainsQuery()); } /** @@ -191,4 +215,10 @@ class DatabaseStorageDriver implements StorageDriver, CanDeleteKeys, CanFindByAn { $this->tenants->deleteMany($keys, $tenant ?? $this->currentTenant()); } + + public function usesCache(): bool + { + // null is also truthy here + return $this->app['config']['tenancy.storage_drivers.db.cache_store'] !== false; + } } 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..1d42ca1b --- /dev/null +++ b/tests/CachedResolverTest.php @@ -0,0 +1,61 @@ +markTestSkipped('This test is only relevant for the DB storage driver.'); + } + } + + /** @test */ + public function a_query_is_not_made_for_tenant_id_once_domain_is_cached() + { + $tenant = Tenant::new()->withDomains(['foo.localhost']); + + // todo assert query is made: + $queried = tenancy()->findByDomain('foo.localhost'); + + // todo assert query is not made but cache call is made: + $cached = tenancy()->findByDomain('foo.localhost'); + } + + /** @test */ + public function a_query_is_not_made_for_tenant_once_id_is_cached() + { + $tenant = Tenant::new()->withData(['id' => '123']); + + // todo assert query is made: + $queried = tenancy()->find('123'); + + // todo assert query is not made but cache call is made: + $cached = tenancy()->find('123'); + } + + /** @test */ + public function modifying_tenants_domains_updates_domains_in_the_cached_domain_to_id_mapping() + { + + } + + /** @test */ + public function modifying_tenants_data_updates_data_in_the_cached_id_to_tenant_data_mapping() + { + $tenant = Tenant::new()->withData(['id' => '123', 'foo' => 'bar']); + + // todo assert cache record is set + $this->assertSame('bar', tenancy()->find('123')->get('foo')); + + // todo assert cache record is updated + $tenant->set('foo', 'xyz'); + + $this->assertSame('xyz', tenancy()->find('123')->get('foo')); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 257964d1..600b7aba 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -99,6 +99,7 @@ abstract class TestCase extends \Orchestra\Testbench\TestCase 'tenancy.redis.prefixed_connections' => ['default'], 'tenancy.migration_paths' => [database_path('../migrations')], 'tenancy.storage_drivers.db.connection' => 'central', + 'tenancy.storage_drivers.db.cache_store' => null, // TODO REMOVE THIS BEFORE MERGING TO 2.X 'tenancy.bootstrappers.redis' => \Stancl\Tenancy\TenancyBootstrappers\RedisTenancyBootstrapper::class, 'queue.connections.central' => [ 'driver' => 'sync',