[4.x] Add tenant parameter to defaults() in `UrlGeneratorBootstrapp…

…er` (#1311)

* Pass tenant parameter using defaults in UrlGeneratorBootstrapper, update tests accordingly (wip)

* Fix code style (php-cs-fixer)

* Update bootstrapper

* Improve TenancyUrlGenerator docblocks

* Improve bootstrapper/TenancyUrlGenerator tests (WIP)

* Improve route() name prefixing test

* Keep  `UrlGeneratorBootstrapper::$addTenantParameterToDefaults` disabled by default

* Add `$override` functionality  to TenancyUrlGenerator

* Test $override functionality, update new defaults in the bootstrapper tests

* Fix code style (php-cs-fixer)

* Update comments

* Update routeNameOverride()

* cleanup

---------

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: Samuel Štancl <[email protected]>

src/Bootstrappers/UrlGeneratorBootstrapper.php

@@ -10,6 +10,7 @@
 use Stancl\Tenancy\Contracts\TenancyBootstrapper;
 use Stancl\Tenancy\Contracts\Tenant;
 use Stancl\Tenancy\Overrides\TenancyUrlGenerator;
+use Stancl\Tenancy\Resolvers\PathTenantResolver;
 
 /**
  * Makes the app use TenancyUrlGenerator (instead of Illuminate\Routing\UrlGenerator) which:
@@ -19,10 +20,20 @@
  * Used with path and query string identification.
  *
  * @see TenancyUrlGenerator
- * @see \Stancl\Tenancy\Resolvers\PathTenantResolver
+ * @see PathTenantResolver
  */
 class UrlGeneratorBootstrapper implements TenancyBootstrapper
 {
+    /**
+     * Should the tenant route parameter get added to TenancyUrlGenerator::defaults().
+     *
+     * This is recommended when using path identification since defaults() generally has better support in integrations,
+     * namely Ziggy, compared to TenancyUrlGenerator::$passTenantParameterToRoutes.
+     *
+     * With query string identification, this has no effect since URL::defaults() only works for route paramaters.
+     */
+    public static bool $addTenantParameterToDefaults = true;
+
     public function __construct(
         protected Application $app,
         protected UrlGenerator $originalUrlGenerator,
@@ -32,7 +43,7 @@ public function bootstrap(Tenant $tenant): void
     {
         URL::clearResolvedInstances();
 
-        $this->useTenancyUrlGenerator();
+        $this->useTenancyUrlGenerator($tenant);
     }
 
     public function revert(): void
@@ -45,15 +56,24 @@ public function revert(): void
      *
      * @see \Illuminate\Routing\RoutingServiceProvider registerUrlGenerator()
      */
-    protected function useTenancyUrlGenerator(): void
+    protected function useTenancyUrlGenerator(Tenant $tenant): void
     {
         $newGenerator = new TenancyUrlGenerator(
             $this->app['router']->getRoutes(),
             $this->originalUrlGenerator->getRequest(),
             $this->app['config']->get('app.asset_url'),
         );
 
-        $newGenerator->defaults($this->originalUrlGenerator->getDefaultParameters());
+        $defaultParameters = $this->originalUrlGenerator->getDefaultParameters();
+
+        if (static::$addTenantParameterToDefaults) {
+            $defaultParameters = array_merge(
+                $defaultParameters,
+                [PathTenantResolver::tenantParameterName() => $tenant->getTenantKey()]
+            );
+        }
+
+        $newGenerator->defaults($defaultParameters);
 
         $newGenerator->setSessionResolver(function () {
             return $this->app['session'] ?? null;

src/Overrides/TenancyUrlGenerator.php

@@ -13,39 +13,77 @@
 /**
  * This class is used in place of the default UrlGenerator when UrlGeneratorBootstrapper is enabled.
  *
- * TenancyUrlGenerator does two extra things:
- * 1. Autofill the {tenant} parameter in the tenant context with the current tenant if $passTenantParameterToRoutes is enabled (enabled by default)
- * 2. Prepend the route name with `tenant.` (or the configured prefix) if $prefixRouteNames is enabled (disabled by default)
+ * TenancyUrlGenerator does a few extra things:
+ *   - Autofills the tenant parameter in the tenant context with the current tenant.
+ *     This is done either by:
+ *       - URL::defaults() -- if UrlGeneratorBootstrapper::$addTenantParameterToDefaults is enabled.
+ *         This generally has the best support since tools like e.g. Ziggy read defaults().
+ *       - 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.
  *
- * Both of these can be skipped by passing the $bypassParameter (`['central' => true]` by default)
+ *   - Prepends route names passed to route() and URL::temporarySignedRoute()
+ *     with `tenant.` (or the configured prefix) 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).
  */
 class TenancyUrlGenerator extends UrlGenerator
 {
     /**
-     * Parameter which bypasses the behavior modification of route() and temporarySignedRoute().
+     * Parameter which works as a flag for bypassing the behavior modification of route() and temporarySignedRoute().
+     *
+     * For example, in tenant context:
+     * Route::get('/', ...)->name('home');
+     * // query string identification
+     * Route::get('/tenant', ...)->middleware(InitializeTenancyByRequestData::class)->name('tenant.home');
+     * - route('home') => app.test/tenant?tenant=tenantKey
+     * - route('home', [$bypassParameter => true]) => app.test/
+     * - route('tenant.home', [$bypassParameter => true]) => app.test/tenant -- no query string added
      *
-     * E.g. route('tenant') => app.test/{tenant}/tenant (or app.test/tenant?tenant=tenantKey if the route doesn't accept the tenant parameter)
-     * route('tenant', [$bypassParameter => true]) => app.test/tenant.
+     * 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
      */
     public static string $bypassParameter = 'central';
 
     /**
-     * Determine if the route names passed to `route()` or `temporarySignedRoute()`
-     * should get prefixed with the tenant route name prefix.
+     * Should route names passed to route() or temporarySignedRoute()
+     * get prefixed with the tenant route name prefix.
      *
-     * This is useful when using path identification with packages that generate URLs,
-     * like Jetstream, so that you don't have to manually prefix route names passed to each route() call.
+     * This is useful when using e.g. path identification with third-party packages
+     * where you don't have control over all route() calls or don't want to change
+     * too many files. Often this will be when using route cloning.
      */
     public static bool $prefixRouteNames = false;
 
     /**
-     * Determine if the tenant parameter should get passed
-     * to the links generated by `route()` or `temporarySignedRoute()` whenever available
-     * (enabled by default – works with both path and query string identification).
+     * Should the tenant parameter be passed to route() or temporarySignedRoute() calls.
+     *
+     * This is useful with path or query parameter identification. The former can be handled
+     * more elegantly using UrlGeneratorBootstrapper::$addTenantParameterToDefaults.
      *
-     * With path identification, you can disable this and use URL::defaults() instead (as an alternative solution).
+     * @see UrlGeneratorBootstrapper
      */
-    public static bool $passTenantParameterToRoutes = true;
+    public static bool $passTenantParameterToRoutes = false;
+
+    /**
+     * Route name overrides.
+     *
+     * Note: This behavior can be bypassed using $bypassParameter just like
+     * $prefixRouteNames and $passTenantParameterToRoutes.
+     *
+     * Example from a Jetstream integration:
+     * [
+     *     'profile.show' => 'tenant.profile.show',
+     *     'two-factor.login' => 'tenant.two-factor.login',
+     * ]
+     *
+     * In the tenant context:
+     * - `route('profile.show')` will return a URL as if you called `route('tenant.profile.show')`.
+     * - `route('profile.show', ['central' => true])` will return a URL as if you called `route('profile.show')`.
+     */
+    public static array $overrides = [];
 
     /**
      * Override the route() method so that the route name gets prefixed
@@ -99,7 +137,7 @@ protected function routeBehaviorModificationBypassed(mixed $parameters): bool
     protected function prepareRouteInputs(string $name, array $parameters): array
     {
         if (! $this->routeBehaviorModificationBypassed($parameters)) {
-            $name = $this->prefixRouteName($name);
+            $name = $this->routeNameOverride($name) ?? $this->prefixRouteName($name);
             $parameters = $this->addTenantParameter($parameters);
         }
 
@@ -124,10 +162,15 @@ protected function prefixRouteName(string $name): string
     }
 
     /**
-     * If `tenant()` isn't null, add tenant paramter to the passed parameters.
+     * If `tenant()` isn't null, add the tenant parameter to the passed parameters.
      */
     protected function addTenantParameter(array $parameters): array
     {
         return tenant() && static::$passTenantParameterToRoutes ? array_merge($parameters, [PathTenantResolver::tenantParameterName() => tenant()->getTenantKey()]) : $parameters;
     }
+
+    protected function routeNameOverride(string $name): string|null
+    {
+        return static::$overrides[$name] ?? null;
+    }
 }

tests/Bootstrappers/UrlGeneratorBootstrapperTest.php

@@ -19,12 +19,14 @@
     Event::listen(TenancyInitialized::class, BootstrapTenancy::class);
     Event::listen(TenancyEnded::class, RevertToCentralContext::class);
     TenancyUrlGenerator::$prefixRouteNames = false;
-    TenancyUrlGenerator::$passTenantParameterToRoutes = true;
+    TenancyUrlGenerator::$passTenantParameterToRoutes = false;
+    UrlGeneratorBootstrapper::$addTenantParameterToDefaults = false;
 });
 
 afterEach(function () {
     TenancyUrlGenerator::$prefixRouteNames = false;
-    TenancyUrlGenerator::$passTenantParameterToRoutes = true;
+    TenancyUrlGenerator::$passTenantParameterToRoutes = false;
+    UrlGeneratorBootstrapper::$addTenantParameterToDefaults = false;
 });
 
 test('url generator bootstrapper swaps the url generator instance correctly', function() {
@@ -41,36 +43,28 @@
         ->not()->toBeInstanceOf(TenancyUrlGenerator::class);
 });
 
-test('url generator bootstrapper can prefix route names passed to the route helper', function() {
+test('tenancy url generator can prefix route names passed to the route helper', function() {
     Route::get('/central/home', fn () => route('home'))->name('home');
     // Tenant route name prefix is 'tenant.' by default
-    Route::get('/{tenant}/home', fn () => route('tenant.home'))->name('tenant.home')->middleware(['tenant', InitializeTenancyByPath::class]);
+    Route::get('/tenant/home', fn () => route('tenant.home'))->name('tenant.home');
 
     $tenant = Tenant::create();
-    $tenantKey = $tenant->getTenantKey();
     $centralRouteUrl = route('home');
-    $tenantRouteUrl = route('tenant.home', ['tenant' => $tenantKey]);
-    TenancyUrlGenerator::$bypassParameter = 'bypassParameter';
+    $tenantRouteUrl = route('tenant.home');
 
     config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);
 
     tenancy()->initialize($tenant);
 
-    // Route names don't get prefixed when TenancyUrlGenerator::$prefixRouteNames is false
-    expect(route('home'))->not()->toBe($centralRouteUrl);
-    // When TenancyUrlGenerator::$passTenantParameterToRoutes is true (default)
-    // The route helper receives the tenant parameter
-    // So in order to generate central URL, we have to pass the bypass parameter
-    expect(route('home', ['bypassParameter' => true]))->toBe($centralRouteUrl);
-
+    // Route names don't get prefixed when TenancyUrlGenerator::$prefixRouteNames is false (default)
+    expect(route('home'))->toBe($centralRouteUrl);
 
+    // When $prefixRouteNames is true, the route name passed to the route() helper ('home') gets prefixed with 'tenant.' automatically.
     TenancyUrlGenerator::$prefixRouteNames = true;
-    // The $prefixRouteNames property is true
-    // The route name passed to the route() helper ('home') gets prefixed prefixed with 'tenant.' automatically
+
     expect(route('home'))->toBe($tenantRouteUrl);
 
-    // The 'tenant.home' route name doesn't get prefixed because it is already prefixed with 'tenant.'
-    // Also, the route receives the tenant parameter automatically
+    // The 'tenant.home' route name doesn't get prefixed -- it is already prefixed with 'tenant.'
     expect(route('tenant.home'))->toBe($tenantRouteUrl);
 
     // Ending tenancy reverts route() behavior changes
@@ -79,6 +73,76 @@
     expect(route('home'))->toBe($centralRouteUrl);
 });
 
+test('the route helper can receive the tenant parameter automatically', function (
+    string $identification,
+    bool $addTenantParameterToDefaults,
+    bool $passTenantParameterToRoutes,
+) {
+    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);
+
+    $appUrl = config('app.url');
+
+    UrlGeneratorBootstrapper::$addTenantParameterToDefaults = $addTenantParameterToDefaults;
+
+    // When the tenant parameter isn't added to defaults, the tenant parameter has to be passed "manually"
+    // by setting $passTenantParameterToRoutes to true. This is only preferable with query string identification.
+    // With path identification, this ultimately doesn't have any effect
+    // if UrlGeneratorBootstrapper::$addTenantParameterToDefaults is true,
+    // but TenancyUrlGenerator::$passTenantParameterToRoutes can still be used instead.
+    TenancyUrlGenerator::$passTenantParameterToRoutes = $passTenantParameterToRoutes;
+
+    $tenant = Tenant::create();
+    $tenantKey = $tenant->getTenantKey();
+
+    Route::get('/central/home', fn () => route('home'))->name('home');
+
+    $tenantRoute = $identification === InitializeTenancyByPath::class ? "/{tenant}/home" : "/tenant/home";
+
+    Route::get($tenantRoute, fn () => route('tenant.home'))
+        ->name('tenant.home')
+        ->middleware(['tenant', $identification]);
+
+    tenancy()->initialize($tenant);
+
+    $expectedUrl = match (true) {
+        $identification === InitializeTenancyByRequestData::class && $passTenantParameterToRoutes => "{$appUrl}/tenant/home?tenant={$tenantKey}",
+        $identification === InitializeTenancyByRequestData::class => "{$appUrl}/tenant/home", // $passTenantParameterToRoutes is false
+        $identification === InitializeTenancyByPath::class && ($addTenantParameterToDefaults || $passTenantParameterToRoutes) => "{$appUrl}/{$tenantKey}/home",
+        $identification === InitializeTenancyByPath::class => null, // Should throw an exception -- route() doesn't receive the tenant parameter in this case
+    };
+
+    if ($expectedUrl === null) {
+        expect(fn () => route('tenant.home'))->toThrow(UrlGenerationException::class, 'Missing parameter: tenant');
+    } else {
+        expect(route('tenant.home'))->toBe($expectedUrl);
+    }
+})->with([InitializeTenancyByPath::class, InitializeTenancyByRequestData::class])
+    ->with([true, false]) // UrlGeneratorBootstrapper::$addTenantParameterToDefaults
+    ->with([true, false]); // TenancyUrlGenerator::$passTenantParameterToRoutes
+
+test('url generator can override specific route names', function() {
+    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);
+
+    Route::get('/foo', fn () => 'foo')->name('foo');
+    Route::get('/bar', fn () => 'bar')->name('bar');
+    Route::get('/baz', fn () => 'baz')->name('baz'); // Not overridden
+
+    TenancyUrlGenerator::$overrides = ['foo' => 'bar'];
+
+    expect(route('foo'))->toBe(url('/foo'));
+    expect(route('bar'))->toBe(url('/bar'));
+    expect(route('baz'))->toBe(url('/baz'));
+
+    tenancy()->initialize(Tenant::create());
+
+    expect(route('foo'))->toBe(url('/bar'));
+    expect(route('bar'))->toBe(url('/bar')); // not overridden
+    expect(route('baz'))->toBe(url('/baz')); // not overridden
+
+    // Bypass the override
+    expect(route('foo', ['central' => true]))->toBe(url('/foo'));
+});
+
 test('both the name prefixing and the tenant parameter logic gets skipped when bypass parameter is used', function () {
     $tenantParameterName = PathTenantResolver::tenantParameterName();
 
@@ -105,54 +169,8 @@
         ->not()->toContain('bypassParameter');
 
     // When the bypass parameter is false, the generated route URL points to the prefixed route ('tenant.home')
-    expect(route('home', ['bypassParameter' => false]))->toBe($tenantRouteUrl)
+    // The tenant parameter is not passed automatically since both
+    // UrlGeneratorBootstrapper::$addTenantParameterToDefaults and TenancyUrlGenerator::$passTenantParameterToRoutes are false by default
+    expect(route('home', ['bypassParameter' => false, 'tenant' => $tenant->getTenantKey()]))->toBe($tenantRouteUrl)
         ->not()->toContain('bypassParameter');
 });
-
-test('url generator bootstrapper can make route helper generate links with the tenant parameter', function() {
-    Route::get('/query_string', fn () => route('query_string'))->name('query_string')->middleware(['universal', InitializeTenancyByRequestData::class]);
-    Route::get('/path', fn () => route('path'))->name('path');
-    Route::get('/{tenant}/path', fn () => route('tenant.path'))->name('tenant.path')->middleware([InitializeTenancyByPath::class]);
-
-    $tenant = Tenant::create();
-    $tenantKey = $tenant->getTenantKey();
-    $queryStringCentralUrl = route('query_string');
-    $queryStringTenantUrl = route('query_string', ['tenant' => $tenantKey]);
-    $pathCentralUrl = route('path');
-    $pathTenantUrl = route('tenant.path', ['tenant' => $tenantKey]);
-
-    // Makes the route helper receive the tenant parameter whenever available
-    // Unless the bypass parameter is true
-    TenancyUrlGenerator::$passTenantParameterToRoutes = true;
-
-    TenancyUrlGenerator::$bypassParameter = 'bypassParameter';
-
-    config(['tenancy.bootstrappers' => [UrlGeneratorBootstrapper::class]]);
-
-    expect(route('path'))->toBe($pathCentralUrl);
-    // Tenant parameter required, but not passed since tenancy wasn't initialized
-    expect(fn () => route('tenant.path'))->toThrow(UrlGenerationException::class);
-
-    tenancy()->initialize($tenant);
-
-    // Tenant parameter is passed automatically
-    expect(route('path'))->not()->toBe($pathCentralUrl); // Parameter added as query string – bypassParameter needed
-    expect(route('path', ['bypassParameter' => true]))->toBe($pathCentralUrl);
-    expect(route('tenant.path'))->toBe($pathTenantUrl);
-
-    expect(route('query_string'))->toBe($queryStringTenantUrl)->toContain('tenant=');
-    expect(route('query_string', ['bypassParameter' => 'true']))->toBe($queryStringCentralUrl)->not()->toContain('tenant=');
-
-    tenancy()->end();
-
-    expect(route('query_string'))->toBe($queryStringCentralUrl);
-
-    // Tenant parameter required, but shouldn't be passed since tenancy isn't initialized
-    expect(fn () => route('tenant.path'))->toThrow(UrlGenerationException::class);
-
-    // Route-level identification
-    pest()->get("http://localhost/query_string")->assertSee($queryStringCentralUrl);
-    pest()->get("http://localhost/query_string?tenant=$tenantKey")->assertSee($queryStringTenantUrl);
-    pest()->get("http://localhost/path")->assertSee($pathCentralUrl);
-    pest()->get("http://localhost/$tenantKey/path")->assertSee($pathTenantUrl);
-});