Description

This bug/improvement request is related to ZF-7455 , but goes beyond it as this is more than a documentation problem. Zend_Test_PHPUnit_ControllerTestCase seems to be unable to test redirects generated by preDispatch() hooks in action controllers and in controller plugins. That is to say, you cannot apparently use the redirect assertions provided by Zend_Test to test for redirects issued by preDispatch() hooks and plug-ins.

The default behavior of Zend_Controller_Action_Helper_Redirector is to call exit() after a redirect has been issued. This halts further processing of any code after the redirect. Zend_Test disables this default behavior.

This practice is sufficient to allow redirects from within actions. However, it seems there is no clean way to use those same assertions on redirects that are performed within a Zend_Controller_Action 's preDispatch() method, or,to test a redirect issued within a preDispatch() method on a Zend_Controller_Plugin that is attached to the controller.

For example, I have an ACL plugin which checks to see whether the user must be logged in to view the requested action. If the user must be logged in, then the preDispatch() method of the plugin calls the redirector helper to redirect to the login controller. However, since Zend_Test has disabled the exit() functionality of the redirector helper, the dispatcher still proceeds to dispatch to the originally requested action. Since that originally requested action expected the user to be logged in, it throws errors of its own. The end result being (I think because I have an error controller specified?) that an HTTP 500 error is thrown and Zend_Test's assertRedirect() methods return false instead of true ...

There has got to be a cleaner way to do this... Surely there are cases where Zend_Test needs to be used for integration testing to confirm that preDispatch() code which issues redirects is playing well with the code in the actions?

Comments

Posted by Marc Hodgins (mjh_ca) on 2009-08-05T19:18:03.000+0000

Fixing formatting...

Posted by Benjamin Eberlei (beberlei) on 2009-09-18T01:41:31.000+0000

You are right, personally i think redirector action helper has to be refactored to skip the dispatching look somehow and call the redirect and exit after that, so that it behaves equally in the enviroments.

Posted by Michael Stillwell (mjs) on 2009-09-30T06:26:39.000+0000

Maybe it's possible to do something with an exception? That is, instead of exiting, a third return technique is added to the redirect helper (on top of "continue" and "exit) so that in test mode it throws a custom exception from the redirect helper to effect the jump. This exception could be caught somewhere in test harness. It's a nasty fix though...

Posted by Olivier Doucet (ezameku) on 2010-10-19T02:25:48.000+0000

I am also experiencing this problem. I have a authentification control in preDispatch()
Everything is working fine in production, but when I'm executing the tests, my action is still triggered.
I am currently using ZF 1.10.8

As far as I can tell, this exhibits the same behaviour in production and testing.

Posted by Nicolas Quiénot (niqo) on 2011-01-19T05:04:33.000+0000

Having the same issue, any news on this ?

Posted by alexvandam (alex505) on 2011-02-06T06:06:54.000+0000

This problem isn't just limited to preDispatch.

I am using a routeShutdown hook, for language detection.

And I redirect to a url witch contains the url.

Like: /nl/controller/action

This isn't detected by zend_test either!

Posted by alexvandam (alex505) on 2011-02-06T06:12:47.000+0000

I found out that it works sometimes. But for some reason, when the error controller is used, it isn't detected as a redirect.

So when i go to /abc (this doesn't exists, so it produces a 404 page), it redirects to /nl/abc. But that isn't detected! But when i go to /index, which redirects to /nl/index, it ís detected properly in zend_test.

Posted by Rob Morgan (robsta) on 2011-06-24T06:25:38.000+0000

Hi Guys,

I've developed a patch which seems to fix the behaviour for me (I'm only using redirects in preDispatch methods). We might need to tweak it a bit to work with routeShutdown etc. All I'm doing is 'returning early' in Zend_Controller_Action::dispatch() if the preDispatch method has turned the response into a redirect.

Rob - Have you considered that it may be bad practice to do redirecting from {{preDispatch()}} or {{init()}} etc.? See what I have commented in ZF-5619. Thoughts?

Posted by Rob Morgan (robsta) on 2011-06-24T14:22:58.000+0000

bq. Rob - Have you considered that it may be bad practice to do redirecting from preDispatch() or init() etc.? See what I have commented in ZF-5619. Thoughts?

Hi Kim,

No I haven't. Why is it bad practice? For example if I wanted to check if a user is not 'logged-in' then deny them from the rest of the controller actions, what should I be doing? Ideally I don't want to repeat code in every controller action to check whether the user is logged in or not. It would be preferable to place this logic in one location and automatically deny access to all of the controller actions.

bq. The primary issue I'm seeing right now is actually in your code. When you call _forward() or one of the _redirect() or redirector methods, you should always return immediately; if you do not do so, the action continues to execute, which can lead to the problems you're describing.

and Olek (ZF-7455)

bq. If I redirect from an action helper in init or preDispatch, I want the action controller not to even call the action. Unfortunately, this is not the case, and the regular action logic like rendering happens as well.

So have you tested that if you are calling redirector in {{preDispatch()}} and return there immediately, does it prevent action to be executed?

Posted by Rob Morgan (robsta) on 2011-06-25T07:18:13.000+0000

bq. So have you tested that if you are calling redirector in preDispatch() and return there immediately, does it prevent action to be executed?

Yes I have. The problem is this works in production, but if you are using {{Zend_Test_PHPUnit_ControllerTestCase}} to unit-test your controllers it won't work. This is because the {{setExit}} method on {{Zend_Controller_Action_Helper_Redirector}} is set to {{false}}, so performing a redirect won't call PHP's {{exit}} function - hence {{Zend_Controller_Action::dispatch()}} will call the controller action.

Does that make sense?

Regards,

Rob

Posted by Kim Blomqvist (kblomqvist) on 2011-06-25T07:38:11.000+0000

Ok then the patch looks good to me. Nice work, thanks.

Posted by Rob Morgan (robsta) on 2011-06-25T07:46:16.000+0000

bq. Ok then the patch looks good to me. Nice work, thanks.

Kim,

Keep in mind we might need to play with it a bit to cater for some of the other issues the guys have mentioned above. e.g. I'm not sure if it fixes redirects in the {{routeShutdown}} hook for example as I didn't test for this.

It should be trivial to add another test though. I'll do it if I get some time.

Or alternatively maybe we should open separate issues for these hooks? What are your thoughts?

Regards,

Rob

Posted by Kim Blomqvist (kblomqvist) on 2011-06-25T07:56:24.000+0000

bq. I'm not sure if it fixes redirects in the routeShutdown hook for example as I didn't test for this.

But that's the plugin method and thus beyound the issue addressed here? Though I could also test this in next week.

edit:

bq. Or alternatively maybe we should open separate issues for these hooks? What are your thoughts?

Not sure how this fits in the scenario, so i'll explain and let you judge.

I have a test that does the following actions:

Build dependencies

Calls a dispatch on url X to setup dependencies (this url does a header-location redirect)

Dispatches url Y

Checks result.

Since this commit was done this case is now failing and fails to ever execute the action in Url Y. I might be doing something wrong, i should probably clear the dispatcher after the first call but i'm looking into how to do this.

I just wanted to share the scenario so you guys can look at another usercase.

Could you provide a short code sample illustrating your use case? The test code plus the code in the action method for "url X" (specifically the redirect) should be sufficient to allow me to get a good handle on how to correct the issue.

One of the changes introduced for this issue's fix is that action methods are not executed after a redirect has been invoked, and this is where you're problem lies. We may want to revisit that decision, as it appears to break the pre-existing functionality you outlined

@Rafael
Could you still show the code where you build your dependencies and then dispatch to the final action? I will update the ZF manual to include a note advising users that they must use resetResponse in that situation, but I would like to make sure I understand the use case properly first. Thanks!

@Rafael, @Matthew
The fix for the issue has changed that facet of framework's behavior: you now have to resetResponse between redirects, which (AFAIK) you didn't need to do before. Would this be considered a BC break, or is it OK?