Repeated AbstractFactory may occur in ServiceManager.

[Moln]

See #104

[Ocramius]

Needs a test case before being applied.

[Moln]

Added a unit test.

[samsonasik]

ping @Ocramius

[Ocramius]

I just realized that this change is incorrect.

Following code is perfectly legit, for example:

$abstractFactory1 = new MyConfiguredAbstractFactory($config1);
$abstractFactory2 = new MyConfiguredAbstractFactory($config2);

$sm = new ServiceManager([
    'abstract_factories' => [
        $abstractFactory1,
        $abstractFactory2,
    ],
]);

This scenario seems legit to me, and is actually even used within stuff like DoctrineModule, now that I look closer.

I think the key used to store the factory internally should be based on spl_object_hash() instead.

A test for this scenario is also needed, IMO...

[Ocramius]

This should be 2.

[Ocramius]

A new test with the same instance of CallOnlyOnceAbstractFactory should be provided. In that scenario, only one factory should remain.

[Ocramius]

And even there, strong reasoning should be given as to why a duplicate abstract factory is actually a problem...

[Moln]

Ocramius is right.
It is my new pull.

ServiceManager:

1. The key used spl_object_hash() instead.
2. Add $cachedAbstractFactories property for cached some instances.

Unit test:

1. Remove testCallOnlyOnceWithMultipleIdenticalAbstractFactory
2. Add testCallTwiceWithDifferentInstanceAbstractFactories and testCallOnlyOnceWithSameInstanceAbstractFactory.

[Ocramius]

Ok, now that the patch is correct (and sorry if it feels like I'm dragging you around, I really get to check this stuff when it pops up in my inbox): what kind of problem does #104 cause? Is it just a performance issue?

Assuming idempotent factories (big assumptions, so I might be wrong here), I don't see a problem with having two factories referencing the same instance inside the ServiceManager...

[Moln]

I thought for a while. May be I am too sensitive to performance.
Yes, it a performance issue. May be a very little performance impact.

At that time I wrote two ZF modules (Module A & Module B). They all have the same configuration.

'service_manager' => array(
 'abstract_factories' => array(
    'Zend\Db\Adapter\AdapterAbstractServiceFactory',
    'Zend\Cache\Service\StorageCacheAbstractServiceFactory',
    '...',
 ),
),

I just want to get another service, but call every AbstractFactory::canCreateServiceWithName() twice. I just feel no need.

Now this is another solution.

class Module
{
    public function init(ModuleManager $manager)
    {
        $manager->getEventManager()->attach(ModuleEvent::EVENT_MERGE_CONFIG, function (ModuleEvent $event) {
            /** @var \Zend\ModuleManager\Listener\ConfigListener $configListener */
            $configListener =$event->getParam('configListener');
            $configs = $configListener->getMergedConfig(false);

            $configs['service_manager']['abstract_factories'] =
                array_unique($configs['service_manager']['abstract_factories']);

            $configListener->setMergedConfig($configs);
        });
    }
}

😄 😄

[snapshotpl]

It can be problem when canCreate will be more expensive and will return false.

[Moln]

Or may be it is zend-modulemanager or zend-mvc issue, not zend-servicemanager.

[weierophinney]

Assigning to 3.2.0 release, as the feature is technically new behavior.

[weierophinney]

Merged to develop for release with 3.2.0