[Bugfix] Check if container contains method before calling it

[boesing]

Do not blindly call getServiceLocator on ContainerInterface which must not implement that method.

[boesing]

@asgrim Could you have a look at this or probably ping someone who might be responsible?
This would allow to merge zendframework/zend-mvc#223

[mwillbanks]

Please resolve the issue of the interface and additionally please add a test that provides the existing failure condition.

[mwillbanks]

Generally, in this case, I would not suggest looking to see if the method exists but rather only checking against the ServiceLocatorAwareInterface as that is the interface that defines this definition rather than allowing duck typing of something that may not be following the interface. The interface gives us a contractual obligation.

[weierophinney]

This change is incorrect! That particular class should only ever be in play if zend-servicemanager v2 is in use, in which case we can depend on that method being present.

I think you've found an incorrect solution to the problem you've identified.

[boesing]

@weierophinney Exactly. And since zend-servicemanager v2 ServiceManager class does not have getServiceLocator()-Method (which got introduced as deprecated in v3, dunno why since there is/was no such method in v2 either), I get:

( ! ) Fatal error: Call to undefined method Zend\ServiceManager\ServiceManager::getServiceLocator() in vendor/zendframework/zend-form/src/FormElementManager/FormElementManagerV2Polyfill.php on line 194

Using the following versions:

• zendframework/zend-form: v2.10.0
• zendframework/zend-servicemanager: v2.7.8

[boesing]

I've reverted my changes and added a failing unit-test instead.

[Xerkus]

problem is it expects abstract plugin manager

[boesing]

So is the FormElementManager not meant to be used in other services which are created in the base ServiceManager?

I actually want to use the FormAnnotationBuilderFactory.
This factory is registered in the ServiceListenerFactory configuration, which configures the default ServiceManager.

So I could ensure that injectFactory is not being called from FormAnnotationBuilderFactory at all, or we just ensure that getServiceLocator is not being called blindly. I would prefer the latter.

[weierophinney]

@boesing — The following:

since zend-servicemanager v2 ServiceManager class does not have getServiceLocator()-Method

is irrelevant. The FormElementManager is an AbstractPluginManager instance, and under zend-servicemanager v2 those are all ServiceLocatorAwareInterface implementations, and define getServiceLocator().

Under zend-servicemanager v2, when a service is being pulled from a plugin manager, the $container passed to initializers, factories, etc. are all instances of the plugin_manager. The injectFactory() method you were changing is receiving the FormElementManager instance for the $container argument, and we thus need to pull its parent container (via getServiceLocator()) so we do things like retrieve the InputFilterManager.

Under zend-servicemanager v3, the $container passed to initializers and factories is the parent container; as such, we do not need to call getServiceLocator().

I've looked at the test case you wrote, and noted that it is not written correctly for zend-servicemanager v2; the second argument to injectFactory() should correctly be $this->manager, to mimic how initializers are executed for plugin managers under v2.

As such, this points to the core issue: the FormAnnotationBuilderFactory under 2.7 is not correct. The line you linked reads as follows:

$formElementManager->injectFactory($container, $annotationBuilder);

This is incorrect in two ways:

1. It is passing the arguments in v3 order (container, than builder). This is actually the source of the getServiceLocator() problems for you, as the arguments are in the wrong order, and it's attempting to pull the service locator from the $annotationBuilder argument!
2. It should be passing the $formElementManager and not the parent $container instance.

It should more correctly read:

method_exists($container, 'configure'))
    ? $formElementManager->injectFactory($container, $annotationBuilder) // zend-servicemanager v3
    : $formElementManager->injectFactory($annotationBuilder, $formElementManager); // zend-servicemanager v2

I'll submit a PR against the 2.7 branch shortly to do this.

[weierophinney]

@boesing I've just seen your zendframework/zend-mvc#223, and that does it correctly as well, so that'll be the one I merge.

[boesing]

I guess you meant zendframework/zend-mvc#223.

[weierophinney]

I guess you meant zendframework/zend-mvc#223.

Indeed. Edited my comment to reflect that. 😄