Clone routes action improvements (#4)

* Use "clone" instead of "re-register" in UniversalRouteTest

* Fix `$shouldCloneRoute` condition, improve tenant flagging logic

* Use clone instead of reregister in stub

assets/TenancyServiceProvider.stub.php

@@ -171,12 +171,12 @@ class TenancyServiceProvider extends ServiceProvider
         if (InitializeTenancyByPath::inGlobalStack()) {
             TenancyUrlGenerator::$prefixRouteNames = true;
 
-            /** @var CloneRoutesAsTenant $reregisterRoutes */
+            /** @var CloneRoutesAsTenant $cloneRoutes */
-            $reregisterRoutes = app(CloneRoutesAsTenant::class);
+            $cloneRoutes = app(CloneRoutesAsTenant::class);
 
             /**
-             * You can provide a closure for re-registering a specific route, e.g.:
+             * You can provide a closure for cloning a specific route, e.g.:
-             * $reregisterRoutes->reregisterUsing('welcome', function () {
+             * $cloneRoutes->cloneUsing('welcome', function () {
              *      Route::get('/tenant-welcome', fn () => 'Current tenant: ' . tenant()->getTenantKey())
              *          ->middleware(['universal', InitializeTenancyByPath::class])
              *          ->name('tenant.welcome');
@@ -185,17 +185,17 @@ class TenancyServiceProvider extends ServiceProvider
              * To make Livewire v2 (2.12.2+) work with kernel path identification,
              * use this closure to override the livewire.message-localized route:
              *
-             * $reregisterRoutes->reregisterUsing('livewire.message-localized', function (Route $route) {
+             * $cloneRoutes->cloneUsing('livewire.message-localized', function (Route $route) {
              *     $route->setUri(str($route->uri())->replaceFirst('locale', $tenantParameter = PathTenantResolver::tenantParameterName()));
              *     $route->parameterNames[0] = $tenantParameter;
              *     $route->middleware('tenant');
              * });
              *
-             * To see the default behavior of re-registering the universal routes, check out the reregisterRoute() method in ReregisterRoutesAsTenant.
+             * To see the default behavior of re-registering the universal routes, check out the cloneRoute() method in CloneRoutesAsTenant.
              * @see CloneRoutesAsTenant
              */
 
-            $reregisterRoutes->handle();
+            $cloneRoutes->handle();
         }
     }
 

src/Actions/CloneRoutesAsTenant.php

@@ -105,24 +105,27 @@ class CloneRoutesAsTenant
         }
 
         $routesAreUniversalByDefault = $this->config->get('tenancy.default_route_mode') === RouteMode::UNIVERSAL;
-        $routeHasIdentificationMiddleware = tenancy()->routeHasIdentificationMiddleware($route);
         $routeHasPathIdentification = PathIdentificationManager::pathIdentificationOnRoute($route);
         $pathIdentificationMiddlewareInGlobalStack = PathIdentificationManager::pathIdentificationInGlobalStack();
+        $routeHasNonPathIdentificationMiddleware = tenancy()->routeHasIdentificationMiddleware($route) && ! $routeHasPathIdentification;
 
         // Determine if the passed route should get cloned
         // The route should be cloned if it has path identification middleware
         // Or if the route doesn't have identification middleware and path identification middleware
         // Is not used globally or the routes are universal by default
-        $shouldCloneRoute = $routeHasPathIdentification ||
+        $shouldCloneRoute = ! $routeHasNonPathIdentificationMiddleware &&
-            (! $routeHasIdentificationMiddleware && ($routesAreUniversalByDefault || $pathIdentificationMiddlewareInGlobalStack));
+            ($routesAreUniversalByDefault || $routeHasPathIdentification || $pathIdentificationMiddlewareInGlobalStack);
 
         if ($shouldCloneRoute) {
             $newRoute = $this->createNewRoute($route);
-            $routeIsUniversal = tenancy()->routeHasMiddleware($newRoute, 'universal');
+            $routeConsideredUniversal = tenancy()->routeHasMiddleware($newRoute, 'universal') || $routesAreUniversalByDefault;
 
-            // Add the 'tenant' flag to the new route if the route is universal
+            if ($routeHasPathIdentification && ! $routeConsideredUniversal && ! tenancy()->routeHasMiddleware($newRoute, 'tenant')) {
-            // Or if it isn't universal and it doesn't have the identification middlware (= it isn't "flagged" as tenant by having the MW)
+                // Skip adding tenant flag
-            if ((! $routeHasPathIdentification && ! $routeIsUniversal) || $routeIsUniversal || $routesAreUniversalByDefault) {
+                // Non-universal routes with identification middleware are already considered tenant
+                // Also skip adding the flag if the route already has the flag
+                // So that the route only has the 'tenant' middleware group once
+            } else {
                 $newRoute->middleware('tenant');
             }
 

tests/UniversalRouteTest.php

@@ -321,7 +321,7 @@ test('ReregisterRoutesAsTenant registers prefixed duplicates of universal routes
     config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_parameter_name' => $tenantParameterName = 'team']);
     config(['tenancy.identification.resolvers.' . PathTenantResolver::class . '.tenant_route_name_prefix' => $tenantRouteNamePrefix = 'team-route.']);
 
-    // Test that routes with controllers as well as routes with closure actions get re-registered correctly
+    // Test that routes with controllers as well as routes with closure actions get cloned correctly
     $universalRoute = RouteFacade::get('/home', $useController ? Controller::class : fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')->middleware($routeMiddleware)->name('home');
     $centralRoute = RouteFacade::get('/central', fn () => true)->name('central');
 
@@ -372,16 +372,16 @@ test('tenant resolver methods return the correct names for configured values', f
     ['tenant_route_name_prefix', 'prefix']
 ]);
 
-test('ReregisterRoutesAsTenant only re-registers routes with path identification by default', function () {
+test('CloneRoutesAsTenant only clones routes with path identification by default', function () {
     app(Kernel::class)->pushMiddleware(InitializeTenancyByPath::class);
 
     $currentRouteCount = fn () => count(RouteFacade::getRoutes()->get());
 
     $initialRouteCount = $currentRouteCount();
 
-    // Path identification is used globally, and this route doesn't use a specific identification middleware, meaning path identification is used and the route should get re-registered
+    // Path identification is used globally, and this route doesn't use a specific identification middleware, meaning path identification is used and the route should get cloned
     RouteFacade::get('/home', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')->middleware('universal')->name('home');
-    // The route uses a specific identification middleware other than InitializeTenancyByPath – the route shouldn't get re-registered
+    // The route uses a specific identification middleware other than InitializeTenancyByPath – the route shouldn't get cloned
     RouteFacade::get('/home-domain-id', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')->middleware(['universal', InitializeTenancyByDomain::class])->name('home-domain-id');
 
     expect($currentRouteCount())->toBe($newRouteCount = $initialRouteCount + 2);
@@ -391,7 +391,7 @@ test('ReregisterRoutesAsTenant only re-registers routes with path identification
 
     $reregisterRoutesAction->handle();
 
-    // Only one of the two routes gets re-registered
+    // Only one of the two routes gets cloned
     expect($currentRouteCount())->toBe($newRouteCount + 1);
 });
 
@@ -403,7 +403,7 @@ test('custom callbacks can be used for reregistering universal routes', function
     $currentRouteCount = fn () => count(RouteFacade::getRoutes()->get());
     $initialRouteCount = $currentRouteCount();
 
-    // Skip re-registering the 'home' route
+    // Skip cloning the 'home' route
     $reregisterRoutesAction->cloneUsing($routeName, function (Route $route) {
         return;
     })->handle();
@@ -427,7 +427,7 @@ test('reregistration of specific routes can get skipped', function () {
     $currentRouteCount = fn () => count(RouteFacade::getRoutes()->get());
     $initialRouteCount = $currentRouteCount();
 
-    // Skip re-registering the 'home' route
+    // Skip cloning the 'home' route
     $reregisterRoutesAction->skipRoute($routeName)->handle();
 
     // Expect route count to stay the same because the 'home' route re-registration gets skipped