[8.x] Fix array keys from cached routes

[glamorous]

Introduced by #35714.
Removed the slash to match the same array key as in
src/Illuminate/Routing/RouteCollection.php:60

We noticed a different result when we used route caching for in this code piece:

$url = parse_url($requestUri, PHP_URL_PATH);

 if (!is_string($url)) {
        return true;
 }

$routeKey = '{tenant}.' . trim(config('app.domain')) . trim($url, '/');

$routes = Route::getRoutes()->get($requestMethod);

return array_key_exists($routeKey, $routes);

Only routes with a specified domain where affected and a the result array of "->get('GET')" would not have the same array keys. Check next example with uri "test" and domain "{tenant}.laravel.test", this would give these two different results if you use cached router or not:

"{tenant}.laravel.test/test" => cached
"{tenant}.laravel.testtest" => not cached

The check for the existing of a domain is not necessary because the "->getDomain" function can return null as well and wil result in empty string (just like it behaves in the non compiled RouteCollection).

If approved & merged, I can cherry-pick it for 9.x release as well.

[driesvints]

I don't think this is valid? Seems like existing tests are breaking because of this PR.

[taylorotwell]

@glamorous please provide a bullet point list of steps to take in a fresh Laravel application to recreate the bug. Also please mark this PR as ready for review when you have done so.

[glamorous]

1. Create new Laravel application (8.x or 9.x)
2. Create a route (important to set a "domain" on the route)

Route::middleware(['web'])
    ->domain('{tenant}.' . 'laravel.test')
    ->where(['tenant' => '^((?!(admin|api|webhooks)).)*$'])
    ->get('/test', function () {
       return view('welcome');
    });

3. Open tinker-session:

array_keys(Route::getRoutes()->get('GET'));

4. Result:

[
     "sanctum/csrf-cookie",
     "_ignition/health-check",
     "api/user",
     "/",
     "{tenant}.laravel.testtest",
   ]

4. Close tinker session
5. Run php artisan route:cache in console
6. Redo step 3 (tinker session)
7. Result:

[
     "sanctum/csrf-cookie",
     "_ignition/health-check",
     "api/user",
     "/",
     "{tenant}.laravel.test/test",
   ]

8. See the difference in array keys from both results.

[driesvints]

It indeed seems that this is the correct fix. We'll need to adjust the tests and also version the routes file anew to make sure existing cached route files aren't conflicting with this change.

[glamorous]

I'll try to find some time this weekend to check which tested needs an update

[taylorotwell]

Can we get a test on this?

[glamorous]

There was just a test that needed the fix as well to match the changed code. If merged, I can cherry-pick for 9.x

[driesvints]

@taylorotwell there were tests for this, they've now been changed to the new behavior 👍