From 97e45ab9ccf0a51ab7b7570c2ef49674b7e9fcbd Mon Sep 17 00:00:00 2001 From: Erik Gaal Date: Sat, 24 Sep 2022 20:15:47 +0200 Subject: [PATCH] Refactor cached tenant resolvers with decorator pattern --- assets/config.php | 24 +++++ src/Repository/IlluminateTenantRepository.php | 52 ++++++++++ src/Repository/TenantRepository.php | 22 +++++ src/Resolvers/CachedTenantResolver.php | 34 +++++++ .../Contracts/CachedTenantResolver.php | 78 --------------- src/Resolvers/DomainTenantResolver.php | 53 ++--------- src/Resolvers/PathTenantResolver.php | 33 +++---- src/Resolvers/RequestDataTenantResolver.php | 24 ++--- src/TenancyServiceProvider.php | 63 ++++++++++++ tests/CachedTenantResolverTest.php | 95 ------------------- tests/Repository/InMemoryTenantRepository.php | 41 ++++++++ tests/Resolvers/CachedTenantResolverTest.php | 50 ++++++++++ tests/Resolvers/DomainTenantResolverTest.php | 33 +++++++ tests/Resolvers/PathTenantResolverTest.php | 40 ++++++++ .../RequestDataTenantResolverTest.php | 33 +++++++ 15 files changed, 424 insertions(+), 251 deletions(-) create mode 100644 src/Repository/IlluminateTenantRepository.php create mode 100644 src/Repository/TenantRepository.php create mode 100644 src/Resolvers/CachedTenantResolver.php delete mode 100644 src/Resolvers/Contracts/CachedTenantResolver.php delete mode 100644 tests/CachedTenantResolverTest.php create mode 100644 tests/Repository/InMemoryTenantRepository.php create mode 100644 tests/Resolvers/CachedTenantResolverTest.php create mode 100644 tests/Resolvers/DomainTenantResolverTest.php create mode 100644 tests/Resolvers/PathTenantResolverTest.php create mode 100644 tests/Resolvers/RequestDataTenantResolverTest.php diff --git a/assets/config.php b/assets/config.php index 2a54e0b9..dd380805 100644 --- a/assets/config.php +++ b/assets/config.php @@ -196,4 +196,28 @@ return [ '--class' => 'DatabaseSeeder', // root seeder class // '--force' => true, ], + + /** + * + */ + 'tenant_resolvers' => [ + \Stancl\Tenancy\Resolvers\DomainTenantResolver::class => [ + 'cache' => false, + 'cache_ttl' => 3600, + 'cache_store' => null, + ], + + \Stancl\Tenancy\Resolvers\PathTenantResolver::class => [ + 'parameter_name' => 'tenant', + 'cache' => false, + 'cache_ttl' => 3600, + 'cache_store' => null, + ], + + \Stancl\Tenancy\Resolvers\RequestDataTenantResolver::class => [ + 'cache' => false, + 'cache_ttl' => 3600, + 'cache_store' => null, + ] + ] ]; diff --git a/src/Repository/IlluminateTenantRepository.php b/src/Repository/IlluminateTenantRepository.php new file mode 100644 index 00000000..b58ce8ad --- /dev/null +++ b/src/Repository/IlluminateTenantRepository.php @@ -0,0 +1,52 @@ + */ + public readonly string $modelClass, + ) { + } + + public function find(int|string $id): ?Tenant + { + return $this->query()->find($id); + } + + public function findForDomain(string $domain): ?Tenant + { + return $this->query() + ->whereRelation('domains', 'domain', $domain) + ->first(); + } + + /** + * @inheritDoc + */ + public function all(): iterable + { + return $this->query()->cursor(); + } + + /** + * @inheritDoc + */ + public function whereKeyIn(string|int ...$ids): iterable + { + return $this->query()->whereKey($ids)->cursor(); + } + + /** + * @return Builder + */ + private function query(): Builder + { + return (new $this->modelClass)::query(); + } +} diff --git a/src/Repository/TenantRepository.php b/src/Repository/TenantRepository.php new file mode 100644 index 00000000..5c9ee8d7 --- /dev/null +++ b/src/Repository/TenantRepository.php @@ -0,0 +1,22 @@ + + */ + public function all(): iterable; + + /** + * @return iterable + */ + public function whereKeyIn(string|int ...$ids): iterable; +} diff --git a/src/Resolvers/CachedTenantResolver.php b/src/Resolvers/CachedTenantResolver.php new file mode 100644 index 00000000..99a7a43b --- /dev/null +++ b/src/Resolvers/CachedTenantResolver.php @@ -0,0 +1,34 @@ +cache->remember( + key: $this->getCacheKey(...$args), + ttl: $this->ttl, + callback: fn() => $this->tenantResolver->resolve(...$args) + ); + } + + public function getCacheKey(mixed ...$args): string + { + return sprintf('%s:%s', $this->prefix, json_encode($args)); + } +} diff --git a/src/Resolvers/Contracts/CachedTenantResolver.php b/src/Resolvers/Contracts/CachedTenantResolver.php deleted file mode 100644 index f93d7bb5..00000000 --- a/src/Resolvers/Contracts/CachedTenantResolver.php +++ /dev/null @@ -1,78 +0,0 @@ -cache = $cache->store(static::$cacheStore); - } - - public function resolve(mixed ...$args): Tenant - { - if (! static::$shouldCache) { - return $this->resolveWithoutCache(...$args); - } - - $key = $this->getCacheKey(...$args); - - if ($this->cache->has($key)) { - $tenant = $this->cache->get($key); - - $this->resolved($tenant, ...$args); - - return $tenant; - } - - $tenant = $this->resolveWithoutCache(...$args); - $this->cache->put($key, $tenant, static::$cacheTTL); - - return $tenant; - } - - public function invalidateCache(Tenant $tenant): void - { - if (! static::$shouldCache) { - return; - } - - foreach ($this->getArgsForTenant($tenant) as $args) { - $this->cache->forget($this->getCacheKey(...$args)); - } - } - - public function getCacheKey(mixed ...$args): string - { - return '_tenancy_resolver:' . static::class . ':' . json_encode($args); - } - - abstract public function resolveWithoutCache(mixed ...$args): Tenant; - - public function resolved(Tenant $tenant, ...$args): void - { - } - - /** - * Get all the arg combinations for resolve() that can be used to find this tenant. - * - * @return array[] - */ - abstract public function getArgsForTenant(Tenant $tenant): array; -} diff --git a/src/Resolvers/DomainTenantResolver.php b/src/Resolvers/DomainTenantResolver.php index 926c02c0..7eb989f9 100644 --- a/src/Resolvers/DomainTenantResolver.php +++ b/src/Resolvers/DomainTenantResolver.php @@ -4,57 +4,24 @@ declare(strict_types=1); namespace Stancl\Tenancy\Resolvers; -use Illuminate\Database\Eloquent\Builder; -use Stancl\Tenancy\Contracts\Domain; use Stancl\Tenancy\Contracts\Tenant; +use Stancl\Tenancy\Contracts\TenantResolver; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedOnDomainException; +use Stancl\Tenancy\Repository\TenantRepository; -class DomainTenantResolver extends Contracts\CachedTenantResolver +class DomainTenantResolver implements TenantResolver { - /** The model representing the domain that the tenant was identified on. */ - public static Domain $currentDomain; // todo |null? + public function __construct( + private readonly TenantRepository $tenantRepository, + ) { + } - public static bool $shouldCache = false; - - public static int $cacheTTL = 3600; // seconds - - public static string|null $cacheStore = null; // default - - public function resolveWithoutCache(mixed ...$args): Tenant + public function resolve(mixed ...$args): Tenant { $domain = $args[0]; - /** @var Tenant|null $tenant */ - $tenant = config('tenancy.tenant_model')::query() - ->whereHas('domains', fn (Builder $query) => $query->where('domain', $domain)) - ->with('domains') - ->first(); + $tenant = $this->tenantRepository->findForDomain($domain); - if ($tenant) { - $this->setCurrentDomain($tenant, $domain); - - return $tenant; - } - - throw new TenantCouldNotBeIdentifiedOnDomainException($args[0]); - } - - public function resolved(Tenant $tenant, ...$args): void - { - $this->setCurrentDomain($tenant, $args[0]); - } - - protected function setCurrentDomain(Tenant $tenant, string $domain): void - { - static::$currentDomain = $tenant->domains->where('domain', $domain)->first(); - } - - public function getArgsForTenant(Tenant $tenant): array - { - $tenant->unsetRelation('domains'); - - return $tenant->domains->map(function (Domain $domain) { - return [$domain->domain]; - })->toArray(); + return $tenant ?? throw new TenantCouldNotBeIdentifiedOnDomainException($args[0]); } } diff --git a/src/Resolvers/PathTenantResolver.php b/src/Resolvers/PathTenantResolver.php index 2ac2a59f..b5571829 100644 --- a/src/Resolvers/PathTenantResolver.php +++ b/src/Resolvers/PathTenantResolver.php @@ -6,38 +6,31 @@ namespace Stancl\Tenancy\Resolvers; use Illuminate\Routing\Route; use Stancl\Tenancy\Contracts\Tenant; +use Stancl\Tenancy\Contracts\TenantResolver; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedByPathException; +use Stancl\Tenancy\Repository\TenantRepository; -class PathTenantResolver extends Contracts\CachedTenantResolver +class PathTenantResolver implements TenantResolver { - public static string $tenantParameterName = 'tenant'; + public function __construct( + private readonly TenantRepository $tenantRepository, + private readonly string $tenantParameterName = 'tenant', + ) { + } - public static bool $shouldCache = false; - - public static int $cacheTTL = 3600; // seconds - - public static string|null $cacheStore = null; // default - - public function resolveWithoutCache(mixed ...$args): Tenant + public function resolve(mixed ...$args): Tenant { /** @var Route $route */ - $route = $args[0]; + [$route] = $args; - if ($id = $route->parameter(static::$tenantParameterName)) { - $route->forgetParameter(static::$tenantParameterName); + if ($id = $route->parameter($this->tenantParameterName)) { + $route->forgetParameter($this->tenantParameterName); - if ($tenant = tenancy()->find($id)) { + if ($tenant = $this->tenantRepository->find($id)) { return $tenant; } } throw new TenantCouldNotBeIdentifiedByPathException($id); } - - public function getArgsForTenant(Tenant $tenant): array - { - return [ - [$tenant->id], - ]; - } } diff --git a/src/Resolvers/RequestDataTenantResolver.php b/src/Resolvers/RequestDataTenantResolver.php index 5ed65495..d5780039 100644 --- a/src/Resolvers/RequestDataTenantResolver.php +++ b/src/Resolvers/RequestDataTenantResolver.php @@ -5,31 +5,25 @@ declare(strict_types=1); namespace Stancl\Tenancy\Resolvers; use Stancl\Tenancy\Contracts\Tenant; +use Stancl\Tenancy\Contracts\TenantResolver; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedByRequestDataException; +use Stancl\Tenancy\Repository\TenantRepository; -class RequestDataTenantResolver extends Contracts\CachedTenantResolver +class RequestDataTenantResolver implements TenantResolver { - public static bool $shouldCache = false; + public function __construct( + private readonly TenantRepository $tenantRepository, + ) { + } - public static int $cacheTTL = 3600; // seconds - - public static string|null $cacheStore = null; // default - - public function resolveWithoutCache(mixed ...$args): Tenant + public function resolve(mixed ...$args): Tenant { $payload = $args[0]; - if ($payload && $tenant = tenancy()->find($payload)) { + if ($payload && $tenant = $this->tenantRepository->find($payload)) { return $tenant; } throw new TenantCouldNotBeIdentifiedByRequestDataException($payload); } - - public function getArgsForTenant(Tenant $tenant): array - { - return [ - [$tenant->id], - ]; - } } diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index 3850720c..e3f4339a 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -5,6 +5,7 @@ declare(strict_types=1); namespace Stancl\Tenancy; use Illuminate\Cache\CacheManager; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Event; use Illuminate\Support\ServiceProvider; use Stancl\Tenancy\Bootstrappers\FilesystemTenancyBootstrapper; @@ -12,7 +13,12 @@ use Stancl\Tenancy\Contracts\Domain; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Enums\LogMode; use Stancl\Tenancy\Events\Contracts\TenancyEvent; +use Stancl\Tenancy\Repository\IlluminateTenantRepository; +use Stancl\Tenancy\Repository\TenantRepository; +use Stancl\Tenancy\Resolvers\CachedTenantResolver; use Stancl\Tenancy\Resolvers\DomainTenantResolver; +use Stancl\Tenancy\Resolvers\PathTenantResolver; +use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; class TenancyServiceProvider extends ServiceProvider { @@ -40,6 +46,63 @@ class TenancyServiceProvider extends ServiceProvider return $app[Tenancy::class]->tenant; }); + $this->app->bind(TenantRepository::class, function () { + return new IlluminateTenantRepository(config('tenancy.tenant_model')); + }); + + $this->app->singleton(DomainTenantResolver::class, function () { + $tenantResolver = new DomainTenantResolver( + tenantRepository: $this->app->make(TenantRepository::class), + ); + + if (($config = config('tenancy.tenant_resolvers.' . DomainTenantResolver::class)) && ($config['cache'] ?? false)) { + return new CachedTenantResolver( + tenantResolver: $tenantResolver, + cache: Cache::store(config('tenancy.tenant_resolvers')), + prefix: '_tenancy_resolver:domain:', + ttl: $config['ttl'] ?? 3600, + ); + } else { + return $tenantResolver; + } + }); + + $this->app->singleton(PathTenantResolver::class, function () { + $config = config('tenancy.tenant_resolvers.' . PathTenantResolver::class); + $tenantResolver = new PathTenantResolver( + tenantRepository: $this->app->make(TenantRepository::class), + tenantParameterName: $config['parameter_name'] ?? 'tenant', + ); + + if ($config['cache'] ?? false) { + return new CachedTenantResolver( + tenantResolver: $tenantResolver, + cache: Cache::store(config('tenancy.tenant_resolvers')), + prefix: '_tenancy_resolver:path:', + ttl: $config['ttl'] ?? 3600, + ); + } else { + return $tenantResolver; + } + }); + + $this->app->singleton(RequestDataTenantResolver::class, function () { + $tenantResolver = new RequestDataTenantResolver( + tenantRepository: $this->app->make(TenantRepository::class), + ); + + if (($config = config('tenancy.tenant_resolvers.' . RequestDataTenantResolver::class)) && ($config['cache'] ?? false)) { + return new CachedTenantResolver( + tenantResolver: $tenantResolver, + cache: Cache::store(config('tenancy.tenant_resolvers')), + prefix: '_tenancy_resolver:request_data:', + ttl: $config['ttl'] ?? 3600, + ); + } else { + return $tenantResolver; + } + }); + $this->app->bind(Domain::class, function () { return DomainTenantResolver::$currentDomain; }); diff --git a/tests/CachedTenantResolverTest.php b/tests/CachedTenantResolverTest.php deleted file mode 100644 index d71375be..00000000 --- a/tests/CachedTenantResolverTest.php +++ /dev/null @@ -1,95 +0,0 @@ -domains()->create([ - 'domain' => 'acme', - ]); - - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue()->toBeTrue(); -}); - -test('the underlying resolver is not touched when using the cached resolver', function () { - $tenant = Tenant::create(); - $tenant->domains()->create([ - 'domain' => 'acme', - ]); - - DB::enableQueryLog(); - - DomainTenantResolver::$shouldCache = false; - - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - DB::flushQueryLog(); - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - pest()->assertNotEmpty(DB::getQueryLog()); // not empty - - DomainTenantResolver::$shouldCache = true; - - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - DB::flushQueryLog(); - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - expect(DB::getQueryLog())->toBeEmpty(); // empty -}); - -test('cache is invalidated when the tenant is updated', function () { - $tenant = Tenant::create(); - $tenant->createDomain([ - 'domain' => 'acme', - ]); - - DB::enableQueryLog(); - - DomainTenantResolver::$shouldCache = true; - - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - DB::flushQueryLog(); - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - expect(DB::getQueryLog())->toBeEmpty(); // empty - - $tenant->update([ - 'foo' => 'bar', - ]); - - DB::flushQueryLog(); - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - pest()->assertNotEmpty(DB::getQueryLog()); // not empty -}); - -test('cache is invalidated when a tenants domain is changed', function () { - $tenant = Tenant::create(); - $tenant->createDomain([ - 'domain' => 'acme', - ]); - - DB::enableQueryLog(); - - DomainTenantResolver::$shouldCache = true; - - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - DB::flushQueryLog(); - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - expect(DB::getQueryLog())->toBeEmpty(); // empty - - $tenant->createDomain([ - 'domain' => 'bar', - ]); - - DB::flushQueryLog(); - expect($tenant->is(app(DomainTenantResolver::class)->resolve('acme')))->toBeTrue(); - pest()->assertNotEmpty(DB::getQueryLog()); // not empty - - DB::flushQueryLog(); - expect($tenant->is(app(DomainTenantResolver::class)->resolve('bar')))->toBeTrue(); - pest()->assertNotEmpty(DB::getQueryLog()); // not empty -}); diff --git a/tests/Repository/InMemoryTenantRepository.php b/tests/Repository/InMemoryTenantRepository.php new file mode 100644 index 00000000..5b3dfa1d --- /dev/null +++ b/tests/Repository/InMemoryTenantRepository.php @@ -0,0 +1,41 @@ + */ + private Collection $tenants = new Collection(), + ) { + } + + public function find(int|string $id): ?Tenant + { + return $this->tenants->first(fn (Tenant $tenant) => $tenant->getTenantKey() == $id); + } + + public function findForDomain(string $domain): ?Tenant + { + return $this->tenants->firstWhere(fn (Tenant $tenant) => in_array($domain, $tenant->domains ?? [])); + } + + public function all(): iterable + { + return $this->tenants->lazy(); + } + + public function whereKeyIn(string|int ...$ids): iterable + { + return $this->tenants->filter(fn (Tenant $tenant) => in_array($tenant->getTenantKey(), $ids))->lazy(); + } + + public function store(Tenant $tenant): void + { + $this->tenants->push($tenant); + } +} diff --git a/tests/Resolvers/CachedTenantResolverTest.php b/tests/Resolvers/CachedTenantResolverTest.php new file mode 100644 index 00000000..7d6856ec --- /dev/null +++ b/tests/Resolvers/CachedTenantResolverTest.php @@ -0,0 +1,50 @@ + 1, + ]; + + $resolver = new CachedTenantResolver( + tenantResolver: $underlying, + cache: $cache, + prefix: '_tenant_resolver', + ); + + $store->expects('get')->withAnyArgs()->andReturnNull(); + $underlying->expects('resolve')->andReturn($tenant = new Tenant()); + $store->expects('put')->withSomeOfArgs($tenant); + + expect($resolver->resolve($args))->toBe($tenant); +}); + +it('skips the underlying resolver if cache is valid', function () { + $underlying = Mockery::mock(TenantResolver::class); + $cache = new Repository($store = Mockery::mock(Store::class)); + + $args = [ + 'id' => 1, + ]; + + $resolver = new CachedTenantResolver( + tenantResolver: $underlying, + cache: $cache, + prefix: '_tenant_resolver', + ); + + $cache->expects('get')->withAnyArgs()->andReturn($tenant = new Tenant()); + $underlying->expects('resolve')->never(); + + expect($resolver->resolve($args))->toBe($tenant); +}); diff --git a/tests/Resolvers/DomainTenantResolverTest.php b/tests/Resolvers/DomainTenantResolverTest.php new file mode 100644 index 00000000..9e8eb8b0 --- /dev/null +++ b/tests/Resolvers/DomainTenantResolverTest.php @@ -0,0 +1,33 @@ +repository = new InMemoryTenantRepository(); + + $this->tenant = new Tenant(); + $this->tenant->domains = ['acme']; + + $this->repository->store($this->tenant); +}); + +it('resolves the tenant with domain', function () { + $resolver = new DomainTenantResolver( + tenantRepository: $this->repository, + ); + + $result = $resolver->resolve('acme'); + + expect($result)->toBe($this->tenant); +}); + +it('throws when unable to find tenant', function () { + $resolver = new DomainTenantResolver( + tenantRepository: $this->repository, + ); + + $resolver->resolve('foo'); +})->throws(TenantCouldNotBeIdentifiedOnDomainException::class); diff --git a/tests/Resolvers/PathTenantResolverTest.php b/tests/Resolvers/PathTenantResolverTest.php new file mode 100644 index 00000000..bb3843f0 --- /dev/null +++ b/tests/Resolvers/PathTenantResolverTest.php @@ -0,0 +1,40 @@ +repository = new InMemoryTenantRepository(); + + $this->tenant = new Tenant(); + $this->tenant->id = 1; + + $this->repository->store($this->tenant); +}); + +it('resolves the tenant from path', function () { + $resolver = new PathTenantResolver( + tenantRepository: $this->repository, + ); + + $route = (new Route('get', '/{tenant}/foo', fn () => null)) + ->bind(Request::create('/1/foo')); + + $result = $resolver->resolve($route); + + expect($result)->toBe($this->tenant); +}); + +it('throws when unable to find tenant', function () { + $resolver = new PathTenantResolver( + tenantRepository: new InMemoryTenantRepository(), + ); + + $route = (new Route('GET', '/{tenant}/foo', fn () => null)) + ->bind(Request::create('/2/foo')); + + $resolver->resolve($route); +})->throws(TenantCouldNotBeIdentifiedByPathException::class); diff --git a/tests/Resolvers/RequestDataTenantResolverTest.php b/tests/Resolvers/RequestDataTenantResolverTest.php new file mode 100644 index 00000000..1ce4b460 --- /dev/null +++ b/tests/Resolvers/RequestDataTenantResolverTest.php @@ -0,0 +1,33 @@ +repository = new InMemoryTenantRepository(); + + $this->tenant = new Tenant(); + $this->tenant->id = 1; + + $this->repository->store($this->tenant); +}); + +it('resolves the tenant', function () { + $resolver = new DomainTenantResolver( + tenantRepository: $this->repository, + ); + + $result = $resolver->resolve(id: 1); + + expect($result)->toBe($this->tenant); +}); + +it('throws when unable to find tenant', function () { + $resolver = new DomainTenantResolver( + tenantRepository: $this->repository, + ); + + $resolver->resolve('foo'); +})->throws(TenantCouldNotBeIdentifiedOnDomainException::class);