http-interop / http-middleware

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

Add decorator interfaces #73

Closed mindplay-dk closed 7 years ago

mindplay-dk commented 7 years ago

Now that the scope of the package has changed from "just middleware" to the broader "server" scope, I'd suggest adding two generally useful interfaces for pre-processing of requests and post-processing of responses - e.g. the request->request and response->response lambdas.

interface PreProcessorInterface {
    public function preProcess(ServerRequestInterface $request): ServerRequestInterface;
}

interface PostProcessorInterface {
    public function postProcess(ResponseInterface $request): ResponseInterface;
}

These two interfaces were part of the http-message-strategies package, along with the handler-interface - they used different, more abstract terminology: request and response "operators", whereas I'm proposing interface names that describe the intended use in a server-context.

I've been using the post-processor interface on a few projects - for example, I use it in a session-abstraction (middleware) which uses a post-processor to commit the session ID to the response, which gives me the benefit of being able to bootstrap the session abstraction (in a DI container) in such a way that neither the post-processor or the session abstraction are loaded at all, unless they're actually activated in the DI container during the request.

I haven't personally had any real use-case for the pre-processor interface as of yet, but I feel it should be included for completeness.

danizord commented 7 years ago

Hey @mindplay-dk! Maybe I'm missing something about your proposal, but not sure if we need yet another way of doing pre/post processing here. That's exactly the point of MiddlewareInterface after all, both pre and post processors can be implemented using existing MiddlewareInterface:

class MyPreProcessor implements MiddlewareInterface
{
    public function process(
        ServerRequestInterface $request,
        DelegateInterface $delegate
    ): ResponseInterface {
        $request = $this->doSomePreProsessing($request);

        return $delegate->process($request);
    }
}
class MyPostProcessor implements MiddlewareInterface
{
    public function process(
        ServerRequestInterface $request,
        DelegateInterface $delegate
    ): ResponseInterface {
        $response = $delegate->process($request);

        return $this->doSomePostProcessing($response);
    }
}
mindplay-dk commented 7 years ago

I was expecting someone would bring this up :-)

Yes, you can implement the same with middleware - but I believe we need these interfaces for the same reason we need the handler-interface, as a low-level abstraction: middleware is a more high-level abstraction that requires a framework for dispatch, whereas handlers provide the low-level request->response abstraction. To complement that, I believe we ought to have other two natural low-level abstractions, request->request and response->response.

See this for a longer discussion.

In my own projects, I use post-processors to avoid loading and initializing my cookie and session abstractions, unless or until they're actually invoked in the DI container - at which point the activation function in the DI container creates and injects the post-processor into a request context object, and the post-processors get applied after the middleware stack has been dispatched, e.g. at a lower level than middleware. In other words, the pre/post processor (and handler) abstractions provide potential for low-level interop - they're useful as "plumbing" in application layers below the middleware layer, as well as of course in applications that don't use middleware at all.

This is part of the reason we were proposing these three interfaces as a separate standard - they're not dependent on middleware. But now, in the name of moving things ahead more quickly, the scope of this PSR has been broadened to include "server" abstractions, and I believe these two interfaces provide two important server abstractions.

Of course, we could still do a separate PSR for these, but since this PSR has engulfed part of the other PSR, it would make more sense for this PSR to provide the same completeness the other PSR was aiming for and absorb the other two abstractions as well. It would be difficult to even define the scope of the other package now, since this PSR has broadened it's scope to cover the same general domain, "the server" - there would be a clear scope overlap, and no clear reason why the request/response (handler) abstraction doesn't exist in the same space as the request/request and response/response abstractions.

mnapoli commented 7 years ago

In my opinion and the needs I've seen, the whole point of middlewares is that this is not an interface written to represent something, but this is an interface to connect things together (interoperability).

I personally don't see a need for these interfaces, MiddlewareInterface is all I want and need.

oscarotero commented 7 years ago

I see some useful things here. There're some middlewares that can fit perfectly in some of these interfaces. For example, TrailinSlash just modify the request's uri, and Encoder compress the response. And that simplifies the error backtrace because there are less inner middlewares in the app flow.

On the other hand, this complicated a bit the middleware handler because it's needed to check the type of the object in order to insert it at the begining, middle or end of the middleware stack.

I also don't like the names (PreProccesor / PostProccesor) because say that these interfaces must be used with a middleware processor. But this is a minor issue.

danizord commented 7 years ago

@mindplay-dk

In my own projects, I use post-processors to avoid loading and initializing my cookie and session abstractions, unless or until they're actually invoked in the DI container

That's possible with MiddlewareInterface as well, for instance Zend\Expressive does that by lazy loading the middleware only when they are actually used.

Yes, you can implement the same with middleware - but I believe we need these interfaces for the same reason we need the handler-interface, as a low-level abstraction: middleware is a more high-level abstraction that requires a framework for dispatch, whereas handlers provide the low-level request->response abstraction. To complement that, I believe we ought to have other two natural low-level abstractions, request->request and response->response.

I see, but given that the same can be achieved using high level abstraction, I see no benefit in having multiple ways of doing the same thing being recommended by the same PSR, as this may cause confusion for some users.

mindplay-dk commented 7 years ago

I also don't like the names (PreProccesor / PostProccesor) because say that these interfaces must be used with a middleware processor

I was trying to place it in a middleware context, which may have been a mistake.

That's possible with MiddlewareInterface as well

With a mutable middleware stack, maybe. I don't want that - for the sake of long-running applications, the middleware stack needs to be immutable. A request-context, on the other hand, can be mutable, since it gets created and lives for the duration of the request only.

I see, but given that the same can be achieved using high level abstraction

It can't. You can't achieve low-level abstraction in a framework (or app) using a high-level abstraction that depends on a framework.

Pre-processors needs to run before the request is processed (whether by a middleware stack or something else) and post-processors need to run after the request.

And both are non-optional - they must perform an operation, just a handler must - there's no "next" component you can delegate to.

The use-cases are wildly different.

It's less so intended for users and more for low-level interop among framework/application authors.

shadowhand commented 7 years ago

both pre and post processors can be implemented using existing MiddlewareInterface

Agreed. The result of RequestHandler and Middleware is the same. The result of pre/post would not, which would broaden the scope of the PSR a bit.

On the other hand, this complicated a bit the middleware handler because it's needed to check the type of the object in order to insert it at the begining, middle or end of the middleware stack.

This is my biggest concern. Right now middleware is only one interface and you don't need runtime (duck type) checks anywhere. By having 3 possible interfaces inside of a middleware stack, everything gets more complicated and you probably need to use instanceof multiple times. That's a šŸ‘Ž in my book.

mindplay-dk commented 7 years ago

By having 3 possible interfaces inside of a middleware stack, everything gets more complicated and you probably need to use instanceof multiple times. That's a šŸ‘Ž in my book.

That is not what I'm proposing, at all.

You broadened the scope of the PSR when you decided to take over the far more general handler interface, which isn't middleware-dependent or middleware-specific at all.

You clearly indicated the broadening of the scope from just middleware to server-concerns in general by changing the namespace to Interop\Http\Server. You did not change the name of the PSR from "HTTP Server Middleware", so the way I see it, you've all acknowledged that owning the handler interface broadens the scope from middleware to server, but you're not willing to own the rest of those interfaces?

I understand you did this to accelerate the process, but you've created a problem. If we proceed with a separate PSR for the other interfaces, what will we name it? "the rest of server"? Gah.

In my opinion, there are only two options: you either propose (and wait for) the complete server PSR with the full set of interfaces - or you own the entire server domain and add the two interfaces for completeness.

I know these interfaces have nothing to do with middleware, but neither does the handler interface, it's merely a dependency that you weren't willing to wait for - it belongs to the server domain.

I really think we're creating a messy situation here, by partially absorbing another PSR with a broader scope - it will be very difficult to proceed with that PSR describing it as a PSR for the server domain, when you're already (as indicated by the namespace) taking ownership of that domain.

jschreuder commented 7 years ago

What adventage are these supposed to deliver?

class PreProcessor implements PreProcessorInterface {
    public function preProcess(ServerRequestInterface $request)
    {
        return proces_in_some_way($request);
    }
}

Instead of:

class PreProcessor implements MiddlewareInterface {
    public function process(ServerRequestInterface $request, RequestHandlerInterface $handler)
    {
        return $handler->handle(proces_in_some_way($request));
    }
}

Do you really need to double the number of interfaces for this? It seems like added complexity without meaningful benefit.

If this is about convenience, the implementation using the middlewares might always add priorities or separate lists for preprocessing middlewares, actual middlewares and postprocessing middlewares. But there's no need to require implementation of all those concepts from all middleware consumers.

pmall commented 7 years ago

@jschreuder Sometimes you want to abstract request response processing without using a middleware. Lets imagine a router which updates the incoming request method according to a '_method' post parameter. Sure you can do it with a middleware, but it implies the middleware must be processed before the router each time. To me the term middleware implies something which can be plugged in or plugged out without much trouble by the end developer, but in my example it is required for the router to work as expected.

Here what's @mindplay-dk suggest is interfaces for the proces_in_some_way part. I don't know if it needs to be standardized or if it belongs here, but I can see use cases which would be awkward with a middleware.

shadowhand commented 7 years ago

@mindplay-dk

by partially absorbing another PSR with a broader scope

And what PSR are you referring to? I don't see anything in proposed that relates to this.

shadowhand commented 7 years ago

If you're referring to http-message-strategies then I think that is a fine name for it.

mindplay-dk commented 7 years ago

@shadowhand yeah, that.

Well, nobody wants the other two interfaces here, fine, we'll remove the handler-interface from the strategies PSR then I suppose. Live and let live.

mindplay-dk commented 7 years ago

You're still creating a somewhat weird situation, in which I need to install the middleware PSR in order to get the handler-interface, whether my package has anything to do with middleware or not. I'd still strongly prefer to see all the strategy interfaces in one package and this one depending on it. There's an unclear division of responsibility. But, whatever, I guess - nobody seems to care. I rest my case.