Exploring a verifiable provider format

[mindplay-dk]

Following the discussion here and a preliminary exploration of a concept here, I'd like to open a discussion about the possibility of a verifiable provider format.

What I mean by verifiable, is: a provider with external dependencies (dependencies that aren't defined by the provider itself) would be verifiable by a container, ahead of time - before actually attempting to look up a component with a missing dependency.

Consider the following stubs for simple use-case:

class Connection
{
    public function __construct(PDO $pdo)
    {
        // ...
    }
}

class UserRepository
{
    public function __construct(Connection $db)
    {
        // ...
    }

    public function addLogger(LoggerInterface $logger)
    {
        // ...
    }
}

For this example, UserRepository will be registered as a singleton, whereas Connection and it's PDO dependency will be registered under provider-specific names with a user-service prefix, e.g. allowing for more than one Connection and PDO instance for different dependents. The PDO instance needs to be registered by someone outside the provider. The UserRepository::addLogger method demonstrates setter-injection via getExtensions.

Consider a status quo service provider for these dependencies:

class UserServiceProvider implements ServiceProviderInterface
{
    public function getFactories()
    {
        return [
            "user-service.connection" => function (ContainerInterface $container) {
                return new Connection(
                    $container->get("user-service.pdo")
                );
            },
            UserRepository::class => function (ContainerInterface $container) {
                return new UserRepository(
                    $container->get("user-service.connection")
                );
            },
        ];
    }
    
    public function getExtensions()
    {
        return [
            UserRepository::class => function (ContainerInterface $container, UserRepository $repo) {
                $repo->addLogger($container->get(LoggerInterface::class));
                return $repo;
            }
        ];
    }
}

When you bootstrap this provider, you immediately have two problems:

1. Attempting to resolve UserRepository will fail for the missing user-service.pdo.
2. It will fail again for the missing LoggerInterface dependency.

You have to discover and fix these, one at a time, as you uncover the failures.

After that, you could have two more problems: the types of user-service.pdo and LoggerInterface aren't checked, so those might fail when passed to their dependents - and that's assuming their dependents have type-hints, otherwise you could have more obscure problems already.

Finally, here's the verifiable provider format I'd like to discuss:

class UserServiceProvider implements ServiceProviderInterface
{
    public function getFactories()
    {
        return [
            "user-service.connection" => [
                function (PDO $pdo) {
                    return new Connection($pdo);
                },
                ["user-service.pdo"]
            ],
            UserRepository::class => [
                function (Connection $connection) {
                    return new UserRepository($connection);
                },
                ["user-service.connection"]
            ],
        ];
    }
    
    public function getExtensions()
    {
        return [
            UserRepository::class => [
                function (UserRepository $repo, LoggerInterface $logger) {
                    $repo->addLogger($logger);
                    return $repo;
                },
                // implied: [UserRepository::class, LoggerInterface::class]
            ],
        ];
    }
}

Each factory definition consists of tuple with two elements: a factory function, and an (optional) array of dependency names, each corresponding to a factory-function argument.

The list of dependency names in this example are positional: ["user-service.pdo"] - but they could also be type-name maps: [PDO::class => "user-service.pdo"], or they could be argument-name maps: ["pdo" => "user-service.pdo"], or possibly a mix of any of those. There are pros and cons for each.

There are three benefits to this approach:

1. Containers now have an actual list of dependencies, which they can verify ahead of time.
2. Type-checking happen as the closure-level, before attempting to call constructors, etc.
3. Type-hinted functory-functions allow for static analysis.

Note that this format does not preclude dynamic lookups. Factory functions that dynamically (for example, conditionally) determines component names, can still look up components by requesting ContainerInterface via the closure, same as the current proposal. Likewise, containers that provide service-providers, and don't have the dependency list available, can look up everything dynamically, same as before.

In both those cases, of course, there's no validation a container can perform. But the option to build or write verifiable containers could be made backwards compatible with the current proposal.

This example is actually less verbose than the original, though I'm not sure if that's generally the case, or just this particular scenario. But factory-functions could be even more abbreviated with PHP 7.4 function expressions, e.g.:

            "user-service.connection" => [
                fn (PDO $pdo) => new Connection($pdo),
                ["user-service.pdo"]
            ],

I'm not crazy about using tuples, by the way - it might be better to introduce a class or a function/interface to construct them, e.g.:

            "user-service.connection" => factory(
                fn (PDO $pdo) => new Connection($pdo),
                ["user-service.pdo"]
            ),

Based on the concept in the experimental container I mentioned above, one might also consider PHP 8 attributes, which could remove the need for either tuples or a class/function/interface, e.g.:

            "user-service.connection" => 
                fn (#[name("user-service.pdo")] PDO $pdo) => new Connection($pdo),
            ,

Anyways, there's some ideas to explore.

Let me know what you make of this? 🙂

[XedinUnknown]

I absolutely acknowledge the problem. I remember that before this last edition with just getFactories() and getExtensions(), there used to be a much more complex version that defined what a service has to look like. However, this was viewed as too complex, and so we moved to a simpler definition of the form (ContainerInterface $c) => mixed.

The problem still presents, however. That's why I have been using dhii/services; please have a look. Interface extraction is underway in Dhii/services-interface#1, but I'm sure you can already see how this works. In particular, please note the getDependencies() method.

With regard to having something like this in SP, I'm not sure this will be allowed through if we make it part of this standard. I'm thinking that the best approach could be to keep the service definition to a callable with a specific signature, and maybe add something like dhii/services on top - that's how I use it right now - maybe in another standard. The question is then: how much burden does the fact that the definition's deps declaration is optional put onto compatible DI container implementations?

[mecha]

To add to what @XedinUnknown said:

The approach implemented by dhii/services is 100% optional and compliant with the current spec, since it merely provides invocable classes that can replace closures, and can expose their dependencies via a getDependencies() method.

They look something like this:

class UserServiceProvider implements ServiceProviderInterface
{
    public function getFactories()
    {
        return [
            "user-service.connection" => new Factory(
				['user-service.pdo'],
				function (PDO $pdo) {
                	return new Connection($pdo);
	            }
			),
			// Alternative to `Factory`, calls the ctor and passes the deps
            UserRepository::class => new Constructor(UserRepository::class, [
				'user-service.connection'
			]),
        ];
    }
    
    public function getExtensions()
    {
        return [
            UserRepository::class => new Extension(
				[LoggerInterface::class], // UserRepository::class is implied
                function (UserRepository $repo, LoggerInterface $logger) {
                    $repo->addLogger($logger);
                    return $repo;
                },
            ),
        ];
    }
}

The end result isn't all that different from the format that @mindplay-dk proposed - just slightly more verbose - but provides the following benefits:

1. Instances of Factory, Extension, Constructor, Alias, Value, ServiceList and all other provided classes, are valid callable values. This makes them compliant with the current standard, which wouldn't need to change to support array services and therefore break backwards-compatibility.
2. It gives each service semantic meaning, which could influence what a container, or any consumer really, can do with the information.

As @XedinUnknown said, because SP currently provides callable[] from its methods, consumers that support this implementation will be burdened with catering for callable|Service. For instance:

$deps = ($service instanceof Service)
	? $service->getDependencies()
	: [];

It's not ideal, but IMO it's relatively minor. It's what I've had to resort to in order to detect non-existent services, circular dependencies, and even be able to reuse multiple instances of the same provider (yes, really).

So it would be great if an equivalent to Service was part of the spec, even if everything else remains the same and continues to depend on callable.

[mindplay-dk]

Yes, this looks very similar to what I'm suggesting.

The main difference is this doesn't leverage reflection to resolve singletons.

So, where dhii/services requires this:

new Factory(
    [Foo::class, Bar::class, "my.cache"],
    function (Foo $foo, Bar $bar, ICache $cache) {
        // ...
    }
);

I'd prefer something more along the lines of this:

new Factory(
    [ICache::class => "my.cache"],
    function (Foo $foo, Bar $bar, ICache $cache) {
        // ...
    }
);

That is, avoid restating your assumptions - if the type is equivalent to the name, as it typically is for singleton registrations, there's no need to restate that. And it's not the verbosity I'm opposed to - just the fact that stating the same information twice creates another source of errors. Someone looking at code with an error isn't necessarily going to know which of a conflicting pair of type-hints and names is right or wrong.

I like what I'm seeing here though - it's very simple and elegant. 🙂

[XedinUnknown]

I don't think that re-stating type assummptions is a problem here. The type of the service is completely unrelated to the name of the service. Although this is a convention in some frameworks, I for one do not name my services after their type, but instead the name relfects the significance of the service in the application, I've got quite a few projects working this way, and I don't think that this standard should work against any particular naming convention. In this light, the fact that you mention the same type twice in one definition is incidental. Cases where this will happen often are numerous, and here we can even talk about auto-wiring, but this is a case for the implementation, rather than the standard IMHO.

[mindplay-dk]

The type of the service is completely unrelated to the name of the service.

It can be, but it doesn't have to be.

For multitons, you need to invent a name - for example, if two different classes depends on an ICache, I will register those under distinct names. Whereas, for singletons, there's no reason to invent another name than the singleton type-name, so I register them under their class-name, or under an interface name.

I would do this with any container, as a convention - even something simple like Pimple, regardless of whether it does any sort of auto-wiring based on the names.

I don't think that this standard should work against any particular naming convention.

What I'm suggesting does not work against your convention - it just works using this convention also.

I don't use auto-wiring myself, but I do rely on this naming convention.

I'm pretty sure a lot of people do.

[XedinUnknown]

Yes, of course, like I wrote - there are many cases where the class name is used for a service name. And yet, they are not related: it's just the framework or personal choice to do this. There can be tooling that allows you to leverage that convention, if it is used, such as autowiring, or you can simply make a helper function that encodes that convention. But I really don't think we should relate the service name and the class name in the SP standard. The same flexibility that allows us to have a multitude of useful implementations, like dhii/services, allows anyone to have their own conventions and supporting implementations.

The point of dhii/services-interface is to simply add the awareness of dependencies to SP, and does not encode any convention. I'm just not sure if it belongs here, or in potentially a separate PSR. I've added a description, and made a public release - please take a look.

[mindplay-dk]

I don't know, I go back and forth on this right now. 🤔

On the one hand, I can see your point, that registering services under class/interface names, as a convention, is a personal choice - and I get that it isn't strictly about interoperability, per se.

On the other hand, while the current provider proposal does not preclude verifiability, it only permits it as an implementation detail.

That is, it's up to the individual authoring a provider, or publishing one from a container, whether or how they validate dependencies. If containers define and validate verifiable dependencies in different ways, that verification becomes non-interoperable. The implementations used by individual providers will report errors differently, throw different exceptions, and so on. One provider can't register dependencies defined by another provider - and a container can't verify them.

What I'm proposing is one way of making verifiable requirements interoperable.

There might be other ways.

For example, taking inspiration from dhii/services, we might consider simply adding an interface like this:

interface ServiceInterface
{
    /**
     * Retrieves the keys of known, dependent services.
     *
     * @return string[] A list of strings each representing the key of a service.
     */
    public function getDependencies(): array;
}

The return-types in ServiceProviderInterface would change from callable[] to (callable|ServiceInterface)[].

Using a class to implement this interface would be optional.

When implemented, the list of keys does not have to be complete, and does not have to correspond with the argument order of a callable - it's provides a means of specifying verifiable dependencies only.

The callable signature remains (ContainerInterface $c) => mixed as it is today. No complexity added to providers that don't have this information, nor to containers that don't wish to verify - they can simply ignore the interface.

The factory type in dhii/services could implement this interface, since you have all your dependencies specified explicitly - you actually have the method implementation already, so this would instantly be verifiable.

The container prototype I mentioned in my original post could be refactored, abstracting away the reflection and inference features, while exposing them via this interface - the reflection/inference becomes an implementation detail.

Maybe that's a direction worth considering?

[XedinUnknown]

One provider can't register dependencies defined by another provider

Not sure about this one, and not sure I understood it correctly. Multiple providers can in fact declare services with same names as from other providers. My modularity and loading standard defines a strict resolution mechanic based on load order for this, where the latter provider overrides same name services from earlier providers.

With regard to adding a ServiceInterface, that's exactl what I did in Dhii/services#4. I would be happy if this was added to the SP standard here. What I'm not sure about are these points:

1. Whether FIG would allow such an extension. Earlier they were quite clear that the standard has to be extremely simple.
2. Whether this actually solves the problem. Declaring e.g. array<callable|ServiceInterface> means that services can still be simple callables, and so it's still optional for providers to return ServiceInterface - just like if it was in a different package.

If ServiceInterface were to be included in the standard, I contend that we should mention that the list of dependency service names is order-sensitive. This adds opportunities, like the one implemented in Factory, whereby you can have type-safe parameters for your definitions. This means that it's possible to even verify service type in advance by comparing the return type declaration with what is expected in dependencies. Yay static analysis!

With regard to reflections, I'm not sure I understand. As far as I know, dhii/services does not use reflections anywhere, and I don't remember having a need to do that.

[mecha]

The Factory in dhii/services wasn't built with singleton resolution in mind - the arguments given to the function simply replace what would have been $c->get() calls inside the function.

But another implementation of ServiceInterface could do this. It doesn't need to share the same constructor signature. So if you really want to resolve singletons using reflection, you could just implement it the way you want. I think the proposed interface would support such an implementation.

And at the end of the day, the implementation of the service doesn't matter to anyone except to the author of the SP. So long as we have the interface as part of the spec, of course.

[XedinUnknown]

Maybe it's time to make some implementations:

1. Something that can build a dependency graph, and warn about unfulfilled dependencies, as well as maybe detect recursion.
2. Something that can auto-wire implementations based on the convention when the service name is same as class name.

Pretty sure @mecha made some progress in the direction of reursion detection some time ago in Dhii/containers#13, and we should probably finalize it. @mindplay-dk, would you like to make a proof of concept that uses reflections, based on what we have already discussed?

[mecha]

Pretty sure @mecha made some progress in the direction of reursion detection some time ago in Dhii/containers#13, and we should probably finalize it.

Sure! I had even done some work on statically analyzing a service graph and report errors. I'll dig up the work I had already completed and polish it up.

[mindplay-dk]

If ServiceInterface were to be included in the standard, I contend that we should mention that the list of dependency service names is order-sensitive. This adds opportunities, like the one implemented in Factory, whereby you can have type-safe parameters for your definitions.

This doesn't make sense to me.

It's first of all not simple - it's a hidden requirements that can't be expressed in a type-declaration.

But it's also irrelevant to the standard. The closure that accepts these dependency-names, in that order, is an implementation detail - so now we're back to imposing requirements based on ideas and implementation details that aren't a part of this abstraction. Which you just talked me out of. 😏

This means that it's possible to even verify service type in advance by comparing the return type declaration with what is expected in dependencies. Yay static analysis!

Yes, that's what I was trying to accomplish with my original post.

I think that will still be possible at the level of the individual library/container implementation - but it doesn't necessarily have anything to do with interoperability, as far as validation is concerned.

With regard to reflections, I'm not sure I understand. As far as I know, dhii/services does not use reflections anywhere, and I don't remember having a need to do that.

No, this was in relation to my own prototype. 🙂

would you like to make a proof of concept that uses reflections, based on what we have already discussed?

I might play around with this, yes - so we can have something more concrete to discuss.

[mindplay-dk]

I've added the interface to a branch here, so we can play around and compare notes.

You can install it via composer.json using e.g.:

    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/mindplay-dk/service-provider.git"
        }
    ],
    "require": {
        "container-interop/service-provider": "dev-add-service-interface as 0.4.0"
    },

[mindplay-dk]

Alright, I have a working prototype that implements the ServiceInterface - have a look:

https://bitbucket.org/mindplaydk/mindplay-funbox/src/provider-interop/

Factory is now something similar to the factory in dhii/services, but with the reflection-based inference I wanted. It implements ServiceInterface.

The Container constructor now verifies any factories that implement ServiceInterface. (test here)

Note that, while this Factory implementation does return keys in function-order, that's an internal implementation detail to Factory - Container does not depend on the result of ServiceInterface for any other purpose than verifying dependencies, since Factory is responsible for internally resolving parameters to the original closure given to it's constructor.

ServiceInterface might be the wrong name, btw - it doesn't describe it's role or responsibility. Implementing ServiceInterface is not what makes it a service - being callable makes it a service, this interface is an optional facet of factories capable of reporting their dependencies. I can't think of a meaningful name at the moment though...

[mecha]

bitbucket.org/mindplaydk/mindplay-funbox/src/provider-interop

Oh that's much simpler than I imagined. You're basically just "translating" the dependencies. Cool!

ServiceInterface might be the wrong name, btw ... I can't think of a meaningful name at the moment though...

Hah! Why do you think we gave it such as generic name? 🤣

[XedinUnknown]

It's first of all not simple - it's a hidden requirements that can't be expressed in a type-declaration.
... it's also irrelevant to the standard

Totally fair points. I concede.

ServiceInterface might be the wrong name, btw - it doesn't describe it's role or responsibility. Implementing ServiceInterface is not what makes it a service - being callable makes it a service,

That's why ServiceInterface should declare __invoke(ContainerInterface $c): mixed though :). At the same time I agree that whether withDependencies() should be included in this interface is debatable. Seeing how we must keep the standard simple and yet extensible, we could perhaps follow somewhat the example of PSR-14, where the event dispatcher leaves great freedom to what the event should be, but allows it to optionally be a StoppableEventInterface(): it's perfectly legal to pass an (object)['my-thing' => 'my-value'] to it, if that's all you need. And it leaves the door open for another standard, like dhii/event-interface or any framework-specific standard, to define how to interact with the event itself. Perhaps we should do something similar here: a ServiceInterface with just __invoke(ContainerInterface $c): mixed, then an e.g. DependentServiceInterface extends ServiceInterface with getDependencies() (maybe with withDependencies() too, like in e.g. PSR-7).

Alright, I have a working prototype that implements the ServiceInterface - have a look

Looks very useful if all of the dependencies are services that correspond to the param types by name. And brings you a step closer to autowiring too, if that's needed. If you would like to contribute it to dhii/services, I'd gladly accept 😅, because it already has value today. I was unable to infer the type of $attrs though, because ReflectionParameter does not declare a getAttributes(). What am I missing here?

[mindplay-dk]

ServiceInterface might be the wrong name, btw - it doesn't describe it's role or responsibility. Implementing ServiceInterface is not what makes it a service - being callable makes it a service,

That's why ServiceInterface should declare __invoke(ContainerInterface $c): mixed though :).

Ah, what's what's missing! Yeah, I couldn't make sense of it, since the type union callable|ServiceInterface makes them alternatives - adding the __invoke method to the interface basically is equivalent to ServiceInterface extends callable, so this makes perfect sense. I will add that.

At the same time I agree that whether withDependencies() should be included in this interface is debatable.

I left this out because it's unrelated to consuming a factory - it's about building the factory, which isn't essential for interoperability. It belongs to the implementation details of each library, I think.

Looks very useful if all of the dependencies are services that correspond to the param types by name.

If you missed it, you can (and must) specify a name in any case where the param-types do not correlate with keys.

And brings you a step closer to autowiring too, if that's needed.

Hard pass. 😉

All of my applications have at least two contextual layers of IOC containers - one for long-lived services that survive multiple requests (when running under Swoole, etc.) and another for request-specific dependencies, which gets disposed after each request.

Auto-wiring is extremely risky in applications with two layers, as you have no idea about the life-cycle of an auto-wired service. (Unbox specifically lists it as a non-feature, since $provider->register(Some::class) is literally all that is required when the dependencies can all be resolved using type-hints. I don't consider this "auto-wiring", which to me implies the dependencies are automatically registered for you - which is where things get risky.)

If you would like to contribute it to dhii/services, I'd gladly accept 😅, because it already has value today.

I already maintain an IOC container (unbox) - the experimental container (funbox) that I've been tinkering with here is just an experiment to play with PHP 8 patterns and try out some ideas. 🙂

I was unable to infer the type of $attrs though, because ReflectionParameter does not declare a getAttributes(). What am I missing here?

PHP 8 - attributes 🙂

[mindplay-dk]

So I added __invoke to that branch, but it occurs to me, the name is now even more wrong - this now really represents a factory and definitely not a service, in the terminology of this proposal.

We probably should rename it to FactoryInterface or perhaps more specifically ServiceFactoryInterface or EntryFactoryInterface?

The terms "service" and "entry" seems to be used interchangeably in the spec?

I generally favor "entry", because it doesn't imply that it's a service - it could be just a value.

On the other hand, value entries usually don't have dependencies - there is no practical reason to use this interface for values, where getDependencies() would just return an empty array anyhow.

On the other other hand, I have seen examples where a registered value is an array, where the initialization of that array does have dependencies, e.g. on one or more services that provide elements that get collected into the resulting array.

I guess I'm leaning towards ServiceFactoryInterface though, because this jives better with ServiceProviderInterface.

All those in favor say aye?

[mindplay-dk]

I've opened a draft PR, so we can discuss more details in context.

I've added some documentation to the README as well - referring to the interface as ServiceFactoryInterface, though I'm waiting for your feedback before renaming it.

[XedinUnknown]

Hmmmm. It is a factory, indeed. But as far as I've read, the callable itself is called a "service definition", and if it implements an additional interface it is no different - still a definition, by the terminology that I'm familiar with. So need to think about this. Maybe ServiceDefinitionInterface is better? @mecha, what do you think?

[mindplay-dk]

Quote the "Factories" section in the README:

the value is a callable that will return the entry, aka the factory

Factories have the following signature

Factories accept one parameter

etc.

Based on that, the documentation change seemed to fit. (?)

[mecha]

Regarding withDependencies():

It probably shouldn't, and I say this while crying, be a part of this standard. However, I feel it should be part of some standard.

This method is very important for applications that need to be composed of multiple providers - especially when a provider can be reused. I use this method to prefix services in a provider. So I would like it to be something that I could depend on, and not be proprietary.

I guess I'm leaning towards ServiceFactoryInterface though, because this jives better with ServiceProviderInterface.

But as far as I've read, the callable itself is called a "service definition"

I'd tend towards definition, so as to not confuse the terminology with what is used in ServiceProviderInterface::getFactories().

But where does that leave extensions? They require a second parameter in __invoke(), so having an extension implement ServiceFactoryInterface doesn't work.

[mecha]

I think this is a problem of terminology definitions. The way I understand it is like this:

$definition = function(ContainerInterface $c) {};
$service = $definition($c);

[
	// a definition in a list is an "entry"
    'key' => $definition,
]

class SP implements ServiceProviderInterface
{
	public function getFactories() {
    	// Entries returned from here are "factory" definitions
	}


	public function getExtensions() {
    	// Entries returned from here are "extension" definitions
	}
}

[mindplay-dk]

Regarding withDependencies():

[...]

This method is very important for applications that need to be composed of multiple providers [...]

I think that's a matter of opinion.

I considered this feature for my own container (unbox) in the past, and always landed on "no".

One of my motivations for writing a container was to strike a balance between useful and necessary. Where something like Pimple lands firmly on the side of "necessary", and where something like PHP-DI, having "all the bells and whistles", lands pretty heavily on the side of "useful", I tried to land somewhere in the middle - in a place where a given feature has to be very useful to make the cut.

A lot of branches and pull requests were opened, tried, tested, discussed, and ultimately rejected if they served only a few use-cases. I worked in an very modular environment where unbox was used for some 20-30 providers, many inter-dependencies between them, many optional add-ons, etc.

We found very few use-cases (one or two) where someone needed to actually override a single dependency of a service definition.

Typically, if a dependency of a given service definition needs to be replaced, that same dependency needs to be replaced for all dependents in all providers - I've found it's much more common to override the dependency itself, than to replace a single dependency.

I also suspect (not from experience) that it's less transparent when there's a problem you need to debug. Which provider is responsible for this service definition? When it's definition is pieced together by several providers, things can get pretty complex.

With all that in mind, is it a big deal to have to replace the entire definition of a service? In my opinion, this type of override typically happens in an application-specific provider, not in a library provider - so I think it's actually safer and simpler to replace whole definitions.

So I think this comes down to opinions, really? And I think community standards should try to stay away from things that come down to mostly opinion.

But where does that leave extensions? They require a second parameter in __invoke(), so having an extension implement ServiceFactoryInterface doesn't work.

Yes, extensions are topic we haven't explored yet - I omitted them from my prototype for now.

They will most likely need a separate interface, since their __invoke signature would be different.

[mindplay-dk]

I think this is a problem of terminology definitions. The way I understand it is like this:

$definition = function(ContainerInterface $c) {};

The "Factories" section uses phrases like, "Factories accept one parameter".

The method-name in the existing interface is getFactories.

True, "definition" and "entry" are used in other sections - but this needs to fit into that section.

I'd rather not challenge the existing documentation or it's terminology. 🤔

[XedinUnknown]

Well, from what I gathered, both factories and extensions are definitions. So probably ServiceDefinitionInterface would declare __invoke(ContainerInterface $c): mixed, and then an ServiceExtensionInterface extends ServiceDefinitionInterface would extend that with __invoke(ContainerInterface $c, mixed $prev = null): mixed

[mecha]

I think that's a matter of opinion.

I completely agree. I think you misunderstood that I want such replacement to be supported by a container - I do not. This is about handing control back over to the application, allowing it to decide how to use service providers before constructing its container.

I'd rather not challenge the existing documentation or it's terminology

Me neither. But like you said

[extensions] will most likely need a separate interface

And for that we'd need an interface from which both factories and extensions derive from, to allow a consumer to expect either one or the other. And since an ExtensionInterface cannot extend a FactoryInterface due to incompatible __invoke() signatures, the only route is to have a 3rd interface that acts as a parent. The names we have available to us are "service", "definition" or "entry".

Or we could keep what we have now: ExtensionInterface makes the second argument optional. It's not quite inline with what the SP spec says; extensions MUST get a second argument. But it may be the only solution.

[mecha]

@XedinUnknown Yeah so 2 interfaces, like what we currently have in dhii/service-interface

interface WhateverWeDecideToCallIt {
	public function getDependencies(): array;
	public function __invoke(ContainerInterface $c): mixed;
}

interface ExtensionInterface extends WhateverWeDecideToCallIt {
	public function __invoke(ContainerInterface $c, mixed $prev = null): mixed;
}

Looking at it from a different perspective, calling the primary interface FactoryInterface would make sense. You can view extensions as a factories (because they still create some value) but ones that need a previous value (hence why they're called extensions). So I'm a bit less opposed to the name FactoryInterface now.

But if this will be a proposal to integrate into the container-interop/service-provider spec, it would be worth getting the opinion of the current contributors.

[mindplay-dk]

Well, from what I gathered, both factories and extensions are definitions.

Aha, the terminology makes sense now. 🙂

I'm not sure I understand the examples you both posted - the factory/extension interfaces override __invoke using a $previous that defaults to null? This should be a required argument.

I think the parent interface needs to be a proper facet - it's what both interfaces have in common, which is getDependencies and not __invoke, which has a distinct signature in each interface.

That is, __invoke has the same name in both interfaces, but it doesn't represent the same method in the OOP sense - it doesn't make sense for someone to implement both the factory and extension interface in the same class, right? These types are essentially types of functions in the FP sense, so they must be mutually exclusive - a function can't be two functions, right?

So, short version in semi-pseudo-code, we end up with something like:

interface ServiceDefinitionInterface
{
    public function getDependencies(): string[];
}

interface ServiceFactoryInterface extends ServiceDefinitionInterface
{
    public function __invoke(ContainerInterface $container): mixed;
}

interface ServiceExtensionInterface extends ServiceDefinitionInterface
{
    public function __invoke(ContainerInterface $container, mixed $previous): mixed;
}

Now, if you type-hint against ServiceDefinitionInterface, all you have access to is getDependencies - which is the only thing the two interfaces have in common, so that seems correct?

[XedinUnknown]

the factory/extension interfaces override __invoke using a $previous that defaults to null? This should be a required argument.

According to the docs, $previous is an optional argument. Also, making it non-optional looks like it would break LSP.

[XedinUnknown]

I'm going to add comments relating to e.g. ServiceInterface directly on the PR from now, FYI.

[mecha]

I'm not sure if there would be a case where ServiceDefinitionInterface should be invoked. But I guess if the consumer can't tell what it is (factory or extension), it can't pass the right args anyway.

So I'm inclined to agree with @mindplay-dk on the interfaces.

[mecha]

I'm going to add comments relating to e.g. ServiceInterface directly on the PR from now, FYI.

That's a good idea. This thread is already quite long.

[mindplay-dk]

the factory/extension interfaces override __invoke using a $previous that defaults to null? This should be a required argument.

According to the docs, $previous is an optional argument. Also, making it non-optional looks like it would break LSP.

Hmmmmmyeah, but then it goes on to explain:

This parameter CAN be typehinted on the expected service type and CAN be nullable

I think maybe the intention with the default null was actually just to make it nullable? And that's how you did that in older versions of PHP, before it was possible to declare something as nullable without implicitly declaring it optional?

I mean, it wouldn't make any sense for this to be optional - an extension can't get invoked unless there's a factory definition for whatever it's trying to modify. Even if that factory produces a null, that has to make it nullable only, not necessarily optional.

To put it another way, if passing the previous value was optional, the interface wouldn't make any sense at all - you can't extend something if passing that something to your function is optional.

I think we need to hear from the original authors on this. 🤔

[mindplay-dk]

Hm, well, here's some sort of explanation, I guess, maybe:

service-provider/src/ServiceProviderInterface.php

Lines 29 to 36 in 4969b9e

	* Callables have the following signature:
	* function(Psr\Container\ContainerInterface $container, $previous)
	* or function(Psr\Container\ContainerInterface $container, $previous = null)
	*
	* About factories parameters:
	*
	* - the container (instance of `Psr\Container\ContainerInterface`)
	* - the entry to be extended. If the entry to be extended does not exist and the parameter is nullable, `null` will be passed.

"If the entry to be extended does not exist" - this seems to imply containers should ignore missing factories and just invoke the extensions with null. I would have assume any container would throw for missing factories - mine certainly does, and I wouldn't want to change that, but I suppose nothing is stopping container authors from allowing it and just invoking extensions.

Either way, that probably explains why it's nullable - but still doesn't explain why it's optional.

I mean, the end result is the same - whether an extension gets called with null or gets called with null by default, the parameter will be null either way.

So I don't believe there's any reason for this be nullable, or any real reason why this inline documentation needs to stamp out two different callable signatures here? Either of those would get called with null if the factory is missing, or if it's registered as null.

I would think this should be corrected, but will wait for confirmation here before touching it.

[mindplay-dk]

Regarding nullables, this has been clarified in the ongoing first draft of the PSR.

To the original subject of this thread: there is an ongoing PR #54 which would make dependencies enumerable.

I don't think there's anything further to discuss on that subject, so I'm closing this issue - if more discussion is required, please start a thread in Discussions.