Handle case of static controller method and controllers using magic __call() method
merged branch sli-systems/dc-1 (PR #6080)

This PR was squashed before being merged into the master branch (closes #6080).

Commits
-------

e477a2e Handle case of static controller method and controllers using magic __call() method

Discussion
----------

Handle case of static controller method and controllers using magic __call() method

Improve collecting controller details for edge cases where:
- controller is array, but contains class name and static method
- method doesn't exist, but is handled by magic __call() method

---------------------------------------------------------------------------

by fabpot at 2012-11-21T08:12:08Z

Can you add some unit tests?

---------------------------------------------------------------------------

by sli-systems at 2012-11-21T22:19:17Z

@pierredup
I disagree with the your comment about is_callable() only working with objects. The PHP docs state that the first  argument is a callable, so it can be a string, array, closure, and perhaps more.

The test I added also shows that the code works as is :)

I've thought about your suggestion of adding reflection to look up the location of __call(). However, I think this doesn't really  add a lot and only complicates matters. Also, as you can see in the new test, there is also __callStatic() to consider.

The fact that file/line are n/a is  correct, because the most typical case will be that __call() and __callStatic() will delegate to some other method that might not even be in the same class/file (a subclass I would expect), IMHO.

@fabpot
Good catch  about the '/'. I hope the test is complete enough. Looks more like an exercise on PHP callables than anything else, tho ;)

---------------------------------------------------------------------------

by pierredup at 2012-11-22T04:56:18Z

True that ````is_callable```` takes any callable argument, except in the one specific case where you have a ````__call()```` method, and pass an array with the first paramater as a string.

Take the following example:

    class Controller {
        public function __call($method, $arguments) {}
    }

    $controller = array('Controller', 'action');

    var_dump(is_callable($controller));

Here ````is_callable($controller)```` will actually return ````false````, where if you have ````$controller = array(new Controller, 'action');```` it would return true.

Of course if you have a ````__callStatic```` method, then it would always return true.

Your tests doesn't seem to cover this use case

---------------------------------------------------------------------------

by sli-systems at 2012-11-22T20:27:05Z

Hmm, maybe. I have to admin that I do not know about this case. OTOH, if is_callable returns false is it really callable then? I would think this more of a PHP bug then?

I think I might have come across this case during coding, but then dismissed it because in that case FilterControllerEvent failed already before the data collector code is reached.

In FilterControllerEvent there is a check on is_callable and a LogicException is thrown if $controller is not callable.

So, is FilterControllerEvent wrong  too then?

---------------------------------------------------------------------------

by pierredup at 2012-11-22T20:41:14Z

One would think that if is_callable returns false, then the controller isn't callable, but in the case I mentioned above, the controller is in fact callable. I also thought it was a bug with php, but the php-internals don't seem to think so.

The problem is, if you specify the class as a string, php looks for a static method, even if you have a __call method, it won't be registered.

I will have a look at the FilterControllerEvent to see if this use case applies there as well.

---------------------------------------------------------------------------

by sli-systems at 2012-11-22T20:50:32Z

Rather strange - if that is the case then using is_callable seems pretty pointless and the only way would be to try to execute the controller to find out if it is, in fact, callable...

---------------------------------------------------------------------------

by pierredup at 2012-11-22T20:51:07Z

Okay so it actually seems that the case above isn't callable after all. If the controller is specified as a string, then a static method need to exist. Hence why it works with __callStatic. Only when an instance of the class is specified, will it handle the __call method.

---------------------------------------------------------------------------

by sli-systems at 2012-11-22T20:57:55Z

So the tests are sufficient then?

---------------------------------------------------------------------------

by pierredup at 2012-11-22T20:59:22Z

Yes it is.
This happens when you just assume something without actually testing it :)

Sorry for the hassle