From dfb0e1ad66c6610253032968bb793855747bb391 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Sat, 6 Jun 2026 23:52:37 +0200 Subject: [PATCH] TenancyUrlGenerator: override `toRoute()`, refactor (#1439) This PR adds the `toRoute()` method override to `TenancyUrlGenerator`. `toRoute()` now attempts to find a tenant equivalent of the passed route (= a route with the same name as the passed one, but with the tenant prefix) and generates URL for the tenant route. This behavior can be bypassed using the bypass parameter, like with the `route()` method override `TenancyUrlGenerator` had until now. The primary reason for adding this is that Livewire v4 no longer uses the `route()` helper (which automatically prefixes the passed route name because of the override in `TenancyUrlGenerator`) in `Livewire::getUpdateUri()`. Now, it uses `toRoute()` (https://github.com/livewire/livewire/commit/544aa3dfb8195f342ef0adbf179139841ad817b7#diff-e7609f8b0a60bde5a85067803d4e2f08f235c7cee9225a51ea67a85ff9a1d694R52), which didn't automatically swap the route for its 'tenant.'-prefixed equivalent in tenant context (until now). So for the Livewire integration to work with path identification, we need to override `toRoute()` as described. The `temporarySignedRoute()` override got removed because `temporarySignedRoute()` calls `route()` under the hood, there's no need to specifically override `temporarySignedRoute()`. > Note: Browsing old convos, it seems like the `temporarySignedRoute()` override was needed to make Livewire file uploads work with path identification, but it's not needed anymore. TenancyUrlGenerator had some changes since then, and now, I can't see the _exact_ reason why we needed the override (`temporarySignedRoute()` uses `route()` under the hood, so the only thing that should really matter is overriding `route()`/`toRoute()`). It was likely a leftover from some older implementation. The `route()` override got simplified. Since `route()` uses `toRoute()` under the hood, the `route()` override only has to have the prefixing logic. The rest is delegated to `toRoute()`. > Note: Even though we override `toRoute()` now which `route()` uses for generating the URLs, we still need to override `route()` for its `$this->routes->getByName($name)` call to receive the prefixed name. For example, if `route()` wasn't overridden, and we only had one route: `tenant.foo` (no central `foo` route), and we'd call `route('foo')`, we'd get an exception saying that route "foo" wasn't found, even if automatic route name prefixing was enabled and `toRoute()` was overridden. With the `route()` override, `route('foo')` acts as if we passed 'tenant.foo' instead of 'foo'. Comments in TenancyUrlGenerator and UrlGeneratorBootstrapper got updated to be more accurate. All _intentionally_ affected methods are listed in TenancyUrlGenerator's docblock. --------- Co-authored-by: Samuel Stancl --- .../UrlGeneratorBootstrapper.php | 2 +- src/Overrides/TenancyUrlGenerator.php | 71 +++++++++++-------- .../UrlGeneratorBootstrapperTest.php | 44 ++++++++++++ 3 files changed, 86 insertions(+), 31 deletions(-) diff --git a/src/Bootstrappers/UrlGeneratorBootstrapper.php b/src/Bootstrappers/UrlGeneratorBootstrapper.php index 3708d636..ba1a6d05 100644 --- a/src/Bootstrappers/UrlGeneratorBootstrapper.php +++ b/src/Bootstrappers/UrlGeneratorBootstrapper.php @@ -16,7 +16,7 @@ use Stancl\Tenancy\Resolvers\PathTenantResolver; /** * Makes the app use TenancyUrlGenerator (instead of Illuminate\Routing\UrlGenerator) which: * - prefixes route names with the tenant route name prefix (PathTenantResolver::tenantRouteNamePrefix() by default) - * - passes the tenant parameter to the link generated by route() and temporarySignedRoute() (PathTenantResolver::tenantParameterName() by default). + * - passes the tenant parameter (PathTenantResolver::tenantParameterName() by default) to the link generated by the affected methods like route() and temporarySignedRoute(). * * Used with path and query string identification. * diff --git a/src/Overrides/TenancyUrlGenerator.php b/src/Overrides/TenancyUrlGenerator.php index f7ed9a84..bcf1bd3f 100644 --- a/src/Overrides/TenancyUrlGenerator.php +++ b/src/Overrides/TenancyUrlGenerator.php @@ -22,16 +22,18 @@ use Stancl\Tenancy\Resolvers\RequestDataTenantResolver; * - Automatically passing ['tenant' => ...] to each route() call -- if TenancyUrlGenerator::$passTenantParameterToRoutes is enabled * This is a more universal solution since it supports both path identification and query parameter identification. * - * - Prepends route names passed to route() and URL::temporarySignedRoute() - * with `tenant.` (or the configured prefix) if $prefixRouteNames is enabled. + * - Prepends route names with the tenant route name prefix ('tenant.' by default, + * configurable at tenant_route_name_prefix under PathTenantResolver) if $prefixRouteNames is enabled. * This is primarily useful when using route cloning with path identification. * - * To bypass this behavior on any single route() call, pass the $bypassParameter as true (['central' => true] by default). + * Affected methods: route(), toRoute(), temporarySignedRoute(), signedRoute() (the last two via the route() override). + * + * To bypass this behavior on any single affected method call, pass the $bypassParameter as true (['central' => true] by default). */ class TenancyUrlGenerator extends UrlGenerator { /** - * Parameter which works as a flag for bypassing the behavior modification of route() and temporarySignedRoute(). + * Parameter which works as a flag for bypassing the behavior modification of the affected methods. * * For example, in tenant context: * Route::get('/', ...)->name('home'); @@ -44,12 +46,12 @@ class TenancyUrlGenerator extends UrlGenerator * Note: UrlGeneratorBootstrapper::$addTenantParameterToDefaults is not affected by this, though * it doesn't matter since it doesn't pass any extra parameters when not needed. * - * @see UrlGeneratorBootstrapper + * @see Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper */ public static string $bypassParameter = 'central'; /** - * Should route names passed to route() or temporarySignedRoute() + * Should route names passed to the affected methods * get prefixed with the tenant route name prefix. * * This is useful when using e.g. path identification with third-party packages @@ -59,12 +61,12 @@ class TenancyUrlGenerator extends UrlGenerator public static bool $prefixRouteNames = false; /** - * Should the tenant parameter be passed to route() or temporarySignedRoute() calls. + * Should the tenant parameter be passed to the affected methods. * * This is useful with path or query parameter identification. The former can be handled * more elegantly using UrlGeneratorBootstrapper::$addTenantParameterToDefaults. * - * @see UrlGeneratorBootstrapper + * @see Stancl\Tenancy\Bootstrappers\UrlGeneratorBootstrapper */ public static bool $passTenantParameterToRoutes = false; @@ -105,8 +107,18 @@ class TenancyUrlGenerator extends UrlGenerator public static bool $passQueryParameter = true; /** - * Override the route() method so that the route name gets prefixed - * and the tenant parameter gets added when in tenant context. + * Override the route() method to prefix the route name before $this->routes->getByName($name) is called + * in the parent route() call. + * + * This is necessary because $this->routes->getByName($name) is called to retrieve the route + * before passing it to toRoute(). If only the prefixed route (e.g. 'tenant.foo') is registered + * and the original ('foo') isn't, route() would throw a RouteNotFoundException. + * So route() has to be overridden to prefix the passed route name, even though toRoute() is overridden already. + * + * Only the name is taken from prepareRouteInputs() here — parameter handling + * (adding tenant parameter, removing bypass parameter) is delegated to toRoute(). + * + * Affects temporarySignedRoute() and signedRoute() as well since they call route() under the hood. */ public function route($name, $parameters = [], $absolute = true) { @@ -114,32 +126,28 @@ class TenancyUrlGenerator extends UrlGenerator throw new InvalidArgumentException('Attribute [name] expects a string backed enum.'); } - [$name, $parameters] = $this->prepareRouteInputs($name, Arr::wrap($parameters)); // @phpstan-ignore argument.type + [$name] = $this->prepareRouteInputs(Arr::wrap($parameters), $name); // @phpstan-ignore argument.type return parent::route($name, $parameters, $absolute); } /** - * Override the temporarySignedRoute() method so that the route name gets prefixed - * and the tenant parameter gets added when in tenant context. + * Override the toRoute() to prefix the route name + * and add the tenant parameter when in tenant context. + * + * Also affects route(). Even though route() is overridden separately, it delegates parameter handling to toRoute(). */ - public function temporarySignedRoute($name, $expiration, $parameters = [], $absolute = true) + public function toRoute($route, $parameters, $absolute) { - if ($name instanceof BackedEnum && ! is_string($name = $name->value)) { - throw new InvalidArgumentException('Attribute [name] expects a string backed enum.'); + $name = $route->getName(); + + [$prefixedName, $parameters] = $this->prepareRouteInputs(Arr::wrap($parameters), $name); + + if ($name && $prefixedName !== $name && $tenantRoute = $this->routes->getByName($prefixedName)) { + $route = $tenantRoute; } - $wrappedParameters = Arr::wrap($parameters); - - [$name, $parameters] = $this->prepareRouteInputs($name, $wrappedParameters); // @phpstan-ignore argument.type - - if (isset($wrappedParameters[static::$bypassParameter])) { - // If the bypass parameter was passed, we need to add it back to the parameters after prepareRouteInputs() removes it, - // so that the underlying route() call in parent::temporarySignedRoute() can bypass the behavior modification as well. - $parameters[static::$bypassParameter] = $wrappedParameters[static::$bypassParameter]; - } - - return parent::temporarySignedRoute($name, $expiration, $parameters, $absolute); + return parent::toRoute($route, $parameters, $absolute); } /** @@ -155,16 +163,19 @@ class TenancyUrlGenerator extends UrlGenerator } /** - * Takes a route name and an array of parameters to return the prefixed route name + * Takes an array of parameters and a route name to return the prefixed route name * and the route parameters with the tenant parameter added. * * To skip these modifications, pass the bypass parameter in route parameters. * Before returning the modified route inputs, the bypass parameter is removed from the parameters. */ - protected function prepareRouteInputs(string $name, array $parameters): array + protected function prepareRouteInputs(array $parameters, string|null $name): array { if (! $this->routeBehaviorModificationBypassed($parameters)) { - $name = $this->routeNameOverride($name) ?? $this->prefixRouteName($name); + if (! is_null($name)) { + $name = $this->routeNameOverride($name) ?? $this->prefixRouteName($name); + } + $parameters = $this->addTenantParameter($parameters); } diff --git a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php index f089207a..18664b06 100644 --- a/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php +++ b/tests/Bootstrappers/UrlGeneratorBootstrapperTest.php @@ -401,3 +401,47 @@ test('the bypass parameter works correctly with temporarySignedRoute', function( ->toContain('localhost/foo') ->not()->toContain('central='); // Bypass parameter gets removed from the generated URL }); + +test('toRoute can automatically prefix the passed route name', function () { + config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); + + Route::get('/central/home', fn () => 'central')->name('home'); + Route::get('/tenant/home', fn () => 'tenant')->name('tenant.home'); + + TenancyUrlGenerator::$prefixRouteNames = true; + + $tenant = Tenant::create(); + + tenancy()->initialize($tenant); + + $centralRoute = Route::getRoutes()->getByName('home'); + + // url()->toRoute() prefixes the name of the passed route ('home') with the tenant prefix + // and generates the URL for the tenant route (as if the 'tenant.home' route was passed to the method) + expect(url()->toRoute($centralRoute, [], true))->toBe('http://localhost/tenant/home'); + + // Passing the bypass parameter skips the name prefixing, so the method returns the central route URL + expect(url()->toRoute($centralRoute, ['central' => true], true))->toBe('http://localhost/central/home'); +}); + +test('toRoute modifies parameters even when the route has no name', function () { + config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]); + + TenancyUrlGenerator::$passTenantParameterToRoutes = true; + + $unnamedRoute = Route::get('/unnamed', fn () => 'unnamed'); + + $tenant = Tenant::create(); + + tenancy()->initialize($tenant); + + // The tenant parameter is added to the URL even for unnamed routes + expect(url()->toRoute($unnamedRoute, [], true)) + ->toBe("http://localhost/unnamed?tenant={$tenant->getTenantKey()}"); + + // The bypass parameter prevents passing the tenant parameter and is stripped from the URL + expect(url()->toRoute($unnamedRoute, ['central' => true], true)) + ->toBe("http://localhost/unnamed") + ->not()->toContain('tenant=') + ->not()->toContain('central='); +});