Workaround for double-template rendering with forward() plug-in. #2206

Merged
merged 1 commit into from Aug 20, 2012

Projects

None yet

3 participants

@demiankatz

If a caller returns the value of $this->forward()->dispatch() in a controller action while the InjectViewModelListener is active, the same view model gets attached to the layout twice, and the PHP template will be rendered twice, wasting resources and possibly causing strange side effects. This commit detaches the problem listener before dispatching a forwarded action so that the output of dispatch() can be safely returned by the calling action. It also provides a generic, extensible mechanism for detaching other listeners in different use cases.

@demiankatz demiankatz If a caller returns the value of $this->forward()->dispatch() in a co…
…ntroller action while the InjectViewModelListener is active, the same view model gets attached to the layout twice, and the PHP template will be rendered twice, wasting resources and possibly causing strange side effects. This commit detaches the problem listener before dispatching a forwarded action. It also provides a generic, extensible mechanism for detaching other listeners in different use cases.
0b8fc7a
@travisbot

This pull request passes (merged 0b8fc7a into 5dd58b8).

@weierophinney weierophinney commented on the diff Aug 20, 2012
library/Zend/Mvc/Controller/Plugin/Forward.php
@@ -58,6 +64,46 @@ public function setMaxNestedForwards($maxNestedForwards)
}
/**
+ * Get information on listeners that need to be detached before dispatching.
+ *
+ * Each entry in the array contains three keys:
+ *
+ * id (identifier for event-emitting component),
+ * event (the hooked event)
+ * and class (the class of listener that should be detached).
+ *
+ * @return array
+ */
+ public function getListenersToDetach()
+ {
+ // If a blacklist has not been explicitly set, return the default:
+ if (is_null($this->listenersToDetach)) {
@weierophinney
weierophinney Aug 20, 2012

The convention within the framework is to use "null === $this->listenersToDetach", or, if you despise yoda conditions, "$this->listenersToDetach === null", instead of is_null().

@weierophinney weierophinney commented on the diff Aug 20, 2012
library/Zend/Mvc/Controller/Plugin/Forward.php
+ * and class (the class of listener that should be detached).
+ *
+ * @return array
+ */
+ public function getListenersToDetach()
+ {
+ // If a blacklist has not been explicitly set, return the default:
+ if (is_null($this->listenersToDetach)) {
+ // We need to detach the InjectViewModelListener to prevent templates
+ // from getting attached to the ViewModel twice when a calling action
+ // returns the output generated by a forwarded action.
+ $this->listenersToDetach = array(
+ array(
+ 'id' => 'Zend\Stdlib\DispatchableInterface',
+ 'event' => MvcEvent::EVENT_DISPATCH,
+ 'class' => 'Zend\Mvc\View\Http\InjectViewModelListener'
@weierophinney
weierophinney Aug 20, 2012

Add a comma after last element in array (allows re-ordering or adding new items with minimal changeset in the VCS).

@weierophinney weierophinney added a commit that referenced this pull request Aug 20, 2012
@weierophinney weierophinney [#2206] consistency fixes
- Per comments on issue.
c6ea13d
@weierophinney
Zend Framework member

I took care of all suggestions above when merging.

@weierophinney weierophinney merged commit 0b8fc7a into zendframework:master Aug 20, 2012

1 check passed

Details default The Travis build passed
@ghost Unknown pushed a commit that referenced this pull request Jul 14, 2013
@weierophinney weierophinney [#2206] consistency fixes
- Per comments on issue.
a4ebab0
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment