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]);