Typehint mapping and setter injection

[jverdeyen]

I have a question/issue about/with Typehint mapping and setter injection.

I implement an interface on all of my handler classes:

    _instanceof:
        AppBundle\Service\HandlerInterface:
            tags:
                - { name: tactician.handler, typehints: false }
            autowire: true
            public: true

All goes fine by this.

But one handler requires a setter injection, for example.

    /**
     * @param EntityManagerInterface $em
     *
     * @required
     */
    public function setEm(EntityManagerInterface $em): void
    {
        $this->em = $em;
    }

The problem here is that the findCommandsForService in TypeHintMapping will find this function and tries to map it. This will trigger the following error:

Can not route Doctrine\ORM\EntityManagerInterface to AppBundle\Service\Membership\MembershipRegistrationHandler, class Doctrine\ORM\EntityManagerInterface does not exist!

Because of

if (!class_exists($commandClassName)) {
    throw new InvalidArgumentException("Can not route $commandClassName to $serviceId, class $commandClassName does not exist!");
}

I could create a custom mapping, which only checks for a handle function instead of all other function in my class. But is this the expected behaviour? Or should we filter out functions with interface typehinting?

Thanks!

[rosstuck]

Hi @jverdeyen, thanks for opening such a detailed issue, much appreciated! :)

This is definitely not the intended behavior, we tried to exclude a lot of edge cases for this but interface typehints slipped through, my apologies!

If you (or anyone else) is interested, this seems like a fairly straightforward one to fix, we need an extra check here: https://github.com/thephpleague/tactician-bundle/blob/master/src/DependencyInjection/HandlerMapping/TypeHintMapping.php#L56 Probably something like $parameter->getClass()->isInterface() plus an extra test.

If you'd like to give it a go, let me know. Otherwise, I'll pick it up when I can but that may be a little while 😅

Also, purely from curiosity and no judgment intended, I'm curious why any handler services need setter injection as opposed to constructor injection? I'm a big fan of all required deps being passed in there and would like to make it easy, so if there's any shortcomings or edge cases, I'd love to see if we can improve them.

Thanks again for such a nicely researched issue and for using this bundle. :)

[jverdeyen]

@rosstuck thanks for your feedback!

I'll try to create a PR with this solution and a test.

I'm using a EntityManagerAwareTrait in my project. The only was to properly inject the em by a trait is using a setter injection. This way it doesn't change the constructor of the classes using this trait. Might not be a good practice, but in rapid development it's a win-win.