Skip to content
This repository has been archived by the owner on Jan 8, 2020. It is now read-only.

Remove SM-Aware requirement from Forward plugin #4330

Merged
merged 3 commits into from
Apr 30, 2013
Merged

Remove SM-Aware requirement from Forward plugin #4330

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
96ea4b7
[#4314] Inject ControllerManager in Forward plugin
weierophinney Apr 26, 2013
3c0ce99
[#4330] Constructor injection
weierophinney Apr 26, 2013
2917ef0
Incorporate feedback
weierophinney Apr 30, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 15 additions & 52 deletions library/Zend/Mvc/Controller/Plugin/Forward.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,25 +10,23 @@
namespace Zend\Mvc\Controller\Plugin;

use Zend\EventManager\SharedEventManagerInterface as SharedEvents;
use Zend\Mvc\Controller\ControllerManager;
use Zend\Mvc\Exception;
use Zend\Mvc\InjectApplicationEventInterface;
use Zend\Mvc\MvcEvent;
use Zend\Mvc\Router\RouteMatch;
use Zend\ServiceManager\ServiceLocatorAwareInterface;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\Stdlib\DispatchableInterface as Dispatchable;

class Forward extends AbstractPlugin
{
/**
* @var MvcEvent
* @var ControllerManager
*/
protected $event;
protected $controllers;

/**
* @var ServiceLocatorInterface
* @var MvcEvent
*/
protected $locator;
protected $event;

/**
* @var int
Expand All @@ -45,6 +43,14 @@ class Forward extends AbstractPlugin
*/
protected $listenersToDetach = null;

/**
* @param ControllerManager $controllers
*/
public function __construct(ControllerManager $controllers)
{
$this->controllers = $controllers;
}

/**
* Set maximum number of nested forwards allowed
*
Expand Down Expand Up @@ -98,7 +104,7 @@ public function setListenersToDetach($listeners)
/**
* Dispatch another controller
*
* @param string $name Controller name; either a class name or an alias used in the DI container or service locator
* @param string $name Controller name; either a class name or an alias used in the controller manager
* @param null|array $params Parameters with which to seed a custom RouteMatch object for the new controller
* @return mixed
* @throws Exception\DomainException if composed controller does not define InjectApplicationEventInterface
Expand All @@ -107,28 +113,11 @@ public function setListenersToDetach($listeners)
public function dispatch($name, array $params = null)
{
$event = clone($this->getEvent());
$locator = $this->getLocator();
$scoped = false;

// Use the controller loader when possible
if ($locator->has('ControllerLoader')) {
$locator = $locator->get('ControllerLoader');
$scoped = true;
}

$controller = $locator->get($name);
if (!$controller instanceof Dispatchable) {
throw new Exception\DomainException('Can only forward to DispatchableInterface classes; class of type ' . get_class($controller) . ' received');
}
$controller = $this->controllers->get($name);
if ($controller instanceof InjectApplicationEventInterface) {
$controller->setEvent($event);
}
if (!$scoped) {
if ($controller instanceof ServiceLocatorAwareInterface) {
$controller->setServiceLocator($locator);
}
}


// Allow passing parameters to seed the RouteMatch with & copy matched route name
if ($params !== null) {
Expand All @@ -137,7 +126,6 @@ public function dispatch($name, array $params = null)
$event->setRouteMatch($routeMatch);
}


if ($this->numNestedForwards > $this->maxNestedForwards) {
throw new Exception\DomainException("Circular forwarding detected: greater than $this->maxNestedForwards nested forwards");
}
Expand Down Expand Up @@ -222,31 +210,6 @@ protected function reattachProblemListeners(SharedEvents $sharedEvents, array $l
}
}

/**
* Get the locator
*
* @return ServiceLocatorInterface
* @throws Exception\DomainException if unable to find locator
*/
protected function getLocator()
{
if ($this->locator) {
return $this->locator;
}

$controller = $this->getController();

if (!$controller instanceof ServiceLocatorAwareInterface) {
throw new Exception\DomainException('Forward plugin requires controller implements ServiceLocatorAwareInterface');
}
$locator = $controller->getServiceLocator();
if (!$locator instanceof ServiceLocatorInterface) {
throw new Exception\DomainException('Forward plugin requires controller composes Locator');
}
$this->locator = $locator;
return $this->locator;
}

/**
* Get the event
*
Expand Down
46 changes: 46 additions & 0 deletions library/Zend/Mvc/Controller/Plugin/Service/ForwardFactory.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
<?php
/**
* Zend Framework (http://framework.zend.com/)
*
* @link http://github.com/zendframework/zf2 for the canonical source repository
* @copyright Copyright (c) 2005-2013 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/

namespace Zend\Mvc\Controller\Plugin\Service;

use Zend\ServiceManager\Exception\ServiceNotCreatedException;
use Zend\ServiceManager\FactoryInterface;
use Zend\ServiceManager\ServiceLocatorInterface;
use Zend\Mvc\Controller\Plugin\Forward;

class ForwardFactory implements FactoryInterface
{
/**
* {@inheritDoc}
*
* @return Forward
* @throws ServiceNotCreatedException if ControllerLoader service is not found in application service locator
*/
public function createService(ServiceLocatorInterface $plugins)
{
$services = $plugins->getServiceLocator();
if (!$services instanceof ServiceLocatorInterface) {
throw new ServiceNotCreatedException(sprintf(
'%s requires that the application service manager has been injected; none found',
__CLASS__
));
}

if (!$services->has('ControllerLoader')) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling has or directly getting the ControllerLoader makes no real difference in terms of exception clarity, no?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except that it allows us to give a more detailed and specific exception message.

throw new ServiceNotCreatedException(sprintf(
'%s requires that the application service manager contains a "%s" service; none found',
__CLASS__,
'ControllerLoader'
));
}
$controllers = $services->get('ControllerLoader');

return new Forward($controllers);
}
}
2 changes: 1 addition & 1 deletion library/Zend/Mvc/Controller/PluginManager.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class PluginManager extends AbstractPluginManager
* @var array
*/
protected $factories = array(
'forward' => 'Zend\Mvc\Controller\Plugin\Service\ForwardFactory',
'identity' => 'Zend\Mvc\Controller\Plugin\Service\IdentityFactory',
);

Expand All @@ -40,7 +41,6 @@ class PluginManager extends AbstractPluginManager
'acceptableviewmodelselector' => 'Zend\Mvc\Controller\Plugin\AcceptableViewModelSelector',
'filepostredirectget' => 'Zend\Mvc\Controller\Plugin\FilePostRedirectGet',
'flashmessenger' => 'Zend\Mvc\Controller\Plugin\FlashMessenger',
'forward' => 'Zend\Mvc\Controller\Plugin\Forward',
'layout' => 'Zend\Mvc\Controller\Plugin\Layout',
'params' => 'Zend\Mvc\Controller\Plugin\Params',
'postredirectget' => 'Zend\Mvc\Controller\Plugin\PostRedirectGet',
Expand Down
56 changes: 34 additions & 22 deletions tests/ZendTest/Mvc/Controller/Plugin/ForwardTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
use Zend\EventManager\StaticEventManager;
use Zend\Http\Request;
use Zend\Http\Response;
use Zend\Mvc\Controller\ControllerManager;
use Zend\Mvc\Controller\PluginManager;
use Zend\Mvc\Controller\Plugin\Forward as ForwardPlugin;
use Zend\Mvc\MvcEvent;
use Zend\Mvc\Router\RouteMatch;
Expand Down Expand Up @@ -46,14 +48,37 @@ public function setUp()
$routeMatch->setMatchedRouteName('some-route');
$event->setRouteMatch($routeMatch);

$locator = new Locator;
$locator->add('forward', function () {
return new ForwardController();
$services = new Locator();
$plugins = $this->plugins = new PluginManager();
$plugins->setServiceLocator($services);

$controllers = $this->controllers = new ControllerManager();
$controllers->setFactory('forward', function () use ($plugins) {
$controller = new ForwardController();
$controller->setPluginManager($plugins);
return $controller;
});
$controllers->setServiceLocator($services);
$services->add('ControllerLoader', function () use ($controllers) {
return $controllers;
});
$services->add('ControllerPluginManager', function () use ($plugins) {
return $plugins;
});
$services->add('Zend\ServiceManager\ServiceLocatorInterface', function () use ($services) {
return $services;
});
$services->add('EventManager', function () use ($mockEventManager) {
return $mockEventManager;
});
$services->add('SharedEventManager', function () use ($mockSharedEventManager) {
return $mockSharedEventManager;
});

$this->controller = new SampleController();
$this->controller->setEvent($event);
$this->controller->setServiceLocator($locator);
$this->controller->setServiceLocator($services);
$this->controller->setPluginManager($plugins);

$this->plugin = $this->controller->plugin('forward');
}
Expand All @@ -66,28 +91,17 @@ public function tearDown()
public function testPluginWithoutEventAwareControllerRaisesDomainException()
{
$controller = new UneventfulController();
$plugin = new ForwardPlugin();
$plugin = new ForwardPlugin($this->controllers);
$plugin->setController($controller);
$this->setExpectedException('Zend\Mvc\Exception\DomainException', 'InjectApplicationEventInterface');
$plugin->dispatch('forward');
}

public function testPluginWithoutLocatorAwareControllerRaisesDomainException()
{
$controller = new UnlocatableEventfulController();
$controller->setEvent($this->controller->getEvent());
$plugin = new ForwardPlugin();
$plugin->setController($controller);
$this->setExpectedException('Zend\Mvc\Exception\DomainException', 'implements ServiceLocatorAwareInterface');
$plugin->dispatch('forward');
}

public function testPluginWithoutControllerLocatorRaisesDomainException()
public function testPluginWithoutControllerLocatorRaisesServiceNotCreatedException()
{
$controller = new SampleController();
$this->setExpectedException('Zend\ServiceManager\Exception\ServiceNotCreatedException');
$plugin = $controller->plugin('forward');
$this->setExpectedException('Zend\Mvc\Exception\DomainException', 'composes Locator');
$plugin->dispatch('forward');
}

public function testDispatchRaisesDomainExceptionIfDiscoveredControllerIsNotDispatchable()
Expand All @@ -96,17 +110,15 @@ public function testDispatchRaisesDomainExceptionIfDiscoveredControllerIsNotDisp
$locator->add('bogus', function () {
return new stdClass;
});
$this->setExpectedException('Zend\Mvc\Exception\DomainException', 'DispatchableInterface');
$this->setExpectedException('Zend\ServiceManager\Exception\ServiceNotFoundException');
$this->plugin->dispatch('bogus');
}

public function testDispatchRaisesDomainExceptionIfCircular()
{
$this->setExpectedException('Zend\Mvc\Exception\DomainException', 'Circular forwarding');
$sampleController = $this->controller;
$sampleController->getServiceLocator()->add('sample', function () use ($sampleController) {
return $sampleController;
});
$this->controllers->setService('sample', $sampleController);
$this->plugin->dispatch('sample', array('action' => 'test-circular'));
}

Expand Down