Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

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

Merged
merged 1 commit into from

3 participants

Demian Katz Don't Add Me To Your Organization a.k.a The Travis Bot Matthew Weier O'Phinney
Demian Katz

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.

Demian Katz 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
Don't Add Me To Your Organization a.k.a The Travis Bot

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

Matthew Weier O'Phinney weierophinney commented on the diff
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)) {
Matthew Weier O'Phinney Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Matthew Weier O'Phinney weierophinney commented on the diff
library/Zend/Mvc/Controller/Plugin/Forward.php
((10 lines not shown))
+ * 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'
Matthew Weier O'Phinney Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Matthew Weier O'Phinney weierophinney referenced this pull request from a commit
Matthew Weier O'Phinney weierophinney [#2206] consistency fixes
- Per comments on issue.
c6ea13d
Matthew Weier O'Phinney

I took care of all suggestions above when merging.

Matthew Weier O'Phinney weierophinney merged commit 0b8fc7a into from
Deleted user Unknown referenced this pull request from a commit
Matthew Weier O'Phinney 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
Commits on Aug 20, 2012
  1. Demian Katz

    If a caller returns the value of $this->forward()->dispatch() in a co…

    demiankatz authored
    …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.
This page is out of date. Refresh to see the latest.
118 library/Zend/Mvc/Controller/Plugin/Forward.php
View
@@ -10,6 +10,7 @@
namespace Zend\Mvc\Controller\Plugin;
+use Zend\EventManager\SharedEventManagerInterface as SharedEvents;
use Zend\Mvc\Exception;
use Zend\Mvc\InjectApplicationEventInterface;
use Zend\Mvc\MvcEvent;
@@ -46,6 +47,11 @@ class Forward extends AbstractPlugin
protected $numNestedForwards = 0;
/**
+ * @var array
+ */
+ protected $listenersToDetach = null;
+
+ /**
* Set maximum number of nested forwards allowed
*
* @param int $maxNestedForwards
@@ -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)) {
Matthew Weier O'Phinney Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ // 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'
Matthew Weier O'Phinney Owner

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ )
+ );
+ }
+ return $this->listenersToDetach;
+ }
+
+ /**
+ * Set information on listeners that need to be detached before dispatching.
+ *
+ * @param array $listeners Listener information; see getListenersToDetach() for details on format.
+ * @return void
+ */
+ public function setListenersToDetach($listeners)
+ {
+ $this->listenersToDetach = $listeners;
+ }
+
+ /**
* Dispatch another controller
*
* @param string $name Controller name; either a class name or an alias used in the DI container or service locator
@@ -101,14 +147,86 @@ public function dispatch($name, array $params = null)
}
$this->numNestedForwards++;
+ // Detach listeners that may cause problems during dispatch:
+ $sharedEvents = $event->getApplication()->getEventManager()->getSharedManager();
+ $listeners = $this->detachProblemListeners($sharedEvents);
+
$return = $controller->dispatch($event->getRequest(), $event->getResponse());
+ // If we detached any listeners, reattach them now:
+ $this->reattachProblemListeners($sharedEvents, $listeners);
+
$this->numNestedForwards--;
return $return;
}
/**
+ * Detach problem listeners specified by getListenersToDetach() and return an array of information that will
+ * allow them to be reattached.
+ *
+ * @param SharedEvents $sharedEvents Shared event manager
+ * @return array
+ */
+ protected function detachProblemListeners(SharedEvents $sharedEvents)
+ {
+ // Convert the problem list from two-dimensional array to more convenient id => event => class format:
+ $formattedProblems = array();
+ foreach ($this->getListenersToDetach() as $current) {
+ if (!isset($formattedProblems[$current['id']])) {
+ $formattedProblems[$current['id']] = array();
+ }
+ if (!isset($formattedProblems[$current['id']][$current['event']])) {
+ $formattedProblems[$current['id']][$current['event']] = array();
+ }
+ $formattedProblems[$current['id']][$current['event']][] = $current['class'];
+ }
+
+ // Loop through the class blacklist, detaching problem events and remembering their CallbackHandlers
+ // for future reference:
+ $results = array();
+ foreach ($formattedProblems as $id => $eventArray) {
+ $results[$id] = array();
+ foreach ($eventArray as $eventName => $classArray) {
+ $results[$id][$eventName] = array();
+ $events = $sharedEvents->getListeners($id, $eventName);
+ foreach ($events as $currentEvent) {
+ $currentCallback = $currentEvent->getCallback();
+ if (!isset($currentCallback[0])) {
+ continue;
+ }
+ foreach ($classArray as $class) {
+ if (is_a($currentCallback[0], $class)) {
+ $sharedEvents->detach($id, $currentEvent);
+ $results[$id][$eventName][] = $currentEvent;
+ }
+ }
+ }
+ }
+ }
+
+ return $results;
+ }
+
+ /**
+ * Reattach all problem listeners detached by detachProblemListeners(), if any.
+ *
+ * @param SharedEvents $sharedEvents Shared event manager
+ * @param array $listeners Output of detachProblemListeners()
+ * @return void
+ */
+ protected function reattachProblemListeners(SharedEvents $sharedEvents, array $listeners)
+ {
+ foreach ($listeners as $id => $eventArray) {
+ foreach ($eventArray as $eventName => $callbacks) {
+ foreach ($callbacks as $current) {
+ $sharedEvents->attach($id, $eventName, $current->getCallback(), $current->getMetadatum('priority'));
+ }
+ }
+ }
+ }
+
+ /**
* Get the locator
*
* @return ServiceLocatorInterface
9 tests/ZendTest/Mvc/Controller/Plugin/ForwardTest.php
View
@@ -29,7 +29,16 @@ class ForwardTest extends TestCase
public function setUp()
{
StaticEventManager::resetInstance();
+
+ $mockSharedEventManager = $this->getMock('Zend\EventManager\SharedEventManagerInterface');
+ $mockSharedEventManager->expects($this->any())->method('getListeners')->will($this->returnValue(array()));
+ $mockEventManager = $this->getMock('Zend\EventManager\EventManagerInterface');
+ $mockEventManager->expects($this->any())->method('getSharedManager')->will($this->returnValue($mockSharedEventManager));
+ $mockApplication = $this->getMock('Zend\Mvc\ApplicationInterface');
+ $mockApplication->expects($this->any())->method('getEventManager')->will($this->returnValue($mockEventManager));
+
$event = new MvcEvent();
+ $event->setApplication($mockApplication);
$event->setRequest(new Request());
$event->setResponse(new Response());
$event->setRouteMatch(new RouteMatch(array('action' => 'test')));
Something went wrong with that request. Please try again.