From 947894fa1d40b7f651f55d45bc56746eaacc60d2 Mon Sep 17 00:00:00 2001 From: Hayatunnabi Nabil Date: Sat, 8 Nov 2025 05:52:08 +0600 Subject: [PATCH 01/10] [4.x] Fix dropRLSPolicies() (#1413) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `?` parameters are not supported in these statements, so we have to use string interpolation like in other related code. --------- Co-authored-by: Samuel Štancl --- src/Concerns/ManagesRLSPolicies.php | 2 +- tests/RLS/PolicyTest.php | 17 +++++++++++++++++ 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/Concerns/ManagesRLSPolicies.php b/src/Concerns/ManagesRLSPolicies.php index 6b804fb7..f6329d0e 100644 --- a/src/Concerns/ManagesRLSPolicies.php +++ b/src/Concerns/ManagesRLSPolicies.php @@ -26,7 +26,7 @@ trait ManagesRLSPolicies $policies = static::getRLSPolicies($table); foreach ($policies as $policy) { - DB::statement('DROP POLICY ? ON ?', [$policy, $table]); + DB::statement("DROP POLICY {$policy} ON {$table}"); } return count($policies); diff --git a/tests/RLS/PolicyTest.php b/tests/RLS/PolicyTest.php index ee9bf5cc..b790343e 100644 --- a/tests/RLS/PolicyTest.php +++ b/tests/RLS/PolicyTest.php @@ -17,6 +17,7 @@ use Stancl\Tenancy\Commands\CreateUserWithRLSPolicies; use Stancl\Tenancy\RLS\PolicyManagers\TableRLSManager; use Stancl\Tenancy\RLS\PolicyManagers\TraitRLSManager; use Stancl\Tenancy\Bootstrappers\PostgresRLSBootstrapper; +use Stancl\Tenancy\Tenancy; use function Stancl\Tenancy\Tests\pest; beforeEach(function () { @@ -189,6 +190,22 @@ test('rls command recreates policies if the force option is passed', function (s TraitRLSManager::class, ]); +test('dropRLSPolicies only drops RLS policies', function () { + DB::statement('CREATE POLICY "comments_dummy_rls_policy" ON comments USING (true)'); + DB::statement('CREATE POLICY "comments_foo_policy" ON comments USING (true)'); // non-RLS policy + + $policyCount = fn () => count(DB::select("SELECT policyname FROM pg_policies WHERE tablename = 'comments'")); + + expect($policyCount())->toBe(2); + + $removed = Tenancy::dropRLSPolicies('comments'); + + expect($removed)->toBe(1); + + // Only the non-RLS policy remains + expect($policyCount())->toBe(1); +}); + test('queries will stop working when the tenant session variable is not set', function(string $manager, bool $forceRls) { CreateUserWithRLSPolicies::$forceRls = $forceRls; From 69bf76842496a9b29b48b04dacae38c2e34f1f83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 8 Nov 2025 01:07:53 +0100 Subject: [PATCH 02/10] Cloning: remove route context middleware flags during cloning Previously, if a universal route was cloned without a cloneRoutesWithMiddleware(['universal']) call, i.e. it had both 'clone' and 'universal' flags, with only the former triggering cloning, the 'universal' flag would be included in the middleware of the cloned route. Now, we make sure to remove all context flags -- central, tenant, universal -- in the first step of processing middleware, before adding just 'tenant'. --- src/Actions/CloneRoutesAsTenant.php | 6 +++--- tests/CloneActionTest.php | 21 +++++++++++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index f1cb1450..87afe1d7 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -39,7 +39,7 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * 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. * - * After cloning, only top-level middleware in $cloneRoutesWithMiddleware will be removed + * After cloning, only top-level middleware in $cloneRoutesWithMiddleware (as well as any route context flags) will be removed * from the new route (so by default, 'clone' will be omitted from the new route's MW). * Middleware groups are preserved as-is, even if they contain cloning middleware. * @@ -258,12 +258,12 @@ class CloneRoutesAsTenant return $newRoute; } - /** Removes top-level cloneRoutesWithMiddleware and adds 'tenant' middleware. */ + /** Removes top-level cloneRoutesWithMiddleware and context flags, adds 'tenant' middleware. */ protected function processMiddlewareForCloning(array $middleware): array { $processedMiddleware = array_filter( $middleware, - fn ($mw) => ! in_array($mw, $this->cloneRoutesWithMiddleware) + fn ($mw) => ! in_array($mw, $this->cloneRoutesWithMiddleware) && ! in_array($mw, ['central', 'tenant', 'universal']) ); $processedMiddleware[] = 'tenant'; diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 28a8ccd3..b50a1b2f 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -401,3 +401,24 @@ test('tenant parameter addition can be controlled by setting addTenantParameter' $this->withoutExceptionHandling()->get('http://central.localhost/foo')->assertSee('central'); } })->with([true, false]); + +test('existing context flags are removed during cloning', function () { + RouteFacade::get('/foo', fn () => true)->name('foo')->middleware(['clone', 'central']); + RouteFacade::get('/bar', fn () => true)->name('bar')->middleware(['clone', 'universal']); + + $cloneAction = app(CloneRoutesAsTenant::class); + + // Clone foo route + $cloneAction->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName()) + ->toContain('tenant.foo'); + expect(tenancy()->getRouteMiddleware(RouteFacade::getRoutes()->getByName('tenant.foo'))) + ->not()->toContain('central'); + + // Clone bar route + $cloneAction->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName()) + ->toContain('tenant.foo', 'tenant.bar'); + expect(tenancy()->getRouteMiddleware(RouteFacade::getRoutes()->getByName('tenant.foo'))) + ->not()->toContain('universal'); +}); From 97c5afd2cfe36e4f1e999ea10fd703d858303dd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 8 Nov 2025 18:39:28 +0100 Subject: [PATCH 03/10] Cloning: clarify case where neither paths nor domains differ In such a case, the cloned route will actually *override* the original route, rather than being unused as the original docblock claimed. Also adds a static make() function for convenience. --- src/Actions/CloneRoutesAsTenant.php | 7 ++++++- tests/CloneActionTest.php | 15 +++++++++++++++ 2 files changed, 21 insertions(+), 1 deletion(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index 87afe1d7..120ab0d0 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -71,7 +71,7 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; * // 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. + * // 2. If the original route has no domain, the cloned route will override the original route as they will directly conflict. * $cloneAction->addTenantParameter(false)->cloneRoutesWithMiddleware(['clone'])->cloneRoute('no-tenant-parameter')->handle(); * ``` * @@ -96,6 +96,11 @@ class CloneRoutesAsTenant protected Router $router, ) {} + public static function make(): static + { + return app(static::class); + } + /** Clone routes. This resets routesToClone() but not other config. */ public function handle(): void { diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index b50a1b2f..74625994 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -422,3 +422,18 @@ test('existing context flags are removed during cloning', function () { expect(tenancy()->getRouteMiddleware(RouteFacade::getRoutes()->getByName('tenant.foo'))) ->not()->toContain('universal'); }); + +test('cloning a route without a prefix or differing domains overrides the original route', function () { + RouteFacade::get('/foo', fn () => true)->name('foo')->middleware(['clone']); + + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('foo'); + + $cloneAction = CloneRoutesAsTenant::make(); + $cloneAction->cloneRoute('foo') + ->addTenantParameter(false) + ->tenantParameterBeforePrefix(false) + ->handle(); + + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.foo'); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->not()->toContain('foo'); +}); From 197513dd84285c1ce9abe07c0bc59ab6ccb4d597 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sat, 8 Nov 2025 18:39:28 +0100 Subject: [PATCH 04/10] Cloning: addTenantMiddleware() for specifying ID MW for cloned route Previously, tenant identification middleware was typically specified for the cloned route by "inheriting" it from the central route, which necessarily meant that the central route had to also be marked as universal so it could continue working in the central context -- despite presumably not being usable in the tenant context, thus being universal for no proper reason. In such cases, universal routes were used mainly as a mechanism for specifying the tenant identification middleware to use on the cloned tenant route. Given that recent refactors of the cloning feature have made it more customizable and a bit nicer to use "multiple times", i.e. run handle() with a few different configurations of the action, letting the developer specify the used tenant middleware using a method like this only makes sense. The feature also becomes more independently usable and not just a "hack for universal routes with path identification". --- src/Actions/CloneRoutesAsTenant.php | 19 ++++++++++++++++--- tests/CloneActionTest.php | 27 +++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index 120ab0d0..abe2cbcd 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -30,6 +30,8 @@ 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' middleware can be controlled using addTenantMiddleware(array). You can specify the identification + * middleware to be used on the cloned route using that method -- instead of using the approach that "inherits" it from a universal route. * * 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 @@ -91,6 +93,7 @@ class CloneRoutesAsTenant 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']; + protected array $addTenantMiddleware = ['tenant']; public function __construct( protected Router $router, @@ -148,6 +151,18 @@ class CloneRoutesAsTenant return $this; } + /** + * The tenant middleware to be added to the cloned route. + * + * If used with early identification, make sure to include 'tenant' in this array. + */ + public function addTenantMiddleware(array $middleware): static + { + $this->addTenantMiddleware = $middleware; + + 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 { @@ -271,9 +286,7 @@ class CloneRoutesAsTenant fn ($mw) => ! in_array($mw, $this->cloneRoutesWithMiddleware) && ! in_array($mw, ['central', 'tenant', 'universal']) ); - $processedMiddleware[] = 'tenant'; - - return array_unique($processedMiddleware); + return array_unique(array_merge($processedMiddleware, $this->addTenantMiddleware)); } /** Check if route already has tenant parameter or name prefix. */ diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 74625994..8fc66c56 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -437,3 +437,30 @@ test('cloning a route without a prefix or differing domains overrides the origin expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.foo'); expect(collect(RouteFacade::getRoutes()->get())->map->getName())->not()->toContain('foo'); }); + +test('addTenantMiddleware can be used to specify the tenant middleware for the cloned route', function () { + RouteFacade::get('/foo', fn () => true)->name('foo')->middleware(['clone']); + RouteFacade::get('/bar', fn () => true)->name('bar')->middleware(['clone']); + + $cloneAction = app(CloneRoutesAsTenant::class); + + $cloneAction->cloneRoute('foo')->addTenantMiddleware([InitializeTenancyByPath::class])->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.foo'); + $cloned = RouteFacade::getRoutes()->getByName('tenant.foo'); + expect($cloned->uri())->toBe('{tenant}/foo'); + expect($cloned->getName())->toBe('tenant.foo'); + expect(tenancy()->getRouteMiddleware($cloned))->toBe([InitializeTenancyByPath::class]); + + $cloneAction->cloneRoute('bar') + ->addTenantMiddleware([InitializeTenancyByDomain::class]) + ->domain('foo.localhost') + ->addTenantParameter(false) + ->tenantParameterBeforePrefix(false) + ->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.bar'); + $cloned = RouteFacade::getRoutes()->getByName('tenant.bar'); + expect($cloned->uri())->toBe('bar'); + expect($cloned->getName())->toBe('tenant.bar'); + expect($cloned->getDomain())->toBe('foo.localhost'); + expect(tenancy()->getRouteMiddleware($cloned))->toBe([InitializeTenancyByDomain::class]); +}); From 6ef4b91744d8745f90ddde3d511bb0c325530351 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Sun, 9 Nov 2025 01:27:29 +0100 Subject: [PATCH 05/10] Cloning: improve type annotations, add cloneRoutes() for convenience --- src/Actions/CloneRoutesAsTenant.php | 55 +++++++++++++++++++++++++---- tests/CloneActionTest.php | 11 ++++++ 2 files changed, 60 insertions(+), 6 deletions(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index abe2cbcd..6e988907 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/src/Actions/CloneRoutesAsTenant.php @@ -86,13 +86,27 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; */ class CloneRoutesAsTenant { + /** @var list */ protected array $routesToClone = []; + protected bool $addTenantParameter = true; protected bool $tenantParameterBeforePrefix = true; protected string|null $domain = null; - protected Closure|null $cloneUsing = null; // The callback should accept Route instance or the route name (string) + + /** + * The callback should accept a Route instance or the route name (string). + * + * @var ?Closure(Route|string): void + */ + protected Closure|null $cloneUsing = null; + + /** @var ?Closure(Route): bool */ protected Closure|null $shouldClone = null; + + /** @var list */ protected array $cloneRoutesWithMiddleware = ['clone']; + + /** @var list */ protected array $addTenantMiddleware = ['tenant']; public function __construct( @@ -110,9 +124,12 @@ class CloneRoutesAsTenant // If no routes were specified using cloneRoute(), get all routes // and for each, determine if it should be cloned if (! $this->routesToClone) { - $this->routesToClone = collect($this->router->getRoutes()->get()) + /** @var list */ + $routesToClone = collect($this->router->getRoutes()->get()) ->filter(fn (Route $route) => $this->shouldBeCloned($route)) ->all(); + + $this->routesToClone = $routesToClone; } foreach ($this->routesToClone as $route) { @@ -126,7 +143,9 @@ class CloneRoutesAsTenant if (is_string($route)) { $this->router->getRoutes()->refreshNameLookups(); - $route = $this->router->getRoutes()->getByName($route); + $routeName = $route; + $route = $this->router->getRoutes()->getByName($routeName); + assert(! is_null($route), "Route [{$routeName}] was meant to be cloned but does not exist."); } $this->createNewRoute($route); @@ -155,6 +174,8 @@ class CloneRoutesAsTenant * The tenant middleware to be added to the cloned route. * * If used with early identification, make sure to include 'tenant' in this array. + * + * @param list $middleware */ public function addTenantMiddleware(array $middleware): static { @@ -171,7 +192,11 @@ class CloneRoutesAsTenant return $this; } - /** Provide a custom callback for cloning routes, instead of the default behavior. */ + /** + * Provide a custom callback for cloning routes, instead of the default behavior. + * + * @param ?Closure(Route|string): void $cloneUsing + */ public function cloneUsing(Closure|null $cloneUsing): static { $this->cloneUsing = $cloneUsing; @@ -179,7 +204,11 @@ class CloneRoutesAsTenant return $this; } - /** Specify which middleware should serve as "flags" telling this action to clone those routes. */ + /** + * Specify which middleware should serve as "flags" telling this action to clone those routes. + * + * @param list $middleware + */ public function cloneRoutesWithMiddleware(array $middleware): static { $this->cloneRoutesWithMiddleware = $middleware; @@ -190,7 +219,9 @@ class CloneRoutesAsTenant /** * Provide a custom callback for determining whether a route should be cloned. * Overrides the default middleware-based detection. - * */ + * + * @param Closure(Route): bool $shouldClone + */ public function shouldClone(Closure|null $shouldClone): static { $this->shouldClone = $shouldClone; @@ -213,6 +244,18 @@ class CloneRoutesAsTenant return $this; } + /** + * Clone individual routes. + * + * @param list $routes + */ + public function cloneRoutes(array $routes): static + { + $this->routesToClone = array_merge($this->routesToClone, $routes); + + return $this; + } + protected function shouldBeCloned(Route $route): bool { // Don't clone routes that already have tenant parameter or prefix diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 8fc66c56..ab9c5e9b 100644 --- a/tests/CloneActionTest.php +++ b/tests/CloneActionTest.php @@ -464,3 +464,14 @@ test('addTenantMiddleware can be used to specify the tenant middleware for the c expect($cloned->getDomain())->toBe('foo.localhost'); expect(tenancy()->getRouteMiddleware($cloned))->toBe([InitializeTenancyByDomain::class]); }); + +test('cloneRoutes can be used to clone multiple routes', function () { + RouteFacade::get('/foo', fn () => true)->name('foo'); + $bar = RouteFacade::get('/bar', fn () => true)->name('bar'); + RouteFacade::get('/baz', fn () => true)->name('baz'); + + CloneRoutesAsTenant::make()->cloneRoutes(['foo', $bar])->handle(); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.foo'); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->toContain('tenant.bar'); + expect(collect(RouteFacade::getRoutes()->get())->map->getName())->not()->toContain('tenant.baz'); +}); From 2aca784c0b723606f55a4aeea6bf4b5f4cd6b0d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Mon, 10 Nov 2025 17:31:02 +0100 Subject: [PATCH 06/10] Cloning: remove comments in TSP stub in favor of referencing class docs --- assets/TenancyServiceProvider.stub.php | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index e0b69e6e..46f35515 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -242,24 +242,7 @@ class TenancyServiceProvider extends ServiceProvider /** @var CloneRoutesAsTenant $cloneRoutes */ $cloneRoutes = $this->app->make(CloneRoutesAsTenant::class); - // The cloning action has two modes: - // 1. Clone all routes that have the middleware present in the action's $cloneRoutesWithMiddleware property. - // You can customize the middleware that triggers cloning by using cloneRoutesWithMiddleware() on the action. - // - // By default, the middleware is ['clone'], but using $cloneRoutes->cloneRoutesWithMiddleware(['clone', 'universal'])->handle() - // will clone all routes that have either 'clone' or 'universal' middleware (mentioning 'universal' since that's a common use case). - // - // Also, you can use the shouldClone() method to provide a custom closure that determines if a route should be cloned. - // - // 2. Clone only the routes that were manually added to the action using cloneRoute(). - // - // Regardless of the mode, you can provide a custom closure for defining the cloned route, e.g.: - // $cloneRoutesAction->cloneUsing(function (Route $route) { - // RouteFacade::get('/cloned/' . $route->uri(), fn () => 'cloned route')->name('cloned.' . $route->getName()); - // })->handle(); - // This will make all cloned routes use the custom closure to define the cloned route instead of the default behavior. - // See Stancl\Tenancy\Actions\CloneRoutesAsTenant for more details. - + /** See CloneRoutesAsTenant for usage details. */ $cloneRoutes->handle(); } From 0cd0bc44b1c0bdb41d26b7e902755d2daaa4b795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Tue, 11 Nov 2025 02:06:03 +0100 Subject: [PATCH 07/10] config: ignore port in default central_domains value Recent Laravel installations often have http://localhost:8000 as APP_URL, so we make sure to strip any port suffix from the default central domain derived from APP_URL. --- assets/config.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/config.php b/assets/config.php index f15a843a..2a3a07e2 100644 --- a/assets/config.php +++ b/assets/config.php @@ -64,7 +64,7 @@ return [ * Only relevant if you're using the domain or subdomain identification middleware. */ 'central_domains' => [ - str(env('APP_URL'))->after('://')->before('/')->toString(), + str(env('APP_URL'))->after('://')->before('/')->before(':')->toString(), ], /** From 45cf7029af2ed4785ed779005fd710c3e74f9b64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Samuel=20=C5=A0tancl?= Date: Fri, 14 Nov 2025 10:58:35 +0100 Subject: [PATCH 08/10] globalUrl: useAssetOrigin() instead of setAssetRoot() This change was prompted by a phpstan failure after a recent update. While making this change, I noticed we don't need the macro anymore as useAssetOrigin() was added to the UrlGenerator earlier this year, simplifying our implementation. --- phpstan.neon | 4 ---- .../FilesystemTenancyBootstrapper.php | 14 +++----------- src/TenancyServiceProvider.php | 10 ++++++---- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/phpstan.neon b/phpstan.neon index 2c6e3d69..bb97e3a0 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -46,10 +46,6 @@ parameters: message: '#PHPDoc tag \@param has invalid value \(dynamic#' paths: - src/helpers.php - - - message: '#Illuminate\\Routing\\UrlGenerator#' - paths: - - src/Bootstrappers/FilesystemTenancyBootstrapper.php - '#Method Stancl\\Tenancy\\Tenancy::cachedResolvers\(\) should return array#' - '#Access to an undefined property Stancl\\Tenancy\\Middleware\\IdentificationMiddleware\:\:\$tenancy#' - '#Access to an undefined property Stancl\\Tenancy\\Middleware\\IdentificationMiddleware\:\:\$resolver#' diff --git a/src/Bootstrappers/FilesystemTenancyBootstrapper.php b/src/Bootstrappers/FilesystemTenancyBootstrapper.php index 2c2d9ec9..faa02de7 100644 --- a/src/Bootstrappers/FilesystemTenancyBootstrapper.php +++ b/src/Bootstrappers/FilesystemTenancyBootstrapper.php @@ -5,7 +5,6 @@ declare(strict_types=1); namespace Stancl\Tenancy\Bootstrappers; use Illuminate\Foundation\Application; -use Illuminate\Routing\UrlGenerator; use Illuminate\Session\FileSessionHandler; use Illuminate\Support\Facades\Storage; use Stancl\Tenancy\Contracts\TenancyBootstrapper; @@ -22,13 +21,6 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper ) { $this->originalAssetUrl = $this->app['config']['app.asset_url']; $this->originalStoragePath = $app->storagePath(); - - $this->app['url']->macro('setAssetRoot', function ($root) { - /** @var UrlGenerator $this */ - $this->assetRoot = $root; - - return $this; - }); } public function bootstrap(Tenant $tenant): void @@ -107,16 +99,16 @@ class FilesystemTenancyBootstrapper implements TenancyBootstrapper if ($suffix === false) { $this->app['config']['app.asset_url'] = $this->originalAssetUrl; - $this->app['url']->setAssetRoot($this->originalAssetUrl); + $this->app['url']->useAssetOrigin($this->originalAssetUrl); return; } if ($this->originalAssetUrl) { $this->app['config']['app.asset_url'] = $this->originalAssetUrl . "/$suffix"; - $this->app['url']->setAssetRoot($this->app['config']['app.asset_url']); + $this->app['url']->useAssetOrigin($this->app['config']['app.asset_url']); } else { - $this->app['url']->setAssetRoot($this->app['url']->route('stancl.tenancy.asset', ['path' => ''])); + $this->app['url']->useAssetOrigin($this->app['url']->route('stancl.tenancy.asset', ['path' => ''])); } } diff --git a/src/TenancyServiceProvider.php b/src/TenancyServiceProvider.php index 9b32f088..afd20fb6 100644 --- a/src/TenancyServiceProvider.php +++ b/src/TenancyServiceProvider.php @@ -6,6 +6,7 @@ namespace Stancl\Tenancy; use Closure; use Illuminate\Cache\CacheManager; +use Illuminate\Contracts\Container\Container; use Illuminate\Database\Console\Migrations\FreshCommand; use Illuminate\Routing\Events\RouteMatched; use Illuminate\Support\Facades\Event; @@ -157,12 +158,13 @@ class TenancyServiceProvider extends ServiceProvider $this->loadRoutesFrom(__DIR__ . '/../assets/routes.php'); } - $this->app->singleton('globalUrl', function ($app) { + $this->app->singleton('globalUrl', function (Container $app) { if ($app->bound(FilesystemTenancyBootstrapper::class)) { - $instance = clone $app['url']; - $instance->setAssetRoot($app[FilesystemTenancyBootstrapper::class]->originalAssetUrl); + /** @var \Illuminate\Routing\UrlGenerator */ + $instance = clone $app->make('url'); + $instance->useAssetOrigin($app->make(FilesystemTenancyBootstrapper::class)->originalAssetUrl); } else { - $instance = $app['url']; + $instance = $app->make('url'); } return $instance; From ff95c92134e46cd3de4320e90a6501a3444be49e Mon Sep 17 00:00:00 2001 From: lukinovec Date: Mon, 3 Nov 2025 17:33:12 +0100 Subject: [PATCH 09/10] Add SyncedResourceDeleted event and DeleteResourceMapping listener Also move pivot record deletion to that listener and improve tests --- assets/TenancyServiceProvider.stub.php | 3 + .../Events/SyncedResourceDeleted.php | 18 ++++++ .../Listeners/DeleteResourceMapping.php | 60 +++++++++++++++++++ .../Listeners/DeleteResourcesInTenants.php | 7 --- src/ResourceSyncing/ResourceSyncing.php | 11 ++-- src/ResourceSyncing/SyncMaster.php | 2 - src/ResourceSyncing/Syncable.php | 2 + tests/Etc/ResourceSyncing/CentralUser.php | 1 + ...05_11_000002_create_tenant_users_table.php | 3 - tests/ResourceSyncingTest.php | 43 ++++++++++++- 10 files changed, 133 insertions(+), 17 deletions(-) create mode 100644 src/ResourceSyncing/Events/SyncedResourceDeleted.php create mode 100644 src/ResourceSyncing/Listeners/DeleteResourceMapping.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 46f35515..2e7819a5 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -129,6 +129,9 @@ class TenancyServiceProvider extends ServiceProvider ResourceSyncing\Events\SyncedResourceSaved::class => [ ResourceSyncing\Listeners\UpdateOrCreateSyncedResource::class, ], + ResourceSyncing\Events\SyncedResourceDeleted::class => [ + ResourceSyncing\Listeners\DeleteResourceMapping::class, + ], ResourceSyncing\Events\SyncMasterDeleted::class => [ ResourceSyncing\Listeners\DeleteResourcesInTenants::class, ], diff --git a/src/ResourceSyncing/Events/SyncedResourceDeleted.php b/src/ResourceSyncing/Events/SyncedResourceDeleted.php new file mode 100644 index 00000000..941e1841 --- /dev/null +++ b/src/ResourceSyncing/Events/SyncedResourceDeleted.php @@ -0,0 +1,18 @@ +getCentralResource($event->model); + + if (! $centralResource) { + return; + } + + // Delete pivot records if the central resource doesn't use soft deletes + // or the central resource was deleted using forceDelete() + if ($event->forceDelete || ! in_array(SoftDeletes::class, class_uses_recursive($centralResource::class), true)) { + Pivot::withoutEvents(function () use ($centralResource, $event) { + // $event->tenant is null when the deleted resource is a SyncMaster - all mappings are deleted in that case + // When $event->tenant is not null (= a Syncable was deleted), only delete the mapping for that tenant + $centralResource->tenants()->detach($event->tenant); + }); + } + } + + public function getCentralResource(Syncable&Model $resource): SyncMaster|null + { + if ($resource instanceof SyncMaster) { + return $resource; + } + + $centralResourceClass = $resource->getCentralModelName(); + + /** @var (SyncMaster&Model)|null $centralResource */ + $centralResource = $centralResourceClass::firstWhere( + $resource->getGlobalIdentifierKeyName(), + $resource->getGlobalIdentifierKey() + ); + + return $centralResource; + } +} diff --git a/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php b/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php index 6876f476..7b071a27 100644 --- a/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php +++ b/src/ResourceSyncing/Listeners/DeleteResourcesInTenants.php @@ -4,7 +4,6 @@ declare(strict_types=1); namespace Stancl\Tenancy\ResourceSyncing\Listeners; -use Illuminate\Database\Eloquent\SoftDeletes; use Stancl\Tenancy\Listeners\QueueableListener; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterDeleted; @@ -21,12 +20,6 @@ class DeleteResourcesInTenants extends QueueableListener tenancy()->runForMultiple($centralResource->tenants()->cursor(), function () use ($centralResource, $forceDelete) { $this->deleteSyncedResource($centralResource, $forceDelete); - - // Delete pivot records if the central resource doesn't use soft deletes - // or the central resource was deleted using forceDelete() - if ($forceDelete || ! in_array(SoftDeletes::class, class_uses_recursive($centralResource::class), true)) { - $centralResource->tenants()->detach(tenant()); - } }); } } diff --git a/src/ResourceSyncing/ResourceSyncing.php b/src/ResourceSyncing/ResourceSyncing.php index f0d8cc12..824d909c 100644 --- a/src/ResourceSyncing/ResourceSyncing.php +++ b/src/ResourceSyncing/ResourceSyncing.php @@ -11,6 +11,7 @@ use Stancl\Tenancy\Contracts\UniqueIdentifierGenerator; use Stancl\Tenancy\Database\Contracts\TenantWithDatabase; use Stancl\Tenancy\ResourceSyncing\Events\CentralResourceAttachedToTenant; use Stancl\Tenancy\ResourceSyncing\Events\CentralResourceDetachedFromTenant; +use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceSaved; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncMasterRestored; @@ -25,8 +26,8 @@ trait ResourceSyncing } }); - static::deleting(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + static::deleted(function (Syncable&Model $model) { + if ($model->shouldSync()) { $model->triggerDeleteEvent(); } }); @@ -42,13 +43,13 @@ trait ResourceSyncing if (in_array(SoftDeletes::class, class_uses_recursive(static::class), true)) { static::forceDeleting(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + if ($model->shouldSync()) { $model->triggerDeleteEvent(true); } }); static::restoring(function (Syncable&Model $model) { - if ($model->shouldSync() && $model instanceof SyncMaster) { + if ($model->shouldSync()) { $model->triggerRestoredEvent(); } }); @@ -67,6 +68,8 @@ trait ResourceSyncing /** @var SyncMaster&Model $this */ event(new SyncMasterDeleted($this, $forceDelete)); } + + event(new SyncedResourceDeleted($this, tenant(), $forceDelete)); } public function triggerRestoredEvent(): void diff --git a/src/ResourceSyncing/SyncMaster.php b/src/ResourceSyncing/SyncMaster.php index 882aeb54..710bc097 100644 --- a/src/ResourceSyncing/SyncMaster.php +++ b/src/ResourceSyncing/SyncMaster.php @@ -25,7 +25,5 @@ interface SyncMaster extends Syncable public function triggerAttachEvent(TenantWithDatabase&Model $tenant): void; - public function triggerDeleteEvent(bool $forceDelete = false): void; - public function triggerRestoredEvent(): void; } diff --git a/src/ResourceSyncing/Syncable.php b/src/ResourceSyncing/Syncable.php index 3d5288f1..c38b02ea 100644 --- a/src/ResourceSyncing/Syncable.php +++ b/src/ResourceSyncing/Syncable.php @@ -16,6 +16,8 @@ interface Syncable public function triggerSyncEvent(): void; + public function triggerDeleteEvent(bool $forceDelete = false): void; + /** * Get the attributes used for creating the *other* model (i.e. tenant if this is the central one, and central if this is the tenant one). * diff --git a/tests/Etc/ResourceSyncing/CentralUser.php b/tests/Etc/ResourceSyncing/CentralUser.php index 1533bd21..ece09550 100644 --- a/tests/Etc/ResourceSyncing/CentralUser.php +++ b/tests/Etc/ResourceSyncing/CentralUser.php @@ -12,6 +12,7 @@ use Stancl\Tenancy\ResourceSyncing\SyncMaster; class CentralUser extends Model implements SyncMaster { use ResourceSyncing, CentralConnection; + protected $guarded = []; public $timestamps = false; diff --git a/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php b/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php index 0aafd23c..dcd667a6 100644 --- a/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php +++ b/tests/Etc/synced_resource_migrations/2020_05_11_000002_create_tenant_users_table.php @@ -16,9 +16,6 @@ class CreateTenantUsersTable extends Migration $table->string('global_user_id'); $table->unique(['tenant_id', 'global_user_id']); - - $table->foreign('tenant_id')->references('id')->on('tenants')->onUpdate('cascade')->onDelete('cascade'); - $table->foreign('global_user_id')->references('global_id')->on('users')->onUpdate('cascade')->onDelete('cascade'); }); } diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index 3250c37a..d41ed7ee 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -46,6 +46,10 @@ use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceSavedInForeignDatabase; use Illuminate\Database\Eloquent\Scope; use Illuminate\Database\QueryException; use function Stancl\Tenancy\Tests\pest; +use Illuminate\Support\Facades\Schema; +use Illuminate\Database\Schema\Blueprint; +use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; +use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteResourceMapping; beforeEach(function () { config(['tenancy.bootstrappers' => [ @@ -92,6 +96,7 @@ beforeEach(function () { CentralUser::$creationAttributes = $creationAttributes; Event::listen(SyncedResourceSaved::class, UpdateOrCreateSyncedResource::class); + Event::listen(SyncedResourceDeleted::class, DeleteResourceMapping::class); Event::listen(SyncMasterDeleted::class, DeleteResourcesInTenants::class); Event::listen(SyncMasterRestored::class, RestoreResourcesInTenants::class); Event::listen(CentralResourceAttachedToTenant::class, CreateTenantResource::class); @@ -890,9 +895,13 @@ test('deleting SyncMaster automatically deletes its Syncables', function (bool $ 'basic pivot' => false, ]); -test('tenant pivot records are deleted along with the tenants to which they belong to', function() { +test('tenant pivot records are deleted along with the tenants to which they belong to', function(bool $dbLevelOnCascadeDelete) { [$tenant] = createTenantsAndRunMigrations(); + if ($dbLevelOnCascadeDelete) { + addFkConstraintsToTenantUsersPivot(); + } + $syncMaster = CentralUser::create([ 'global_id' => 'cascade_user', 'name' => 'Central user', @@ -907,6 +916,30 @@ test('tenant pivot records are deleted along with the tenants to which they belo // Deleting tenant deletes its pivot records expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); +})->with([ + 'db level on cascade delete' => true, + 'event-based on cascade delete' => false, +]); + +test('pivot record is automatically deleted with the tenant resource', function() { + [$tenant] = createTenantsAndRunMigrations(); + + $syncMaster = CentralUser::create([ + 'global_id' => 'cascade_user', + 'name' => 'Central user', + 'email' => 'central@localhost', + 'password' => 'password', + 'role' => 'cascade_user', + ]); + + $syncMaster->tenants()->attach($tenant); + + $tenant->run(function () { + TenantUser::firstWhere('global_id', 'cascade_user')->delete(); + }); + + // Deleting tenant resource deletes its pivot record + expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); }); test('trashed resources are synced correctly', function () { @@ -1265,6 +1298,14 @@ test('global scopes on syncable models can break resource syncing', function () expect($tenant1->run(fn () => TenantUser::first()->name))->toBe('tenant2 user'); }); +function addFkConstraintsToTenantUsersPivot(): void +{ + Schema::table('tenant_users', function (Blueprint $table) { + $table->foreign('tenant_id')->references('id')->on('tenants')->onDelete('cascade'); + $table->foreign('global_user_id')->references('global_id')->on('users')->onDelete('cascade'); + }); +} + /** * Create two tenants and run migrations for those tenants. * From 53c7d4988c94e30aca821039a9e2c1a9ae876ad9 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 4 Nov 2025 16:52:39 +0100 Subject: [PATCH 10/10] Add DeleteAllTenantMappings listener --- assets/TenancyServiceProvider.stub.php | 2 + .../Listeners/DeleteAllTenantMappings.php | 53 ++++++++++++++++ tests/ResourceSyncingTest.php | 63 +++++++++++++++---- 3 files changed, 107 insertions(+), 11 deletions(-) create mode 100644 src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php diff --git a/assets/TenancyServiceProvider.stub.php b/assets/TenancyServiceProvider.stub.php index 2e7819a5..a1e681d7 100644 --- a/assets/TenancyServiceProvider.stub.php +++ b/assets/TenancyServiceProvider.stub.php @@ -81,6 +81,8 @@ class TenancyServiceProvider extends ServiceProvider ])->send(function (Events\TenantDeleted $event) { return $event->tenant; })->shouldBeQueued(false), + + // ResourceSyncing\Listeners\DeleteAllTenantMappings::class, ], Events\TenantMaintenanceModeEnabled::class => [], diff --git a/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php new file mode 100644 index 00000000..3a572dd9 --- /dev/null +++ b/src/ResourceSyncing/Listeners/DeleteAllTenantMappings.php @@ -0,0 +1,53 @@ +onDelete('cascade') + * on the constraint (otherwise, deleting a tenant will throw a foreign key constraint violation). + * That way, the cleanup will happen on the database level, and this listener will essentially + * just perform an extra 'where' query. + */ +class DeleteAllTenantMappings extends QueueableListener +{ + /** + * Pivot tables to clean up after a tenant is deleted, + * formatted like ['table_name' => 'tenant_key_column']. + * + * Since we cannot automatically detect which pivot tables + * you want to clean up, they have to be specified here. + * + * By default, resource syncing uses the tenant_resources table, and the records are associated + * to tenants by the tenant_id column (thus the ['tenant_resources' => 'tenant_id'] default). + * + * To customize this, set this property, e.g. in TenancyServiceProvider: + * DeleteAllTenantMappings::$pivotTables = [ + * 'tenant_users' => 'tenant_id', + * // You can also add more pivot tables here + * ]; + * + * Non-existent tables specified in the property will be skipped. + */ + public static array $pivotTables = ['tenant_resources' => 'tenant_id']; + + public function handle(TenantDeleted $event): void + { + foreach (static::$pivotTables as $table => $tenantKeyColumn) { + if (Schema::hasTable($table)) { + DB::table($table)->where($tenantKeyColumn, $event->tenant->getTenantKey())->delete(); + } + } + } +} diff --git a/tests/ResourceSyncingTest.php b/tests/ResourceSyncingTest.php index d41ed7ee..ac6b18fd 100644 --- a/tests/ResourceSyncingTest.php +++ b/tests/ResourceSyncingTest.php @@ -48,7 +48,9 @@ use Illuminate\Database\QueryException; use function Stancl\Tenancy\Tests\pest; use Illuminate\Support\Facades\Schema; use Illuminate\Database\Schema\Blueprint; +use Stancl\Tenancy\Events\TenantDeleted; use Stancl\Tenancy\ResourceSyncing\Events\SyncedResourceDeleted; +use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteAllTenantMappings; use Stancl\Tenancy\ResourceSyncing\Listeners\DeleteResourceMapping; beforeEach(function () { @@ -73,6 +75,7 @@ beforeEach(function () { CreateTenantResource::$shouldQueue = false; DeleteResourceInTenant::$shouldQueue = false; UpdateOrCreateSyncedResource::$scopeGetModelQuery = null; + DeleteAllTenantMappings::$pivotTables = ['tenant_resources' => 'tenant_id']; // Reset global scopes on models (should happen automatically but to make this more explicit) Model::clearBootedModels(); @@ -895,30 +898,51 @@ test('deleting SyncMaster automatically deletes its Syncables', function (bool $ 'basic pivot' => false, ]); -test('tenant pivot records are deleted along with the tenants to which they belong to', function(bool $dbLevelOnCascadeDelete) { +test('tenant pivot records are deleted along with the tenants to which they belong to', function (bool $dbLevelOnCascadeDelete, bool $morphPivot) { [$tenant] = createTenantsAndRunMigrations(); - if ($dbLevelOnCascadeDelete) { - addFkConstraintsToTenantUsersPivot(); + if ($morphPivot) { + config(['tenancy.models.tenant' => MorphTenant::class]); + $centralUserModel = BaseCentralUser::class; + + // The default pivot table, no need to configure the listener + $pivotTable = 'tenant_resources'; + } else { + $centralUserModel = CentralUser::class; + + // Custom pivot table + $pivotTable = 'tenant_users'; } - $syncMaster = CentralUser::create([ - 'global_id' => 'cascade_user', + if ($dbLevelOnCascadeDelete) { + addTenantIdConstraintToPivot($pivotTable); + } else { + // Event-based cleanup + Event::listen(TenantDeleted::class, DeleteAllTenantMappings::class); + + DeleteAllTenantMappings::$pivotTables = [$pivotTable => 'tenant_id']; + } + + $syncMaster = $centralUserModel::create([ + 'global_id' => 'user', 'name' => 'Central user', 'email' => 'central@localhost', 'password' => 'password', - 'role' => 'cascade_user', + 'role' => 'user', ]); $syncMaster->tenants()->attach($tenant); + // Pivot records should be deleted along with the tenant $tenant->delete(); - // Deleting tenant deletes its pivot records - expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); + expect(DB::select("SELECT * FROM {$pivotTable} WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); })->with([ 'db level on cascade delete' => true, 'event-based on cascade delete' => false, +])->with([ + 'polymorphic pivot' => true, + 'basic pivot' => false, ]); test('pivot record is automatically deleted with the tenant resource', function() { @@ -942,6 +966,24 @@ test('pivot record is automatically deleted with the tenant resource', function( expect(DB::select("SELECT * FROM tenant_users WHERE tenant_id = ?", [$tenant->getTenantKey()]))->toHaveCount(0); }); +test('DeleteAllTenantMappings handles incorrect configuration correctly', function() { + Event::listen(TenantDeleted::class, DeleteAllTenantMappings::class); + + [$tenant1, $tenant2] = createTenantsAndRunMigrations(); + + // Existing table, non-existent tenant key column + // The listener should throw an 'unknown column' exception + DeleteAllTenantMappings::$pivotTables = ['tenant_users' => 'non_existent_column']; + + // Should throw an exception when tenant is deleted + expect(fn() => $tenant1->delete())->toThrow(QueryException::class, "Unknown column 'non_existent_column' in 'where clause'"); + + // Non-existent table, the listener skips it, no exception to throw + DeleteAllTenantMappings::$pivotTables = ['nonexistent_pivot' => 'non_existent_column']; + + expect(fn() => $tenant2->delete())->not()->toThrow(Exception::class); +}); + test('trashed resources are synced correctly', function () { [$tenant1, $tenant2] = createTenantsAndRunMigrations(); migrateUsersTableForTenants(); @@ -1298,11 +1340,10 @@ test('global scopes on syncable models can break resource syncing', function () expect($tenant1->run(fn () => TenantUser::first()->name))->toBe('tenant2 user'); }); -function addFkConstraintsToTenantUsersPivot(): void +function addTenantIdConstraintToPivot(string $pivotTable): void { - Schema::table('tenant_users', function (Blueprint $table) { + Schema::table($pivotTable, function (Blueprint $table) { $table->foreign('tenant_id')->references('id')->on('tenants')->onDelete('cascade'); - $table->foreign('global_user_id')->references('global_id')->on('users')->onDelete('cascade'); }); }