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>
archtechx · Aug 6, 2024 · 0f7cd2e · 0f7cd2e
1 parent a9ab646
commit 0f7cd2e
Show file tree

Hide file tree

Showing 2 changed files with 73 additions and 18 deletions.
diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php
@@ -150,9 +150,10 @@ protected function cloneRoute(Route $route): void
     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 @@ protected function createNewRoute(Route $route): Route
             return $action
                 ->put('as', $newRouteNamePrefix)
                 ->put('middleware', $newRouteMiddleware)
-                ->put('prefix', $route->getPrefix() . '/{' . PathTenantResolver::tenantParameterName() . '}');
+                ->put('prefix', $prefix . '/{' . PathTenantResolver::tenantParameterName() . '}');
         })->toArray();
 
         /** @var Route $newRoute */

diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php
@@ -246,28 +246,82 @@
 })->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.')
-        ->middleware(['universal', InitializeTenancyByPath::class])
-        ->name($routeName = 'home')
-        ->prefix($prefix = 'prefix');
+    $routes = [
+        RouteFacade::get('/home', fn () => tenant() ? 'Tenancy initialized.' : 'Tenancy not initialized.')
+            ->middleware(['universal', InitializeTenancyByPath::class])
+            ->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);
-
-    $clonedRouteUrl = route($clonedRouteName, ['tenant' => $tenant = Tenant::create()]);
+    $clonedRoutes = [
+        RouteFacade::getRoutes()->getByName('tenant.home'),
+        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}");
+
+        $clonedRouteUrl = route($route->getName(), ['tenant' => $tenant = Tenant::create()]);
+
+        expect($clonedRouteUrl)
+            // 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($clonedRouteUrl)
-        // 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");
+    expect($clonedLandingUrl)
+        ->not()->toContain("prefix//")
+        ->toBe("http://localhost/prefix/{$tenant->getTenantKey()}");
 
-    // The cloned route is accessible
-    pest()->get($clonedRouteUrl)->assertSee('Tenancy initialized.');
+    expect($clonedHomeRouteUrl)
+        ->not()->toContain("prefix//")
+        ->toBe("http://localhost/prefix/{$tenant->getTenantKey()}/home");
 });
 
 class CustomInitializeTenancyByPath extends InitializeTenancyByPath