Trim trailing / from route prefixes during route cloning (#55)

* route cloning: Trim '/' from original route prefixes

* Add test for the trimming of route prefixes

* Revert "Add test for the trimming of route prefixes"

This reverts commit 568ae17d2bf8d5542a0e46840f7604c6a0df236d.

* Add test for the trimming of route prefixes

* Delete extra comments [ci skip]

* Fix regression test [ci skip]

* trigger CI

* Add routes with trailing slashes to the cloned route prefixing test

* Test nested '/' route cloning

* Update cloned route creation as suggested

* fix terminology

* add comment to test

---------

Co-authored-by: Samuel Štancl <samuel.stancl@gmail.com>
Co-authored-by: Samuel Štancl <samuel@archte.ch>

src/Actions/CloneRoutesAsTenant.php

@@ -150,9 +150,10 @@ class CloneRoutesAsTenant
     protected function createNewRoute(Route $route): Route
     {
         $method = strtolower($route->methods()[0]);
-        $uri = $route->getPrefix() ? Str::after($route->uri(), $route->getPrefix()) : $route->uri();
+        $prefix = trim($route->getPrefix() ?? '', '/');
+        $uri = $route->getPrefix() ? Str::after($route->uri(), $prefix) : $route->uri();
 
-        $newRouteAction = collect($route->action)->tap(function (Collection $action) use ($route) {
+        $newRouteAction = collect($route->action)->tap(function (Collection $action) use ($route, $prefix) {
             /** @var array $routeMiddleware */
             $routeMiddleware = $action->get('middleware') ?? [];
 
@@ -174,7 +175,7 @@ class CloneRoutesAsTenant
             return $action
                 ->put('as', $newRouteNamePrefix)
                 ->put('middleware', $newRouteMiddleware)
-                ->put('prefix', $route->getPrefix() . '/{' . PathTenantResolver::tenantParameterName() . '}');
+                ->put('prefix', $prefix . '/{' . PathTenantResolver::tenantParameterName() . '}');
         })->toArray();
 
         /** @var Route $newRoute */

tests/CloneActionTest.php

@@ -246,28 +246,82 @@ test('routes with the clone flag get cloned without making the routes universal'
 })->with([InitializeTenancyByPath::class, CustomInitializeTenancyByPath::class]);
 
 test('the clone action prefixes already prefixed routes correctly', function () {
-    RouteFacade::get('/home', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')
+    $routes = [
-        ->middleware(['universal', InitializeTenancyByPath::class])
+        RouteFacade::get('/home', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')
-        ->name($routeName = 'home')
+            ->middleware(['universal', InitializeTenancyByPath::class])
-        ->prefix($prefix = 'prefix');
+            ->name('home')
+            ->prefix('prefix'),
+
+        RouteFacade::get('/leadingAndTrailingSlash', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')
+            ->middleware(['universal', InitializeTenancyByPath::class])
+            ->name('leadingAndTrailingSlash')
+            ->prefix('/prefix/'),
+
+        RouteFacade::get('/leadingSlash', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')
+            ->middleware(['universal', InitializeTenancyByPath::class])
+            ->name('leadingSlash')
+            ->prefix('/prefix'),
+
+        RouteFacade::get('/trailingSlash', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')
+            ->middleware(['universal', InitializeTenancyByPath::class])
+            ->name('trailingSlash')
+            ->prefix('prefix/'),
+    ];
 
     app(CloneRoutesAsTenant::class)->handle();
 
-    $clonedRoute = RouteFacade::getRoutes()->getByName($clonedRouteName = 'tenant.' . $routeName);
+    $clonedRoutes = [
-
+        RouteFacade::getRoutes()->getByName('tenant.home'),
-    $clonedRouteUrl = route($clonedRouteName, ['tenant' => $tenant = Tenant::create()]);
+        RouteFacade::getRoutes()->getByName('tenant.leadingAndTrailingSlash'),
+        RouteFacade::getRoutes()->getByName('tenant.leadingSlash'),
+        RouteFacade::getRoutes()->getByName('tenant.trailingSlash'),
+    ];
 
     // The cloned route is prefixed correctly
-    expect($clonedRoute->getPrefix())->toBe("{$prefix}/{tenant}");
+    foreach ($clonedRoutes as $key => $route) {
+        expect($route->getPrefix())->toBe("prefix/{tenant}");
 
-    expect($clonedRouteUrl)
+        $clonedRouteUrl = route($route->getName(), ['tenant' => $tenant = Tenant::create()]);
-        // Original prefix does not occur in the cloned route's URL
-        ->not()->toContain("/{$prefix}/{$tenant->getTenantKey()}/{$prefix}")
-        // Route is prefixed correctly
-        ->toBe("http://localhost/{$prefix}/{$tenant->getTenantKey()}/home");
 
-    // The cloned route is accessible
+        expect($clonedRouteUrl)
-    pest()->get($clonedRouteUrl)->assertSee('Tenancy initialized.');
+            // Original prefix does not occur in the cloned route's URL
+            ->not()->toContain("prefix/{$tenant->getTenantKey()}/prefix")
+            ->not()->toContain("//prefix")
+            ->not()->toContain("prefix//")
+            // Route is prefixed correctly
+            ->toBe("http://localhost/prefix/{$tenant->getTenantKey()}/{$routes[$key]->getName()}");
+
+        // The cloned route is accessible
+        pest()->get($clonedRouteUrl)->assertSee('Tenancy initialized.');
+    }
+});
+
+test('clone action trims trailing slashes from prefixes given to nested route groups', function () {
+    RouteFacade::prefix('prefix')->group(function () {
+        RouteFacade::prefix('')->group(function () {
+            // This issue seems to only happen when there's a group with a prefix, then a group with an empty prefix, and then a / route
+            RouteFacade::get('/', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')
+                ->middleware(['universal', InitializeTenancyByPath::class])
+                ->name('landing');
+
+            RouteFacade::get('/home', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')
+                ->middleware(['universal', InitializeTenancyByPath::class])
+                ->name('home');
+        });
+    });
+
+    app(CloneRoutesAsTenant::class)->handle();
+
+    $clonedLandingUrl = route('tenant.landing', ['tenant' => $tenant = Tenant::create()]);
+    $clonedHomeRouteUrl = route('tenant.home', ['tenant' => $tenant]);
+
+    expect($clonedLandingUrl)
+        ->not()->toContain("prefix//")
+        ->toBe("http://localhost/prefix/{$tenant->getTenantKey()}");
+
+    expect($clonedHomeRouteUrl)
+        ->not()->toContain("prefix//")
+        ->toBe("http://localhost/prefix/{$tenant->getTenantKey()}/home");
 });
 
 class CustomInitializeTenancyByPath extends InitializeTenancyByPath