Adding a `$container` attribute to getServices()?

[moufmouf]

I spent the day implementing a universal service provider for Stash.

The result can be seen here: https://github.com/thecodingmachine/stash-universal-module

Here is a quick feedback on my experience with the current interface.

Overall, it is quite easy to provide a default cache pool. In my service provider implementation, I decided to make that cache pool a configurable "composite" pool, where one can register many drivers (by default, the in-memory driver and the APC drivers are enabled).

Now, I'm comparing my service-provider with the Stash bundle that you can see here: https://github.com/tedious/TedivmStashBundle#multiple-services

Everything is quite similar up to the point where we need to deal with "multiple services". The Stash Bundle has a feature allowing the user to create multiple pool instances based only on configuration.

For instance:

stash:
    caches:
        first:
            drivers: [ FileSystem ]
            FileSystem: ~
        second:
            drivers: [ APC, Memcache ]
            Memcache:
                servers: ['127.0.0.1', 11211]

This will generate 2 instances: stash.first and stash.second.

I like this feature. For some people, it is not uncommon to request 2 different cache pools depending on the type of information you want to cache (maybe one default very fast small APC local cache, but also another slower, bigger Memcache cache for shared caching on a cluster?)

Anyway, the fact is that since the getServices method does not take any argument, my only possible solution is to register the service provider twice, passing in argument the name of the cache.

$container = new Simplex\Container();
$container->register(new StashServiceProvider('first'));
$container->register(new StashServiceProvider('second'));

This is a neat trick, but I have the feeling it is really that... a trick. I would love to be able to register only once the StashServiceProvider, and based on the configuration fetched from the container, create one or many instances. But of course, accessing the container is tricky... since it is not fully configured yet.

So, here, I'm turning to you.

What would you think about trying to change the getServices signature to:

public function getServices(ContainerInterface $container);

Assuming the configuration is registered in the container BEFORE the service providers, this would allow a service provider to inject a different number of entries based on container content.

On the plus side: I like how we can mimick existing providers from other frameworks. This offers really more power to service provider writers (I've been writing quite a few and I feel a lack of flexibility so far).

On the minus side: I fear that accessing the container while it is being created might be dangerous though.

What do you think about this idea?

[mnapoli]

getServices(ContainerInterface $container) will be very hard to support in containers IMO.

The problem you faced is, in my opinion, not a problem that needs to be solved by a module. Modules allow us to get started easily.

If I had to use several caches in my app, my first thought would be to do something like this:

'cache.first' => function ($c) {
    return Stash\CacheFactory::create($c->get('cache.first.config'));
},
'cache.first.config' => [
    'drivers' => [ 'FileSystem' ]
],

This is a pseudo PHP-DI config, because I would do this entirely in userland. If the package provides a good-enough factory then I can use it directly. I don't want the service provider to be the only API I can use to create caches (if that's the case then I can expect to be limited in the long term, so I'll definitely not want to use such library). If there is complex factory logic then the service provider should be some sort of proxy to that code (whether it's in the class constructor or a separate factory class), not contain it. Just like controllers should be skinny and not contain domain logic. I don't know if I make sense :)

Service providers are not a "ultimate" solution IMO. Only helpers, that's what modules are about.

[moufmouf]

I've been doing some homework to see how difficult this might be to implement.

Now, the hard part is that we might resolve services that are later modified. This is troublesome.

Symfony has currently a ContainerBuilder on which we can call the get method (just like a regular container) so the signature I'm proposing would be moderately easy to adapt right now, but according to @stof here:

Symfony 3.2 is deprecating the possibility to retrieve a service from a ContainerBuilder which is not yet compiled (and Symfony 4.0 will forbid it entirely), as it causes many issues (and potentially even fatal errors in case the configuration of this service is only half done)

This move from Symfony seems completely logical to me and I understand why they are doing it.

Now, this means that:

• either we completely forget this whole idea (but then, we cant' provide different services based on the configuration) => @mnapoli : I understand this is something you can live with :)
• or we find a way to push only the configuration in the getServices() method, which is kind of weird, since we assumed from the start that the configuration was in the container.

I'll need to give it some more thinking!

[moufmouf]

Ok, I may have an idea that both satisfies @skalpa comments here and the problem of the current issue.

Based on @skalpa comment, we could split the interface in 2 methods:

interface ServiceProvider
{
    public function getServices(ContainerInterface $container);
    public function getServicesExtensions(ContainerInterface $container);
}

Notice that I put the $container argument in both methods. Having the $container is getServices allows to provide a service only if it is not already there (no override).

Now, there is a catch. As noted @stof, when getServices or getServicesExtensions is called, the container does not completely exists. It is in the process of being built. Calling the get method on a container service is a big "no no" as the service might be extended by a service provider at some point later. Also, for compiled container, it is definitely hard to provide a "half-baked" container.

But at the same time, we know that:

• we can safely call the has method of the service
• we can safely call get on parameters (anything that returns a scalar or an array of scalars)

So we could phrase something like this:

When the getServices method is called, the $container passed to getServices is not yet fully configured. As such, if the $container's get method is called, and if the fetched entry is an object or an array of objects, the container MUST throw a ContainerNotReadyException. The $container has method is expected to run as usual, and if the get method is referencing a configuration parameter (i.e. a scalar or an array of scalars), it is expected to work correctly.

By forcing the container to throw a ContainerNotReadyException if we are getting a service, we are forcing a consistent behaviour across all implementations.

Also, this is trivial to implement for runtime container, they can just provide a wrapper like this:

class HalfBakedContainer implements ContainerInterface {

    private $container;
    public function __construct(ContainerInterface $container) {
        $this->container = $container;
    }

    public function has($id) {
        return $this->container->has($id);
    }

    public function get($id) {
        $entry = $this->container->get($id);
        if (is_object($entry)) {
            throw new ContainerNotReadyException("...");
        }
        return $entry;
    }

}

What do you guys think of this idea?

[stof]

@moufmouf note that Symfony will indeed not forbid has or getParameter (be aware that someone could overwrite the parameter later though)

[moufmouf]

@stof Yes, that's what I expected from Symfony. In container-interop/service-provider we do not have a separate notion of parameter and services (parameters and services are both container entries for us). Writing an adapter for Symfony is trivial enough though: https://github.com/thecodingmachine/service-provider-bridge-bundle/blob/1.0/src/SymfonyContainerAdapter.php

You are right to say that a parameter could be overwritten, but I'm not sure this is really a big issue. It should be rare enough (I can't think of a valid use case) and this should be less problematic than having half-baked services. Let me know if you think of something that could go terribly wrong because of overwritten parameters.

[stof]

nothing could go badly wrong (except WTF moments if you retrieve the param early and use this value to configure your service, and then wonder why your overwrite is not applied, leading to hard debugging)

[mnapoli]

This is getting way too complicated for something that I don't believe should be supported by a simple standard for module interoperability. We are really getting away from a simple service provider that's understandable by everybody, and easily implemented everywhere.

[mindplay-dk]

We are really getting away from a simple service provider that's understandable by everybody, and easily implemented everywhere

@mnapoli I'm honestly not sure if we're chasing rainbows? I mean, at least half (but probably more) of the reason why you choose a specific DI container, is because of it's configuration features - if we standardize on providers, aren't we more or less erasing the differences between DI containers entirely? If both the configuration and consumption interfaces are fully standardized, how will it even matter which implementation I choose? I'm sure you guys have already thought about this and discussed it in the past, but perhaps figuring out how to make multiple instances of different container types interoperate is a much meaningful (or realistic) pursuit than trying to erase their differences?

(kind of off-topic, sorry - not really sure where better to bring this up...)

[moufmouf]

Hey @mindplay-dk ,

As much as possible, I'd like to keep this issue focused on its initial topic. Yet, a quick answer to your questions: the point of container-interop/service-provider is not to standardize the way containers work. Some will use configuration files of various formats, others will use callbacks, others a web-based UI... that's fine. Containers consuming the service-provider interface will have an "additional" way to define services that is shared amongst containers. But it's really just that. An additional way. By no mean is it "the right way" (there is no such thing).

I'm sure you guys have already thought about this and discussed it in the past, but perhaps figuring out how to make multiple instances of different container types interoperate is a much meaningful (or realistic) pursuit than trying to erase their differences?

We already followed that path. That lead to the notion of "delegate lookup feature" in container-interop. It works great and enables containers to share entries, which is awesome. But it lacks one key feature: a container cannot easily "extend" a service defined in another container. And we did not find any way to do it easily (I bloged about it here: https://www.thecodingmachine.com/psr-11-an-overview-of-interoperable-php-modules/). Hence the need for service-providers.

If you want to pursue this issue further, would you mind opening a new issue? This way, we can kep this issue focused on passing or not passing the container in the getServices method.

[mnapoli]

@mindplay-dk see #31

[XedinUnknown]

Forgive me, I have not read the whole thread yet, but soon will read it and everything related. I just wanted to point out that I often use containers and providers with a FactoryInterface. My container uses a FactoryInterface implementation to create new instances, which are then returned from the container. The ServiceProviderInterface implementation is required by the factory, and not so much by the container. But it appears to me that right now the direction is to use ServiceProviderInterface with ContainerInterface specifically. So just wanted to provide an alternative use case, as I feel that delegating instantiation to a factory is more structured, and should be pretty common, due to it being the only way of making sure that a new instance is created every time.

[mindplay-dk]

the point of container-interop/service-provider is not to standardize the way containers work. Some will use configuration files of various formats, others will use callbacks, others a web-based UI... that's fine. Containers consuming the service-provider interface will have an "additional" way to define services that is shared amongst containers. But it's really just that. An additional way. By no mean is it "the right way" (there is no such thing).

My problem is not with your mission or vision - you have the right objectives, it's merely that I don't believe that's how the community will use it or perceive it.

If there's a "standard" way to implement providers, it will effectively become the standard, it will displace proprietary providers - it's mere existence will create a sense of pressure to write portable "standard" providers, and proprietary provider formats will most likely be relegated to proprietary/closed-source projects, perhaps not even that.

I don't believe it matters how you describe it or qualify it - being the only/closest thing to interoperable providers, it will effectively mean that you can't really use proprietary provider formats for anything open-source anymore, or (at least) that no one will.

I believe it creates a situation where you will be perceived as community-non-friendly for choosing a proprietary provider-format for open-source projects.

As said, you might as well provide a reference implementation, because the provider-format completely dictates the implementation - the only implementation difference between containers will be performance, so there will be no reason to choose any specific container besides mere performance.

That's my belief, and I understand you don't share my concerns, and I'm not going to oppose you or anything, but, suffice to say, this is a PSR I will personally refuse for those reasons. I don't believe it's a good idea, but that's my personal point of view, so, good luck to you :-)

[mindplay-dk]

Wow, I really did not understand this PSR when I wrote those comments in '17. 😅

To the original subject, I'm pretty sure there would be lots of different ways to solve this, without complicating the current PSR?

For example, you could register all the available drivers as named service factories/functions, e.g. stash.factory.apc etc. - as well as registering factory functions for cache.first etc. which would then look up the named service factory functions and use those.

You could also use __invoke callables to reuse the same function instances in the provider, internally, if needed. You could use proper methods as well, via Closure::fromCallable.

I think there are plenty of ways to structure something like this?

I don't think the current proposal gets in the way of doing more generic or reusable service providers, if that's what you want?

Let me know if you agree, so we can close the issue, please? 🙂

[mindplay-dk]

I've read through the entire thread again.

I don't see anything really actionable here - since this idea didn't go anywhere, and since nothing new has been added to the discussion in years, I'm going to close this thread.

Feel free to reopen the issue if you disagree with this decision. 🙂