From 0f7cd2e8680a64bcf1f4ac9fd14087ae242ecdb0 Mon Sep 17 00:00:00 2001 From: lukinovec Date: Tue, 6 Aug 2024 02:15:18 +0200 Subject: [PATCH] Trim trailing / from route prefixes during route cloning (#55) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 Co-authored-by: Samuel Štancl --- src/Actions/CloneRoutesAsTenant.php | 7 +-- tests/CloneActionTest.php | 84 +++++++++++++++++++++++------ 2 files changed, 73 insertions(+), 18 deletions(-) diff --git a/src/Actions/CloneRoutesAsTenant.php b/src/Actions/CloneRoutesAsTenant.php index fdd2a822..c2c22508 100644 --- a/src/Actions/CloneRoutesAsTenant.php +++ b/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 */ diff --git a/tests/CloneActionTest.php b/tests/CloneActionTest.php index 8e08ef32..2d27bdf1 100644 --- a/tests/CloneActionTest.php +++ b/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.') - ->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}"); - 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"); + $clonedRouteUrl = route($route->getName(), ['tenant' => $tenant = Tenant::create()]); - // The cloned route is accessible - pest()->get($clonedRouteUrl)->assertSee('Tenancy initialized.'); + 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($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