[PSR-15] Feedback on Middleware proposal
565 views
Skip to first unread message
Matthew Weier O'Phinney
Jun 8, 2016, 5:27:25 PM6/8/16
to php...@googlegroups.com
I've been busy lately with trying to shepherd the last ZF3 initiatives, so many
apologies for being late to the game on the Middleware/PSR-7 topics. Things are
starting to slow down, so I set aside today to go through the various threads
and comb through the proposals in detail. What follows is lengthy; for that
reason, I created a "tl;dr" at the end. If you want the "why" behind those
bullet points, keep reading.
The following comments are on the PSR-15 proposal and metadocument.
## Proposal document
### Section 1
Why type-hint against callable?
Middleware stacks can always allow callable middleware, *and then wrap it in an
implementation of the interface*. Anthony demonstrated it in his blog post, and
I've done similarly already when wrapping middleware of different signatures.
The problem I see with allowing any callable is type safety. While it's
admittedly rare, I've seen cases where a callable with an incorrect argument
order or declarations causes problems. Having this strictly typed solves this.
Additionally, most of the implementations I've seen *do* define an interface
already, even if they do not use it. If middleware implements these, wrapping
them becomes fairly trivial when accepting them to the pipeline:
```php
if ($middleware instanceof SomeSupportedThirdPartyMiddlewareInterface) {
$middleware = new
WrapperMiddlewareForSomeSupportedThirdPartyMiddlewareInterface($middleware);
}
```
### Section 2
Two big questions:
1. **Why __invoke()?**
One issue with specifying `__invoke()` in the interface at this point is if
an implementing library defined its own interface, if there are *any*
differences in the signature (e.g., if `$next` is nullable; I know, I f@!#ed
that up in Stratigility, but ç'est la vie), then those implementations cannot
extend the proposed interface.
If we use a concrete method name, current implementations can extend it, and
provide wrappers that will decorate invokable objects with an implementing
method that proxies to their own `__invoke()`. Additionally, existing
dispatch libraries could easily test for the proposal interface vs callables
in order to determine how to dispatch. *This would help adoption
immediately, as libraries could likely add support within a minor version,
instead of a backwards-incompatible major version.*
2. **Why no interface for $next?**
Again, in the interest of type safety, this seems necessary. Additionally,
having such an interface define a concrete, non-magic method would allow for
immediate interoperability. As an example, Stratigility could implement the
new method as a proxy to its own `__invoke()` (or vice versa), preventing
conflicts in signatures.
### Section 2.2
I fully disagree with having the same interface for both client- and server-side
middleware.
Server-side middleware should be able to assume that the passed request is a
`ServerRequestInterface`. Having to test for capabilities within each middleware
will be a nightmare to explain to newcomers, and, frankly, impossible to
rationalize. If users just *assume* that they have received a
`ServerRequestInterface`, and get a plain `RequestInterface`, they'll have some
ugly surprises during execution.
## Metadocument
### Section 4
The reason ExpressJS passes the response is because there's no easy way to
create a new one in node, because the instance ties into the HTTP server's
event system. So, while we can use it as a justification, *the context is
different*.
Yes, I did port the exact signature when creating phly/conduit /
zendframework/zend-stratigility; I was originally doing a literal port, and,
even after switching PSR-7 to immutable instances, found the idea of passing the
response interesting in terms of eliminating dependencies in consuming
middleware.
However, I think there are some compelling reasons to drop the response
argument.
Most cases I've seen where a modified response was passed to a lower layer via
`$next()` have a fair amount of brittleness:
- You cannot assume the response returned is the same response or contains the
same modifications.
- Often, the modifications you need to do *should* be propagated, regardless of
the inner layers dispatched, or
- Modifications should be conditional based on the state of the response
returned from inner layers.
There are ways to rewrite the various content negotiation and caching examples
I've seen on the list to work in the lambda style, and these approaches tend to
be more robust and ensure the validity of the response.
Additionally, there's a learning curve to passing it in. As Rasmus Schulz noted,
What doesn't seem logical, is manipulating output (the response) on the way
in - in terms of control, it doesn't make any sense, because in doing so,
you don't really have any control at all, unless the next component on the
stack is accepting input from the response, which isn't intuitive or easy to
explain at all.
With the HTTP factories proposal (PSR-16) gaining traction (essentially, unless
the rest of FIG members who haven't voted vote now and vote against, or a lot of
folks change their votes), I see less and less reason to keep the argument.
Moreover, it would be reasonably easy to adapt existing middleware libraries to
the lambda style:
```php
/**
* Assumes MiddlewareInterface is now lambda-style
*/
class DoublePassToLambdaMiddleware implements MiddlewareInterface
{
private $middleware;
private $response;
/**
* Accepts the double-pass middleware, and a blank response
*/
public function __construct($middleware, $response)
{
$this->middleware = $middleware;
$this->response = $response;
}
/**
* Assumes that the interface uses a method other than __invoke.
* Just spit-balling here, and chose something not widely used.
*/
public function respond(ServerRequestInterface $request,
MiddlewareStack $next)
{
$middleware = $this->middleware;
return $middleware(
$request,
$this->response,
function ($request, $response) use ($next) {
return $next->respond($request, $response);
}
);
}
}
```
Existing libraries could easily wrap existing middleware in something like the
above to adapt it to PSR-15.
## Final notes
**tl;dr**: I've read through all the threads and had a thorough look at both the
PSR-15 proposal and metadocument, as well as the factories proposal (PSR-16). My
take is:
- The factories largely remove the requirement for passing the response when
invoking middleware.
- The brittleness of design that occurs when pre-modifying a response to pass
when invoking middleware convinces me the argument should be removed.
- I definitely feel separate interfaces are required for client- vs. server-side
middleware. It makes the signatures self-enforcing at the engine-level, makes
the arguments self-documenting, and prevents the need to type-check within
middleware when writing server-side middleware.
- I'd like to see a method other than `__invoke()` in the `MiddlewareInterface`
as it will make adapting existing libraries to be PSR-15 compliant easier.
- I'd like to see an interface for the `$next` argument, also with a method
other than `__invoke()`; again, this will be to allow adapting existing
libraries to PSR-15.
--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/
apologies for being late to the game on the Middleware/PSR-7 topics. Things are
starting to slow down, so I set aside today to go through the various threads
and comb through the proposals in detail. What follows is lengthy; for that
reason, I created a "tl;dr" at the end. If you want the "why" behind those
bullet points, keep reading.
The following comments are on the PSR-15 proposal and metadocument.
## Proposal document
### Section 1
Why type-hint against callable?
Middleware stacks can always allow callable middleware, *and then wrap it in an
implementation of the interface*. Anthony demonstrated it in his blog post, and
I've done similarly already when wrapping middleware of different signatures.
The problem I see with allowing any callable is type safety. While it's
admittedly rare, I've seen cases where a callable with an incorrect argument
order or declarations causes problems. Having this strictly typed solves this.
Additionally, most of the implementations I've seen *do* define an interface
already, even if they do not use it. If middleware implements these, wrapping
them becomes fairly trivial when accepting them to the pipeline:
```php
if ($middleware instanceof SomeSupportedThirdPartyMiddlewareInterface) {
$middleware = new
WrapperMiddlewareForSomeSupportedThirdPartyMiddlewareInterface($middleware);
}
```
### Section 2
Two big questions:
1. **Why __invoke()?**
One issue with specifying `__invoke()` in the interface at this point is if
an implementing library defined its own interface, if there are *any*
differences in the signature (e.g., if `$next` is nullable; I know, I f@!#ed
that up in Stratigility, but ç'est la vie), then those implementations cannot
extend the proposed interface.
If we use a concrete method name, current implementations can extend it, and
provide wrappers that will decorate invokable objects with an implementing
method that proxies to their own `__invoke()`. Additionally, existing
dispatch libraries could easily test for the proposal interface vs callables
in order to determine how to dispatch. *This would help adoption
immediately, as libraries could likely add support within a minor version,
instead of a backwards-incompatible major version.*
2. **Why no interface for $next?**
Again, in the interest of type safety, this seems necessary. Additionally,
having such an interface define a concrete, non-magic method would allow for
immediate interoperability. As an example, Stratigility could implement the
new method as a proxy to its own `__invoke()` (or vice versa), preventing
conflicts in signatures.
### Section 2.2
I fully disagree with having the same interface for both client- and server-side
middleware.
Server-side middleware should be able to assume that the passed request is a
`ServerRequestInterface`. Having to test for capabilities within each middleware
will be a nightmare to explain to newcomers, and, frankly, impossible to
rationalize. If users just *assume* that they have received a
`ServerRequestInterface`, and get a plain `RequestInterface`, they'll have some
ugly surprises during execution.
## Metadocument
### Section 4
The reason ExpressJS passes the response is because there's no easy way to
create a new one in node, because the instance ties into the HTTP server's
event system. So, while we can use it as a justification, *the context is
different*.
Yes, I did port the exact signature when creating phly/conduit /
zendframework/zend-stratigility; I was originally doing a literal port, and,
even after switching PSR-7 to immutable instances, found the idea of passing the
response interesting in terms of eliminating dependencies in consuming
middleware.
However, I think there are some compelling reasons to drop the response
argument.
Most cases I've seen where a modified response was passed to a lower layer via
`$next()` have a fair amount of brittleness:
- You cannot assume the response returned is the same response or contains the
same modifications.
- Often, the modifications you need to do *should* be propagated, regardless of
the inner layers dispatched, or
- Modifications should be conditional based on the state of the response
returned from inner layers.
There are ways to rewrite the various content negotiation and caching examples
I've seen on the list to work in the lambda style, and these approaches tend to
be more robust and ensure the validity of the response.
Additionally, there's a learning curve to passing it in. As Rasmus Schulz noted,
What doesn't seem logical, is manipulating output (the response) on the way
in - in terms of control, it doesn't make any sense, because in doing so,
you don't really have any control at all, unless the next component on the
stack is accepting input from the response, which isn't intuitive or easy to
explain at all.
With the HTTP factories proposal (PSR-16) gaining traction (essentially, unless
the rest of FIG members who haven't voted vote now and vote against, or a lot of
folks change their votes), I see less and less reason to keep the argument.
Moreover, it would be reasonably easy to adapt existing middleware libraries to
the lambda style:
```php
/**
* Assumes MiddlewareInterface is now lambda-style
*/
class DoublePassToLambdaMiddleware implements MiddlewareInterface
{
private $middleware;
private $response;
/**
* Accepts the double-pass middleware, and a blank response
*/
public function __construct($middleware, $response)
{
$this->middleware = $middleware;
$this->response = $response;
}
/**
* Assumes that the interface uses a method other than __invoke.
* Just spit-balling here, and chose something not widely used.
*/
public function respond(ServerRequestInterface $request,
MiddlewareStack $next)
{
$middleware = $this->middleware;
return $middleware(
$request,
$this->response,
function ($request, $response) use ($next) {
return $next->respond($request, $response);
}
);
}
}
```
Existing libraries could easily wrap existing middleware in something like the
above to adapt it to PSR-15.
## Final notes
**tl;dr**: I've read through all the threads and had a thorough look at both the
PSR-15 proposal and metadocument, as well as the factories proposal (PSR-16). My
take is:
- The factories largely remove the requirement for passing the response when
invoking middleware.
- The brittleness of design that occurs when pre-modifying a response to pass
when invoking middleware convinces me the argument should be removed.
- I definitely feel separate interfaces are required for client- vs. server-side
middleware. It makes the signatures self-enforcing at the engine-level, makes
the arguments self-documenting, and prevents the need to type-check within
middleware when writing server-side middleware.
- I'd like to see a method other than `__invoke()` in the `MiddlewareInterface`
as it will make adapting existing libraries to be PSR-15 compliant easier.
- I'd like to see an interface for the `$next` argument, also with a method
other than `__invoke()`; again, this will be to allow adapting existing
libraries to PSR-15.
--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/
Woody Gilk
Jun 13, 2016, 9:30:45 AM6/13/16
to PHP Framework Interoperability Group
On Wed, Jun 8, 2016 at 4:27 PM, Matthew Weier O'Phinney
<mweiero...@gmail.com> wrote:
> **tl;dr**: I've read through all the threads and had a thorough look at both the
> PSR-15 proposal and metadocument, as well as the factories proposal (PSR-16). My
> take is:
Thanks for the comprehensive feedback, Matthew. I've read through a
<mweiero...@gmail.com> wrote:
> **tl;dr**: I've read through all the threads and had a thorough look at both the
> PSR-15 proposal and metadocument, as well as the factories proposal (PSR-16). My
> take is:
couple of times and agree with you on all points.
Despite my initial reluctance to do anything other than standardize
the current status quo, having HTTP factory will allow us to create a
much better standard than would be possible within the current
ecosystem. Defining something that makes the future better is far more
useful than locking us into a system that leaves too much open to
interpretation.
Expect some updates to the proposal within the next couple of days
that include all the feedback I've received.
--
Woody Gilk
http://about.me/shadowhand
Larry Garfield
Jun 13, 2016, 10:33:05 AM6/13/16
to php...@googlegroups.com
On 06/13/2016 08:30 AM, Woody Gilk wrote:
> Defining something that makes the future better is far more
> useful than locking us into a system that leaves too much open to
> interpretation.
I'm half tempted to work that sentence into the mission statement
> Defining something that makes the future better is far more
> useful than locking us into a system that leaves too much open to
> interpretation.
somehow. Well-said.
--Larry Garfield
Matthew Weier O'Phinney
Jun 13, 2016, 3:39:46 PM6/13/16
to php...@googlegroups.com
looking forward to reviewing the next set of changes!
--
https://mwop.net/
Woody Gilk
Jun 14, 2016, 11:13:02 PM6/14/16
to PHP Framework Interoperability Group
On Wed, Jun 8, 2016 at 4:27 PM, Matthew Weier O'Phinney
<mweiero...@gmail.com> wrote:
> - I definitely feel separate interfaces are required for client- vs. server-side
> middleware. It makes the signatures self-enforcing at the engine-level, makes
> the arguments self-documenting, and prevents the need to type-check within
> middleware when writing server-side middleware.
Coming back to this particular point, the biggest difficulty that I
<mweiero...@gmail.com> wrote:
> - I definitely feel separate interfaces are required for client- vs. server-side
> middleware. It makes the signatures self-enforcing at the engine-level, makes
> the arguments self-documenting, and prevents the need to type-check within
> middleware when writing server-side middleware.
see with separate interfaces for client and server (which we explored
once already) is that ServerMiddlewareInterface _cannot_ extend
MiddlewareInterface to due conflicts in the signature. In an ideal
world, PHP would understand that ServerRequestInterface is an
extension of RequestInterface and therefore an acceptable overload,
but the engine currently does not allow for this:
https://3v4l.org/7hj2E
Which means that in order to resolve this, implementations would
either have to declare that they are _strictly_ for usage with client
requests or server requests, since the middleware stack container
would type hint against a single interface:
public function withMiddleware(MiddlewareInterface $middleware);
So that leaves another possibility, wherein MiddlewareInterface is a
completely empty construct that is extended by
ClientMiddlewareInterface and ServerMiddlewareInterface. This scenario
has two obvious downsides:
1. It requires an extra interface just for compatibility.
2. Since the interface is empty it could be implemented by any class,
even if no handle() method was present.
Both scenarios impact the ability to properly type hint against
interfaces at some part of the stack. While it is very nice from an
implementation standpoint, it makes the stack handling much more
complicated and possibly error prone. Not type hinting on the stack is
certainly possible, but that just defers type checking to the stack
container:
public function withMiddleware($middleware)
{
if (false === ($middleware instanceof ClientMiddlewareInterface ||
$middleware instanceof ServerMiddlewareInterface)) {
// ... kaboom!
}
// ... everything is fine, carry on
}
Or you just accept MiddlewareInterface and assume that all the
implementations are sane.
Despite all of the potential problems with having an empty
MiddlewareInterface, I am still in favor of a "three interface"
solution with clear documentation that MiddlewareInterface MUST NOT be
implemented independently. This is how I will be proceeding with my
proposal and I welcome a better solution for dealing with the stack
container type hint problem.
My latest changes are now available in a new PR#771 [1] and hopefully
everyones concerns have been adequately addressed.
[1]: https://github.com/php-fig/fig-standards/pull/771/files
Looking forward to feedback from all involved!
Matthew Weier O'Phinney
Jun 15, 2016, 10:28:44 AM6/15/16
to php...@googlegroups.com
On Tue, Jun 14, 2016 at 10:12 PM, Woody Gilk <woody...@gmail.com> wrote:
> On Wed, Jun 8, 2016 at 4:27 PM, Matthew Weier O'Phinney
> <mweiero...@gmail.com> wrote:
> > - I definitely feel separate interfaces are required for client- vs. server-side
> > middleware. It makes the signatures self-enforcing at the engine-level, makes
> > the arguments self-documenting, and prevents the need to type-check within
> > middleware when writing server-side middleware.
>
> Coming back to this particular point, the biggest difficulty that I
> see with separate interfaces for client and server (which we explored
> once already) is that ServerMiddlewareInterface _cannot_ extend
> MiddlewareInterface to due conflicts in the signature. In an ideal
> world, PHP would understand that ServerRequestInterface is an
> extension of RequestInterface and therefore an acceptable overload,
> but the engine currently does not allow for this:
> https://3v4l.org/7hj2E
The problem, though, is that that would violate the LSP, as implementations of
> On Wed, Jun 8, 2016 at 4:27 PM, Matthew Weier O'Phinney
> <mweiero...@gmail.com> wrote:
> > - I definitely feel separate interfaces are required for client- vs. server-side
> > middleware. It makes the signatures self-enforcing at the engine-level, makes
> > the arguments self-documenting, and prevents the need to type-check within
> > middleware when writing server-side middleware.
>
> Coming back to this particular point, the biggest difficulty that I
> see with separate interfaces for client and server (which we explored
> once already) is that ServerMiddlewareInterface _cannot_ extend
> MiddlewareInterface to due conflicts in the signature. In an ideal
> world, PHP would understand that ServerRequestInterface is an
> extension of RequestInterface and therefore an acceptable overload,
> but the engine currently does not allow for this:
> https://3v4l.org/7hj2E
the server-specific interface could not work in a stack that passes the base
RequestInterface. The only way I could see this working at the engine level is
if union types are ever implemented.
> Which means that in order to resolve this, implementations would
> either have to declare that they are _strictly_ for usage with client
> requests or server requests, since the middleware stack container
> would type hint against a single interface:
>
> public function withMiddleware(MiddlewareInterface $middleware);
>
> So that leaves another possibility, wherein MiddlewareInterface is a
> completely empty construct that is extended by
> ClientMiddlewareInterface and ServerMiddlewareInterface. This scenario
> has two obvious downsides:
StackInterface?
The main purpose I see around PSR-15 is to allow usage of middleware within
different stacks; as long as they follow the interfaces, and as long as the
stack invokes the middleware with the request and a Frame, the stack
implementation itself is not relevant. As such, having a suggested stack
interface as part of the proposal seems out of scope; perhaps this should be
moved to the meta document? Doing so would allow removing the base
MiddlewareInterface, simplifying the scope of the proposal.
<snip>
Other feedback on the latest pull request
(https://github.com/php-fig/fig-standards/pull/771):
- I like the FrameInterface. I was unsure at first about not having separate
versions for client- vs server-side stacks, but on reflection, it makes sense;
any given stack will only be working with one type, and if a user switches
from ServerRequestInterface to RequestInterface implementations within a
server-side stack, PHP will fatal all over them on the invocation of the next
ServerMiddlewareInterface implementation.
- As I noted in my "big post" at the start of this thread, I'd recommend a
different interface method name than `handle()`, as that is already used by a
number of projects (HttpKernelInterface, Laravel middleware). If we use a
different name, existing middleware in these projects would be able to adapt
to be consumable by PSR-15 without breaking backwards compatibility.
Suggestions off the top of my head: execute, run, dispatch (though this
conflicts with some libraries), process, processRequest.
- I think the meta document needs to document:
- the decision and rationale for choosing lambda-style
- the decision and rationale for not using callables for the middleware and
"$next"/"$frame" interfaces
Thanks for the great work on the proposal so far, Woody!
--
https://mwop.net/
Woody Gilk
Jun 15, 2016, 2:59:32 PM6/15/16
to PHP Framework Interoperability Group
On Wed, Jun 15, 2016 at 9:28 AM, Matthew Weier O'Phinney
<mweiero...@gmail.com> wrote:
> The problem, though, is that that would violate the LSP, as implementations of
> the server-specific interface could not work in a stack that passes the base
> RequestInterface. The only way I could see this working at the engine level is
> if union types are ever implemented.
I was looking at it from a slightly different perspective... a client
<mweiero...@gmail.com> wrote:
> The problem, though, is that that would violate the LSP, as implementations of
> the server-specific interface could not work in a stack that passes the base
> RequestInterface. The only way I could see this working at the engine level is
> if union types are ever implemented.
middleware is perfectly acceptable to use as a server middleware,
since Request extends ServerRequest. Whereas a server middleware
cannot be used with Request but only ServerRequest.
> So, that raises a question: does the proposal need to include the
> StackInterface?
>
> The main purpose I see around PSR-15 is to allow usage of middleware within
> different stacks; as long as they follow the interfaces, and as long as the
> stack invokes the middleware with the request and a Frame, the stack
> implementation itself is not relevant. As such, having a suggested stack
> interface as part of the proposal seems out of scope; perhaps this should be
> moved to the meta document? Doing so would allow removing the base
> MiddlewareInterface, simplifying the scope of the proposal.
feel that it is valuable to provide a recommendation for it because of
the aforementioned concerns around being able to use client middleware
in a server stack. Having the StackInterface clearly demonstrates why
the empty MiddlewareInterface is useful for resolving the type
declaration problem.
> - I like the FrameInterface. I was unsure at first about not having separate
> versions for client- vs server-side stacks, but on reflection, it makes sense;
> any given stack will only be working with one type, and if a user switches
> from ServerRequestInterface to RequestInterface implementations within a
> server-side stack, PHP will fatal all over them on the invocation of the next
> ServerMiddlewareInterface implementation.
server middleware in a stack that processes a server request. Having
separate interfaces for server and client frames would complicate
matters too much in my opinion.
> - As I noted in my "big post" at the start of this thread, I'd recommend a
> different interface method name than `handle()`, as that is already used by a
> number of projects (HttpKernelInterface, Laravel middleware). If we use a
> different name, existing middleware in these projects would be able to adapt
> to be consumable by PSR-15 without breaking backwards compatibility.
> Suggestions off the top of my head: execute, run, dispatch (though this
> conflicts with some libraries), process, processRequest.
process() the middleware using the same name would be logical. Here is
a PR with that change:
https://github.com/php-fig/fig-standards/pull/773
> - I think the meta document needs to document:
> - the decision and rationale for choosing lambda-style
> - the decision and rationale for not using callables for the middleware and
> "$next"/"$frame" interfaces
https://github.com/php-fig/fig-standards/pull/774
This could likely be improved but should be a good starting point.
> Thanks for the great work on the proposal so far, Woody!
Matthew Weier O'Phinney
Jun 16, 2016, 4:24:47 PM6/16/16
to php...@googlegroups.com
On Wed, Jun 15, 2016 at 1:58 PM, Woody Gilk <woody...@gmail.com> wrote:
> On Wed, Jun 15, 2016 at 9:28 AM, Matthew Weier O'Phinney
> <mweiero...@gmail.com> wrote:
> > So, that raises a question: does the proposal need to include the
> > StackInterface?
> >
> > The main purpose I see around PSR-15 is to allow usage of middleware within
> > different stacks; as long as they follow the interfaces, and as long as the
> > stack invokes the middleware with the request and a Frame, the stack
> > implementation itself is not relevant. As such, having a suggested stack
> > interface as part of the proposal seems out of scope; perhaps this should be
> > moved to the meta document? Doing so would allow removing the base
> > MiddlewareInterface, simplifying the scope of the proposal.
>
> It definitely does not need to include the StackInterface. However, I
> feel that it is valuable to provide a recommendation for it because of
> the aforementioned concerns around being able to use client middleware
> in a server stack.
I was stumbling on this, trying to figure out why client middleware wouldn't be
> On Wed, Jun 15, 2016 at 9:28 AM, Matthew Weier O'Phinney
> <mweiero...@gmail.com> wrote:
> > So, that raises a question: does the proposal need to include the
> > StackInterface?
> >
> > The main purpose I see around PSR-15 is to allow usage of middleware within
> > different stacks; as long as they follow the interfaces, and as long as the
> > stack invokes the middleware with the request and a Frame, the stack
> > implementation itself is not relevant. As such, having a suggested stack
> > interface as part of the proposal seems out of scope; perhaps this should be
> > moved to the meta document? Doing so would allow removing the base
> > MiddlewareInterface, simplifying the scope of the proposal.
>
> It definitely does not need to include the StackInterface. However, I
> feel that it is valuable to provide a recommendation for it because of
> the aforementioned concerns around being able to use client middleware
> in a server stack.
able to be used, as the ServerRequest fulfills the Request required by the
client middleware ... and then realized it's because of the separated
interfaces bit (you wouldn't be able to compose the client middleware in your
stack if you typehint on ServerMiddleware).
As such, two pieces of feedback:
- Would this functionality be useful? Is anybody actually doing that today? I
see some potential problems with it (e.g., what if the middleware
creates a new
`RequestInterface` instance and passes it to `$frame->next()`?), and have yet
to see a use case where I wouldn't just invoke that middleware directly within
my server-side middleware.
I'm willing to be convinced its useful, but I think we need to demonstrate the
need in the meta document if it really is.
- Second, if it is useful, and thus we're keeping the MiddlewareInterface as a
marker interface, we need more narrative around this in the meta document
detailing what I wrote above. Those reading the spec and meta document should
not need to fill in the blanks and reason it out; the meta document should do
that.
> Having the StackInterface clearly demonstrates why
> the empty MiddlewareInterface is useful for resolving the type
> declaration problem.
>
> > - I like the FrameInterface. I was unsure at first about not having separate
> > versions for client- vs server-side stacks, but on reflection, it makes sense;
> > any given stack will only be working with one type, and if a user switches
> > from ServerRequestInterface to RequestInterface implementations within a
> > server-side stack, PHP will fatal all over them on the invocation of the next
> > ServerMiddlewareInterface implementation.
>
> As per my above comment, it is entirely possible to mix client and
> server middleware in a stack that processes a server request. Having
> separate interfaces for server and client frames would complicate
> matters too much in my opinion.
>> > - As I noted in my "big post" at the start of this thread, I'd recommend a
>> > different interface method name than `handle()`, as that is already used by a
>> > number of projects (HttpKernelInterface, Laravel middleware). If we use a
>> > different name, existing middleware in these projects would be able to adapt
>> > to be consumable by PSR-15 without breaking backwards compatibility.
>> > Suggestions off the top of my head: execute, run, dispatch (though this
>> > conflicts with some libraries), process, processRequest.
>
> Ah, I missed this point before. Since the StackInterface uses
> process() the middleware using the same name would be logical. Here is
> a PR with that change:
> https://github.com/php-fig/fig-standards/pull/773
> > - I think the meta document needs to document:
> > - the decision and rationale for choosing lambda-style
> > - the decision and rationale for not using callables for the middleware and
> > "$next"/"$frame" interfaces
>
> I've added some explanation to the meta document in
> https://github.com/php-fig/fig-standards/pull/774
>
> This could likely be improved but should be a good starting point.
--
https://mwop.net/
Woody Gilk
Jun 20, 2016, 10:14:46 AM6/20/16
to PHP Framework Interoperability Group
document this in the meta document without making it too heavy on
implementation details. Would you be willing to create a PR that fills
in that section? I've started to detail it in my latest changes to the
PR [1].
[1]: https://github.com/php-fig/fig-standards/pull/774
Regarding this specifically:
> what if the middleware creates a new `RequestInterface` instance and passes it to `$frame->next()`?
replace the request. It should be assumed that the request can be
modified and passed, but not completely replaced. Do you think this
needs to be explicitly stated in the documentation?
Matthew Weier O'Phinney
Jun 20, 2016, 12:20:38 PM6/20/16
to php...@googlegroups.com
`MiddlewareInterface` is provided as a common marker interface that may be
used in middleware stacks to allow middleware targeting either clients or
server-side requests. While `ServerRequestInterface` request instances may
always be passed to `ClientMiddlewareInterface`, as they extend from
`RequestInterface`, the opposite (passing a `RequestInterface` to a
`ServerMiddlewareInterface`) is not. `MiddlewareInterface` thus provides a
common base interface for each middleware type for server-side stacks that
wish to allow usage of client middleware (e.g., to proxy to another server).
> Regarding this specifically:
>
>> what if the middleware creates a new `RequestInterface` instance and passes it to `$frame->next()`?
>
> I have never seen a situation where the middleware would completely
> replace the request. It should be assumed that the request can be
> modified and passed, but not completely replaced. Do you think this
> needs to be explicitly stated in the documentation?
server (e.g., to serve a file from S3); in that case, you need to replace the
URI to reference the proxied file at the very least, and may decide to create a
bare `RequestInterface` instance to delegate (to ensure it contains appropriate
headers, content, etc.).
That said, I'd likely just dispatch a client stack within my server-side
middleware to do this, and return the response from it.
My thought is that in `ServerMiddlewareInterface::process()`'s docblock, we may
want to indicate that implementations should always pass a
`ServerRequestInterface` to `$frame->next()`, and never a non-server variant, to
ensure proper execution fo the frame.
--
https://mwop.net/
Oscar Otero
Jun 20, 2016, 1:41:39 PM6/20/16
to php...@googlegroups.com
Hello.
IMHO, I think an empty interface is a bad thing, because the main aim of an interface is to define the api of an object (the public methods and their signatures). If it’s going to be two interfaces (for client and server middleware) I’d create two separate middleware stack and frame, because it makes no sense to typehint the middleware in some places and not in others.
--
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/CAJp_myWZvjAsUvMm9aw7iM8nC14Y7jOaZ72s1fzsSkFKqzUmcA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Woody Gilk
Jun 20, 2016, 2:13:20 PM6/20/16
to PHP Framework Interoperability Group
On Mon, Jun 20, 2016 at 12:41 PM, Oscar Otero <oscar...@gmail.com> wrote:
> I’d create two separate middleware stack and frame, because it makes no
> sense to typehint the middleware in some places and not in others.
Without the empty MiddlewareInterface, how would you declare type for
> I’d create two separate middleware stack and frame, because it makes no
> sense to typehint the middleware in some places and not in others.
a stack container that mixes both client and server middleware?
Oscar Otero
Jun 20, 2016, 3:04:55 PM6/20/16
to php...@googlegroups.com
I didn’t allow that. A RequestStack should accept only RequestMiddleware instances and ServerRequestStack only ServerRequestMiddleware.
For middleware creators, this is not hard to implement:
abstract class DualMiddleware
{
protected function run($request, $frame)
{
//here the code
}
}
class RequestMiddleware extend DualMiddleware
{
public function handle(RequestInterface $request, FrameInterface $frame)
{
return $this->run($request, $frame);
}
}
class ServerRequestMiddleware extend DualMiddleware
{
public function handle(ServerRequestInterface $request, FrameInterface $frame)
{
return $this->run($request, $frame);
}
}
I know that this can be a bit repetitive, but I my oppinion, better than having an empty interface.
For middleware creators, this is not hard to implement:
abstract class DualMiddleware
{
protected function run($request, $frame)
{
//here the code
}
}
class RequestMiddleware extend DualMiddleware
{
public function handle(RequestInterface $request, FrameInterface $frame)
{
return $this->run($request, $frame);
}
}
class ServerRequestMiddleware extend DualMiddleware
{
public function handle(ServerRequestInterface $request, FrameInterface $frame)
{
return $this->run($request, $frame);
}
}
I know that this can be a bit repetitive, but I my oppinion, better than having an empty interface.
> --
> 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/CAGOJM6K9hAiFUQXk4gidtOgxZHJim9efrswXKDLN%3DMwg%2BzZCDQ%40mail.gmail.com.
> 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.
Woody Gilk
Jun 20, 2016, 3:09:36 PM6/20/16
to PHP Framework Interoperability Group
On Mon, Jun 20, 2016 at 2:04 PM, Oscar Otero <oscar...@gmail.com> wrote:
> I didn’t allow that. A RequestStack should accept only RequestMiddleware instances and ServerRequestStack only ServerRequestMiddleware.
I've discussed this at length earlier in the thread. It is perfectly
> I didn’t allow that. A RequestStack should accept only RequestMiddleware instances and ServerRequestStack only ServerRequestMiddleware.
acceptable to mix client middleware with server request middleware and
process a ServerRequest, because ServerRequest is an extension of
Request. Arbitrarily splitting server and client into separate
interfaces makes it impossible to do this.
I definitely don't love having an empty interface, but it is best
solution we have available in PHP without resorting to wrappers.
Reply all
Reply to author
Forward
0 new messages