`ServerMiddlewareInterface::process` vs. Migration

[schnittstabil]

The main reason for process instead of __invoke, I've seen so far was stated by @weierophinney stated at https://github.com/http-interop/http-middleware/pull/24#issuecomment-261794002:

But more difficult is adapting middleware written by end-users or implemented in third-party libraries: if they are using our existing interface (Zend\Stratigility\MiddlewareInterface [which uses __invoke]), they cannot then go and adapt their existing middleware to also work with http-middleware, as the signatures conflict.

Furthermore, @atanvarno69 and @DaGhostman have expressed concerns against __invoke. I hope, I summarize these correctly with: __invoke (may) lead to callable and lead to bad code style/design patterns – at least bad in PHP.

---

Well then, we should give __invoke at least one shot:

class LegacyMiddleware implements \Zend\Stratigility\MiddlewareInterface
{
    function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $next) {
        // modify $response [1]
        $response = $next($request, $response);
        // modify $response [2]
        return $response;
    }
}

For Single Pass, we must refactor the method body similar to this:

- // modify $response [1]
  $response = $next($request, $response);
+ // modify $response [1]

However, because of the name conflict, we must create a new PsrMiddleware, and because we want to be DRY we must refactor the repeated code out into a new BusinessLogic class.

Is this bad design? I guess not, it perfectly aligns with the Open/closed principle: supporting the next awesome middleware type would be really simple.

---

– But ewww, 3 classes instead of 1? Is it worth it, especially if we do not plan to support more than one middleware type? With ServerMiddlewareInterface::process it would be much easier to fulfill both contracts in a single class:

class LegacyMiddleware implements ServerMiddlewareInterface, \Zend\Stratigility\MiddlewareInterface
{
    function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $next) {
        // modify $response [1]
        $response = $next($request, $response);
        // modify $response [2]
        return $response;
    }

    function process(ServerRequestInterface $request, DelegateInterface $delegate) {
        $response = $delegate->($request);
        // modify $response [1]
        // modify $response [2]
        return $response;
    }
}

– Ah, this looks much better: it can be used in a legacy stack and a PSR stack – no problem!

Mhmm, wait, above reminds me of something, which seems a bit unrelated to our problem at the first glance: The diamond problem of Multiple Inheritance. Applied to middlewares, it leads to the question:

Should a Middleware Container which is capable to handle both call __invoke or process?

– Err, process of course! It is the new way, isn't it?

Sadly, that is not that easy. Modifying $response before calling $next is not only a common pattern in Double Pass, it is also a valid one. That is because of the Double Pass Policy: no middleware creates and returns new responses – they only modify the provided $response.

If we cannot easily change the // modify $response order, then we have encountered a problem with the Double Pass Policy. As the policy no longer holds, we must change our business logic to support Single Pass, i.e. other middlewares in the stack must change as well.

Therefore it depends on your concrete stack setup: Does it manages the new Single or the the old Double Pass business logic? Even worse, if one naively adds implement ServerMiddlewareInterface then he may created a BC: the middleware does not work with the old Double Pass business logic anymore.

---

My summary: As most frameworks use __invoke interfaces or callable we should use an __invoke interface as well. Because of the naming conflict, it leads to better design. And furthermore, debugging type errors in mixed setups is much easier than debugging BCs because of naively added implement ServerMiddlewareInterface.

[atanvarno69]

The current meta adequately summarizes why __invoke() is not preferred for ServerMiddlewareInterface. In order to sell __invoke(), you would need to explain:

• why those two points are wrong, and/or
• the concrete benefit(s) of __invoke() over process()

Instead, you go from the quote from @weierophinney to concluding the best solution is to disallow adapting existing middlewares by making the interfaces incompatible, which is the opposite of the point @weierophinney was making. The only justification given is:

Well then, we should give __invoke at least one shot

Why? As far as I can see, all this does is make adoption harder.

[schnittstabil]

Sorry, I should have mentioned the current meta as well:

Why doesn't middleware use __invoke?

Doing so would conflict with existing middleware that implements the double-pass approach and may want to implement the middleware interface.

Maybe, my point was not clear enough:

the concrete benefit(s) of __invoke() over process() As far as I can see, all this does is make adoption harder.

I claim exactly that, process() makes adoption harder!

It means if we only add functionality – in this case a process implementation – e.g. a simple stub:

class LegacyMiddleware implements ServerMiddlewareInterface, \Zend\Stratigility\MiddlewareInterface
{
    // …
    function process(ServerRequestInterface $request, DelegateInterface $delegate) {
        return $delegate->($request);
    }
}

Then this will break existing applications! (Maybe Stratigility always calls the old __invoke and is unaffected by this today, but when they drop support for their __invoke the same happens.)

In my opinion, that would be really, really hard to debug.

[schnittstabil]

Side note: Personally, I prefer the 3 classes solution, in my opinion it is the best OO way one may take. Furthermore this is only about middlewares which are intended to be reused, and of course, we can do other things as well. In most cases we can use a simple adapter, like Zend\Stratigility\Middleware\CallableMiddlewareWrapper that is fine as long as our business logic allows that. Furthermore, it is just an interim solution: we can migrate smoothly to PSR-15:

class LegacyMiddleware implements \Zend\Stratigility\MiddlewareInterface
{
    function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $next) {
        //…
    }
}

// Not exactly, but I hope you get my point:
$psrMiddleware = new CallableMiddlewareWrapper(new LegacyMiddleware());

But the main difference: Now the developer is in charge, he decides to use the legacy Middleware or the PSR-Middleware – and I believe he is the only right one and we cannot remove the burden of responsibility from him.

[weierophinney]

It means if we only add functionality – in this case a process implementation – e.g. a simple stub:

class LegacyMiddleware implements ServerMiddlewareInterface, \Zend\Stratigility\MiddlewareInterface
{
    // …
    function process(ServerRequestInterface $request, DelegateInterface $delegate) {
        return $delegate->($request);
    }
}

Then this will break existing applications! (Maybe Stratigility always calls the old __invoke and is unaffected by this today, but when they drop support for their __invoke the same happens.)

You've lost me on this example. So, I'll give a concrete example of adapting double-pass middleware to http-interop middleware.

Let's say we have the following:

class LegacyMiddleware
{
    public function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $next = null)
    {
        // do something and return a response
    }
}

The above does not explicitly implement Zend\Stratigility\MiddlewareInterface, but could at this point, and work.

Now, let's adapt it:

use Zend\Stratigility\Delegator\CallableDelegateDecorator;

class LegacyMiddleware implements ServerMiddlewareInterface
{
    public function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $next = null)
    {
        return $this->process($request, new CallableDelegateDecorator($next, $response));
    }

    public function process(ServerRequestInterface $request, DelegateInterface $delegate)
    {
        // do something and return a response
    }
}

This middleware now will work in either paradigm — double-pass or single-pass. It also means that when we remove support for callable middleware, we can remove the __invoke() method.

What we've also done is provide a way to decorate existing legacy middleware such that it will then fulfill the new specification:

use Zend\Stratigility\Middleware\CallableMiddlewareDecorator;

$middleware = new CallableMiddlewareDecorator(
    new LegacyMiddleware(),
    new Response()
);

(With our proposed 2.0 changes, you'd do that either before passing the middleware to the middleware pipeline, or we will auto-detect it and decorate it for you — assuming we have a response prototype composed.)

If the specification defines the interface using __invoke(), the user now is forced to update their middleware immediately for it to work, or use a decorator; there is no way to update their code within an existing project prior to adopting the specification; it has to be done all-or-nothing.

As such, I refute your point that defining process() as the method will break existing applications; I have provided evidence above that demonstrates it allows them to be both backwards compatible with existing projects and forwards-compatible with the spec.

[schnittstabil]

@weierophinney I already wrote:

In most cases we can use a simple adapter, like Zend\Stratigility\Middleware\CallableMiddlewareWrapper that is fine as long as our business logic allows that.

The crucial point was this:

It means if we only add functionality … then this will break existing applications!

And these bugs are really, really hard to find and debug, e.g:

-class ContentNegotiationMiddleware implements MiddlewareInterface
+class ContentNegotiationMiddleware implements MiddlewareInterface, ServerMiddlewareInterface
 {
+    public function process(ServerRequestInterface $request, DelegateInterface $delegate)
+    {
+        $accept = $request->getHeaderLine('Accept');
+        $type = $this->negotiate($accept);
+        $request = $request->withHeader('Accept', $type);
+
+        return $delegate->process($request);
+    }

You do not believe it? I've set up a repo just for you: Stratigility Migration Issue.

[schnittstabil]

With our proposed 2.0 changes, you'd do that either before passing the middleware to the middleware pipeline, or we will auto-detect it and decorate it for you.

Sorry, but in my opinion this shows that you do not need process for migration. If developers can/will wrap their legacy middlewares with a CallableMiddlewareDecorator then they could instead wrap them into a Adapter/Proxy as well. Auto-detection creates unforeseeable breaking changes.