http-interop / http-middleware

PSR-15 interfaces for HTTP Middleware
MIT License
73 stars 7 forks source link

Allow Stack to implement Middleware #3

Closed shadowhand closed 8 years ago

shadowhand commented 8 years ago

An interesting suggestion made by @mnapoli was that a middleware Stack could be used as a Middleware to allow deep chaining. In order to achieve this, Stack and Middleware would need to have compatible signatures. Currently both define a process() method with an incompatible signature.

Is this something that should be allowed?

If it is allowed, what would be appropriate names for each method?

shadowhand commented 8 years ago

Personally I am in favor of allowing Stack to be a Middleware. For naming:

Middleware::process($request, $next);
Stack::dispatch($request);

Thoughts @http-interop/http-middleware-contributors ?

mindplay-dk commented 8 years ago

I already implement dispatcher as middleware - I believe it's important to achieve better modularity. (you can see here how I did it in my dispatcher.)

I've noticed something interesting about what's currently referred to as the "stack" interface - it's actually identical to a controller interface:

interface Controller {
    public function dispatch(RequestInterface $request): ResponseInterface;
}

"Takes a Request and produces a Response" is probably the broadest, most general definition you can have of a controller interface?

This was one of the reasons I changed my mind about lambda vs double-pass - it fits much better with controllers. (whereas with double-pass, would-be controllers had to be "response builders" which were given the existing incoming Response and had to populate it, which seemed wrong.)

Either way, what I'm thinking is that this pattern is so common and so generally useful, we should not refer to it as a "stack interface" - a stack happens to be a controller, but so do many other types.

shadowhand commented 8 years ago

@mindplay-dk I don't want to mix middleware with MVC. In the case of client middleware, calling it a controller doesn't make sense. Those of us that use ADR will find it confusing.

The "stack" name also has foundation in the StackPHP project.

mindplay-dk commented 8 years ago

I don't want to mix middleware with MVC

Understood and agreed.

In the case of client middleware, calling it a controller doesn't make sense

Okay, but calling it a "stack" doesn't make sense either.

The ability to dispatch a request and return a response does not make something a stack. The methods previously present were about manipulating a stack - the remaining interface method signature has nothing directly to do with being a stack.

So how about a name that describes only the remaining responsibility, e.g.Dispatchable?

My concern remains though, that this interface signature is so general, it's not even middleware-specific in the first place - having multiple identical interfaces might create interoperability problems.

I can picture having to write dummy adapters, just to get around typing issues - like, say:

use Psr\Middleware\DispatchableInterface;
use Foo\Bar\ControllerInterface;

class ControllerAdapter implements DispatchableInterface {
    public function __construct(ControllerInterface $controller) {
        $this->controller = $controller;
    }

    public function dispatch(RequestInterface $request) {
        return $this->controller($request);
    }
}

I'm honestly not even convinced that this interface rightfully belongs in a middleware standard?

It could potentially be an isolated standard based on PSR-7 - or an amendment to PSR-7, which, even that might be a bit of a stretch, though I suppose you could argue that this interface is about the exchange of HTTP messages, which would make it possibly fit into PSR-7?

I know that this distracts from the goal of getting to a PSR standard for middleware, but please do give some serious thought to the implications if we standardize on something this fundamental as part of an architectural standard.

shadowhand commented 8 years ago

Renaming Stack to Dispatchable works for me.

mnapoli commented 8 years ago

What will this interface solve in terms of interoperability?

The middleware is useful because middlewares can be written once and reused.

If we standardize a "Dispatchable", it means about the same thing as the HttpKernelInterface right? Which is an HTTP application? What do we gain by standardizing that? We don't want reusable and composable applications, because that's exactly what middlewares are for (like Stack before we had PSR-7).

mindplay-dk commented 8 years ago

What will this interface solve in terms of interoperability?

Dispatchers can implement it, which makes it every easy to exchange dispatchers in frameworks.

What do we gain by standardizing that?

I think that HttpKernelInterface is more about interoperability with Symfony - this interface is about interoperability of dispatchers.

We don't want reusable and composable applications

We don't? What is it we're doing then? :-)

mindplay-dk commented 8 years ago

It's something close to the run() method in Expressive maybe?

https://github.com/zendframework/zend-expressive/blob/master/src/Application.php#L553

Except without compounding the responsibility of generating a request for a response with the responsibility of creating the request, which leads to dependency on a specific the PSR-7 implementation; we'll have PSR-17 for that.

And without depending on an emitter - something we don't have a story for at the moment. Which again makes me think that there is an entire area of responsibility which isn't covered by any PSR yet, which is the exchange of messages, which would include also an interface for an emitter. I'm not convinced that this belongs either in PSR-7 or (especially) not here.

I feel increasingly like these responsibilities are not specific to middleware, but are more generally about HTTP message exchanges, which would fit better in PSR-7, or as an entirely separate (and very simple) PSR...

mnapoli commented 8 years ago

We don't want reusable and composable applications

We don't? What is it we're doing then? :-)

Come on that's exactly what I said in the part that you cut:

We don't want reusable and composable applications, because that's exactly what middlewares are for


Dispatchers can implement it, which makes it every easy to exchange dispatchers in frameworks.

OK but why put that interface in a middleware PSR? Why not include a Router interface too for example? I think a Dispatcher interface is fine (without the mutable methods) but in a separate PSR, because a dispatcher doesn't have to be middleware-based.

It seems we are thinking in the same direction:

I feel increasingly like these responsibilities are not specific to middleware, but are more generally about HTTP message exchanges, which would fit better in PSR-7, or as an entirely separate (and very simple) PSR...

mindplay-dk commented 8 years ago

Come on that's exactly what I said in the part that you cut

Haha, sorry, I did that to emphasize how you already made that point ;-)

OK but why put that interface in a middleware PSR?

And now you're making my point ;-)

Okay, so, as I said in my first comment on this topic, I know that this distracts from the goal of getting to a PSR standard for middleware - but it really seems like at least the Dispatchable interface is actually a dependency of this PSR.

It sounds like @mnapoli and I agree that putting it in this PSR seems wrong - I'm not sure @shadowhand is on board with that idea yet? I know it's really annoying to have to switch tracks, but I think we might be creating a real problem for our future selves and the community if we slack on this one.

We also identified another responsibility, Emitter, which might be about exchange too, but perhaps more appropriately belongs as an amendment to PSR-7? Or perhaps yet another PSR, although that one isn't a dependency of this PSR either way, so I think we can set aside that one for now.

There is probably also some reluctance to start a PSR with just one interface with one method? On the other hand though, this one is likely a slam-dunk, as rare as those might be in the land of PSR quibbles.

interface DispatchableInterface
{
    /**
     * @return ResponseInterface
     */
    public function dispatch(ServerRequestInterface $request);
}

Am I missing something obvious? I've never used client middleware at all, so you can guess where I'm going with that question... do only server-requests get "dispatched", or client-request too?

If so, would it be a problem if there's just one interface and it was type-hinted against RequestInterface, or does that take us right back to this discussion?

Crell commented 8 years ago

If a middleware PSR is basically unusable without a dispatcher/stack/queue/collection/thing interface to go with it, then it's not appropriate to split off, IMO. The HTTP Factory is reasonably stand-alone and can be used in places other than middlewares so it makes sense as its own PSR. But if there's no way to connect middlewares together, they're useless. So whatever that mechanism is or gets called it should be included in the same spec, not split off to a separate one.

mnapoli commented 8 years ago

Mmh now there's something I don't get. If we have standard middlewares, then we can use them in all [compatible] frameworks and applications out there. It's all good!

Why then:

but it really seems like at least the Dispatchable interface is actually a dependency of this PSR.

I don't see what prevents us from using PSR-15 middlewares in frameworks/applications that will accept them? Why do we need a dispatchable for this PSR (not questioning the interface itself), why is it a dependency?

If a middleware PSR is basically unusable without a dispatcher/stack/queue/collection/thing interface to go with it, then it's not appropriate to split off, IMO.

Why would PSR-15 middlewares be unusable without the dispatch/stack/… ? That's exactly the point: everyone is free to do whatever (pipe, stack, route, …) they want with middlewares because those have a standard interface.

To me it's like saying PSR-7 is unusable because we don't have standard routers or HTTP clients. That's a totally different level of interoperability.

mindplay-dk commented 8 years ago

@Crell @mnapoli the PSR isn't useless without this piece - the issue isn't interop of middleware with frameworks/applications, it's the ability to exchange implementations of the actual middleware stacks.

In other words, you are correct, this doesn't (strictly speaking) have anything to do with middleware - not directly, but the StackInterface was brought up originally, and I pointed out that this interface is extremely general, and not specific to middleware at all. It's presently part of the PSR, is why I brought it up.

@mnapoli you're right, this isn't really about interop at the middleware level, though it does pertain to middleware dispatchers - which this PSR doesn't strictly need to mention at all, in order to be useful; without any mention of what a stack should look like, middleware would be interoperable without it, but middleware stacks themselves, as components in an application/framework, would not.

I would think that a separate PSR for message dispatch would be at least a soft dependency after all though - if nothing more, then at least by referencing it with a recommendation, along the lines of, "a dispatcher capable of processing PSR-15 middleware SHOULD implement the PSR-(?) Dispatchable interface".

If a dispatch PSR is not a hard dependency, I suppose we could simply remove this interface from the specification for the time being, and add a note to the meta-doc stating why the PSR does not at this time include such an interface, and that the PSR may be amended with a recommendation, at a later time, if such as PSR becomes available?

shadowhand commented 8 years ago

The stack/dispatcher part of PSR-15 has always been optional. I think it should be included simply for completeness. If no one ends up using it, there is no problem. If people do end up using it, there is no problem. Why continue to debate this?

mindplay-dk commented 8 years ago

Why continue to debate this?

Because if we include it, it collides head-on with a future dispatch PSR - and/or hampers the creation of such a PSR. Whether people use it or not. The mere existence of it is the problem.

And because it's not essential to this PSR, or to the functioning or consumption or middleware at all - as proven by the fact that it's optional.

It's not about interop of middleware - it's not even about interop of middleware stacks, it's about interop of any kind of message dispatch mechanism, whether that happens to provide middleware support or not.

While I think it's an important interface, it's not specific to middleware at all, and I think this PSR is complete without it.

@http-interop/http-middleware-contributors do you disagree?

shadowhand commented 8 years ago

it collides head-on with a future dispatch PSR

To my knowledge, no one has suggested such a PSR, and if it did exist it would not be a problem since it wouldn't share the same namespace. This implied PSR could also state that it deprecates the PSR-15 interface, would be a separate package, and would therefore allow an upgrade path.

mindplay-dk commented 8 years ago

no one has suggested such a PSR

we just did :-)

if it did exist it would not be a problem since it wouldn't share the same namespace

A name collision is not the issue - overlapping concerns is the issue. Once people start supporting this interface, and then there's another identical interface in a different namespace for a different PSR, we have problems.

It'll never get to that though, because this PSR would hamper that PSR - everyone will be like, "but PSR-15 already covers that". It won't seem necessary. Only, PSR is about middleware, and has a much larger scope, dispatch being just a detail. So people won't rely on it to standardize dispatch - except maybe of middleware dispatchers - because it's a middleware PSR.

Interop at this level is not a pressing matter as far as this PSR goes, correct? So why should we press it.

Returning to this general issue at a later time, outside the scope of middleware, is perfectly practical. So I think this is an opportunity to reduce the scope and move this PSR along. Why not?

mnapoli commented 8 years ago

+1 with everything @mindplay-dk said

With the logic "why not?" we could include anything in the PSR. We need to think the other way around: "why?", and include only what's needed for middleware interoperability. This is not directly related to using middlewares, it's related to using dispatchers.

To me it's the same thing as PSR-15 should not have been in PSR-7.

We are still debating this here, this will be way worse when the PSR goes to review and hundreds of people don't understand why there's this interface :)

mindplay-dk commented 8 years ago

@http-interop/http-middleware-contributors is this still an open issue for anyone, or can we close it?