Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarification: null "will be passed" for a "service that does not exist" #66

Closed
mindplay-dk opened this issue Dec 23, 2023 · 7 comments
Closed

Comments

@mindplay-dk
Copy link
Collaborator

@moufmouf in the "Entry Extension" section of the README, I found the following requirement:

If an extension is defined for a service that does not exist, null will be passed as a second argument.

While this isn't written in specification language (e.g. "SHOULD") this might create some confusion.

It is of course predicated on the preceding example of optional service dependency:

public static function getLogger(ContainerInterface $container, Logger $logger = null)
public static function getLogger(ContainerInterface $container, ?Logger $logger)

But the language here seems problematic: "for a service that does not exist".

I believe the intention with this example was to demonstrate compatibility with the use of "NULL services" - meaning, a service that has been explicitly registered as NULL, and as such does exist?

If a service "does not exist", meaning it isn't registered at all, I don't believe most existing DI containers would even attempt to invoke any extensions? So this requirement would create an interoperability problem.

As said though, this isn't written in specification language, so it was probably just meant as an example, and not as a requirement that DI containers attempt to invoke extensions for services that "do not exist"?

(I just want to make sure I understand the intention here, as I am working on extracting everything in "loose form" from the README into the PSR and meta document drafts.)

@moufmouf
Copy link
Contributor

Gosh, I must admit I absolutely don't remember what triggered this.

Reading this in retrospect, it seems weird, indeed.

I remember that at some point, I was wondering how a service provider could inject a HTTP middleware at a given point in the stack of middlewares (for instance, a middleware doing error handling needs to come first in the stack, while CORS middleware can come later, etc...)
My idea was to have a service named "middlewares" that returned a SplPriorityQueue of PSR-15 middlewares. Then, each service provider providing the middleware could extend the "middlewares" service and inject a middelware at a given position.
In this case, it could be (moderately) useful for a service provider to create the "middlewares" service if it does not already exists.
But let's face it, this is really only moderately useful. If you want to get rid of the "NULL" service wording, I don't have a strong objection.

What is haunting me however, is how we can resolve this "middlewares list" problem in a compiled fashion.
Ideally, compiled containers (i.e. Symfony) should be able to build the container with the list of middlewares almost hard coded in the container (instead of having to make a function call for each middleware so that the middleware can extend the middlewares list). This optimization should happen at compile time, not at runtime. This is something we never got right so far and one of the biggest issues with all our proposals so far.

@mindplay-dk
Copy link
Collaborator Author

What is haunting me however, is how we can resolve this "middlewares list" problem in a compiled fashion. Ideally, compiled containers (i.e. Symfony) should be able to build the container with the list of middlewares almost hard coded in the container (instead of having to make a function call for each middleware so that the middleware can extend the middlewares list). This optimization should happen at compile time, not at runtime. This is something we never got right so far and one of the biggest issues with all our proposals so far.

We spent considerable time thinking about this in one of my previous jobs, and our conclusion, finally, was that, this is such a small problem, and the potential solutions all have serious drawbacks, that it was just not really worth bothering with.

Numeric priority: fragmented, unreadable code - you'd likely need to var_dump your middleware stack to make sure it ends up calculated exactly the way you want it. Even then, somebody else could change something and break your project.

Topological sorting: more reliable, but a topological sort is kind of a high price to pay, at every request, just to establish your middleware stack.

Using extensions and hand-holding (assuming here middleware is a list of class-names and you have another service that maps those against ContainerInterface::get) for example:

class MyMiddlewareProvider implements ServiceProviderInterface
{
    public function getExtensions()
    {
        return [
            "middleware" => function (ContainerInterface $c, array $middleware) {
                $pos = array_search(PreviousMiddleware::class, $middleware);
                return array_splice($middleware, $pos, 0, MyMiddleware::class);
            }
        ];
    }
}

I don't even know if I wrote that correctly, haha.

No matter what you do, it's going to get fragmented, and in the end, what we opted for was a simple service override in the project provider, which would replace the whole list of middleware - there is typically, what, 10 middlewares in a project, tops? Just get over it and type them out. You'll get human-readable code that does exactly what you think it's going to do.

It doesn't seem like a problem worth solving, unless maybe you're dreaming about auto discovery and all that jazz.

Either way, it seems like more of a framework problem than a DI container problem?

In a framework context, if I had to solve this for the sake of modularity or auto-discover, I would probably have something like a MiddlewareRegistry, and just write extensions that manipulate the registry during bootstrap, e.g.:

class MyMiddlewareProvider implements ServiceProviderInterface
{
    public function getExtensions()
    {
        return [
            MiddlewareRegistry::class => function (ContainerInterface $c, MiddlewareRegistry $registry) {
                $registry->addMiddlewareBefore([PreviousMiddleware::class], MyMiddleware::class)`);
            }
        ];
    }
}

I think there are more than enough ways to solve this problem without this PSR needing to do anything specific to solve it. Would you agree that this is probably outside the scope of this PSR?

@mindplay-dk

This comment was marked as off-topic.

@mindplay-dk
Copy link
Collaborator Author

Sorry, wrong thread, will repost in #65

mindplay-dk added a commit to mindplay-dk/service-provider that referenced this issue Dec 27, 2023
@mindplay-dk
Copy link
Collaborator Author

I'm changing the wording in that phrase to:

This allows an extension to allow (and handle) a service that has been explicitly registered as null.

It's relevant to the example, although the requirement was not.

I'm closing this issue, as the original question has been answered. (I'm sure we'll talk more about the other topics.)

@moufmouf
Copy link
Contributor

I think there are more than enough ways to solve this problem without this PSR needing to do anything specific to solve it. Would you agree that this is probably outside the scope of this PSR?

I remember I had this discussion with @mnapoli a few years back and he was also advocating to put this out of scope of this PSR.

I guess I'm in minority here, but it is indeed a pet peeve of mine. I'm dreaming of something that (in the end) would have autodiscovery of service providers. In my ideal world, you would "composer install" a DI container and a bunch of service providers, and shazaam! everything would be set up automatically.

@mindplay-dk
Copy link
Collaborator Author

I guess I'm in minority here, but it is indeed a pet peeve of mine. I'm dreaming of something that (in the end) would have autodiscovery of service providers. In my ideal world, you would "composer install" a DI container and a bunch of service providers, and shazaam! everything would be set up automatically.

I don't think you're the minority, really - if anything, I'm the minority. 😅

Ideally, I don't want frameworks to magically configure or do anything at all - I want them to implement things that are difficult to implement, but I think they should stay away from anything that's already easy and avoid adding complexity trying to make easy things easier. DI containers solve the hard problems for me already.

Interoperable providers on top of that would take 90% off the setup work and make it more than easy enough, without adding any complexity you wouldn't have needed to add yourself anyway.

Yet, I think most people would prefer what you're describing - if you could just composer install and start filling in the blanks, most people would probably be happy. If you have to additionally add a few providers, ooh, they're already starting to think about writing a meta-framework for your framework, because, yuck, that's 15 lines of boilerplate I've had to write on two different projects this year, oh no, this won't do, hehehe. 😉

But what do I know, maybe you'll figure out a great way to do it that's undeniably more convenient and safe, without adding much complexity at all - just because I don't have the imagination to come up with something like that doesn't mean it's not possible, and this PSR shouldn't get in the way of that. 😄✌️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants