Add support for single action controllers, with __invoke method#645
Add support for single action controllers, with __invoke method#645dbu merged 1 commit intoFriendsOfSymfony:3.xfrom
Conversation
pawel-kowalczyk-alpy
force-pushed
the
3.x
branch
2 times, most recently
from
ee10814 to
83b698e
Compare
|
Looks like PHPStan jobs are broken, not related to my changes |
|
Friendly ping to @dbu |
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
thanks for looking into this, we should indeed support __invoke controllers.
i will have a look at the phpstan failure.
| $controller = $this->controllerResolver->getController($request); | ||
|
|
||
| if (!is_array($controller) || 2 !== count($controller)) { | ||
| if (false === $controller) { |
There was a problem hiding this comment.
this seems a bit dangerous to me. what if the $controller is something weird, like a different kind of array, or some non-array non-object but not false?
should we not add the is_object condition here? that should also avoid the uncertainty below whether method and class are null or set.
| if (false === $controller) { | |
| if (!is_object($controller) && (!is_array($controller) || 2 !== count($controller))) { |
or we could do an if object, elseif array, else return.
There was a problem hiding this comment.
Return type for \Symfony\Component\HttpKernel\Controller\ControllerResolverInterface::getController is callable|false, so return values are limited, but you are right, there can be string for example.
I've changed a logic, now it will work only for object and array with two elements
pawel-kowalczyk-alpy
force-pushed
the
3.x
branch
from
83b698e to
f270e6f
Compare
dbu
left a comment
dbu
left a comment
There was a problem hiding this comment.
great, thanks a lot! i like to be very defensive with these things, you never know what explodes randomly for people...
|
Nice! Any chance for a release soon? Thanks anyway |
|
yes, "soon" :-) i hope that we can wrap up #646 and include that in the release. if that does not happen this week, i will release what we have. |
|
Sure, good luck :) |
|
we have been quick :-D here you go: https://github.com/FriendsOfSymfony/FOSHttpCacheBundle/releases/tag/3.2.0 |
|
Thank you |
2 participants
PR sponsored by AlpinResorts