Request for discussion: Lazy Services Configuration and Separation of Concerns

[fhein]

Currently lazy_services configuration holds the class_map, proxies_namespace, proxies_target_dir and write_proxy_files. The class_map is a service manager concern but the other settings are configuration for @Ocramius ProxyManager library, which is external.

Wouldn't it be cleaner to have a lazy_services configuration for ServiceManager which only holds the classmap like

'lazy_services' => [
    'service1'  => Service1::class, 
    ...
]

and provide the other configuration settings from elsewhere? ServiceManager is ServiceManager, I think, it is out of scope to have to know about configuration details of external libraries.

private function createLazyServiceDelegatorFactory()
{
    ...
}

is a factory embedded in the ServiceManger core. It deals with the class map and the other settings. By means of separation of concerns or modularization wouldn't it be worth to consider to put this factory in a separate class?

[Ocramius]

It used to be an external factory (and namespace), so I suggest looking at where the change was introduced in order to follow the reasoning behind it (I really don't remember when it was squashed into SM internals)

[fhein]

See below commit message when createLazyServiceFactory was introduced to ServiceManager internals was introduced by @weierophinney.

It seems to be a contradiction to separation of concerns.

I'd propose, to move LazyService configuration (paths, etc) out again. LazyServices configuration in ServiceManager could be simplified by removing the requirement of defining a delegator for a lazy services. A configuration like

'lazy_services' => [
    Buzzer::class => Buzzer::class,
    ...
],

holding the lazy_services class map would be all that is needed to make a service lazy. This would maintain and even leverage lazy service's first class citizen status.

Doing so, positive effects on performance could be achieved. Currently, there is a hard string compare in the delegator creation loop:

        if ($delegatorFactory === Proxy\LazyServiceFactory::class) {
            $delegatorFactory = $this->createLazyServiceDelegatorFactory();
        }

which would get obsolete. Technically, lazy_services would remain delegators, but an explicite definition for those delegators would not be necessary.

commit 6f8827efba9c582d528063c38370130d238340ec Author: Matthew Weier O'Phinney matthew@zend.com 2015-09-16 00:06:15 Committer: Matthew Weier O'Phinney matthew@zend.com 2015-09-16 00:06:15 Parent: 7db3473d6ebf5c6365491d9306d64a9c461a334f (Completed second pass on documentation) Branches: ...

Refactor of lazy services

Lazy services were added after the ServiceManager had an established API, and as such had to rely on optional integration. This patch alters that relationship to make lazy service configuration a first-class citizen of the ServiceManager.

This allows:

• Removing the LazyServiceFactoryFactory.
• Passing lazy_services configuration to the constructor or withConfig() method.
• Direct instantiation of the LazyServiceFactory when first encountered as a delegator factory, using the lazy_services configuration composed in the ServiceManager itself.

The usage is thus simplified, as all configuration for all service types is now directly handled by the ServiceManager itself.

[fhein]

And if it was a request to transport pathes et al. also, I'd propose to make delegator configuration flat, so that array_merge_recursive in configure would get obsolete, so all service manager config could get merged with a single array_merge instead of six or so currently. Additional sections supported in the future would get transported automatically.

This could also be used to establish a resolution-logic-cache, so that service resolution could be condensed to a single step. More or less.

I already experiment with that and the measures look promising.

[Ocramius]

@fhein the delegators config is not flat on purpose, because each delegator is added on top of a single service, so you always have:

[
    'delegators' => [
        \Name\Of\The\Service::class => [
            Delegator1::class,
            Delegator2::class,
            Delegator3::class,
        ],
    ],
]

[fhein]

Yes, I know about that cascade. Making that flat would mean to allow delegators referencing delegators.

[fhein]

But I think, that should if at all be discussed separately.

[Ocramius]

delegators referencing delegators.

Don't understand this bit. In general, delegators:

1. shouldn't care about each other
2. are in a stack, so they are already in some sort of relation
3. are executed in a stack also at runtime

Can you make an example of what a flat config would look like, and what it would do?

[fhein]

Idea is like this. Delegator definitions aggregate a linked list of factories (instead of a stack). Drawback would be that the same factory can not appear twice. But that would be almost meaningless anyway.

If we'd allow delegators to be declarative also, we would not even need seperate factory definitions.

    $config = [
        'delegators' => [
            Factory1::class => Factory::class,
            Factory2::class => Factory1::class,
            Factory::class => Factory2::class,
        ]
    ];

    function createDelegatorFromName($name, $options) {

        while (isset($this->delegators[$name])) {
            if (! is_cyclic($this->delegators[$name])
                return $this->createDelegatorFromName($this->delegators[$name], $options);
        }

        // create the factory here and the creationCallback here

        $this->delegators[$name] = $delegatorFactory;

        return function () use ($delegatorFactory, $name, $creationCallback, $options) {
            return $delegatorFactory($this->creationContext, $name, $creationCallback, $options);
        };
    }

Please let me know, if you need a working example.

[Ocramius]

Ah, you want to unify factories and delegators. I don't think these should be unified, as they are completely distinct concepts (although "delegator" is the wrong name, as I've blogged/explained in https://ocramius.github.io/blog/zend-framework-2-delegator-factories-explained/ ).

One applies a decorator around a service, the other builds a service with just the container as aid.

[fhein]

Unification is not the primary goal, or better, not a goal at all (just an option). Delegator linked lists would get resolved. We could remain with that keys which don't reference a delegator must be declared factory (as current).

[fhein]

Having a flat configuration is what I'm striving for here. That would provide a good starting point to make ServiceManager faster. Allthough there's some room for improvement left without changing to the config structure.

[fhein]

What do you think about the original question, lazy services, separation of concerns and simplification of lazy services config? I'd appreciate much if you would like to share your opinion.

[fhein]

@Ocramius, @weierophinney : No response at all? Interesting. I wonder and wonder again, what reasoning could be behind that silence. Quite difficult to imagine. ;) What an interesting market place this is. I love it. :) And I love the platform. Nothing will ever be forgotten.

I am still here and will stay here and I will continue to try to contribute and I will renew my requests for answers and will continue to wait for them. I'd like to see a written answer, what the reasons are why this factory was internalized, and if those reasons still hold.

I am very eager to learn why this was desirable despite of anything everybody outside this pokey context sees as best practice. I am sure there must be very good reasons to do it this way. I'd like to know them. I am here to learn.

[Ocramius]

Closing as per https://github.com/zendframework/zend-servicemanager/pull/254#issuecomment-364026562