ZF-7455: $this->_redirect() behaves differently in test mode

Description

In normal operation, $this->_redirect() exits after performing the redirect. This behaviour is disabled by controller test case via $redirector->setExit(false)--in test mode, the exit is not applied. The result of this is that actions that use _redirect() like a "return" or exit behave differently in test mode.

i.e. actions with the following pattern:

if ($some_condition) { $this->_redirect("foo"); }

if ($other_condition) { $this->_redirect("bar"); }

$this->_redirect("default");

will always be redirected to "default" in test mode.

If this issue cannot be fixed, it would at least be helpful to place a warning in 52.2.3.3 saying that _redirect() must be followed by return in actions, otherwise the redirect assertion may not behave as expected.

Comments

It's best with both _redirect() and _forward() (as well as when using the redirector action helper) to call return():


return $this->_redirect(...);
return $this->_forward(...);

This will ensure that if the exit flag is disabled, the application will work in the same way.

I will add this verbiage to the manual.

Thanks. This seems like the best solution now that exit() has managed to get itself into the library. Looking at "dispatch()" in ControllerTestCase.php it looks like the other point of potentially different behavior is with the JSON helper, which also disables exit.

Added a section on testing and the redirect helper to the manual.

This workaround partly solves the issue and fails when action helpers are involved. 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.

The updated documentation is available at

http://framework.zend.com/manual/en/…

for anyone who's interested.

I think the current text can be improved. At the moment it doesn't really explain, or distinguish between, the problem and the solution. There also doesn't appear to be any meaningful difference between the two "important" boxes and the non-boxed paragraph.

I would prefer something like the following:

There is a potential problem with combining controller testing if using the redirect helper. By default, the redirect helper performs an exit() after issuing the redirect header. This means that the line

$this->_redirect(...);

will typically serve to terminate the script; once $this->_redirect(...) is executed, no statement appearing after the redirect within the action will be executed.

Since exit() also terminates PHPUnit, the testing harness disables this feature of _redirect(...), so that the test harness can continue to run. However, this means that--contrary to non-test mode--statements appearing after $this->_redirect(...) will be executed in test mode. This may result in unexpected behaviour.

To prevent this, you must ensure that you exit the action after performing a _redirect(...). The easiest way to ensure this is to preceded the call to $this->_redirect(...) with "return":

return $this->_redirect(...);

In summary, we recommend that

$this->_redirect(...);

never appears in your action methods. Instead, you should always write:

return $this->_redirect(...);

But what should user do if he has the same rules for every action, e.g. public function preDispatch/init/or something like that() { $auth = Zend_Auth::getInstance(); $role = ($auth->hasIdentity() && !empty($auth->getIdentity()->role)) ? $auth->getIdentity()->role : App_Roles::GUEST; $acl = Zend_Registry::get('acl'); if (!$acl->isAllowed($role, App_Resources::REQUIRED_LOGIN_PAGE)) { $this->_redirect('/error/denied'); } $this->_user = $auth->getIdentity(); } it works fine in application, but in testing mode action method is invoked and can throw error (due to undefined _user properties, or we should check it in every method). Does that mean it never will work in testable applications?

This has obviously not been fixed sufficiently.

I had an idea, we should investigate wheater its possible to replace the redirector helper in the controller testcase with an own implementation that behaves equally.

"Fixing" this really requires overhauls to the way the MVC workflow currently works; there's no way to return early from an action (or helper, or plugin, etc.) due to limitations in how PHP works. In plain and simple language, PHP is not an event-driven language, which makes it impossible to truly short-circuit execution.

The EventManager in ZF2 will help in solving this issue, but there will always be edge cases when it comes to redirection and getting the response returned early.