Skip to content

[5.3] Fix gatherMiddleware() only unique#16185

Merged
taylorotwell merged 1 commit into
laravel:5.3from
actionm:patch-1
Oct 30, 2016
Merged

[5.3] Fix gatherMiddleware() only unique#16185
taylorotwell merged 1 commit into
laravel:5.3from
actionm:patch-1

Conversation

@actionm

@actionm actionm commented Oct 30, 2016

Copy link
Copy Markdown
Contributor

For example:

I have a route group with middleware 'auth' and controller with middleware 'auth'.

Maybe framework/src/Illuminate/Routing/Route.php gatherMiddleware() must return only unique values ?

class MainController extends Controller
{

    public function __construct()
    {
        $this->middleware('auth');
    }
}


Route::group(['middleware' => ['auth']], function () {

    Route::get('/first', 'MainController@index');
}

Route::get('/second', 'MainController@index');

php artisan route:list

before:

| Domain | Method   | URI    | Name  | Action                                      | Middleware |

|        | GET|HEAD | first  |       | App\Http\Controllers\MainController@index   | web,auth,auth
|        | GET|HEAD | second |       | App\Http\Controllers\MainController@index   | web,auth

after fix:

| Domain | Method   | URI    | Name  | Action                                      | Middleware |

|        | GET|HEAD | first  |       | App\Http\Controllers\MainController@index   | web,auth
|        | GET|HEAD | second |       | App\Http\Controllers\MainController@index   | web,auth

Would it be more preferable if artisan route:list displayed middleware from route and controller separately?

For example:

| Domain | Method   | URI    | Name  | Action                                      | Middleware |

|        | GET|HEAD | first  |       | App\Http\Controllers\MainController@index   | [r]web,auth [c]auth
|        | GET|HEAD | second |       | App\Http\Controllers\MainController@index   | [r]web [c]auth

@actionm

actionm commented Oct 30, 2016

Copy link
Copy Markdown
Contributor Author

fixed branch to 5.3 from #16173

@taylorotwell taylorotwell merged commit 778a381 into laravel:5.3 Oct 30, 2016
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.

2 participants