A different approach to middleware

[Alessandro Pellizzari]

Hi all,

I have been thinking a bit about middlewares recently, and I think
everybody sat on javascript conventions regarding what I see as the
elephant in the room: $next.

IMHO it doesn't make sense to have it in the signature. It makes me
think of cooperative multitasking. :P

It also doesn't play well at all with the "functional/reactive style".


TL;DR of what I propose:

```
/** @throws MiddlewareException */
function middleware(Request $req, Response $res) -> Response;

class MiddlewareException extends \Exception
{
public function __construct(
string $msg,
int $httpStatusCode = 500,
Response $res = null,
\Exception $parent = null
);

public function getResponse() -> Response;
}
```

(We can discuss about removing the Response from the parameters and go
ahead with the factory, but I strongly believe $next should go.)

This would allow to:

1. have the middleware signature identical to the controller/actions
signature

2. allow the application/router to manage better the flow of calls.

3. avoid bugs where middlewares forget to call $next, or call it in the
wrong way or at the wrong time.

4. forbids to change $req (some could see this as a disadvantage)


A basic example of application loop could be

```
$req = $this->router->getRequest();
$res = new Response();

try {
foreach ($this->preDispatchMiddlewares as $middleware) {
$res = $middleware($req, $res);
}

$res = $controller->action($req, $res);

foreach ($this->postDispatchMiddlewares as $middleware) {
$res = $middleware($req, $res);
}

} catch (MiddlewareException $e) {
$partialRes = $e->getResponse();
// Decide to return $res, $partialRes or new Response()
}
```

We could also define a MiddlewareInterface (or an abstract class with
bogus implementations) like this:

```
interface Middleware {
/** @throws MiddlewareException */
public function preDispatch(Request $req, Response $res) -> Response;

/** @throws MiddlewareException */
public function postDispatch(Request $req, Response $res) -> Response;
}
```

for example for middleware that need to determine a status during
preDispatch and use it during postDispatch.

What do you think?

Bye.

[Woody Gilk]

I'm not at all in favor of this approach. Being able to modify the request is crucial to how middleware works.

MVC and ADR are totally unrelated to middleware, so I'm not sure why you example includes them.

--
You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
To post to this group, send email to php...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/20160619121331.477b2d2f%40namek.
For more options, visit https://groups.google.com/d/optout.

--

Woody Gilk

http://about.me/shadowhand

[Alessandro Pellizzari]

On Sun, 19 Jun 2016 13:20:27 +0000
Woody Gilk <woody...@gmail.com> wrote:

> I'm not at all in favor of this approach. Being able to modify the
> request is crucial to how middleware works.

I don't think so. The request is what comes from the client. I don't
see why a middleware should change it.

It would become some sort of global, if they could. They could inject
any status in it, hoping there are other middlewares later that can
take on it.

> MVC and ADR are totally unrelated to middleware, so I'm not sure why
> you example includes them.

Why are they unrelated? There must be something managing the request
and returning a response, sooner or later.

My application example was just that, an example of how an application
or router could work.

With my approach there would be no distinction between an action and a
middleware.

If a middleware needs to stop the processing, it throws a
MiddlewareException with statusCode 2xx. If it just needs to add things
to it, it returns it.

The concept could be extended to controllers/actions, making them
exactly the same. Less cognitive burden on the user.

The point is: in functional/reactive programming functions should be as
pure as possible, take an input and return an output, so that functions
can be "piped" without side effects.

Changing the request is a side effect.

The only thing any step should do is "change" the Response. Get the old
one, return a modified version of it or, if needed, discard it and
start from scratch. And, as Response is an immutable object, you avoid
side effects at the application level: the application has access to
the response before calling the mw and after calling it.

That's as pure as it can be.

Other languages, like Rust, have Option, Result and other wrapper
facilities for return values to allow the "manager" to decide what to
do at each step. PHP doesn't, but we can simulate them with Exceptions.

The way middlewares are now, they have several responsibilities:

1- analyze the request and prepare a response (legit)
2- call the next middleware (wrong, IMO)
3- change the request for the next middleware if needed (wrong, IMO)
4- (for the status quo) maintain a "stack" status

That, in my opinion, is a lot for a class, too much for a function.

Bye.

[Woody Gilk]

Modifying the request is critical for several things:

1. Parsing the body of a request and defining "parsed body".
2. Dispatching/Routing and storing the target action/controller in the
middleware as an attribute.
3. Defining the application's default content-type if none was defined.

Possibly more things, those are just the ones I can think of that we
use middleware for.

The problem with having no control in middleware is that you lose the
ability to terminate early. (Using exceptions for this is wrong,
because using exceptions for flow control is an anti-pattern.) When
the middleware is responsible for calling the next middleware, you
gain specific benefits:

- being able to terminate the request
- being able to modify the request before next
- being able to modify the response before return
- being able to replace the response entirely or copy part of it

None of these things are possible with your proposal unless you
completely change the paradigm that has been established for years.
Despite what it looks like, Stack PHP operated under the same general
process as PSR-15. The only difference is that in Stack, the request
was not immutable and therefore didn't need to be passed explicitly.
Each middleware in Stack could modify the request however it wanted
and would impact all later middleware.

--
Woody Gilk
http://about.me/shadowhand

> --
> You received this message because you are subscribed to the Google Groups "PHP Framework Interoperability Group" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to php-fig+u...@googlegroups.com.
> To post to this group, send email to php...@googlegroups.com.

> To view this discussion on the web visit https://groups.google.com/d/msgid/php-fig/20160619151527.3fd272b8%40namek.

[Alessandro Pellizzari]

On Sun, 19 Jun 2016 09:31:30 -0500
Woody Gilk <woody...@gmail.com> wrote:

> Modifying the request is critical for several things:
>
> 1. Parsing the body of a request and defining "parsed body".
> 2. Dispatching/Routing and storing the target action/controller in the
> middleware as an attribute.
> 3. Defining the application's default content-type if none was
> defined.
>
> Possibly more things, those are just the ones I can think of that we
> use middleware for.

All these things are done by the application or the router, not by a
middleware.

If you are using middlewares for these, we should also provide a
factory for Request, not just for Response, in case a mw wants to
rewrite it from scratch.

The Application should prepare the environment. The Router should parse
the URL and populate the Request's parameters. Middlewares should
transform the Response by using info from the Application and the
Router.

> The problem with having no control in middleware is that you lose the
> ability to terminate early. (Using exceptions for this is wrong,
> because using exceptions for flow control is an anti-pattern.)

But in this case, breaking out of the middleware flow IS an
exceptional case.

For example checking if a user is logged and breaking the flow if it's
not, or if the application got an Accept header it does not support, or
a CORS request without proper headers.

> When
> the middleware is responsible for calling the next middleware, you
> gain specific benefits:

Yes, but it's also a pattern most modern languages are getting away
from. Go takes ResponseWriter,Request. Rust takes Request,Response.
Django on Python takes Request,Response

Basically the only other language using next is Javascript.

> - being able to terminate the request

This is done through exceptions.

> - being able to modify the request before next

This is explicitly forbidden, yes.

> - being able to modify the response before return

This is not true, and is the whole point of a middleware

> - being able to replace the response entirely or copy part of it

This is also possible, as the previous response gets passed as
parameter but teh mw can create a new one or add/remove parts in
the old one through normal PSR-7 methods.

So, the only thing you lose is the ability to change the Request,
which, as I say, is a plus, not a downside.

> None of these things are possible with your proposal unless you
> completely change the paradigm that has been established for years.

Removing the Response from the signature would break all the middleware
already written and working, but that doesn't seem a problem.

> Despite what it looks like, Stack PHP operated under the same general
> process as PSR-15. The only difference is that in Stack, the request
> was not immutable and therefore didn't need to be passed explicitly.
> Each middleware in Stack could modify the request however it wanted
> and would impact all later middleware.

But immutability changes everything, and wanting a functional/reactive
behaviour is the natural consequence, and the basics of reactive
programming are in taking the input (the empty or half-baked Response)
and transforming, filtering or mapping it.

The Request is just a (read-only) status each middleware can get info
from. The whole point is to return a Response.

I know it's hard to change programming paradigm, but the way opened by
immutability is this.

Bye.

[Larry Garfield]

Shifting the $next from a parameter to an implicit constructor parameter
makes it, for any practical purpose, Symfony's HttpKernel. I'm not
opposed to that, in concept, as it moves the continuation logic from the
call to the constructor.

However, much as I'd like a direct pipeline rather than an implicit one
that relies on the stack, not letting a middleware pass a request to the
next middleware is an absolute show stopper. Remember,
ServerRequestInterface has not one but two mechanisms that exist for the
sole and express purpose of passing data from one layer to another:
Attributes and parsed body. The entire point of those methods is to
allow one middleware layer to take a request, spawn a slightly-modified
version, and pass the result to the next layer. The next layer then
doesn't know or care if that happened, but we know that it's getting a
modified version. That's exactly how body parsing, cookie encryption,
routing, and such are intended to work with PSR-7.

A PSR-7 middleware design that doesn't allow for that is doomed. That's
before even getting into the exceptions for flow control problem.

Go's ResponseWriter is all well and good, but doesn't work (directly)
with building a response as a message, potentially out of order (body
and then headers). PSR-7 is message-oriented, not stream-oriented, so
any middleware design built for it needs to operate on a message object,
not a simple output stream. (The body can be, though, by design.)

--Larry Garfield

[Alessandro Pellizzari]

On Sun, 19 Jun 2016 17:30:43 -0500
Larry Garfield <la...@garfieldtech.com> wrote:

> However, much as I'd like a direct pipeline rather than an implicit
> one that relies on the stack, not letting a middleware pass a request
> to the next middleware is an absolute show stopper.

> ...

> That's exactly how body
> parsing, cookie encryption, routing, and such are intended to work
> with PSR-7.

This is a different problem.
This means middlewares maintain 2 flows: request and response.

The SOLID solution would then be to have 2 types of middlewares:
those who change the Request and those who change the Response:

function requestMiddleware(Request $req) -> Request;
function responseMiddleware(Request $req, Response $res) -> Response;

The correct way to do it would be to have rich enums. We can simulate
them with classes (names are not final :) :

interface MiddlewareResult;

class AcceptRequest implements MiddlewareResult {
public function __construct(Request $req);
public function getRequest() -> Request;
}

class AcceptResponse implements MiddlewareResult {
public function __construct(Response $res);

public function getResponse() -> Response;
}

class Reject implements MiddlewareResult {
public function __construct(Response $res);

public function getResponse() -> Response;
}

function middleware(Request $req, Response $res) -> MiddlewareResult;


And the dispatcher:

foreach ($middlewares as $mw) {
$result = $mw($req, $res);
switch (true) {
($result instanceof AcceptRequest):
$req = $result->getRequest();
break;

($result instanceof AcceptResponse):
$res = $result->getResponse();
break;

($result instanceof Reject):
return $result->getResponse();
}
}


A couple of middlewares:

function authMiddleware($req, $res) use ($auth) {
$user = $auth->check($req->getHeaderLine('Authorization');
if (is_null($user)) {
return new Reject( $res->withStatus(401) );
}
return new AcceptRequest($req->withAttribute('user', $user));
}

function cryptoCookies($req, $res) use ($crypto) {
return new AcceptRequest(
$req->withCookieParams(
$crypto->decodeCookies($req->getCookieParams())
)
);
}

No exceptions, single flow, single return, static typing, SOLID.

Bye.

[Woody Gilk]

On Mon, Jun 20, 2016 at 2:39 AM, Alessandro Pellizzari <al...@amiran.it> wrote:
>
> The SOLID solution would then be to have 2 types of middlewares:
> those who change the Request and those who change the Response:

SOLID does not mean "a method takes one argument". Single
responsibility states that:

"a class should have only a single responsibility (i.e. only one
potential change in the software's specification should be able to
affect the specification of the class)" [from
https://en.wikipedia.org/wiki/SOLID_(object-oriented_design) ]

I think that we have a different interpretation of what that "single
responsibility" is. In my view, the single purpose of the middleware
is to process a request. That might mean creating a response, or it
might mean doing an authorization check, or it might mean parsing the
request body. Each of these things are a single purpose but the
contract is flexible enough to allow all of them to be implemented
with a single interface.

There is nothing wrong with your proposal per se, but it introduces at
least four more classes and makes dispatching and usage more
complicated.

> No exceptions, single flow, single return, static typing, SOLID.

The current proposal matches all of these requirements too:
fn(request, frame): response. There is a straight forward
directionality, a known return type from each interface, and every
parameter is type hinted.

Regards,

[Matthew Weier O'Phinney]

On Sun, Jun 19, 2016 at 2:55 PM, Alessandro Pellizzari <al...@amiran.it> wrote:
> On Sun, 19 Jun 2016 09:31:30 -0500
> Woody Gilk <woody...@gmail.com> wrote:
>

<snip>

> > When
> > the middleware is responsible for calling the next middleware, you
> > gain specific benefits:
>
> Yes, but it's also a pattern most modern languages are getting away
> from. Go takes ResponseWriter,Request. Rust takes Request,Response.
> Django on Python takes Request,Response
>
> Basically the only other language using next is Javascript.

You're making the assumption that approaches never evolve.

As somebody who ported Connect/Express to PHP, I'll weigh in on why I've adopted
the argument, and why I think it's an important part of the interface.

When you don't have the argument, the way to build your middleware application
is to pass the stack/application to each middleware via either the constructor,
a setter, or a property. While DI containers help with this, it requires that
you know all paths through the application and define this via the dependency
graph. Many frameworks thus have a conventions-based approach for injecting the
stack into middleware (e.g., the application injects itself as a property),
which means that middleware is tied to specific middleware stacks. When you have
a single, language-supported approach, such as WSGI in Python or Rack in Ruby,
then this isn't a problem. When you don't, such as in JS or PHP, it *is* a
problem, as you cannot re-use middleware between the different stacks.

Connect/Express could have taken this approach, but instead opted for passing
the stack as an argument to the middleware during invocation, via a "next"
argument. This approach meant that when Express split from Connect, *middleware
written for Connect continued to work with Express, and vice versa*.
Interoperability works best when it's not based on conventions, but rather
signatures and strict types. Passing the stack as an argument enforces exactly
that.

Another thing to consider is: does your middleware really need to store a
reference to the stack *as a property*? In particular, does this make sense in
an async environment? In Connect/Express, your middleware likely shouldn't keep
a reference to the stack, *as it will change between requests*. When we consider
that one target for PHP middleware are async libraries such as ReactPHP and
Icicle, we definitely want to consider this.

> > - being able to terminate the request
>
> This is done through exceptions.

Exceptions should be used for exceptional circumstances, not expected
application workflows.

<snip>

> So, the only thing you lose is the ability to change the Request,
> which, as I say, is a plus, not a downside.

It's a downside. One aspect of middleware is that you can *nest* middleware in
order to re-use it. As such, I can write middleware that routes for:

- /news
- /news/:date
- /news/:date/:article

and nest it in another application such that it attaches and triggers when under
the path "/devteam", and it will now respond to the following:

- /devteam/news
- /devteam/news/:date
- /devteam/news/:date/:article

*without needing to change any routing internally*. The reason it can do this is
that the middleware stack can strip the subpath "/devteam" before passing the
request on to the nested middleware.

Removing this ability would make such nesting far more difficult, making re-use
less palatable.

--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Alessandro Pellizzari]

On 20/06/2016 11:36, Woody Gilk wrote:

> On Mon, Jun 20, 2016 at 2:39 AM, Alessandro Pellizzari <al...@amiran.it> wrote:
>>
>> The SOLID solution would then be to have 2 types of middlewares:
>> those who change the Request and those who change the Response:
>
> SOLID does not mean "a method takes one argument".

No, it doesn't. And my proposal has several functions with 2 arguments.

> Single
> responsibility states that:
>
> "a class should have only a single responsibility (i.e. only one
> potential change in the software's specification should be able to
> affect the specification of the class)" [from
> https://en.wikipedia.org/wiki/SOLID_(object-oriented_design) ]

Yes, and, as I said, middleware now have at least 3: change the request,
change the response, call the next middleware.

> I think that we have a different interpretation of what that "single
> responsibility" is. In my view, the single purpose of the middleware
> is to process a request. That might mean creating a response, or it
> might mean doing an authorization check, or it might mean parsing the
> request body. Each of these things are a single purpose but the
> contract is flexible enough to allow all of them to be implemented
> with a single interface.

And I agree, but "call the next middleware" should not be its
responsibility.

Bye.

[Alessandro Pellizzari]

On 20/06/2016 17:05, Matthew Weier O'Phinney wrote:

> When you don't have the argument, the way to build your middleware application
> is to pass the stack/application to each middleware via either the constructor,
> a setter, or a property.

And this is the problem.

Why do you need to pass the stack/application to a middleware?

A middleware should be agnostic about it. Just receive a Request and
output a Request, a Response, or signal the intention to stop the flow
and return a different Response.

It could need something to do its job, as a DB connection, or a crypto
component, or a session manager, but not the whole application. And
these can be easily injected in a class in the constructor or in a
function with use().

BTW, we are discussing an interface that will be used by components. If
a component (middleware) needs some object, it will define an interface
and accept only objects adhering to that interface.

It will be the developer's responsibility to write a "bridge" for the
application classes.

> Another thing to consider is: does your middleware really need to store a
> reference to the stack *as a property*?

No, that's the point: it must not store the stack in any way.

> In particular, does this make sense in
> an async environment?

Not having an external state/stack makes a lot of sense exactly in an
async environment. The purest the function/class, the better.

> Exceptions should be used for exceptional circumstances, not expected
> application workflows.

I changed the proposal in the following message to avoid exceptions and
allow to change the request, using functional languages style (wrapped
with objects, because we only have those)

Bye.

[Alexandru Pătrănescu]

Guys, it's all about the flow control and how to handle it while keeping as many things in the stack and not as state on middleware.

For controlling the flow, we have this two approaches, first being the one intended and the second being more or less the one Alessandro propose.

class Middleware1
{
    public function __invoke($request, $response, $next)
    {
        $request = $this->changeRequestBefore($request);
        $response = $this->changeReponseBefore($response, $request);

        $response = $next($request, $response);

        $response = $this->changeReponseAfter($response, $request);

        return $response;
    }
}


class Middleware2
{
    public function requestBefore($request)
    {
        return $this->changeRequestBefore($request);
    }
    
    public function responseBefore($request, $response)
    {
        return $this->changeReponseBefore($response, $request);
   }

    public function responseAfter($request, $response)
    {
        return $this->changeReponseAfter($response, $request);
    }
}

Indeed, the second approach allows to have better single responsibility per method/class but forces usage of the state because in practice, it's often the case when some local variable calculated in requestBefore part or responseBefore part are needed in responseAfter part also. If we have them all in one method, we can keep them locally.

IMO, calling $next is just a way to control the flow, exiting the current method and continuing after. Just how stacks work.

One more thing that sometimes is forgotten: the main purpose of FIG is for framework interoperability and PSRs are sometimes more on how frameworks are and not necessary on best design principles. And that's good, because this brings help related to most helpful design from practice. First approach is more helpful in practice.

Alex