Skip to content

Commit b66acb2

Browse files
andrey-helldarandrey-helldar
committed
Fixed route caching bug
1 parent 8f76e8c commit b66acb2

17 files changed

+175
-86
lines changed

‎README.md‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,13 @@ app('router')->delete('/', [IndexController::class, 'delete']);
5555
app('router')->patch('/', [IndexController::class, 'patch']);
5656
app('router')->options('/', [IndexController::class, 'options']);
5757

58+
app('router')->get('{some}', [IndexController::class, 'index']);
59+
app('router')->post('{some}', [IndexController::class, 'store']);
60+
app('router')->put('{some}', [IndexController::class, 'update']);
61+
app('router')->delete('{some}', [IndexController::class, 'delete']);
62+
app('router')->patch('{some}', [IndexController::class, 'patch']);
63+
app('router')->options('{some}', [IndexController::class, 'options']);
64+
5865
app('router')->get('pages', [PagesController::class, 'index']);
5966
app('router')->post('pages', [PagesController::class, 'store']);
6067
app('router')->put('pages/{page}', [PagesController::class, 'update']);
@@ -65,12 +72,18 @@ app('router')->options('pages/{page}', [PagesController::class, 'options']);
6572

6673
| Method | Url | Name | Helper |
6774
|-----------|--------------|-----------------|--------------------------|
68-
| GET, HEAD | `/` | `index` | `route('index')` |
69-
| POST | `/` | `store` | `route('store')` |
70-
| PUT | `/` | `update` | `route('update')` |
71-
| DELETE | `/` | `destroy` | `route('destroy')` |
72-
| PATCH | `/` | `patch` | `route('patch')` |
73-
| OPTIONS | `/` | `options` | `route('options')` |
75+
| GET, HEAD | `/` | `main.index` | `route('main.index')` |
76+
| POST | `/` | `main.store` | `route('main.store')` |
77+
| PUT | `/` | `main.update` | `route('main.update')` |
78+
| DELETE | `/` | `main.destroy` | `route('main.destroy')` |
79+
| PATCH | `/` | `main.patch` | `route('main.patch')` |
80+
| OPTIONS | `/` | `main.options` | `route('main.options')` |
81+
| GET, HEAD | `{some}` | `some.index` | `route('some.index')` |
82+
| POST | `{some}` | `some.store` | `route('some.store')` |
83+
| PUT | `{some}` | `some.update` | `route('some.update')` |
84+
| DELETE | `{some}` | `some.destroy` | `route('some.destroy')` |
85+
| PATCH | `{some}` | `some.patch` | `route('some.patch')` |
86+
| OPTIONS | `{some}` | `some.options` | `route('some.options')` |
7487
| GET, HEAD | `/pages` | `pages.index` | `route('pages.index')` |
7588
| POST | `/pages` | `pages.store` | `route('pages.store')` |
7689
| PUT | `/pages/123` | `pages.update` | `route('pages.update')` |

‎composer.json‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
"require-dev": {
3939
"orchestra/testbench": "^9.0 || ^10.0",
4040
"pestphp/pest": "^3.8",
41+
"pestphp/pest-plugin-laravel": "^3.2",
4142
"pestphp/pest-plugin-type-coverage": "^3.5"
4243
},
4344
"minimum-stability": "stable",
@@ -80,7 +81,7 @@
8081
"build": "@php vendor/bin/testbench workbench:build --ansi",
8182
"clear": "@php vendor/bin/testbench package:purge-skeleton --ansi",
8283
"prepare": "@php vendor/bin/testbench package:discover --ansi",
83-
"test": "@php vendor/bin/pest --parallel",
84-
"test-coverage": "@php vendor/bin/pest --type-coverage"
84+
"test": "@php vendor/bin/pest",
85+
"test-coverage": "@test --type-coverage --compact"
8586
}
8687
}

‎src/Application.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,10 +29,11 @@ protected function registerBaseServiceProviders(): void
2929
{
3030
$this->register(new EventServiceProvider($this));
3131
$this->register(new LogServiceProvider($this));
32-
$this->register(new RoutingServiceProvider($this));
3332

3433
if (class_exists(ContextServiceProvider::class)) {
3534
$this->register(new ContextServiceProvider($this));
3635
}
36+
37+
$this->register(new RoutingServiceProvider($this));
3738
}
3839
}

‎src/Helpers/Action.php‎

Lines changed: 14 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@
2020
use Illuminate\Support\Str;
2121
use Symfony\Component\HttpFoundation\Request;
2222

23+
use function in_array;
24+
2325
class Action
2426
{
2527
protected array $aliases = [
@@ -41,31 +43,25 @@ class Action
4143
'restore' => [Request::METHOD_POST],
4244
];
4345

44-
protected array $show = [Request::METHOD_GET, Request::METHOD_HEAD];
46+
protected array $show = [
47+
Request::METHOD_GET,
48+
Request::METHOD_HEAD,
49+
];
4550

4651
protected string $default = 'index';
4752

4853
public function get(array $methods, string $uri): string
4954
{
50-
if ($value = $this->show($methods, $uri)) {
51-
return $value;
52-
}
53-
54-
if ($value = $this->collision($methods, $uri)) {
55-
return $value;
56-
}
57-
58-
if ($value = $this->alias($methods)) {
59-
return $value;
60-
}
61-
62-
return $this->default;
55+
return $this->show($methods, $uri)
56+
?? $this->collision($methods, $uri)
57+
?? $this->alias($methods)
58+
?? $this->default;
6359
}
6460

6561
protected function collision(array $methods, string $uri): ?string
6662
{
67-
foreach ($this->collision as $alias => $http_methods) {
68-
if ($this->hasMethods($methods, $http_methods) && $this->hasAlias($uri, $alias)) {
63+
foreach ($this->collision as $alias => $httpMethods) {
64+
if ($this->hasMethods($methods, $httpMethods) && $this->hasAlias($uri, $alias)) {
6965
return $alias;
7066
}
7167
}
@@ -75,8 +71,8 @@ protected function collision(array $methods, string $uri): ?string
7571

7672
protected function alias(array $methods): ?string
7773
{
78-
foreach ($this->aliases as $method => $http_methods) {
79-
if ($this->hasMethods($methods, $http_methods)) {
74+
foreach ($this->aliases as $method => $httpMethods) {
75+
if ($this->hasMethods($methods, $httpMethods)) {
8076
return $method;
8177
}
8278
}

‎src/Helpers/Name.php‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,19 @@
2323

2424
class Name
2525
{
26+
protected string $default = 'main';
27+
2628
public function get(array $methods, string $uri): string
2729
{
2830
$resolved = $this->resolve($uri);
2931
$suffix = $this->getMethodSuffix($methods, $uri);
3032

3133
return $resolved
32-
->when($this->doesntSame($resolved, $suffix), static fn (Collection $items) => $items->push($suffix))
34+
->when(
35+
fn (Collection $items) => $items->isEmpty(),
36+
fn (Collection $items) => $items->push($this->fallback($uri))
37+
)
38+
->when($this->needSuffix($resolved, $suffix), static fn ($items) => $items->push($suffix))
3339
->implode('.');
3440
}
3541

@@ -49,16 +55,25 @@ protected function getMethodSuffix(array $methods, string $uri): string
4955

5056
protected function has(string $value): bool
5157
{
52-
return ! empty($value) && ! Str::contains($value, '{');
58+
return ! empty($value) && Str::doesntContain($value, '{');
5359
}
5460

5561
protected function map(string $value): string
5662
{
5763
return (string) Str::of($value)->lower()->kebab();
5864
}
5965

60-
protected function doesntSame(Collection $haystack, string $needle): bool
66+
protected function needSuffix(Collection $haystack, string $needle): bool
6167
{
62-
return $needle !== $haystack->last();
68+
return $haystack->count() <= 2 || $needle !== $haystack->last();
69+
}
70+
71+
protected function fallback(string $uri): string
72+
{
73+
if (Str::startsWith($uri, '{') && Str::endsWith($uri, '}') && Str::doesntContain($uri, '/')) {
74+
return Str::of($uri)->after('{')->before('}')->value();
75+
}
76+
77+
return $this->default;
6378
}
6479
}

‎src/Routing/Route.php‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
use Illuminate\Routing\Route as BaseRoute;
2222
use Illuminate\Support\Str;
2323

24+
use function app;
25+
use function config;
26+
2427
class Route extends BaseRoute
2528
{
2629
public function getName(): ?string

‎tests/Datasets/cache.php‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
<?php
2+
3+
/**
4+
* This file is part of the "dragon-code/laravel-route-names" project.
5+
*
6+
* For the full copyright and license information, please view the LICENSE
7+
* file that was distributed with this source code.
8+
*
9+
* @author Andrey Helldar <[email protected]>
10+
* @copyright 2025 Andrey Helldar
11+
* @license MIT
12+
*
13+
* @see https://github.com/TheDragonCode/laravel-route-names
14+
*/
15+
16+
declare(strict_types=1);
17+
18+
dataset('cache routes', [
19+
'cache enabled' => [true],
20+
'cache disabled' => [false],
21+
]);

‎tests/Helpers/cache.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
<?php
2+
3+
/**
4+
* This file is part of the "dragon-code/laravel-route-names" project.
5+
*
6+
* For the full copyright and license information, please view the LICENSE
7+
* file that was distributed with this source code.
8+
*
9+
* @author Andrey Helldar <[email protected]>
10+
* @copyright 2025 Andrey Helldar
11+
* @license MIT
12+
*
13+
* @see https://github.com/TheDragonCode/laravel-route-names
14+
*/
15+
16+
declare(strict_types=1);
17+
18+
use Illuminate\Foundation\Console\RouteCacheCommand;
19+
use Illuminate\Foundation\Console\RouteClearCommand;
20+
21+
use function Pest\Laravel\artisan;
22+
23+
function cacheRoutes(bool $use): void
24+
{
25+
$use
26+
? artisan(RouteCacheCommand::class)->run()
27+
: artisan(RouteClearCommand::class)->run();
28+
}

‎tests/Unit/Routes/BasicTest.php‎

Lines changed: 13 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -17,47 +17,41 @@
1717

1818
namespace Tests\Unit\Routes;
1919

20-
it('default', function () {
21-
expect(routeName('foo'))->toBe('index');
22-
expect(routeName('bar'))->toBe('store');
23-
expect(routeName('baz'))->toBe('update');
24-
expect(routeName('baq'))->toBe('destroy');
25-
expect(routeName('baw'))->toBe('patch');
26-
expect(routeName('bae'))->toBe('options');
27-
});
28-
29-
it('simple', function () {
20+
it('default', function (bool $withCache) {
21+
cacheRoutes($withCache);
22+
23+
expect(routeName('foo'))->toBe('main.index');
24+
expect(routeName('bar'))->toBe('main.store');
25+
expect(routeName('baz'))->toBe('main.update');
26+
expect(routeName('baq'))->toBe('main.destroy');
27+
expect(routeName('baw'))->toBe('main.patch');
28+
expect(routeName('bae'))->toBe('main.options');
29+
3030
expect(routeName('simpleFoo'))->toBe('simple.index');
3131
expect(routeName('simpleBar'))->toBe('simple.store');
3232
expect(routeName('simpleBaz'))->toBe('simple.update');
3333
expect(routeName('simpleBaq'))->toBe('simple.destroy');
3434
expect(routeName('simpleBaw'))->toBe('simple.patch');
3535
expect(routeName('simpleBae'))->toBe('simple.options');
36-
});
3736

38-
it('edit', function () {
3937
expect(routeName('simpleEditFoo'))->toBe('simple.edit.show');
4038
expect(routeName('simpleEditBar'))->toBe('simple.edit.store');
4139
expect(routeName('simpleEditBaz'))->toBe('simple.edit.update');
4240
expect(routeName('simpleEditBaq'))->toBe('simple.edit.destroy');
4341
expect(routeName('simpleEditBaw'))->toBe('simple.edit.patch');
4442
expect(routeName('simpleEditBae'))->toBe('simple.edit.options');
45-
});
4643

47-
it('update', function () {
4844
expect(routeName('simpleUpdateFoo'))->toBe('simple.update.show');
4945
expect(routeName('simpleUpdateBar'))->toBe('simple.update.store');
50-
expect(routeName('simpleUpdateBaz'))->toBe('simple.update');
46+
expect(routeName('simpleUpdateBaz'))->toBe('simple.update.update');
5147
expect(routeName('simpleUpdateBaq'))->toBe('simple.update.destroy');
5248
expect(routeName('simpleUpdateBaw'))->toBe('simple.update.patch');
5349
expect(routeName('simpleUpdateBae'))->toBe('simple.update.options');
54-
});
5550

56-
it('destroy', function () {
5751
expect(routeName('simpleDestroyFoo'))->toBe('simple.destroy.show');
5852
expect(routeName('simpleDestroyBar'))->toBe('simple.destroy.store');
5953
expect(routeName('simpleDestroyBaz'))->toBe('simple.destroy.update');
60-
expect(routeName('simpleDestroyBaq'))->toBe('simple.destroy');
54+
expect(routeName('simpleDestroyBaq'))->toBe('simple.destroy.destroy');
6155
expect(routeName('simpleDestroyBaw'))->toBe('simple.destroy.patch');
6256
expect(routeName('simpleDestroyBae'))->toBe('simple.destroy.options');
63-
});
57+
})->with('cache routes');

‎tests/Unit/Routes/CollisionTest.php‎

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,11 +17,13 @@
1717

1818
namespace Tests\Unit\Routes;
1919

20-
it('default', function () {
20+
it('default', function (bool $withCache) {
21+
cacheRoutes($withCache);
22+
2123
expect(routeName('collisionGet'))->toBe('collision.get.show');
2224
expect(routeName('collisionPost'))->toBe('collision.post.store');
2325
expect(routeName('collisionPut'))->toBe('collision.put.update');
2426
expect(routeName('collisionDelete'))->toBe('collision.delete.destroy');
25-
expect(routeName('collisionPatch'))->toBe('collision.patch');
26-
expect(routeName('collisionOptions'))->toBe('collision.options');
27-
});
27+
expect(routeName('collisionPatch'))->toBe('collision.patch.patch');
28+
expect(routeName('collisionOptions'))->toBe('collision.options.options');
29+
})->with('cache routes');

0 commit comments

Comments
 (0)