Added FQCN aliases to `ServiceListenerFactory` so some could use e.g. ReflectionBasedAbstractFactories

[weierophinney]

The ServiceListenerFactory is actually missing some aliases. I've added the missing ones and fixed the old unit tests which provided non-matching configuration to the unit test either.

For ZF4 we should consider switching the FQCN to the factories and moving the old strings to the aliases configuration.

---

Originally posted by @boesing at https://github.com/zendframework/zend-mvc/pull/294

[weierophinney]

@boesing I've started a PR for the latter a month ago https://github.com/zendframework/zend-mvc/pull/292

However, I'm not sure if it would ever be merged, because @Xerkus has another PR for zend-mvc v4, which already does that, by defining a ConfigProvider. Link to the PR: https://github.com/zendframework/zend-mvc/pull/259 Link to the config provider: https://github.com/zendframework/zend-mvc/blob/bf93aa5e64e688a4f2bc7a50679c152bac289cae/src/ConfigProvider.php

---

Originally posted by @thexpand at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-420921708

[weierophinney]

@thexpand Oh, didn't noticed. However, your changes supposed to be BC Break since you are moving the factories to alias and vice versa. I would prefer having a working logic for ZF3 and I'm not sure if there is already any ZF4 ETA at all?

---

Originally posted by @boesing at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-420925034

[weierophinney]

There is no ZF3, actually. There won't be ZF4. Zend Framework is no longer a monolithic framework, but rather a component-based one, as it is now divided into separate repositories, each with its own versioning (i.e. zend-mvc could be 3.1.1, but zend-form may be on 5.0.0).

So I guess you are talking about v4 of zend-mvc. You can track down @Xerkus 's work on the PR, as well as this thread on the forums: https://discourse.zendframework.com/t/rfc-zend-mvc-4-design-changes/447 Honestly, I don't know what the status of zend-mvc v4 is, but once it is ready, I don't even think we would need the aliases anymore. The release will introduce many BC Breaks, so I don't see a problem removing the aliases at all.

The only benefit from having class constants over service strings is to let developers prepare for the upcoming zend-mvc v4 - in other words to provide a transitioning step between the current version and the new major version. I would like to see what the others think, too, so that we can decide which PR to use (if we are going to do it in the first place).

PS: @boesing you have wasted some time doing the same I did in my PR.

/cc @Xerkus @froschdesign @xtreamwayz @weierophinney

---

Originally posted by @thexpand at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-420927946

[weierophinney]

@boesing

However, your changes supposed to be BC Break since you are moving the factories to alias and vice versa.

Can you explain where do you see a BC break? Thanks!

---

Originally posted by @froschdesign at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-420933239

[weierophinney]

@froschdesign I've created a unit test to show what I mean:

Actually, we have this configuration:

    public function testDelegatorIsBeingCalled()
    {
        $controllers = $this->prophesize(ControllerManager::class)->reveal();

        $services = new ServiceManager();
        $factory = function () use ($controllers) {
            return $controllers;
        };

        $services->setFactory('ControllerManager', $factory);

        $delegator = $this->createMock(DelegatorFactoryInterface::class);
        $delegator->expects($this->once())->method('__invoke')->willReturn($controllers);
        $services->addDelegator('ControllerManager', $delegator);

        $this->assertSame($controllers, $services->get('ControllerManager'));
    }

If you change this to the following (which does that PR we are talking about [#292]), it will look like this:

    public function testDelegatorIsBeingCalled()
    {
        $controllers = $this->prophesize(ControllerManager::class)->reveal();

        $services = new ServiceManager();
        $factory = function () use ($controllers) {
            return $controllers;
        };

        $services->setFactory(ControllerManager::class, $factory);
        $services->setAlias('ControllerManager', ControllerManager::class);

        $delegator = $this->createMock(DelegatorFactoryInterface::class);
        $delegator->expects($this->once())->method('__invoke')->willReturn($controllers);
        $services->addDelegator('ControllerManager', $delegator);

        $this->assertSame($controllers, $services->get('ControllerManager'));
    }

Since there is no delegator registered for ControllerManager::class (FQCN), the delegator is never called.

So, in fact, every project which implemented own delegators for the ControllerManager string service name, will break. Or am I missing something?

But if this PR (#294) will get merged, anything supposed to be fine until zend-mvc v4 (where we can swap aliases to make anything correct).

    public function testDelegatorIsBeingCalled()
    {
        $controllers = $this->prophesize(ControllerManager::class)->reveal();

        $services = new ServiceManager();
        $factory = function () use ($controllers) {
            return $controllers;
        };

        $services->setFactory('ControllerManager', $factory);
        $services->setAlias(ControllerManager::class, 'ControllerManager');

        $delegator = $this->createMock(DelegatorFactoryInterface::class);
        $delegator->expects($this->once())->method('__invoke')->willReturn($controllers);
        $services->addDelegator('ControllerManager', $delegator);

        $this->assertSame($controllers, $services->get(ControllerManager::class));
    }

---

Originally posted by @boesing at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-420955840

[weierophinney]

@boesing I think that the main problem is that ServiceManager::addDelegator() does not resolve the name, that was passed to it. I have manually added the following line to the method and ran your test: $name = isset($this->resolvedAliases[$name]) ? $this->resolvedAliases[$name] : $name;

It worked okay, but I'm not sure that I am right. Anyone?

EDIT: Extending a little bit more. Here the delegator is added, without resolving the name. https://github.com/zendframework/zend-servicemanager/blob/master/src/ServiceManager.php#L472-L475

However, when creating a service, the already resolved name is used to access its delegator. https://github.com/zendframework/zend-servicemanager/blob/master/src/ServiceManager.php#L761-L767

---

Originally posted by @thexpand at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-421038733

[weierophinney]

@froschdesign I'm summarizing our short discussion with @boesing on Slack.

Delegators are added to the service manager by the actual name of the service and not by their aliases. Moving the actual service names in the aliases array will be a BC Break, as previously registered delegators to that service won't be called at all.

What we can do is already implemented with this PR. The missing FQCN class constants are added as aliases to the actual service names, which we don't change to keep backwards compatibility.

As we discusses, @boesing remembered doing a mistake more than an year ago with zend-validator, where he moved the ValidatorManager actual service name to the aliases and introduced a new name for the service (FQCN). You can take a look at the commit https://github.com/zendframework/zend-validator/commit/d8f6e43e58cdee0b406ceb22e4318561c2f3c30f

What I suggest is that we close my PR (https://github.com/zendframework/zend-mvc/pull/292) and leave this one, as it does not introduce a BC Break.

As for the failing Travis build - I've created a hotfix on the master branch (https://github.com/zendframework/zend-mvc/pull/295). When accepted and merged to the master branch, it can be merged to this PR, too, so that the tests may pass.

---

Originally posted by @thexpand at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-421050228

[weierophinney]

@boesing Thanks for the explanation and the included unit tests, they help more than 1000 words. 👍

@thexpand Notice: If you open an issue report or a pull request, you can also close it yourself.

---

Originally posted by @froschdesign at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-422645100

[weierophinney]

On hold for now as it potentially conflicts with my work in progress for next major.

---

Originally posted by @Xerkus at https://github.com/zendframework/zend-mvc/pull/294#issuecomment-464413669

[boesing]

@Xerkus As your latest reply was "on hold" in february 2019, is it useful to re-open the PR or do you keep this Issue in mind? Would be fine with both as I actually does not have any issues here (even if it still remains an issue).

[Xerkus]

@boesing New features for the current major will be handled asap. My WIP was delayed at first and scrubbed by now, pretty much, with increasing scope of changes and everything happening. I will have to redo changes so any possible conflicts are a moot point.