Support invokable controllers

[kick-the-bucket]

Currently @Breadcrumb annotations are not read from invokable Controllers:

/**
 * @Breadcrumb("parent")
 */
class CallableController
{
    /**
     * @Breadcrumb("child")
     */
    public function __invoke()
    {
    }
}

[kick-the-bucket]

@rvanlaak please review

[rvanlaak]

Is this dev requirement needed because of the tests? Can we prevent this from being added?

[kick-the-bucket]

Removed, not sure why I added it

[kick-the-bucket]

There's some Symfony dependency injection bug which prevents auto-wiring the FrameworkBundle in the older versions and I'm not sure when it was fixed. On the other hand, I'm not sure if I really need the framework bundle to run the tests

[kick-the-bucket]

Got around it by disabling autoconfigure and autowire in src/Resources/config/services.xml

[rvanlaak]

What if controller isn't an array, but is something different as a callable?

Would an early return in case it isn't callable make sense?

if (!is_callable($controller)) {
    return;
}

[kick-the-bucket]

Both constructor and setter for both events have callable type-hint for $controller, so I guess there isn't any point in such a check

[rvanlaak]

The check wouldn't be needed for $controller, but on the combination of [$controller, '__invoke'] as not every controller does implement that.

[kick-the-bucket]

Added more callable type checks for what can not be handled with Doctrine annotation reader along with this check

[kick-the-bucket]

The "suggest" section in composer.json can probably be amended in the same way as I did with "require-dev" in regards to "doctrine-bundle" - it's a real overkill to install the whole bundle just for the annotation reader/parser.

[kick-the-bucket]

@rvanlaak what do you think about changing the "suggests" as well?

[rvanlaak]

@kick-the-bucket can you rebase on master?

[kick-the-bucket]

Will do tomorrow

…

On Mon 23 Nov 2020, 15:35 Richard van Laak, ***@***.***> wrote: @kick-the-bucket <https://github.com/kick-the-bucket> can you rebase on master? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#51 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABV5XNEFMTLMVO4OZBG2BFLSRJQKTANCNFSM4OJIA7RQ> .

[rvanlaak]

@kick-the-bucket thank you a lot for your PR, it was the inspiration for me in creating a simplified version in #71 Let me know what you think of the implementation!

Will merge the other PR for now and close this PR, so we can soon release v1.7 that supports invokable controllers.

[kick-the-bucket]

Took some 16 months, but I guess better late, than never, right? 😅

[rvanlaak]

Did release v1.7.0-beta.1. Any chance to test it as well? That way we can move to a stable next release asap.

[kick-the-bucket]

I have changed jobs and am no longer using this bundle.