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

Test for v2-v3 PluginManager migration #81

Merged
merged 2 commits into from
Feb 1, 2016

Conversation

kynx
Copy link
Contributor

@kynx kynx commented Jan 24, 2016

I'm sure I'm doing something really, really dumb, but I'm trying to follow the migration docs and from what I've read this test should pass. Instead I get:

1) ZendTest\ServiceManager\AbstractPluginManagerTest::testV2v3PluginManager
Zend\ServiceManager\Exception\ServiceNotFoundException: An alias "ZendTest\ServiceManager\TestAsset\InvokableObject" was requested but no service could be found.

/Users/matt/www/zend-servicemanager/src/ServiceManager.php:551
/Users/matt/www/zend-servicemanager/src/AbstractPluginManager.php:161
/Users/matt/www/zend-servicemanager/test/AbstractPluginManagerTest.php:342

From what I can tell ServiceManager v2 is still looking up factories by their canonicalised name, which would work with:

$serviceManager->setAlias('foo', Foo::class);
$serviceManager->setFactory(Foo::class, Bar::class)

...but not when defined in the alias and factories properties.

Please tell me I'm going mad.

@Ocramius
Copy link
Member

Please tell me I'm going mad.

"I'm going mad."

Jokes apart, seems indeed to be the case :-\ https://travis-ci.org/zendframework/zend-servicemanager/jobs/104510819#L316

The entire aliases system in 2.x was quite of a mess. Is this issue just about the portability components?

@kynx kynx changed the title Added test for V2v3PluginManager v2-v3 PluginManager migration test failure Jan 24, 2016
@kynx
Copy link
Contributor Author

kynx commented Jan 24, 2016

Yes - just trying to follow the docs.

Well, I can see two solutions - migrators alias the 'namespaceclassname' as well as the full name, or SM looks up factories twice. As someone doing migrations I know which I'd prefer 😉

@kynx kynx mentioned this pull request Jan 25, 2016
@svycka
Copy link
Contributor

svycka commented Jan 25, 2016

@kynx, Foo::class this never worked when defined in the alias and factories properties.
related: #50
but unfortunately some zf2 components indeed trying to use this and it partially works because of auto invocables $sm->has(Foo::class) does not work, but$sm->get(Foo::class) works, because of autoInvocable.

@kynx
Copy link
Contributor Author

kynx commented Jan 25, 2016

OK, think I've found the magic sauce. In v2.7 and v3, the plugin manager is never configured. If I override the constructor to do that, it works:

    public function __construct($configInstanceOrParentLocator = null, array $config = [])
    {
        parent::__construct($configInstanceOrParentLocator, $config);
        $config = new Config(['aliases' => $this->aliases, 'factories' => $this->factories]);
        $config->configureServiceManager($this);
    }

In v3, it would be something like:

    public function __construct($configInstanceOrParentLocator = null, array $config = [])
    {
        parent::__construct($configInstanceOrParentLocator, $config);
        $this->configure(['aliases' => $this->aliases, 'factories' => $this->factories]);
    }

Should that be happening by default in AbstractPluginManager?

@kynx
Copy link
Contributor Author

kynx commented Jan 25, 2016

Scratch the above - I've added the canonicalized entry for the factory to the test and it passes. Yay!

Now if only it worked as well in v3. See #87

@kynx kynx changed the title v2-v3 PluginManager migration test failure Test for v2-v3 PluginManager migration Jan 25, 2016
@weierophinney weierophinney added this to the 2.7.5 milestone Feb 1, 2016
@weierophinney weierophinney self-assigned this Feb 1, 2016
@weierophinney weierophinney merged commit 0926f42 into zendframework:release-2.7 Feb 1, 2016
weierophinney added a commit that referenced this pull request Feb 1, 2016
Test for v2-v3 PluginManager migration
weierophinney added a commit that referenced this pull request Feb 1, 2016
weierophinney added a commit that referenced this pull request Feb 1, 2016
@kynx kynx deleted the v2v3-compat-test branch February 2, 2016 00:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants