[5.2] Apply middleware once even in nested groups

[themsaid]

In a previous PR we handled the case of applying a middleware more than once using Route::middleware() however the case of nested groups wasn't handled.

Here's the case:

$router->group(['middleware' => ['web', 'foo']], function () use ($router) {
    $router->group(['middleware' => 'web'], function () use ($router) {
        $router->group(['middleware' => ['web', 'foo', 'baz']], function () use ($router) {
            $router->get('bar', function () { return 'hello'; });
        });
    });
});

The middlewares extracted from this case are:

'web', 'foo', 'baz'

Before this PR:

'web', 'foo', 'web', 'web', 'foo', 'baz'

[GrahamCampbell]

This would be a major breaking change.

[themsaid]

@GrahamCampbell explain please.

[GrahamCampbell]

You already explained it. Your before and afters are different.

[themsaid]

A PR that does the same was merged into 5.2 a few weeks ago #12911

It has the same effect if you use the Route::middleware() method, this PR handles the case where you don't use the method but have nested groups.

However even without this PR, in the same example above if you applied ->middleware(..) to the route, it'll extract only the unique middlewares from the route and the test will pass.

I don't see a case where applying a middleware more than once is needed.

[GrahamCampbell]

Gah, I think we should revert that.

[mark86092]

@GrahamCampbell Is duplicated middleware intentional for some purposes? If not, what I can understand here is that two results of $route->middleware() are different. Thank you for your explaination

[themsaid]

I believe forcing unique middlewares is a safety net that prevents executing a code multiple times while it should be executed once. Anyway if someone is depending on a middleware being applied multiple times intentionally yes this would be considered breaking, it depends on how you see the matter: bug or feature.

[GrahamCampbell]

Is duplicated middleware intentional for some purposes?

Incase someone wants to wrap something more than once. It can't be that common though. I'm fine with this for 5.3, but I think we should revert the existing break on 5.3.

[taylorotwell]

Is $old['middleware'] always available here?

[themsaid]

Actually no, corrected. Sorry.

[taylorotwell]

Yeah let's think about this for 5.3, if at all. I'm inclined to not really limit people in this way at all.

[themsaid]

@taylorotwell then I think #12911 should be reverted as it has the same effect. no?