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

Changes for zend-servicemanager v3 #17

Merged
merged 14 commits into from
Feb 18, 2016
Merged

Conversation

kynx
Copy link
Contributor

@kynx kynx commented Jan 22, 2016

Re #15: against develop this time!

According to https://github.com/zendframework/maintainers/wiki/ZF3-ServiceManager-component-refactors,-phase-2 I'd be throwing an InvalidServiceException from validate() but that breaks tests. Is this the intention for v2?

@svycka
Copy link

svycka commented Jan 26, 2016

@kynx this is because of bug in hydrator module. validatePlugin() should throw Zend\ServiceManager\Exception\RuntimeException as said in https://github.com/zendframework/zend-servicemanager/blob/release-2.7/src/AbstractPluginManager.php#L126 but instead throws Zend\Hydrator\Exception\RuntimeException.

If not then this will be BC break.

@kynx
Copy link
Contributor Author

kynx commented Jan 26, 2016

Thanks @svychka. I've got some changes ready to push that ensure the tests pass in both v2 and v3, but am waiting for zendframework/zend-servicemanager#87 so I can use my shiny new CommonPluginManagerTrait for testing :)

@svycka
Copy link

svycka commented Jan 27, 2016

I don't think it is good idea to throw different exceptions with differrent SM versions. But lets see what ZF team will say

@svycka svycka mentioned this pull request Feb 3, 2016
@weierophinney weierophinney added this to the 2.1.0 milestone Feb 3, 2016
@weierophinney weierophinney self-assigned this Feb 3, 2016
@kynx
Copy link
Contributor Author

kynx commented Feb 6, 2016

@weierophinney Hopefully this one's good to go.

// Assume that this factory is registered with the HydratorManager,
// and just pass it directly on.
return new DelegatingHydrator($serviceLocator);
return new DelegatingHydrator($container);
Copy link
Member

Choose a reason for hiding this comment

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

There's a subtle issue here: with current v2, the container passed to the factory will be the HydratorPluginManager instance, while with v3, it will be the parent container. In the latter case, DelegatingHydrator usage breaks, as it will be attempting to fetch hydrators from the container, but the container will be the application-level container!

Which poses a quandary: how do we get access to the HydratorPluginManager? My suggestion is:

  • Check to see if $container is a HydratorPluginManager instance; if so, done.
  • Check to see if $container->has(HydratorPluginManager::class); if so, pull that, and done.
  • Check to see if $container->has('HydratorManager') (the name used by zend-mvc); if so, pull that, and done.
  • Finally, if all the above fail, create a new instance of HydratorPluginManager and use that.

I'll make that change on merge.

weierophinney added a commit to weierophinney/zend-hydrator that referenced this pull request Feb 18, 2016
Changes for zend-servicemanager v3
@weierophinney weierophinney merged commit 7c52866 into zendframework:develop Feb 18, 2016
weierophinney added a commit that referenced this pull request Feb 18, 2016
weierophinney added a commit that referenced this pull request Feb 18, 2016
@samsonasik samsonasik mentioned this pull request Jul 16, 2016
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants