Closed mindplay-dk closed 10 months ago
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.
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?
My current PSR draft includes this section:
4.2. Idempotency
Calling a factory or extension multiple times with the same container instance SHOULD result in the same service being created each time. Factories and extensions SHOULD NOT maintain internal state that modifies the returned service.
I do agree that service providers should be idempotent - this doesn't mean they're not allowed to have constructors or accept/hold configuration values required for bootstrapping.
You can have two provider instances that don't produce the same result - however, if you have identical provider instances, they must produce the same result, at all times.
@moufmouf is this sufficient to address the issues you're concerned with?
Sorry, wrong thread, will repost in #65
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.)
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.
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. 😄✌️
@moufmouf in the "Entry Extension" section of the README, I found the following requirement:
https://github.com/container-interop/service-provider/blob/a55e71782c7c19b34112296ac22d2ae42b564469/README.md?plain=1#L284
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:
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.)