http-interop / http-middleware

PSR-15 interfaces for HTTP Middleware
MIT License
73 stars 7 forks source link

Issues solved/weakened by PHP RFC: Parameter Type Widening #65

Closed schnittstabil closed 7 years ago

schnittstabil commented 7 years ago

:tada: PHP RFC: Parameter Type Widening got landed and will be available in PHP 7.2 :tada:

As far as I can see that solved/weakened some old issues for us :heart:

Current Interfaces

```php interface DelegateInterface { public function process(ServerRequestInterface $request); } interface MiddlewareInterface { public function process(ServerRequestInterface $request, DelegateInterface $delegate); } ```

Parameter Type Widening allows Middlewares which work on client and server side

interface ClientDelegateInterface
{
    public function process(RequestInterface $request);
}

interface ClientMiddlewareInterface
{
    public function process(RequestInterface $request, ClientDelegateInterface $delegate);
}

class ClientAndServerMiddleware implements ClientMiddlewareInterface, MiddlewareInterface
{
    /**
     * @param RequestInterface|ServerRequestInterface $request
     * @param ClientDelegateInterface|DelegateInterface $delegate
     *
     * @return ResponseInterface
     */
    public function process($request, $delegate)
    {
        if (! $request instanceOf RequestInterface) {
            throw new InvalidArgumentException(…);
        }

        if (!($delegate instanceOf ClientDelegateInterface || $delegate instanceOf DelegateInterface)) {
            throw new InvalidArgumentException(…);
        }

        // do some stuff…
        return $delegate->process($request);
    }
}

Parameter Type Widening allows callable delegates

class CallableDelegateMiddleware implements MiddlewareInterface
{
    /**
     * @param ServerRequestInterface $request
     * @param callable|DelegateInterface $delegate
     */
    public function process(ServerRequestInterface $request, $delegate)
    {
        if (! $delegate instanceOf DelegateInterface) {
            if (! is_callable($delegate)) {
                throw new InvalidArgumentException(…);
            }

            $delegate = new class($delegate) implements DelegateInterface {
                private $d;

                public function __construct($delegate)
                {
                    $this->d = $delegate;
                }

                public function process(ServerRequestInterface $request)
                {
                    return ($this->d)($request);
                }
            };
        }

        // do some stuff…
        return $delegate->process($request);
    }
}

Btw, I'd love to see DelegateInterface::__invoke :wink:


Did I miss issues affected by that RFC? Thoughts?

atanvarno69 commented 7 years ago

This is out of scope (Meta document, section 3.2 Non-Goals):

  • Attempting to define interfaces for client/asynchronous middleware.

Since (section 5.1.3):

external requests are typically handled asynchronously and would typically return a promise of a response. [...] It is outside the scope of this proposal to address the needs of asynchronous request/response life cycles.

Even if it were in scope, this does not provide a mechanism for providing promises.

Parameter Type Widening allows callable delegates [...] Btw, I'd love to see DelegateInterface::__invoke 😉

Make a case at review, succinctly explaining why these things are favorable. As it stands, I doubt this proposal will undergo any significant revision (like altering its scope) until review feedback is being incorporated.

schnittstabil commented 7 years ago

@atanvarno69 I've posted this because long time ago both were not out of scope.

Make a case at review, succinctly explaining why these things are favorable.

You got me wrong, I do not say (all) these are favorable. For example, and to be more explicit, I do not propose callable delegates above – the opposite is the case, I say that RFC weakens arguments for callable!

schnittstabil commented 7 years ago

Even if it were in scope, this does not provide a mechanism for providing promises.

The example above was discussed many times, and If you reread it, then you must admit that it also works in promised land – the response object is not altered, only returned. Btw, promises are not necessarily needed to achieve async client middlewares.

I believe you just do not know, that my [PSR-15] Falsehoods About The Client-side post at the mailing-list made client middlewares out of scope – Hence, I'm fully aware of your arguments, they are my own.

shadowhand commented 7 years ago

It is not enough to allow different request types to be handled by a middleware. Server middleware will return a response, and client middleware will (almost certainly) return a promise. Thus the behavior of server and client middleware will be inherently different. The point of parameter type widening to allow dropping a type when the underlying behavior is the same.