Callable delegates

[mindplay-dk]

I hate to bring this up again, but...

A lot of people have expressed a dislike for the single-method DelegateInterface and want to be able to to just return $next($request) in their middleware implementations. That's not a big deal for me, but several people seemed annoyed, and wanted to put __invoke() in the interface because of that, etc.

In mindplay/middleman, I have this silly implementation of DelegateInterface that simply wraps a callable, which is also completely redundant and wasteful - creating a meaningless extra object wrapper for every middleware that gets dispatched, for perceived type-safety (and, okay, yes, stricter static analysis) but it's really just moving the point of failure one layer away from the middleware itself if it passes a non-request to the delegate.

Well, that's two issues, but then I just ran into another one.

I have a case where I'd like to call this middleware, which does client IP detection, but from outside the context of a middleware stack.

In other words, I want to just run a single middleware component.

With a callable delegate, that was really simple - I might just do:

$client_ip = new ClientIp();

$ip = $client_ip->process($request, function ($r) { return $r; })->getAttribute("client-ip");

But I can't do that.

I need an actual implementation of DelegateInterface and I have to either implement and create an instance of that, or import and instanciate an entire middleware stack - for no real reason, other than to satisfy the type-hint.

This is not an odd case - I've had plenty of cases in the past, where I compose middleware out of several other internal middleware components, for example, conditionally dispatching different middleware instances, etc.

In the light of all these issue, I have to bring up this subject again - sorry.

What I keep coming back to, is that callable for delegates, in de-facto standard PSR-7 middleware, worked fine - it wasn't causing anyone any real headaches, and I think the problem that stricter type-hinting solves in this case is really mostly theoretical.

The DelegateInterface, on the other hand, seems to have rubbed several people the wrong way, and seems to breed other problems and complexities that wouldn't exist with a plain callable.

Who are we supposedly helping with this extra abstraction around delegates?

Maybe the reason this interface (and it's method) was so hard to name, is because it's really honestly serving no real purpose beyond stricter type-hinting - in a place that wasn't really being reported as a problematic pain point by anyone in the first place.

Are we absolutely freakin' sure that this is the right approach?

[shadowhand]

@schnittstabil why would it be a different namespace? The correct thing to do would be to version v2.0.0 against PHP 7 and use strict type hints.

[schnittstabil]

Lets say we have 2 middlewares and a stack:

• middleware A requires "psr/http-middleware": "^1.0"
• middleware B requires "psr/http-middleware": "^2.0"
• the stack requires "psr/http-middleware": "^1.0|^2.0"

If you want to use A and B with the stack, composer must install: "psr/http-middleware": "^1.0" or "psr/http-middleware": "^2.0"

But composer cannot install both :unamused:

[shadowhand]

@schnittstabil I don't really see that as a problem. The transition to PHP 7 versions will certainly be tricky for a while but people will figure it out and move on, or fork middleware to provide the version they need.

[schnittstabil]

For usual packages, I agree. But we want to recommend a standard, and as far as I know the FIG says that it is not a living thing: standards have to be superseded. I can think of a PSR-15.1 standard which only add new things, but we cannot deprecate PSR-15 compatible libraries – I believe the FIG would be really pissed off.

[moufmouf]

@shadowhand I believe @schnittstabil is in sync with what most FIG members think here. When it comes to FIG standards, you never provide a new major version with breaking changes. Instead, you deprecate a PSR and replace it with a new one, but both must be usable side-by-side (like PSR-0 and PSR-4 for instance). It's true we never have had this kind of issue with interface based PSRs but I think the issue was discussed with PSR-6 (regarding a vote to amend the PSR) and the vote was rejected for this very reason if my memory is correct.

[shadowhand]

@moufmouf that policy may need to be revisited. Right now that really doesn't matter for the current discussion, other than previously mentioned issues.

[mindplay-dk]

it breaks static analysis it might break auto-completion of IDEs it makes debugging harder when something breaks

This is all true in theory, but was never a problem for me with de-facto, and I never got the impression that this caused anybody else any hassle.

It's really trivial to address the run-time issue - you can simply type-check arguments and returned value within the callable generated by the middleware-stack.

As for auto-completion, at least Php Storm recognizes implementations of __invoke() and will auto-complete and inspect.

[schnittstabil]

About return type-hinting: I do not know how well it is supported by QA tools and IDEs, but the following should allegedly work with most of them (never trust stackoverlfow):

public function process(ServerRequestInterface $request, callable $delegate)
{
   /* @var $response ResponseInterface */
   $response = $delegate($request);
   //…
}

[shadowhand]

simply type-check arguments and returned value

To play devil's advocate: This implies that every middleware should run these type checks? That is a lot of burden to place on every single middleware and I would never support such an action.

[shadowhand]

This issue is based on a single implementation of DelegateInterface:

I have this silly implementation of DelegateInterface that simply wraps a callable

No one says you need to implement DelegateInterface this way. Just because you chose to implement it that way doesn't mean it is the one true way. equip/dispatch certainly doesn't and (in my biased opinion) is better for it.

The decision to use DelegateInterface::process() over __invoke was made partially to prevent developers from incorrectly attempting to implement DelegateInterface in their middleware. As such, moving to __invoke would remove this inherent security feature and I am not willing to do that.

Those who feel strongly about this should take their case to FIG during the review phase. Far too much time has been spent revisiting the implementation of DelegateInterface and I want to move forward.

[mindplay-dk]

This implies that every middleware should run these type checks?

No, I mean type-check them in the delegate - which is generated by the middleware-stack. I think it's okay to burden the middleware stack with this - of course it would be unacceptable to burden the middleware developers, I agree.

Look, if we can't reach an agreement on this, maybe just keep it as-is and let's move on - at this point I'm more interested in closing this PSR than nit-picking over the details. I don't see the practical point in having this statically typed and I don't care for the garbage code and throw-away objects I have to write in the dispatcher - but on the other hand, the problem is isolated to implementation of middleware-stacks; the middleware part of it is good, and I don't care enough to hold up this PSR indefinitely.

No one says you need to implement DelegateInterface this way

I honestly don't care how it's implemented - my problem is that something like this even has to exist in the first place. It has no real justification to exist. It's not solving any actual relevant domain problem. No matter how you implement it, it's a work-around to an issue we created by trying to impose type-safety in a dynamic language.

Fuck it, let's move on and see how the community responds?

[shadowhand]

@mindplay-dk those implementing stacks in PHP7 have the option of enforcing the strict type check. This is not the case if we use callable.

[schnittstabil]

Wait! Even with callable it is possible to do so:

$delegate = function (RequestInterface $request) : ResponseInterface {
    return new Response();
}

You get the same enforcement – Or, am I'm wrong?

[shadowhand]

But the consumer of that delegate has no assurance. Only an interface can provide strict adherence.

[schnittstabil]

The consumer will (only) require psr/htttp-middleware with the non-strict DelegateInterface – at least with v1.0, thus I believe he has no assurance:

class Middleware implements ServerMiddlewareInterface
{
    public function process(ServerRequestInterface $request, DelegateInterface $delegate)
    {
        // maybe I get a `$delegate->process(…):ResponseInterface`
        // or a simple `$delegate->process(…)`
        // – I cannot be sure, can I?
    }
}

A PHP7 implementation may call it like following. But to me, as long as PHP does not surprises me, they both enforce the same:

// interface
return $this->middleware[$this->index]->process($request, new class implements DelegateInterface {
    public function process(ServerRequestInterface $request):ResponseInterface
    {…}
});

// callable
return $this->middleware[$this->index]->process(
    $request,
    function (ServerRequestInterface $request) : ResponseInterface {
        …
    }
);

EDIT: Parameter type-hinting is of course a bit different. But as long as the stack calls the middleware only with type-hinted $request (i.e. function(ServerRequestInterface $request)) the type error is thrown in the right line of code:

class Middleware implements ServerMiddlewareInterface
{
    public function process(ServerRequestInterface $request, callable $delegate)
    {
        return $delegate(42); // <--- Fatal Error in this line
    }
}

[shadowhand]

The fact remains that if you use PHP7 and if you use a dispatch implementation that enforces types, you will be 100% certain that the return value of $delegate->process() will be a ResponseInterface. This is simply not possible with a callable type hint.

[schnittstabil]

The fact remains that if you use PHP7 and if you use a dispatch implementation that enforces types, you will be 100% certain that the return value of $delegate->process() will be a ResponseInterface.

I fully agree.

This is simply not possible with a callable type hint.

Sorry, I do not understand why this should not be possible – Maybe I misunderstood or I miss something. I'll give it a shot tomorrow and setup a small repository…

[schnittstabil]

@shadowhand Nope, sorry, I don't get it. Every kind of return-type-hinted callable I've used, enforces the same (see this repository, which covers all types mentioned at Callbacks / Callables (php.net)):

PHP Fatal error:  Uncaught TypeError:
Return value of … must be an instance of Psr\Http\Message\ResponseInterface, integer returned… 

Furthermore, I have read PHP RFC: Return Type Declarations (again) – and they talk most of the time about type-hinting anonymous functions – hence, I think this is only misunderstanding…

[shadowhand]

I think we're talking about different things. I am talking about the end user, not the person defining the implementation of the delegate. As the end user, a DelegateInterface type hint enforces static type checks and a callable type hint cannot.

[schnittstabil]

Well then, I believe your comments above and your snippet were at least a bit misleading in this context.

DelegateInterface::__invoke would already solve many, many issues. And I just wanted to be sure that we do not reject callable because of wrong assumptions.

[weierophinney]

@schnittstabil —

DelegateInterface::__invoke would already solve many, many issues.

What issues does it solve, exactly? You keep saying it solves issues, but you've not (a) clearly defined what the issues are, (b) detailed how using process() causes the issue, or (c) how __invoke() solves it.

Right now, it feels more like this is more around a personal preference for __invoke() and callable, versus a solid technical reason. If you can provide that information, succinctly, I think all of us can better evaluate the impact of your proposal.

And I just wanted to be sure that we do not reject callable because of wrong assumptions.

Which wrong assumptions? Can you list them succinctly, please?

I've read through 2 weeks plus of comment threads today (I've been on vacation, hence my silence until today), and I've yet to understand them.

The reasons outlined for not using callable as the typehint are:

• No guarantee that the delegate provided has the correct signature.
• Potential runtime issues when the delegate does not conform to the interface.
• Potential tooling issues with regards to identifying what signature the argument conforms to (though your screencast demonstrates that most IDEs would likely manage this fine).
• Decreased clarity for end-users reading the signature (does the callable need to conform to the interface? can I pass anything I want there? etc.).
• Incompatibility with a future specification that supercedes this one that does provide PHP 7 typehints.

Can you provide a list of technical reasons that support callable as a typehint? Having a concrete list allows everyone involved to more objectively evaluate the pros and cons of each approach.

[schnittstabil]

@weierophinney I believe I have already treated all of your concerns and questions. For this reason, I take your allegations personal. If I find some time the next days I will collect all my github comments and will answer you questions in detail. But I believe as long as #43 and #44 exists it would be rather useless.

[schnittstabil]

Having a concrete list allows everyone involved to more objectively evaluate the pros and cons of each approach.

BTW, this was the very first time – as far as I know – where somebody has written a concrete list against callable – beside the small META sections, which I've already treated too.

It is not my fault, that your current arguments against callable were nowhere succinctly summarized until now. I can only refute and weigh arguments I'm aware of. Hence, do not try to pass the buck to me, because you have lost the overview.