Closed schnittstabil closed 7 years ago
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.
Sorry, I should have mentioned the README.md:
Why doesn't middleware use
__invoke
?… In addition, classes that define
__invoke
can be type hinted ascallable
, 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
.
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.
I don't understand. What is the difference? I believe the same applies to the single-pass approach.
None of them are based on PSR-15. It's comparing apples or oranges, or at least oranges to tangerines.
That is not fair either, PSR-15 targets all of these frameworks. But, okay if you need PSR-15 example:
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)
equip/dispatch
will only accept PSR-15 middleware. There's already a lot of it.
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 :grin:
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? :wink:
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
:unamused:
For my GreenOnion
project __invoke
instead of process
means (using phploc):
– 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);
}
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?_
Your argument about the cyclomatic complexity makes no sense to me;
I can very well imagine that. Zend\Stratigility\Next
(using phploc):
class
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 https://github.com/http-interop/http-middleware/pull/39#issuecomment-263134031)
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.
Do not get confused with callable
. As long as we talk about __invoke
only:
__invoke
we can reuse delegates everywhere, no Adapter needed! That would increase their acceptance, not decreases it.__invoke
interfaces leads to mixed
instead of callable
in practice, and you do the same at MiddlewarePipe::pipepublic function process(RequestInterface $request, DelegateInterface $delegate)
{
$response = $delegate->process($request);
// vs.
$response = $delegate($request);
}
I've already explained at https://github.com/http-interop/http-middleware/pull/24#issuecomment-261751261 and again at https://github.com/http-interop/http-middleware/issues/35#issue-191257563:
[the] current wording [
delegate->process
] … leads to a hard to grasp oddity:
- the
$delegate
parameter is a delegate of the stack- 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 :confused:
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
!
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.
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?
- Doing so would conflict with existing middleware that implements the double-pass approach and may want to implement the middleware interface.
- 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.
process
make the situation even worse.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.
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 :-)
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 https://github.com/http-interop/http-middleware/pull/41#issuecomment-263969406. Nobody, argued against it – not even you, @shadowhand.
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. https://github.com/http-interop/http-middleware/pull/39#issuecomment-263134031), less acceptance and less innovative frameworks (https://github.com/http-interop/http-middleware/issues/37#issuecomment-263069310) in the future with process
.
A small micro-framework survey:
All of them support anonymous functions. With
ServerMiddlewareInterface::__invoke
they may want to type-hint againstcallable
at someday – withServerMiddlewareInterface::process
they will never be able to do – thus they must type-hint tomixed
:unamused:Moreover, the same applies to every single future framework/container, which wants to support anonymous functions as well.