ServerMiddlewareInterface::process leads to lesser strict type-hinting

[schnittstabil]

A small micro-framework survey:

// Slim Framework Team: Slim
$app->get('/hello/{name}', function ($request, $response, $args) {…});

// Sensiolabs: Silex
$app->get('/hello/{name}', function ($name) use($app) {…}); 

// Laravel: Lumen
$app->get('user/{id}', function ($id) {…});

// Zend: Expressive
$app->get('/path', function (ServerRequestInterface $request, ResponseInterface $response, callable $next) {…});

All of them support anonymous functions. With ServerMiddlewareInterface::__invoke they may want to type-hint against callable at someday – with ServerMiddlewareInterface::process they will never be able to do – thus they must type-hint to mixed 😒

Moreover, the same applies to every single future framework/container, which wants to support anonymous functions as well.

[atanvarno69]

They can accept whatever they like and that's not the PSR's problem.

Specifying ServerMiddlewareInterface::process does not prohibit a micro-framework from accepting callable (they could even accept [$someMiddleware, 'process']) or whatever else they want alongside instances of ServerMiddlewareInterface. This is an issue for their backend delegating logic.

The spec says their backend should provide a DelegateInterface instance to provide a response when asked to by a ServerMiddlewareInterface instance. There is no requirement for frameworks to only accept PSR-15 middleware objects. If they choose to accept callable as well, that is a design decision and an implementation detail for their delegate which the PSR is not interested in.

The PSR is only interested in making sure that any PSR-15 compliant middleware can be dropped into any PSR-15 delegate and work as expected, it is not interested in the mechanism by which that middleware is dropped in. Nor does it care if a framework wants to support its own middleware alongside PSR-15 middleware.

[schnittstabil]

Sorry, I should have mentioned the README.md:

Why doesn't middleware use __invoke?

…
In addition, classes that define __invoke can be type hinted as callable, which results in less strict typing. This is generally undesirable, especially when the __invoke method uses strict typing.

In practice, and I hope I showed that above, it would lead to type-hinting against mixed which is much more undesirable than callable.

[shadowhand]

It's not at all fair to compare frameworks that are using the double-pass approach and then claim that this makes current type hinting situation worse.

[schnittstabil]

I don't understand. What is the difference? I believe the same applies to the single-pass approach.

[shadowhand]

None of them are based on PSR-15. It's comparing apples or oranges, or at least oranges to tangerines.

[schnittstabil]

That is not fair either, PSR-15 targets all of these frameworks. But, okay if you need PSR-15 example:

https://github.com/mindplay-dk/middleman/blob/4bb528ac19c397d57da9465b1215db030ea1ca2a/src/Dispatcher.php#L36:

class Dispatcher implements MiddlewareInterface
{
    //…
    /**
     * @var mixed[] unresolved middleware stack
     */
    private $stack;
    /**
     * @param (callable|MiddlewareInterface|mixed)[] $stack middleware stack (with at least one middleware component)
     * …
     */
    public function __construct($stack, callable $resolver = null)

Even your array_map([$this, 'append'], $stack); trick does not help here: the $stack contains mixed middlewares, and the __construct must be type-hinted against mixed as well.

Even with splat:

// not possible:
public function __construct(callable $resolver, callable ...$stack)

[shadowhand]

equip/dispatch will only accept PSR-15 middleware. There's already a lot of it.

[schnittstabil]

I'm a member of https://github.com/middlewares, I would be proud to rewrite them – furthermore we must already change the namespace sooner or later 😁

[schnittstabil]

I tripped over this, because I was working on my GreenOnion stack to support PSR-15+Generators and many things got really messy just because I have to type-check everything.

The current version is not perfect, and I do not claim that with ServerMiddlewareInterface::__invoke everything would be just fine, but callable will make many things easier, and would be more type-safe (type-checking is very error prone IMO)…

Why don't I write one of my beloved adapters? 😉
It is only possible to write a GeneratorToPsr15Adapter, but because of PHP restrictions (no call/cc or similar), it is not possible to write a Psr15ToGeneratorAdapter 😒

[schnittstabil]

For my GreenOnion project __invoke instead of process means (using phploc):

• Removing 67 lines of 163 161, i.e. 41%
• Reduced Cyclomatic Complexity of the whole class from 14.00 to 6.00 (<10 is recommended)
• With the same semantics!

– Thus, at least for my project it would be tremendous.

EDIT/Clarification: results above are regarding to these interfaces (callable is not necessary):

interface ServerMiddlewareInterface
{
    public function __invoke(ServerRequestInterface $request, DelegateInterface $delegate);
}
interface DelegateInterface
{
    public function __invoke(ServerRequestInterface $request);
}

[weierophinney]

For what it's worth, I agree with @atanvarno69 and @shadowhand — it is up to the middleware framework to decide whether or not it accepts callable middleware. We've got code in the develop branch of both Stratigility and Expressive now that accepts callables and then wraps them in an http-middleware proxy. (They also then wrap the $next argument in a proxy to allow it to be used as a delegate.) This approach works, and keeps type safety in the actual dispatcher, making that code simple and robust.

Your argument about the cyclomatic complexity makes no sense to me; it would make more sense if you could provide a diff demonstrating the code changes from implementation via process() vs implementation via __invoke(), and why the latter necessarily reduces LoC by 41%. I've found that when using callable type hints, I had to add code in order to handle edge cases (such as Class::method callables under PHP < 7, error handling to catch errors due to calling "middleware" with invalid signatures, etc.); as such, I'm highly skeptical. You indicate now that the changes were only due to changing the interface method names from process to __invoke, but, again, I really would need to see if this is due to the method chosen, or other design decisions — and you've not linked to any diffs to demonstrate.

So, what it comes down to is quite simply this: I'm really not sure why you are pushing so hard for using __invoke() in the interfaces, other than personal preference. We have demonstrated time and again reasons why it may not be ideal, ranging from making it possible for existing middleware and middleware frameworks to adapt, to type safety, to readability and code clarity, to edge cases with the language. What exactly does defining __invoke() as the interface method improve?

[schnittstabil]

Your argument about the cyclomatic complexity makes no sense to me;

I can very well imagine that. Zend\Stratigility\Next (using phploc):

• 438 lines for a single class
• Cyclomatic Complexity of the whole class 48.00 (<10 is recommended)

Sorry @weierophinney could not resist, you made that too easy. I hope you have had good reasons to introduce that complexity, but I believe we do not agree about the point when software becomes a big ball of mud.

---

I really would need to see if this is due to the method chosen, or other design decisions.

Of course, it is a design decision to use callable/__invoke! It is the same decision all middleware frameworks made before PSR-15 – at least all I'm aware of. And it is the same decision you made at Created interfaces for describing middleware on 21. Jan 2015 because:

We added our MiddlewareInterface because several of our users insisted.

---

I'm really not sure why you are pushing so hard for using __invoke() in the interfaces, other than personal preference.

Of course, it is about my personal preference! I love clean code; I love reusable code; I love KISS!

To state it simple: I would love Middlewares wich are usable by their own! (E.g see #39 (comment))

---

We have demonstrated time and again reasons why it may not be ideal, ranging from

1. making it possible for existing middleware and middleware frameworks to adapt
2. to type safety
3. to readability and code clarity
4. to edge cases with the language.

Do not get confused with callable. As long as we talk about __invoke only:

1. See ServerMiddlewareInterface::process vs. Migration #44 MiddlewarePipe vs. MiddlewareStack #35, [PoC][WiP] callables #39 (comment) and many more - with__invoke we can reuse delegates everywhere, no Adapter needed! That would increase their acceptance, not decreases it.
2. See my comments above: non-__invoke interfaces leads to mixed instead of callable in practice, and you do the same at MiddlewarePipe::pipe
3. Are you kidding me?

public function process(RequestInterface $request, DelegateInterface $delegate)
{
    $response = $delegate->process($request);
    // vs.
    $response = $delegate($request);
}

I've already explained at #24 (comment) and again at #35 (comment):

[the] current wording [delegate->process] … leads to a hard to grasp oddity:

1. the $delegate parameter is a delegate of the stack
2. and the middleware delegates the remaining computation to it

Thus, what a middleware essentially does is $delegate->delegateComputation(), and the first delegate means something quiet different than the second delegate 😕

4. Sorry, which edge cases? I do not see much technical difference in DelegateInterface::proccess DelegateInterface::__invoke.

---

it is up to the middleware framework to decide whether or not it accepts callable middleware.

In the meantime, I've understood that point of view and I've stopped promoting that middleware frameworks MUST accept callable middlewares. But if they decide to implement PSR-15 side by side with their existing callable interfaces, then they must type-hint against mixed and they must manually type-check. And furthermore they must wrap their callable even if those have the same signature as process!

[shadowhand]

Making delegates callable without making middleware callable would allow people to implement both middleware and delegate interfaces in the same class, which is definitely not correct.

From where I sit, it looks like you are picking and choosing details (many already explained and refuted in the META document) that support your argument and ignoring all the others, @schnittstabil. If you still feel strongly and can present your argument concisely, you have the opportunity to do so during the review stage.

[schnittstabil]

From where I sit, it looks like you are picking and choosing details (many already explained and refuted in the META document) that support your argument and ignoring all the others

As far as I can see, this issue thread is only about process vs __invoke of the ServerMiddlewareInterface. And the META only states:

Why doesn't middleware use __invoke?

1. Doing so would conflict with existing middleware that implements the double-pass approach and may want to implement the middleware interface.
2. In addition, classes that define __invoke can be type hinted as callable, which results in less strict typing.
3. This is generally undesirable, especially when the __invoke method uses strict typing.

• ServerMiddlewareInterface::process vs. Migration #44 shows that 1. is a really bad idea and a PSR should not encourage to do that.
• This issue thread shows that 2. is flawed, because process make the situation even worse.
• 3 is just an opinion I do not agree with – but I do not ignore it.

---

If you still feel strongly and can present your argument concisely

Yes, I still feel strongly about it and I've done my best to present my arguments concisely – refuting my arguments because they are not concisely enough or because I pick and choose those details which underpin my arguments would be just irrational. These arguments will neither become false nor vanish in thin air through that.

[mindplay-dk]

At this point I'm only skimming the discussions, as I've pretty arrived at the conclusion that you can argue more or less equally for/against both approaches - because they're both right, and both wrong, in different ways. The only way this debate could ever be settled to the satisfaction of those who argue in favor of one or the other, is if PHP had certain missing language features - such as type-hinted callables or generics.

In absence of the right language features to express the actual constraints, whatever we end up with will have pros and cons. I personally would lean more towards accepting the limitations of the language and use the closest thing available - rather than trying to shoehorn in type-safety, which comes at a cost of complexity, and largely ends up being run-time type-checks anyhow, but I can see it both ways.

I essentially don't really care anymore - the implications in terms of added complexity and overhead largely affect middleware-stacks only, and I'm of the mind that that's not the most important thing; the most important thing is implementation of middleware, and I can't see that working equally well either way.

My main interest is to see the PSR finalized soon, so I can finalize the projects I'm working on. I'm mostly happy with the important parts :-)

[schnittstabil]

And before you claim again, I would ignore arguments:

Making delegates callable without making middleware callable would allow people to implement both middleware and delegate interfaces in the same class, which is definitely not correct.

The inheritance hierarchy and interface were never created to prevent that – The opposite is the case, they provide definitions of methods which objects agree on in order to cooperate.

Preventing cooperation by the same name on purpose is just a irrational hack.

EDIT: Moreover, I gave a realistic example of middlewares which can act as delegates as well at #41 (comment). Nobody, argued against it – not even you, @shadowhand.

[schnittstabil]

My main interest is to see the PSR finalized soon, so I can finalize the projects I'm working on.

Same holds for me. But I also see too much unnecessarily work (e.g. #39 (comment)), less acceptance and less innovative frameworks (#37 (comment)) in the future with process.