search

PHP-DI / Silex-Bridge

PHP-DI integration in Silex
http://php-di.org/doc/frameworks/silex.html
MIT License
24 stars 8 forks source link

Support ServiceControllerServiceProvider #7

Closed jdreesen closed 9 years ago

jdreesen commented 9 years ago

The Silex bridge cannot be used together with Silex\Provider\ServiceControllerServiceProvider because it always returns an empty array in ControllerResolverInterface::getArguments().

The ServiceControllerServiceProvider decorates the ControllerResolver of the app, so that the ControllerResolver::getController() method of this bridge gets only called if the controller is no Silex service. If it is, only ControllerResolver::getArguments() of this bridge gets called, which always returns an empty array and thus the controller action will be called without arguments, resulting in an error.

It seems that we'd need an instance of Invoker\ParameterResolver\ParameterResolver to resolve the parameters of the controller but I don't know how to get it from the container because it's private.

mnapoli commented 9 years ago

Hi! Right I see what you mean thanks for the details. I'm curious as to why you would use the ServiceControllerServiceProvider since this bridge already does the "controller as a service" thing?

Anyway this PR might help: https://github.com/PHP-DI/Invoker/pull/2 With this, we could use CallableResolver in ControllerResolver::getController() and for ControllerResolver::getArguments() we could create a ParameterResolver manually. After all, PHP-DI's invoker is simple: https://github.com/PHP-DI/PHP-DI/blob/master/src/DI/Container.php#L321-L329

            $parameterResolver = new ResolverChain([
                new AssociativeArrayResolver,
                new TypeHintContainerResolver($this->wrapperContainer),
            ]);

That would be an even simpler invoker (so faster, more predictable, …) and we could use it directly.

jdreesen commented 9 years ago

I'm curious as to why you would use the ServiceControllerServiceProvider since this bridge already does the "controller as a service" thing?

You are right and it's not me using the ServiceControllerServiceProvider (at least not directly) but the WebProfilerServiceProvider I tried to include.

Thanks for your suggestions :) I'll play around with them a bit in the next days (if I find the time) and send a PR if I found a solution.

jdreesen commented 9 years ago

I just pushed a first version that implements ControllerResolver::getArguments() to my fork: https://github.com/jdreesen/Silex-Bridge/commit/0ee3381f313d7bd40c526847bc8d6dac350c4109

However, this does not work with controllers that need to be resolved from the container. We would need https://github.com/PHP-DI/Invoker/pull/2 for this, so is there anything preventing it to be merged?

mnapoli commented 9 years ago

:+1: I'll rebase https://github.com/PHP-DI/Invoker/pull/2 and try to merge it

mnapoli commented 9 years ago

@jdreesen rebasing was too painful ;) I created a new PR and merged it: https://github.com/PHP-DI/Invoker/releases/tag/1.2.0

jdreesen commented 9 years ago

Very nice, thx! :)

I don't have free time until next week but then I'll update my commit and send a PR.