From d645a6538ad792f43b89b9c2318ad89aa180e245 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 18 Feb 2016 09:05:46 -0600 Subject: [PATCH 1/5] Ensure Navigation helper PluginManager is BC Adds tests similar to those for the `HelperPluginManager` to ensure that the first argument to the `Navigation\PluginManager` is optional and/or accepts a `Zend\ServiceManager\Config` instance, and updates the constructor to work with both v2 and v3. --- src/Helper/Navigation/PluginManager.php | 11 ++-- .../PluginManagerCompatibilityTest.php | 51 +++++++++++++++++++ 2 files changed, 57 insertions(+), 5 deletions(-) create mode 100644 test/Helper/Navigation/PluginManagerCompatibilityTest.php diff --git a/src/Helper/Navigation/PluginManager.php b/src/Helper/Navigation/PluginManager.php index b48c472e..86c6aa3c 100644 --- a/src/Helper/Navigation/PluginManager.php +++ b/src/Helper/Navigation/PluginManager.php @@ -52,19 +52,20 @@ class PluginManager extends HelperPluginManager ]; /** - * @param ContainerInterface $container - * @param array $config + * @param null|ConfigInterface|ContainerInterface $configOrContainerInstance + * @param array $v3config If $configOrContainerInstance is a container, this + * value will be passed to the parent constructor. */ - public function __construct(ContainerInterface $container, array $config = []) + public function __construct($configOrContainerInstance = null, array $v3config = []) { $this->initializers[] = function ($container, $instance) { if (! $instance instanceof AbstractHelper) { - continue; + return; } $instance->setServiceLocator($container); }; - parent::__construct($container, $config); + parent::__construct($configOrContainerInstance, $v3config); } } diff --git a/test/Helper/Navigation/PluginManagerCompatibilityTest.php b/test/Helper/Navigation/PluginManagerCompatibilityTest.php new file mode 100644 index 00000000..c35f84ab --- /dev/null +++ b/test/Helper/Navigation/PluginManagerCompatibilityTest.php @@ -0,0 +1,51 @@ +helpers = new PluginManager(new ServiceManager()); + } + + /** + * @group 43 + */ + public function testConstructorArgumentsAreOptionalUnderV2() + { + if (method_exists($this->helpers, 'configure')) { + $this->markTestSkipped('zend-servicemanager v3 plugin managers require a container argument'); + } + + $helpers = new PluginManager(); + $this->assertInstanceOf(PluginManager::class, $helpers); + } + + /** + * @group 43 + */ + public function testConstructorAllowsConfigInstanceAsFirstArgumentUnderV2() + { + if (method_exists($this->helpers, 'configure')) { + $this->markTestSkipped('zend-servicemanager v3 plugin managers require a container argument'); + } + + $helpers = new PluginManager(new Config([])); + $this->assertInstanceOf(PluginManager::class, $helpers); + } +} From 263aba6c0f93295b07b3b86ff0449a0749d4f657 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 18 Feb 2016 09:23:09 -0600 Subject: [PATCH 2/5] Full compatibility test for Navigation PluginManager Implements a full compatibility test for the navigation plugin manager. --- src/Helper/Navigation/PluginManager.php | 7 +++++ .../PluginManagerCompatibilityTest.php | 27 +++++++++++++++---- test/HelperPluginManagerCompatibilityTest.php | 2 +- 3 files changed, 30 insertions(+), 6 deletions(-) diff --git a/src/Helper/Navigation/PluginManager.php b/src/Helper/Navigation/PluginManager.php index 86c6aa3c..4f0d3490 100644 --- a/src/Helper/Navigation/PluginManager.php +++ b/src/Helper/Navigation/PluginManager.php @@ -49,6 +49,13 @@ class PluginManager extends HelperPluginManager Links::class => InvokableFactory::class, Menu::class => InvokableFactory::class, Sitemap::class => InvokableFactory::class, + + // v2 canonical FQCNs + + 'zendviewhelpernavigationbreadcrumbs' => InvokableFactory::class, + 'zendviewhelpernavigationlinks' => InvokableFactory::class, + 'zendviewhelpernavigationmenu' => InvokableFactory::class, + 'zendviewhelpernavigationsitemap' => InvokableFactory::class, ]; /** diff --git a/test/Helper/Navigation/PluginManagerCompatibilityTest.php b/test/Helper/Navigation/PluginManagerCompatibilityTest.php index c35f84ab..28454ab4 100644 --- a/test/Helper/Navigation/PluginManagerCompatibilityTest.php +++ b/test/Helper/Navigation/PluginManagerCompatibilityTest.php @@ -7,10 +7,13 @@ * @license http://framework.zend.com/license/new-bsd New BSD License */ -namespace ZendTest\View; +namespace ZendTest\View\Helper\Navigation; use Zend\ServiceManager\Config; use Zend\ServiceManager\ServiceManager; +use Zend\ServiceManager\Test\CommonPluginManagerTrait; +use Zend\View\Exception\InvalidHelperException; +use Zend\View\Helper\Navigation\AbstractHelper; use Zend\View\Helper\Navigation\PluginManager; /** @@ -18,9 +21,21 @@ */ class PluginManagerCompatibilityTest extends \PHPUnit_Framework_TestCase { - public function setUp() + use CommonPluginManagerTrait; + + protected function getPluginManager() + { + return new PluginManager(new ServiceManager()); + } + + protected function getV2InvalidPluginException() + { + return InvalidHelperException::class; + } + + protected function getInstanceOf() { - $this->helpers = new PluginManager(new ServiceManager()); + return AbstractHelper::class; } /** @@ -28,7 +43,8 @@ public function setUp() */ public function testConstructorArgumentsAreOptionalUnderV2() { - if (method_exists($this->helpers, 'configure')) { + $helpers = $this->getPluginManager(); + if (method_exists($helpers, 'configure')) { $this->markTestSkipped('zend-servicemanager v3 plugin managers require a container argument'); } @@ -41,7 +57,8 @@ public function testConstructorArgumentsAreOptionalUnderV2() */ public function testConstructorAllowsConfigInstanceAsFirstArgumentUnderV2() { - if (method_exists($this->helpers, 'configure')) { + $helpers = $this->getPluginManager(); + if (method_exists($helpers, 'configure')) { $this->markTestSkipped('zend-servicemanager v3 plugin managers require a container argument'); } diff --git a/test/HelperPluginManagerCompatibilityTest.php b/test/HelperPluginManagerCompatibilityTest.php index 8034eed3..786c8d79 100644 --- a/test/HelperPluginManagerCompatibilityTest.php +++ b/test/HelperPluginManagerCompatibilityTest.php @@ -20,7 +20,7 @@ use Zend\ServiceManager\ServiceManager; use Zend\ServiceManager\Test\CommonPluginManagerTrait; -class PluginManagerCompatibilityTest extends TestCase +class HelperPluginManagerCompatibilityTest extends TestCase { use CommonPluginManagerTrait; From 50ccd44f6b6883021ab1ef5fe6e671bd1e8650b4 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 18 Feb 2016 10:29:12 -0600 Subject: [PATCH 3/5] Ensure injectTranslator returns early if no parent container found Adds a test for a condition reported by @webimpress, detailing expected behavior if the plugin manager does not have a parent locator when `injectTranslator()` is called (should return null, not error), and adds a conditional into `injectTranslator()` to return early if no container is available. --- src/HelperPluginManager.php | 33 +++++++++++++++++++++++--------- test/HelperPluginManagerTest.php | 12 ++++++++++++ 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/src/HelperPluginManager.php b/src/HelperPluginManager.php index a8a6a470..200ddb50 100644 --- a/src/HelperPluginManager.php +++ b/src/HelperPluginManager.php @@ -273,16 +273,18 @@ public function getRenderer() /** * Inject a helper instance with the registered renderer * - * @param $first - * @param $second + * @param ContainerInterface|Helper\HelperInterface $first helper instance + * under zend-servicemanager v2, ContainerInterface under v3. + * @param ContainerInterface|Helper\HelperInterface $second + * ContainerInterface under zend-servicemanager v3, helper instance + * under v2. Ignored regardless. */ public function injectRenderer($first, $second) { - if ($first instanceof ContainerInterface) { - $helper = $second; - } else { - $helper = $first; - } + $helper = ($first instanceof ContainerInterface) + ? $second + : $first; + $renderer = $this->getRenderer(); if (null === $renderer) { return; @@ -293,22 +295,35 @@ public function injectRenderer($first, $second) /** * Inject a helper instance with the registered translator * - * @param $first - * @param $second + * @param ContainerInterface|Helper\HelperInterface $first helper instance + * under zend-servicemanager v2, ContainerInterface under v3. + * @param ContainerInterface|Helper\HelperInterface $second + * ContainerInterface under zend-servicemanager v3, helper instance + * under v2. Ignored regardless. */ public function injectTranslator($first, $second) { if ($first instanceof ContainerInterface) { + // v3 usage $container = $first; $helper = $second; } else { + // v2 usage; grab the parent container $container = $second->getServiceLocator(); $helper = $first; } + if (! $helper instanceof TranslatorAwareInterface) { return; } + if (! $container) { + // Under zend-navigation v2.5, the navigation PluginManager is + // always lazy-loaded, which means it never has a parent + // container. + return; + } + if ($container->has('MvcTranslator')) { $helper->setTranslator($container->get('MvcTranslator')); return; diff --git a/test/HelperPluginManagerTest.php b/test/HelperPluginManagerTest.php index 251d078f..a17bbda8 100644 --- a/test/HelperPluginManagerTest.php +++ b/test/HelperPluginManagerTest.php @@ -16,6 +16,7 @@ use Zend\ServiceManager\ServiceManager; use Zend\View\Exception\InvalidHelperException; use Zend\View\HelperPluginManager; +use Zend\View\Helper\HeadTitle; use Zend\View\Helper\HelperInterface; use Zend\View\Helper\Url; use Zend\View\Renderer\PhpRenderer; @@ -175,6 +176,17 @@ public function testIfHelperIsTranslatorAwareAndBothMvcTranslatorAndTranslatorAr $this->assertSame($translator, $helper->getTranslator()); } + /** + * @group 47 + */ + public function testInjectTranslatorWillReturnEarlyIfThePluginManagerDoesNotHaveAParentContainer() + { + $helpers = new HelperPluginManager(); + $helper = new HeadTitle(); + $this->assertNull($helpers->injectTranslator($helper, $helpers)); + $this->assertNull($helper->getTranslator()); + } + public function testCanOverrideAFactoryViaConfigurationPassedToConstructor() { $helper = $this->prophesize(HelperInterface::class)->reveal(); From 8c45bed8e44e4b9c04359395ca5bf5a1f4bd6c57 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 18 Feb 2016 10:33:36 -0600 Subject: [PATCH 4/5] Conditionally exclude new test ... when testing against zend-servicemanager v3. --- test/HelperPluginManagerTest.php | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/test/HelperPluginManagerTest.php b/test/HelperPluginManagerTest.php index a17bbda8..9e08eb15 100644 --- a/test/HelperPluginManagerTest.php +++ b/test/HelperPluginManagerTest.php @@ -181,6 +181,12 @@ public function testIfHelperIsTranslatorAwareAndBothMvcTranslatorAndTranslatorAr */ public function testInjectTranslatorWillReturnEarlyIfThePluginManagerDoesNotHaveAParentContainer() { + if (method_exists($this->helpers, 'configure')) { + $this->markTestSkipped( + 'Skip test when testing against zend-servicemanager v3, as that implementation ' + . 'guarantees a parent container in plugin managers' + ); + } $helpers = new HelperPluginManager(); $helper = new HeadTitle(); $this->assertNull($helpers->injectTranslator($helper, $helpers)); From a1228db74fde52682fc9ede8a3f6808e4dcc0e7c Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Thu, 18 Feb 2016 10:44:10 -0600 Subject: [PATCH 5/5] Added CHANGELOG for #47 --- CHANGELOG.md | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 527867b1..5742771f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ All notable changes to this project will be documented in this file, in reverse chronological order by release. -## 2.6.2 - TBD +## 2.6.2 - 2016-02-18 ### Added @@ -18,7 +18,15 @@ All notable changes to this project will be documented in this file, in reverse ### Fixed -- Nothing. +- [#47](https://github.com/zendframework/zend-view/pull/47) fixes + `Navigation\PluginManager` to ensure it is backwards compatible + with zend-servicemanager v2, including: + - fixing the constructor to be BC with v2 and forwards-compatible with v3. + - adding additional, normalized alias/factory pairs. +- [#47](https://github.com/zendframework/zend-view/pull/47) fixes + the behavior of `HelperPluginManager::injectTranslator()` to return + early if no container is provided (fixing an issue with navigation + helpers introduced in 2.6.0). ## 2.6.1 - 2016-02-18