Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[11.x] middleware support for specific method in resource routes #53313

Conversation

MrPunyapal
Copy link
Contributor

@MrPunyapal MrPunyapal commented Oct 26, 2024

Enhancement: Middleware Assignment for Resource & Singleton Routes

This pull request enables assigning middleware to specific resourceful methods on resource and singleton routes within the Laravel framework. Key changes include new method 'middlewareFor` for method-specific middleware assignment and updates to resource registration logic to support these configurations.

Motivation: While most developers prefer using routes for defining middleware, this particular case necessitates defining it at the controller level. Alternatively, we could extract all routes into separate routes. Addressing this challenge will streamline our approach.

Examples after merging:

Route::resource('users', UserController::class)->middlewareFor('show', 'auth');
Route::apiResource('users', UserController::class)->middlewareFor(['show', 'update'], 'auth');
Route::resource('users', UserController::class)
    ->middlewareFor('show', 'auth')
    ->middlewareFor('update', 'auth');
Route::apiResource('users', UserController::class)
    ->middlewareFor(['show', 'update'], ['auth', 'verified']);

Route::singleton('users', UserController::class)->middlewareFor('show', 'auth');
Route::apiSingleton('users', UserController::class)->middlewareFor(['show', 'update'], 'auth');
Route::singleton('users', UserController::class)
    ->middlewareFor('show', 'auth')
    ->middlewareFor('update', 'auth');
Route::apiSingleton('users', UserController::class)
    ->middlewareFor(['show', 'update'], ['auth', 'verified']);

I'm up for withoutMiddlewareFor if it's in demand. added withoutMiddlewareFor.

Route::middleware('auth', 'verified', 'other')->group(function () {
    Route::resource('users', UsersController::class)->withoutMiddlewareFor(['create', 'store'], 'verified')
        ->withoutMiddlewareFor('index', ['auth', 'verified'])
        ->withoutMiddlewareFor('destroy', 'other');

    Route::singleton('user', UserController::class)->withoutMiddlewareFor('show', ['auth', 'verified'])
        ->withoutMiddlewareFor(['create', 'store'], 'verified')
        ->withoutMiddlewareFor('destroy', 'other');
});

… set middleware for specific methods on registered resource
…ability to set middleware for specific methods on registered resource
…t for setting middleware for specific methods on registered resources
@MrPunyapal MrPunyapal changed the title [WIP] [11.x] middleware support for specific method in resource routes [11.x] middleware support for specific method in resource routes Oct 26, 2024
@aniket-magadum
Copy link

Looks good to have 👍

@taylorotwell
Copy link
Member

One thing that gives me pause here is the order in which you call middleware and middlewareFor is not respected. The middlewareFor stuff is always put before middleware. What if you don't want that?

@taylorotwell taylorotwell marked this pull request as draft November 8, 2024 04:10
@MrPunyapal
Copy link
Contributor Author

One thing that gives me pause here is the order in which you call middleware and middlewareFor is not respected. The middlewareFor stuff is always put before middleware. What if you don't want that?

Thanks for reviewing and taking an interest in my PR! I'll take a look soon.

@MrPunyapal MrPunyapal marked this pull request as ready for review January 1, 2025 14:39
@MrPunyapal MrPunyapal marked this pull request as draft January 1, 2025 14:42
@MrPunyapal MrPunyapal force-pushed the feat/middleware-support-for-specific-method-in-resource-routes branch from 4bd83d8 to 6ccf360 Compare January 1, 2025 14:53
@MrPunyapal MrPunyapal force-pushed the feat/middleware-support-for-specific-method-in-resource-routes branch from 240df4b to ce7aa5d Compare January 1, 2025 16:31
@MrPunyapal MrPunyapal force-pushed the feat/middleware-support-for-specific-method-in-resource-routes branch from ce7aa5d to d77b2ba Compare January 1, 2025 16:32
@MrPunyapal MrPunyapal marked this pull request as ready for review January 1, 2025 16:35
@shaedrich
Copy link
Contributor

Route::resource('users', UserController::class)->middlewareFor('show', 'auth');
Route::apiResource('users', UserController::class)->middlewareFor(['show', 'update'], 'auth');
Route::resource('users', UserController::class)
    ->middlewareFor('show', 'auth')
    ->middlewareFor('update', 'auth');
Route::apiResource('users', UserController::class)
    ->middlewareFor(['show', 'update'], ['auth', 'verified']);

Route::singleton('users', UserController::class)->middlewareFor('show', 'auth');
Route::apiSingleton('users', UserController::class)->middlewareFor(['show', 'update'], 'auth');
Route::singleton('users', UserController::class)
    ->middlewareFor('show', 'auth')
    ->middlewareFor('update', 'auth');
Route::apiSingleton('users', UserController::class)
    ->middlewareFor(['show', 'update'], ['auth', 'verified']);

Wouldn't it be more intuitive if this was an associative array? This would be consistent with other resource methods:

Route::resource('photos', PhotoController::class)->names([
    'create' => 'photos.build'
]);

@MrPunyapal
Copy link
Contributor Author

MrPunyapal commented Jan 2, 2025

Wouldn't it be more intuitive if this was an associative array? This would be consistent with other resource methods:

Then we cannot achieve same list of Middleware for 2 or more methods 🤔

So if I need 2 Middleware for show and index then we need to repeat them.

@shaedrich
Copy link
Contributor

That is true, but would there be much harm in that compared to intuitive notation? Also, the idea of using Route::resource() instead of multiple Route::(get|post|put|patch|delete) calls is that all the routes share a significant similarity, but if we start to customized each route anyway, the benefit dwindles.

@MrPunyapal
Copy link
Contributor Author

One thing that gives me pause here is the order in which you call middleware and middlewareFor is not respected. The middlewareFor stuff is always put before middleware. What if you don't want that?

@taylorotwell, I apologize for the delay. I've updated the code to respect the order and revised the tests accordingly.

@taylorotwell taylorotwell merged commit 7a80e6c into laravel:11.x Jan 9, 2025
38 checks passed
@MrPunyapal MrPunyapal deleted the feat/middleware-support-for-specific-method-in-resource-routes branch January 10, 2025 03:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants