From 0ef0104355584a2242fd55113710ffa747087d82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Mon, 28 Jul 2025 17:08:07 +0200 Subject: [PATCH 1/8] Add MariaDB database manager config --- assets/config.php | 1 + 1 file changed, 1 insertion(+) diff --git a/assets/config.php b/assets/config.php index 73becdee..5e231f2a 100644 --- a/assets/config.php +++ b/assets/config.php @@ -203,6 +203,7 @@ return [ 'managers' => [ 'sqlite' => Stancl\Tenancy\Database\TenantDatabaseManagers\SQLiteDatabaseManager::class, 'mysql' => Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager::class, + 'mariadb' => Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager::class, 'pgsql' => Stancl\Tenancy\Database\TenantDatabaseManagers\PostgreSQLDatabaseManager::class, 'sqlsrv' => Stancl\Tenancy\Database\TenantDatabaseManagers\MicrosoftSQLDatabaseManager::class, From b2f2669885c932ccfc2c3d1be7047198e4985216 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 29 Jul 2025 17:17:32 +0200 Subject: [PATCH 2/8] [4.x] Cloning: addTenantParameter(bool), domain(string|null) (#1374) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Add test for the new clone action addTenantParameter property * Add $addTenantParameter properyt to the clone action * Fix code style (php-cs-fixer) * Update clone action annotation * Make addTenantParameter(false) sound by adding domain() logic to route cloning --------- Co-authored-by: github-actions[bot] Co-authored-by: Samuel Štancl --- .php-cs-fixer.php | 1 - src/Actions/CloneRoutesAsTenant.php | 91 ++++++++++++++++++++++------- tests/CloneActionTest.php | 52 +++++++++++++++++ 3 files changed, 122 insertions(+), 22 deletions(-) diff --git a/.php-cs-fixer.php b/.php-cs-fixer.php index c0fe775c..d4ce4e41 100644 --- a/.php-cs-fixer.php +++ b/.php-cs-fixer.php @@ -21,7 +21,6 @@ $rules = [ 'blank_line_before_statement' => [ 'statements' => ['return'] ], - 'braces' => true, 'cast_spaces' => true, 'class_definition' => true, 'concat_space' => [ diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index c5818878..ec60d880 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -30,6 +30,11 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * By providing a callback to shouldClone(), you can change how it's determined if a route should be cloned if you don't want to use middleware flags. * * Cloned routes are prefixed with '/{tenant}', flagged with 'tenant' middleware, and have their names prefixed with 'tenant.'. + * + * The addition of the tenant parameter can be controlled using addTenantParameter(true|false). Note that if you decide to disable + * tenant parameter addition, the routes MUST differ in domains. This can be controlled using the domain(string|null) method. The + * default behavior is to NOT set any domains on cloned routes -- unless specified otherwise using that method. + * * The parameter name and prefix can be changed e.g. to `/{team}` and `team.` by configuring the path resolver (tenantParameterName and tenantRouteNamePrefix). * Routes with names that are already prefixed won't be cloned - but that's just the default behavior. * The cloneUsing() method allows you to completely override the default behavior and customize how the cloned routes will be defined. @@ -43,8 +48,8 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * * Example usage: * ``` - * Route::get('/foo', fn () => true)->name('foo')->middleware('clone'); - * Route::get('/bar', fn () => true)->name('bar')->middleware('universal'); + * Route::get('/foo', ...)->name('foo')->middleware('clone'); + * Route::get('/bar', ...)->name('bar')->middleware('universal'); * * $cloneAction = app(CloneRoutesAsTenant::class); * @@ -54,10 +59,20 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * // Clone bar route as /{tenant}/bar and name it tenant.bar ('universal' middleware won't be present in the cloned route) * $cloneAction->cloneRoutesWithMiddleware(['universal'])->handle(); * - * Route::get('/baz', fn () => true)->name('baz'); + * Route::get('/baz', ...)->name('baz'); * * // Clone baz route as /{tenant}/bar and name it tenant.baz ('universal' middleware won't be present in the cloned route) * $cloneAction->cloneRoute('baz')->handle(); + * + * Route::domain('central.localhost')->get('/no-tenant-parameter', ...)->name('no-tenant-parameter')->middleware('clone'); + * + * // Clone baz route as /no-tenant-parameter and name it tenant.no-tenant-parameter (the route won't have the tenant parameter) + * // This can be useful with domain identification. Importantly, the original route MUST have a domain set. The domain of the + * // cloned route can be customized using domain(string|null). By default, the cloned route will not be scoped to a domain, + * // unless a domain() call is used. It's important to keep in mind that: + * // 1. When addTenantParameter(false) is used, the paths will be the same, thus domains must differ. + * // 2. If the original route (with the same path) has no domain, the cloned route will never be used due to registration order. + * $cloneAction->addTenantParameter(false)->cloneRoutesWithMiddleware(['clone'])->cloneRoute('no-tenant-parameter')->handle(); * ``` * * Calling handle() will also clear the $routesToClone array. @@ -70,6 +85,8 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; class CloneRoutesAsTenant { protected array $routesToClone = []; + protected bool $addTenantParameter = true; + protected string|null $domain = null; protected Closure|null $cloneUsing = null; // The callback should accept Route instance or the route name (string) protected Closure|null $shouldClone = null; protected array $cloneRoutesWithMiddleware = ['clone']; @@ -78,6 +95,7 @@ class CloneRoutesAsTenant protected Router $router, ) {} + /** Clone routes. This resets routesToClone() but not other config. */ public function handle(): void { // If no routes were specified using cloneRoute(), get all routes @@ -102,15 +120,37 @@ class CloneRoutesAsTenant $route = $this->router->getRoutes()->getByName($route); } - $this->copyMiscRouteProperties($route, $this->createNewRoute($route)); + $this->createNewRoute($route); } // Clean up the routesToClone array after cloning so that subsequent calls aren't affected $this->routesToClone = []; $this->router->getRoutes()->refreshNameLookups(); + $this->router->getRoutes()->refreshActionLookups(); } + /** + * Should a tenant parameter be added to the cloned route. + * + * The name of the parameter is controlled using PathTenantResolver::tenantParameterName(). + */ + public function addTenantParameter(bool $addTenantParameter): static + { + $this->addTenantParameter = $addTenantParameter; + + return $this; + } + + /** The domain the cloned route should use. Set to null if it shouldn't be scoped to a domain. */ + public function domain(string|null $domain): static + { + $this->domain = $domain; + + return $this; + } + + /** Provide a custom callback for cloning routes, instead of the default behavior. */ public function cloneUsing(Closure|null $cloneUsing): static { $this->cloneUsing = $cloneUsing; @@ -118,6 +158,7 @@ class CloneRoutesAsTenant return $this; } + /** Specify which middleware should serve as "flags" telling this action to clone those routes. */ public function cloneRoutesWithMiddleware(array $middleware): static { $this->cloneRoutesWithMiddleware = $middleware; @@ -125,6 +166,10 @@ class CloneRoutesAsTenant return $this; } + /** + * Provide a custom callback for determining whether a route should be cloned. + * Overrides the default middleware-based detection. + * */ public function shouldClone(Closure|null $shouldClone): static { $this->shouldClone = $shouldClone; @@ -132,6 +177,7 @@ class CloneRoutesAsTenant return $this; } + /** Clone an individual route. */ public function cloneRoute(Route|string $route): static { $this->routesToClone[] = $route; @@ -171,28 +217,31 @@ class CloneRoutesAsTenant $action->put('as', PathTenantResolver::tenantRouteNamePrefix() . $name); } - $action - ->put('middleware', $middleware) - ->put('prefix', $prefix . '/{' . PathTenantResolver::tenantParameterName() . '}'); + if ($this->domain) { + $action->put('domain', $this->domain); + } elseif ($action->offsetExists('domain')) { + $action->offsetUnset('domain'); + } + + $action->put('middleware', $middleware); + + if ($this->addTenantParameter) { + $action->put('prefix', $prefix . '/{' . PathTenantResolver::tenantParameterName() . '}'); + } /** @var Route $newRoute */ $newRoute = $this->router->$method($uri, $action->toArray()); - return $newRoute; - } - - /** - * Copy misc properties of the original route to the new route. - */ - protected function copyMiscRouteProperties(Route $originalRoute, Route $newRoute): void - { + // Copy misc properties of the original route to the new route. $newRoute - ->setBindingFields($originalRoute->bindingFields()) - ->setFallback($originalRoute->isFallback) - ->setWheres($originalRoute->wheres) - ->block($originalRoute->locksFor(), $originalRoute->waitsFor()) - ->withTrashed($originalRoute->allowsTrashedBindings()) - ->setDefaults($originalRoute->defaults); + ->setBindingFields($route->bindingFields()) + ->setFallback($route->isFallback) + ->setWheres($route->wheres) + ->block($route->locksFor(), $route->waitsFor()) + ->withTrashed($route->allowsTrashedBindings()) + ->setDefaults($route->defaults); + + return $newRoute; } /** Removes top-level cloneRoutesWithMiddleware and adds 'tenant' middleware. */ diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 866babb5..656ad327 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -6,6 +6,7 @@ use Stancl\Tenancy\Actions\CloneRoutesAsTenant; use Stancl\Tenancy\Resolvers\PathTenantResolver; use Illuminate\Support\Facades\Route as RouteFacade; use function Stancl\Tenancy\Tests\pest; +use Illuminate\Routing\Exceptions\UrlGenerationException; test('CloneRoutesAsTenant action clones routes with clone middleware by default', function () { config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_parameter_name' => 'team']); @@ -337,3 +338,54 @@ test('clone action can be used fluently', function() { expect(collect(RouteFacade::getRoutes()->get())->map->getName()) ->toContain('tenant.foo', 'tenant.bar', 'tenant.baz'); }); + +test('the cloned route can be scoped to a specified domain', function () { + RouteFacade::domain('foo.localhost')->get('/foo', fn () => in_array('tenant', request()->route()->middleware()) ? 'tenant' : 'central')->name('foo')->middleware('clone'); + + // Importantly, we CANNOT add a domain to the cloned route *if the original route didn't have a domain*. + // This is due to the route registration order - the more strongly scoped route (= route with a domain) + // must be registered first, so that Laravel tries that route first and only moves on if the domain check fails. + $cloneAction = app(CloneRoutesAsTenant::class); + // To keep the test simple we don't even need a tenant parameter + $cloneAction->domain('bar.localhost')->addTenantParameter(false)->handle(); + + expect(route('foo'))->toBe('http://foo.localhost/foo'); + expect(route('tenant.foo'))->toBe('http://bar.localhost/foo'); +}); + +test('tenant parameter addition can be controlled by setting addTenantParameter', function (bool $addTenantParameter) { + RouteFacade::domain('central.localhost') + ->get('/foo', fn () => in_array('tenant', request()->route()->middleware()) ? 'tenant' : 'central') + ->name('foo') + ->middleware('clone'); + + // By default this action also removes the domain + $cloneAction = app(CloneRoutesAsTenant::class); + $cloneAction->addTenantParameter($addTenantParameter)->handle(); + + $clonedRoute = RouteFacade::getRoutes()->getByName('tenant.foo'); + + // We only use the route() helper here, since once a request is made + // the URL generator caches the request's domain and it affects route + // generation for routes that do not have domain() specified (tenant.foo) + expect(route('foo'))->toBe('http://central.localhost/foo'); + if ($addTenantParameter) + expect(route('tenant.foo', ['tenant' => 'abc']))->toBe('http://localhost/abc/foo'); + else + expect(route('tenant.foo'))->toBe('http://localhost/foo'); + + // Original route still works + $this->withoutExceptionHandling()->get(route('foo'))->assertSee('central'); + + if ($addTenantParameter) { + expect($clonedRoute->uri())->toContain('{tenant}'); + + $this->withoutExceptionHandling()->get('http://localhost/abc/foo')->assertSee('tenant'); + $this->withoutExceptionHandling()->get('http://central.localhost/foo')->assertSee('central'); + } else { + expect($clonedRoute->uri())->not()->toContain('{tenant}'); + + $this->withoutExceptionHandling()->get('http://localhost/foo')->assertSee('tenant'); + $this->withoutExceptionHandling()->get('http://central.localhost/foo')->assertSee('central'); + } +})->with([true, false]); From 475ea10ae906480dd7fa5c6ee74411f5330f3913 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 29 Jul 2025 17:18:14 +0200 Subject: [PATCH 3/8] Delete unused import (#1382) --- tests/DatabasePreparationTest.php | 1 - 1 file changed, 1 deletion(-) diff --git a/tests/DatabasePreparationTest.php b/tests/DatabasePreparationTest.php index d5641af4..1f3a4f09 100644 --- a/tests/DatabasePreparationTest.php +++ b/tests/DatabasePreparationTest.php @@ -9,7 +9,6 @@ use Stancl\Tenancy\Events\TenantCreated; use Stancl\Tenancy\Jobs\CreateDatabase; use Stancl\Tenancy\Jobs\MigrateDatabase; use Stancl\Tenancy\Jobs\SeedDatabase; -use Stancl\Tenancy\Database\TenantDatabaseManagers\MySQLDatabaseManager; use Stancl\Tenancy\Tests\Etc\Tenant; use Illuminate\Foundation\Auth\User as Authenticable; use Stancl\Tenancy\Tests\Etc\TestSeeder; From 0a48767c32be40eee2898ed1f3efc60b4cb1bd81 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Tue, 29 Jul 2025 22:25:07 +0200 Subject: [PATCH 4/8] Bump jobpipeline dependency to rc6 --- composer.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/composer.json b/composer.json index 3d3bd3eb..2ee61fb0 100644 --- a/composer.json +++ b/composer.json @@ -21,7 +21,7 @@ "illuminate/support": "^12.0", "laravel/tinker": "^2.0", "ramsey/uuid": "^4.7.3", - "stancl/jobpipeline": "2.0.0-rc5", + "stancl/jobpipeline": "2.0.0-rc6", "stancl/virtualcolumn": "^1.5.0", "spatie/invade": "*", "laravel/prompts": "0.*" From 81fff15afea5ac1588665f6adbf0f54430e85e7b Mon Sep 17 00:00:00 2001 From: lukinovec Date: Sun, 3 Aug 2025 23:15:34 +0200 Subject: [PATCH 5/8] [4.x] Update ForgetTenantParameter-related comments (#1375) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Update ForgetTenantParameter-related comments * Improve path id mw config docblock * improve docblocks --------- Co-authored-by: Samuel Štancl --- assets/config.php | 7 +++++-- src/Listeners/ForgetTenantParameter.php | 20 ++++++++++---------- src/TenancyServiceProvider.php | 20 ++++++++++++++++---- 3 files changed, 31 insertions(+), 16 deletions(-) diff --git a/assets/config.php b/assets/config.php index 5e231f2a..ba503aad 100644 --- a/assets/config.php +++ b/assets/config.php @@ -93,11 +93,14 @@ return [ * Identification middleware tenancy recognizes as path identification middleware. * * This is used for determining if a path identification middleware is used - * during operations specific to path identification, e.g. forgetting the tenant parameter in ForgetTenantParameter. + * during operations specific to path identification. + * + * This is used for forgetting the tenant parameter using the ForgetTenantParameter listener. + * The listener only has an effect when path identification middleware + * is used in the global middleware stack and certain other conditions are met. * * If you're using a custom path identification middleware, add it here. * - * @see \Stancl\Tenancy\Actions\CloneRoutesAsTenant * @see \Stancl\Tenancy\Listeners\ForgetTenantParameter */ 'path_identification_middleware' => [ diff --git a/src/Listeners/ForgetTenantParameter.php b/src/Listeners/ForgetTenantParameter.php index 0b1d1440..d159b967 100644 --- a/src/Listeners/ForgetTenantParameter.php +++ b/src/Listeners/ForgetTenantParameter.php @@ -11,18 +11,18 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; // todo@earlyIdReview /** - * Remove the tenant parameter from the matched route when path identification is used globally. + * Conditionally removes the tenant parameter from matched routes when using kernel path identification. * - * While initializing tenancy, we forget the tenant parameter (in PathTenantResolver), - * so that the route actions don't have to accept it. + * When path identification middleware is in the global stack, + * the tenant parameter is initially forgotten during tenancy initialization in PathTenantResolver. + * However, because kernel identification occurs before route matching, the route still contains + * the tenant parameter when RouteMatched is fired. This listener removes it to prevent route + * actions from needing to accept an unwanted tenant parameter. * - * With kernel identification, tenancy gets initialized before the route gets matched. - * The matched route gets the tenant parameter again, so we have to forget the parameter again on RouteMatched. - * - * We remove the {tenant} parameter from the matched route when - * 1) the InitializeTenancyByPath middleware is in the global stack, AND - * 2) the matched route does not have identification middleware (so that {tenant} isn't forgotten when using route-level identification), AND - * 3) the route isn't in the central context (so that {tenant} doesn't get accidentally removed from central routes). + * The {tenant} parameter is removed from the matched route only when ALL of these conditions are met: + * 1) A path identification middleware is in the global middleware stack (kernel identification) + * 2) The matched route does NOT have its own identification middleware (route-level identification takes precedence) + * 3) The route is in tenant or universal context (central routes keep their tenant parameter) */ class ForgetTenantParameter { diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index 4059479e..22a81624 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -20,6 +20,8 @@ use Stancl\Tenancy\Resolvers\DomainTenantResolver; class TenancyServiceProvider extends ServiceProvider { public static Closure|null $configure = null; + public static bool $registerForgetTenantParameterListener = true; + public static bool $migrateFreshOverride = true; /* Register services. */ public function register(): void @@ -104,9 +106,11 @@ class TenancyServiceProvider extends ServiceProvider Commands\CreateUserWithRLSPolicies::class, ]); - $this->app->extend(FreshCommand::class, function ($_, $app) { - return new Commands\MigrateFreshOverride($app['migrator']); - }); + if (static::$migrateFreshOverride) { + $this->app->extend(FreshCommand::class, function ($_, $app) { + return new Commands\MigrateFreshOverride($app['migrator']); + }); + } $this->publishes([ __DIR__ . '/../assets/config.php' => config_path('tenancy.php'), @@ -152,6 +156,14 @@ class TenancyServiceProvider extends ServiceProvider Route::middlewareGroup('tenant', []); Route::middlewareGroup('central', []); - Event::listen(RouteMatched::class, ForgetTenantParameter::class); + if (static::$registerForgetTenantParameterListener) { + // Ideally, this listener would only be registered when kernel-level + // path identification is used, however doing that check reliably + // at this point in the lifecycle isn't feasible. For that reason, + // rather than doing an "outer" check, we do an "inner" check within + // that listener. That also means the listener needs to be registered + // always. We allow for this to be controlled using a static property. + Event::listen(RouteMatched::class, ForgetTenantParameter::class); + } } } From f308e2f84d00565e4846ad048d8df11d03f172fd Mon Sep 17 00:00:00 2001 From: lukinovec Date: Sun, 3 Aug 2025 23:21:03 +0200 Subject: [PATCH 6/8] [4.x] Resolve testing todos (#1361) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Test encrypted cookie identification * Add Fortify bootstrapper custom query param passing test * Correct Fortify route bootstrapper test (todo refactor, convoluted) * Clarify Fortify bootstrapper test * Fix encrypted cookie identification test * Move encrypted cookie assertion to "cookie identification works" * Cover configured tenant model columns in cached resolver tests * Refactor testing resolver with default vs custom tenant model name config * Delete resolved todo * Make code more concise * Keep initial formatting (minimize diff noise) * Make dataset/helper method parameter clearer * Clarify fortify test * Clarify assertions, improve comments * Delete excessive comments, make existing comments consistent and clearer * Make cached resolver test file clearer, update outdated comments * Use the tenant model column term consistently * FIx inconsistencies * Provide more info in comment * make comment more clear * static property reset --------- Co-authored-by: Samuel Štancl --- .../Integrations/FortifyRouteBootstrapper.php | 1 - src/Resolvers/RequestDataTenantResolver.php | 1 - .../FortifyRouteBootstrapperTest.php | 78 +++++++--- tests/CachedTenantResolverTest.php | 133 ++++++++++++------ tests/RequestDataIdentificationTest.php | 9 +- 5 files changed, 157 insertions(+), 65 deletions(-) diff --git a/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php b/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php index fb371d6a..12b49f78 100644 --- a/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php +++ b/src/Bootstrappers/Integrations/FortifyRouteBootstrapper.php @@ -86,7 +86,6 @@ class FortifyRouteBootstrapper implements TenancyBootstrapper protected function useTenantRoutesInFortify(Tenant $tenant): void { if (static::$passQueryParameter) { - // todo@tests $tenantParameterName = RequestDataTenantResolver::queryParameterName(); $tenantParameterValue = RequestDataTenantResolver::payloadValue($tenant); } else { diff --git a/src/Resolvers/RequestDataTenantResolver.php b/src/Resolvers/RequestDataTenantResolver.php index 4d8b3277..48ee45ee 100644 --- a/src/Resolvers/RequestDataTenantResolver.php +++ b/src/Resolvers/RequestDataTenantResolver.php @@ -31,7 +31,6 @@ class RequestDataTenantResolver extends Contracts\CachedTenantResolver public function getPossibleCacheKeys(Tenant&Model $tenant): array { - // todo@tests return [ $this->formatCacheKey(static::payloadValue($tenant)), ]; diff --git a/tests/Bootstrappers/FortifyRouteBootstrapperTest.php b/tests/Bootstrappers/FortifyRouteBootstrapperTest.php index 63f0f2a0..bfc835ee 100644 --- a/tests/Bootstrappers/FortifyRouteBootstrapperTest.php +++ b/tests/Bootstrappers/FortifyRouteBootstrapperTest.php @@ -1,6 +1,5 @@ [FortifyRouteBootstrapper::class]]); + config([ + // Parameter name (RequestDataTenantResolver::queryParameterName()) + 'tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.query_parameter' => 'team_query', + // Parameter value (RequestDataTenantResolver::payloadValue() gets the tenant model column value) + 'tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => 'company', + ]); + + config([ + // Parameter name (PathTenantResolver::tenantParameterName()) + 'tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_parameter_name' => 'team_path', + // Parameter value (PathTenantResolver::tenantParameterValue() gets the tenant model column value) + 'tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_model_column' => 'name', + ]); + $originalFortifyHome = config('fortify.home'); $originalFortifyRedirects = config('fortify.redirects'); - Route::get('/home', function () { - return true; - })->name($homeRouteName = 'home'); - - Route::get('/welcome', function () { - return true; - })->name($welcomeRouteName = 'welcome'); + Route::get('/home', fn () => true)->name($homeRouteName = 'home'); + Route::get('/welcome', fn () => true)->name($welcomeRouteName = 'welcome'); FortifyRouteBootstrapper::$fortifyHome = $homeRouteName; FortifyRouteBootstrapper::$fortifyRedirectMap['login'] = $welcomeRouteName; @@ -43,21 +53,53 @@ test('fortify route tenancy bootstrapper updates fortify config correctly', func expect(config('fortify.home'))->toBe($originalFortifyHome); expect(config('fortify.redirects'))->toBe($originalFortifyRedirects); - FortifyRouteBootstrapper::$passTenantParameter = true; - tenancy()->initialize($tenant = Tenant::create()); - expect(config('fortify.home'))->toBe('http://localhost/home?tenant=' . $tenant->getTenantKey()); - expect(config('fortify.redirects'))->toEqual(['login' => 'http://localhost/welcome?tenant=' . $tenant->getTenantKey()]); + /** + * When $passQueryParameter is true, the bootstrapper uses + * the RequestDataTenantResolver config for generating the Fortify URLs + * - tenant parameter is 'team_query' + * - parameter value is the tenant's company ('Acme') + * + * When $passQueryParameter is false, the bootstrapper uses + * the PathTenantResolver config for generating the Fortify URLs + * - tenant parameter is 'team_path' + * - parameter value is the tenant's name ('Foo') + */ + $tenant = Tenant::create([ + 'company' => 'Acme', + 'name' => 'Foo', + ]); - tenancy()->end(); - expect(config('fortify.home'))->toBe($originalFortifyHome); - expect(config('fortify.redirects'))->toBe($originalFortifyRedirects); + // The bootstrapper generates and overrides the URLs in the Fortify config correctly + // (= the generated URLs have the correct tenant parameter + parameter value) + // The bootstrapper should use RequestDataTenantResolver while generating the URLs (default) + FortifyRouteBootstrapper::$passQueryParameter = true; - FortifyRouteBootstrapper::$passTenantParameter = false; tenancy()->initialize($tenant); - expect(config('fortify.home'))->toBe('http://localhost/home'); - expect(config('fortify.redirects'))->toEqual(['login' => 'http://localhost/welcome']); + expect(config('fortify.home'))->toBe('http://localhost/home?team_query=Acme'); + expect(config('fortify.redirects'))->toBe(['login' => 'http://localhost/welcome?team_query=Acme']); + + // The bootstrapper restores the original Fortify config when ending tenancy tenancy()->end(); + expect(config('fortify.home'))->toBe($originalFortifyHome); expect(config('fortify.redirects'))->toBe($originalFortifyRedirects); + + // The bootstrapper should use PathTenantResolver while generating the URLs now + FortifyRouteBootstrapper::$passQueryParameter = false; + + tenancy()->initialize($tenant); + + expect(config('fortify.home'))->toBe('http://localhost/home?team_path=Foo'); + expect(config('fortify.redirects'))->toBe(['login' => 'http://localhost/welcome?team_path=Foo']); + + tenancy()->end(); + + // The bootstrapper can override the home and redirects config without the tenant parameter being passed + FortifyRouteBootstrapper::$passTenantParameter = false; + + tenancy()->initialize($tenant); + + expect(config('fortify.home'))->toBe('http://localhost/home'); + expect(config('fortify.redirects'))->toBe(['login' => 'http://localhost/welcome']); }); diff --git a/tests/CachedTenantResolverTest.php b/tests/CachedTenantResolverTest.php index 558fb345..322a1961 100644 --- a/tests/CachedTenantResolverTest.php +++ b/tests/CachedTenantResolverTest.php @@ -17,59 +17,72 @@ use Stancl\Tenancy\Middleware\InitializeTenancyByPath; use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; use function Stancl\Tenancy\Tests\pest; -test('tenants can be resolved using cached resolvers', function (string $resolver) { - $tenant = Tenant::create(['id' => $tenantKey = 'acme']); +beforeEach($cleanup = function () { + Tenant::$extraCustomColumns = []; +}); - $tenant->domains()->create(['domain' => $tenantKey]); +afterEach($cleanup); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); +test('tenants can be resolved using cached resolvers', function (string $resolver, bool $configureTenantModelColumn) { + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']); + + $tenant->createDomain($tenant->{$tenantModelColumn}); + + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); })->with([ DomainTenantResolver::class, PathTenantResolver::class, RequestDataTenantResolver::class, +])->with([ + 'tenant model column is id (default)' => false, + 'tenant model column is name (custom)' => true, ]); -test('the underlying resolver is not touched when using the cached resolver', function (string $resolver) { - $tenant = Tenant::create(['id' => $tenantKey = 'acme']); +test('the underlying resolver is not touched when using the cached resolver', function (string $resolver, bool $configureTenantModelColumn) { + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']); - $tenant->createDomain($tenantKey); + $tenant->createDomain($tenant->{$tenantModelColumn}); DB::enableQueryLog(); config(['tenancy.identification.resolvers.' . $resolver . '.cache' => false]); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); pest()->assertNotEmpty(DB::getQueryLog()); // not empty config(['tenancy.identification.resolvers.' . $resolver . '.cache' => true]); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->toBeEmpty(); // empty })->with([ DomainTenantResolver::class, PathTenantResolver::class, RequestDataTenantResolver::class, +])->with([ + 'tenant model column is id (default)' => false, + 'tenant model column is name (custom)' => true, ]); -test('cache is invalidated when the tenant is updated', function (string $resolver) { - $tenant = Tenant::create(['id' => $tenantKey = 'acme']); - $tenant->createDomain($tenantKey); +test('cache is invalidated when the tenant is updated', function (string $resolver, bool $configureTenantModelColumn) { + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']); + + $tenant->createDomain($tenant->{$tenantModelColumn}); DB::enableQueryLog(); config(['tenancy.identification.resolvers.' . $resolver . '.cache' => true]); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->not()->toBeEmpty(); DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->toBeEmpty(); // Tenant cache gets invalidated when the tenant is updated @@ -77,41 +90,47 @@ test('cache is invalidated when the tenant is updated', function (string $resolv DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->not()->toBeEmpty(); // Cache was invalidated, so the tenant was retrieved from the DB })->with([ DomainTenantResolver::class, PathTenantResolver::class, RequestDataTenantResolver::class, +])->with([ + 'tenant model column is id (default)' => false, + 'tenant model column is name (custom)' => true, ]); -test('cache is invalidated when the tenant is deleted', function (string $resolver) { +test('cache is invalidated when the tenant is deleted', function (string $resolver, bool $configureTenantModelColumn) { DB::statement('SET FOREIGN_KEY_CHECKS=0;'); // allow deleting the tenant - $tenant = Tenant::create(['id' => $tenantKey = 'acme']); - $tenant->createDomain($tenantKey); + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']); + $tenant->createDomain($tenant->{$tenantModelColumn}); DB::enableQueryLog(); config(['tenancy.identification.resolvers.' . $resolver . '.cache' => true]); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->not()->toBeEmpty(); DB::flushQueryLog(); - expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenantKey))))->toBeTrue(); + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); expect(DB::getQueryLog())->toBeEmpty(); $tenant->delete(); DB::flushQueryLog(); - expect(fn () => app($resolver)->resolve(getResolverArgument($resolver, $tenantKey)))->toThrow(TenantCouldNotBeIdentifiedException::class); + expect(fn () => app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn)))->toThrow(TenantCouldNotBeIdentifiedException::class); expect(DB::getQueryLog())->not()->toBeEmpty(); // Cache was invalidated, so the DB was queried })->with([ DomainTenantResolver::class, PathTenantResolver::class, RequestDataTenantResolver::class, +])->with([ + 'tenant model column is id (default)' => false, + 'tenant model column is name (custom)' => true, ]); test('cache is invalidated when a tenants domain is changed', function () { @@ -307,23 +326,49 @@ test('PathTenantResolver properly separates cache for each tenant column', funct pest()->get("/x/bar/b")->assertSee('foo'); expect(count(DB::getRawQueryLog()))->toBe(4); // unchanged expect(count($redisKeys()))->toBe(4); // unchanged - - Tenant::$extraCustomColumns = []; // reset }); /** - * Return the argument for the resolver – tenant key, or a route instance with the tenant parameter. + * This method is used in generic tests to ensure that caching works correctly both with default and custom resolver config. * - * PathTenantResolver uses a route instance with the tenant parameter as the argument, - * unlike other resolvers which use a tenant key as the argument. - * - * This method is used in the tests where we test all the resolvers - * to make getting the resolver arguments less repetitive (primarily because of the PathTenantResolver). + * If $configureTenantModelColumn is false, the tenant model column is 'id' (default) -- don't configure anything, keep the defaults. + * If $configureTenantModelColumn is true, the tenant model column should be 'name' (custom) -- configure tenant_model_column in the resolvers. */ -function getResolverArgument(string $resolver, string $tenantKey): string|Route +function tenantModelColumn(bool $configureTenantModelColumn): string { + // Default tenant model column is 'id' + $tenantModelColumn = 'id'; + + if ($configureTenantModelColumn) { + // Use 'name' as the custom tenant model column + $tenantModelColumn = 'name'; + + Tenant::$extraCustomColumns = [$tenantModelColumn]; + + Schema::table('tenants', function (Blueprint $table) use ($tenantModelColumn) { + $table->string($tenantModelColumn)->unique(); + }); + + config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_model_column' => $tenantModelColumn]); + config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.tenant_model_column' => $tenantModelColumn]); + } + + return $tenantModelColumn; +} + +/** + * This method is used in generic tests where we test all the resolvers + * to make getting the resolver arguments less repetitive (primarily because of PathTenantResolver). + * + * For PathTenantResolver, return a route instance with the value retrieved using $tenant->{$parameterColumn} as the parameter. + * For RequestDataTenantResolver and DomainTenantResolver, return the value retrieved using $tenant->{$parameterColumn}. + * + * Tenant model column is 'id' by default, but in the generic tests, + * we also configure that to 'name' to ensure everything works both with default and custom config. + */ +function getResolverArgument(string $resolver, Tenant $tenant, string $parameterColumn = 'id'): string|Route { - // PathTenantResolver uses a route instance as the argument if ($resolver === PathTenantResolver::class) { + // PathTenantResolver uses a route instance as the argument $routeName = 'tenant-route'; // Find or create a route instance for the resolver @@ -332,17 +377,21 @@ function getResolverArgument(string $resolver, string $tenantKey): string|Route ->prefix('{tenant}') ->middleware(InitializeTenancyByPath::class); - // To make the tenant available on the route instance - // Make the 'tenant' route parameter the tenant key - // Setting the parameter on the $route->parameters property is required - // Because $route->setParameter() throws an exception when $route->parameters is not set yet - $route->parameters[PathTenantResolver::tenantParameterName()] = $tenantKey; + /** + * To make the tenant available on the route instance, + * set the 'tenant' route parameter to the tenant model column value ('id' or 'name'). + * + * Setting the parameter on the $route->parameters property is required + * because $route->setParameter() throws an exception when $route->parameters isn't set yet. + */ + $route->parameters['tenant'] = $tenant->{$parameterColumn}; - // Return the route instance with the tenant key as the 'tenant' parameter + // Return the route instance with 'id' or 'name' as the 'tenant' parameter return $route; } - // Resolvers other than PathTenantResolver use the tenant key as the argument - // Return the tenant key - return $tenantKey; + // Assuming that: + // - with RequestDataTenantResolver, the tenant model column value is the payload value + // - with DomainTenantResolver, the tenant has a domain with name equal to the tenant model column value (see the createDomain() calls in various tests) + return $tenant->{$parameterColumn}; } diff --git a/tests/RequestDataIdentificationTest.php b/tests/RequestDataIdentificationTest.php index 0f635bf1..63940ed5 100644 --- a/tests/RequestDataIdentificationTest.php +++ b/tests/RequestDataIdentificationTest.php @@ -9,6 +9,7 @@ use Stancl\Tenancy\Middleware\InitializeTenancyByRequestData; use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; use Stancl\Tenancy\Tests\Etc\Tenant; use function Stancl\Tenancy\Tests\pest; +use Illuminate\Cookie\CookieValuePrefix; beforeEach(function () { config([ @@ -88,6 +89,11 @@ test('cookie identification works', function (string|null $tenantModelColumn) { // Default cookie name $this->withoutExceptionHandling()->withUnencryptedCookie('tenant', $payload)->get('test')->assertSee($tenant->id); + // Manually encrypted cookie (encrypt the cookie exactly like MakesHttpRequests does in prepareCookiesForRequest()) + $encryptedPayload = encrypt(CookieValuePrefix::create('tenant', app('encrypter')->getKey()) . $payload, false); + + $this->withoutExceptionHandling()->withUnencryptedCookie('tenant', $encryptedPayload)->get('test')->assertSee($tenant->id); + // Custom cookie name config(['tenancy.identification.resolvers.' . RequestDataTenantResolver::class . '.cookie' => 'custom_tenant_id']); $this->withoutExceptionHandling()->withUnencryptedCookie('custom_tenant_id', $payload)->get('test')->assertSee($tenant->id); @@ -97,10 +103,7 @@ test('cookie identification works', function (string|null $tenantModelColumn) { expect(fn () => $this->withoutExceptionHandling()->withUnencryptedCookie('tenant', $payload)->get('test'))->toThrow(TenantCouldNotBeIdentifiedByRequestDataException::class); })->with([null, 'slug']); -// todo@tests encrypted cookie - test('an exception is thrown when no tenant data is provided in the request', function () { pest()->expectException(TenantCouldNotBeIdentifiedByRequestDataException::class); $this->withoutExceptionHandling()->get('test'); }); - From 8f8af34c320d1bf89792ee02634885b4c6b93a3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Tue, 5 Aug 2025 11:12:25 +0200 Subject: [PATCH 7/8] [4.x] Only revert initialized bootstrappers (#1385) * Only revert initialized bootstrappers (Tenancy::initializedBootstrappers) * Fix use of @property across the codebase --- .../SQLiteDatabaseManager.php | 4 +- src/Listeners/BootstrapTenancy.php | 4 + src/Listeners/RevertToCentralContext.php | 4 +- src/Tenancy.php | 15 +++- tests/InitializedBootstrappersTest.php | 87 +++++++++++++++++++ 5 files changed, 110 insertions(+), 4 deletions(-) create mode 100644 tests/InitializedBootstrappersTest.php diff --git a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php index d7fb8da2..64b96fc1 100644 --- a/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php +++ b/src/Database/TenantDatabaseManagers/SQLiteDatabaseManager.php @@ -46,7 +46,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager * tenant instances passed to $closeInMemoryConnectionUsing closures, * if you're setting that property as well. * - * @property Closure(PDO, string)|null + * @var Closure(PDO, string)|null */ public static Closure|null $persistInMemoryConnectionUsing = null; @@ -59,7 +59,7 @@ class SQLiteDatabaseManager implements TenantDatabaseManager * NOTE: The parameter provided to the closure is the Tenant * instance, not a PDO connection. * - * @property Closure(Tenant)|null + * @var Closure(Tenant)|null */ public static Closure|null $closeInMemoryConnectionUsing = null; diff --git a/src/Listeners/BootstrapTenancy.php b/src/Listeners/BootstrapTenancy.php index 50f38208..957949a4 100644 --- a/src/Listeners/BootstrapTenancy.php +++ b/src/Listeners/BootstrapTenancy.php @@ -20,6 +20,10 @@ class BootstrapTenancy $tenant = $event->tenancy->tenant; $bootstrapper->bootstrap($tenant); + + if (! in_array($bootstrapper::class, $event->tenancy->initializedBootstrappers)) { + $event->tenancy->initializedBootstrappers[] = $bootstrapper::class; + } } event(new TenancyBootstrapped($event->tenancy)); diff --git a/src/Listeners/RevertToCentralContext.php b/src/Listeners/RevertToCentralContext.php index 0a680532..312346ac 100644 --- a/src/Listeners/RevertToCentralContext.php +++ b/src/Listeners/RevertToCentralContext.php @@ -15,7 +15,9 @@ class RevertToCentralContext event(new RevertingToCentralContext($event->tenancy)); foreach (array_reverse($event->tenancy->getBootstrappers()) as $bootstrapper) { - $bootstrapper->revert(); + if (in_array($bootstrapper::class, $event->tenancy->initializedBootstrappers)) { + $bootstrapper->revert(); + } } event(new RevertedToCentralContext($event->tenancy)); diff --git a/src/Tenancy.php b/src/Tenancy.php index f96c0a51..66173cba 100644 --- a/src/Tenancy.php +++ b/src/Tenancy.php @@ -35,6 +35,20 @@ class Tenancy */ public static array $findWith = []; + /** + * A list of bootstrappers that have been initialized. + * + * This is used when reverting tenancy, mainly if an exception + * occurs during bootstrapping, to ensure we don't revert + * bootstrappers that haven't been properly initialized + * (bootstrapped for the first time) previously. + * + * @internal + * + * @var list> + */ + public array $initializedBootstrappers = []; + /** Initialize tenancy for the passed tenant. */ public function initialize(Tenant|int|string $tenant): void { @@ -192,7 +206,6 @@ class Tenancy /** * Run a callback for multiple tenants. - * More performant than running $tenant->run() one by one. * * @param array|array|\Traversable|string|int|null $tenants */ diff --git a/tests/InitializedBootstrappersTest.php b/tests/InitializedBootstrappersTest.php new file mode 100644 index 00000000..549c527f --- /dev/null +++ b/tests/InitializedBootstrappersTest.php @@ -0,0 +1,87 @@ + [ + Initialized_DummyBootstrapperFoo::class, + Initialized_DummyBootstrapperBar::class, + Initialized_DummyBootstrapperBaz::class, + ]]); + + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); + + // Only needs to be done in tests + app()->singleton(Initialized_DummyBootstrapperFoo::class); + app()->singleton(Initialized_DummyBootstrapperBar::class); + app()->singleton(Initialized_DummyBootstrapperBaz::class); + + $tenant = TenantModel::create(); + + try { + $tenant->run(fn() => null); + // Should throw an exception + expect(true)->toBe(false); + } catch (Exception $e) { + // NOT 'baz fail in revert' as was the behavior before + // the commit that added this test + expect($e->getMessage())->toBe('bar fail in bootstrap'); + } + + expect(tenancy()->initializedBootstrappers)->toBe([ + Initialized_DummyBootstrapperFoo::class, + ]); +}); + +class Initialized_DummyBootstrapperFoo implements TenancyBootstrapper +{ + public string $bootstrapped = 'uninitialized'; + + public function bootstrap(Tenant $tenant): void + { + $this->bootstrapped = 'bootstrapped'; + } + + public function revert(): void + { + $this->bootstrapped = 'reverted'; + } +} + +class Initialized_DummyBootstrapperBar implements TenancyBootstrapper +{ + public string $bootstrapped = 'uninitialized'; + + public function bootstrap(Tenant $tenant): void + { + throw new Exception('bar fail in bootstrap'); + } + + public function revert(): void + { + $this->bootstrapped = 'reverted'; + } +} + +class Initialized_DummyBootstrapperBaz implements TenancyBootstrapper +{ + public string $bootstrapped = 'uninitialized'; + + public function bootstrap(Tenant $tenant): void + { + $this->bootstrapped = 'bootstrapped'; + } + + public function revert(): void + { + throw new Exception('baz fail in revert'); + } +} From 3984d64cfaba82cdbef0c87061b28680d5537309 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Tue, 5 Aug 2025 12:33:16 +0200 Subject: [PATCH 8/8] Use globalCache in CachedTenantResolver (fix #1340) --- .../Contracts/CachedTenantResolver.php | 9 ++-- tests/CachedTenantResolverTest.php | 53 +++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/src/Resolvers/Contracts/CachedTenantResolver.php b/src/Resolvers/Contracts/CachedTenantResolver.php index cd982d2f..16966149 100644 --- a/src/Resolvers/Contracts/CachedTenantResolver.php +++ b/src/Resolvers/Contracts/CachedTenantResolver.php @@ -4,8 +4,8 @@ declare(strict_types=1); namespace Stancl\Tenancy\Resolvers\Contracts; -use Illuminate\Contracts\Cache\Factory; use Illuminate\Contracts\Cache\Repository; +use Illuminate\Contracts\Foundation\Application; use Illuminate\Database\Eloquent\Model; use Stancl\Tenancy\Contracts\Tenant; use Stancl\Tenancy\Contracts\TenantCouldNotBeIdentifiedException; @@ -13,12 +13,11 @@ use Stancl\Tenancy\Contracts\TenantResolver; abstract class CachedTenantResolver implements TenantResolver { - /** @var Repository */ - protected $cache; + protected Repository $cache; - public function __construct(Factory $cache) + public function __construct(Application $app) { - $this->cache = $cache->store(static::cacheStore()); + $this->cache = $app->make('globalCache')->store(static::cacheStore()); } /** diff --git a/tests/CachedTenantResolverTest.php b/tests/CachedTenantResolverTest.php index 322a1961..d33a85e5 100644 --- a/tests/CachedTenantResolverTest.php +++ b/tests/CachedTenantResolverTest.php @@ -5,14 +5,21 @@ declare(strict_types=1); use Illuminate\Database\Schema\Blueprint; use Illuminate\Routing\Route; use Illuminate\Support\Facades\DB; +use Illuminate\Support\Facades\Event; use Illuminate\Support\Facades\Redis; use Stancl\Tenancy\Tests\Etc\Tenant; use Stancl\Tenancy\Resolvers\PathTenantResolver; use Stancl\Tenancy\Resolvers\DomainTenantResolver; use Illuminate\Support\Facades\Route as RouteFacade; use Illuminate\Support\Facades\Schema; +use Stancl\Tenancy\Bootstrappers\CacheTagsBootstrapper; +use Stancl\Tenancy\Bootstrappers\CacheTenancyBootstrapper; use Stancl\Tenancy\Contracts\TenantCouldNotBeIdentifiedException; +use Stancl\Tenancy\Events\TenancyEnded; +use Stancl\Tenancy\Events\TenancyInitialized; use Stancl\Tenancy\Exceptions\TenantCouldNotBeIdentifiedOnDomainException; +use Stancl\Tenancy\Listeners\BootstrapTenancy; +use Stancl\Tenancy\Listeners\RevertToCentralContext; use Stancl\Tenancy\Middleware\InitializeTenancyByPath; use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; use function Stancl\Tenancy\Tests\pest; @@ -102,6 +109,52 @@ test('cache is invalidated when the tenant is updated', function (string $resolv 'tenant model column is name (custom)' => true, ]); +// Only testing update here - presumably if this works, deletes (and other things we test here) +// will work as well. The main unique thing about this test is that it makes the change from +// *within* the tenant context. +test('cache is invalidated when tenant is updated from within the tenant context', function (string $cacheBootstrapper) { + config(['tenancy.bootstrappers' => [$cacheBootstrapper]]); + Event::listen(TenancyInitialized::class, BootstrapTenancy::class); + Event::listen(TenancyEnded::class, RevertToCentralContext::class); + + $resolver = PathTenantResolver::class; + + $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn(true) => 'acme']); + + DB::enableQueryLog(); + + config(['tenancy.identification.resolvers.' . $resolver . '.cache' => true]); + + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); + expect(DB::getQueryLog())->not()->toBeEmpty(); + DB::flushQueryLog(); + + // Different cache context + tenancy()->initialize($tenant); + + // Tenant cache gets invalidated when the tenant is updated + $tenant->touch(); + + // If we use 'globalCache' in CachedTenantResolver's constructor (correct) this line has no effect - in either case the global cache is used. + // If we use 'cache' (regression) this line DOES have an effect. If we don't end tenancy, the tenant's cache is used, which + // wasn't populated before, so the test succeeds - a query is made. But if we do end tenancy, the central cache is used, + // which was populated BUT NOT INVALIDATED which makes the test fail. + // + // The actual resolver would only ever run in the central context, but this detail makes it easier to reason about how this test + // confirms the fix. + tenancy()->end(); + + DB::flushQueryLog(); + + expect($tenant->is(app($resolver)->resolve(getResolverArgument($resolver, $tenant, $tenantModelColumn))))->toBeTrue(); + + expect(DB::getQueryLog())->not()->toBeEmpty(); // Cache was invalidated, so the tenant was retrieved from the DB +})->with([ + // todo@samuel test this with the database cache bootstrapper too? + CacheTenancyBootstrapper::class, + CacheTagsBootstrapper::class, +]); + test('cache is invalidated when the tenant is deleted', function (string $resolver, bool $configureTenantModelColumn) { DB::statement('SET FOREIGN_KEY_CHECKS=0;'); // allow deleting the tenant $tenant = Tenant::create([$tenantModelColumn = tenantModelColumn($configureTenantModelColumn) => 'acme']);