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 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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(); }