[REVIEW] PSR-17 HTTP Message Factories

[Matthew Weier O'Phinney]

As of today, we formally begin the REVIEW phase of the proposed PSR-17
(HTTP Message Factories) specification. The proposed specification is
in the fig-standards repository at the following locations:

- Specification:
https://github.com/php-fig/fig-standards/blob/4af55ad2989ff47fb37b5012d61a81c6adca1bfb/proposed/http-factory/http-factory.md

- Metadocument:
https://github.com/php-fig/fig-standards/blob/4af55ad2989ff47fb37b5012d61a81c6adca1bfb/proposed/http-factory/http-factory-meta.md

During the Review phase, acceptable changes to the specification
include wording, typographical fixes, and clarifications. If you feel
major changes are necessary, please bring your arguments to the list
under this thread. As Sponsor of the specification, I will be final
arbiter on what changes we will include. If any major changes are
considered, we will return to the Draft phase.

The Review period will end no sooner than 21 June 2018 at 11:59pm. At
that time, if the working group can demonstrate two viable trial
implementations, and no need for major changes, I will call for an
Acceptance Vote.

--
Matthew Weier O'Phinney
mweiero...@gmail.com
https://mwop.net/

[Robert Lu]

Hi, Matthew.


Could you explain why we need HTTP Factory?

Firstly,

The former is a significant pain point for PSR-7 middleware, as it can leave the response in an incomplete state. If the stream attached to the response body is not seekable or not writable, there is no way to recover from an error condition in which the body has already been written to.

As a middleware, it has to assume that ResponseInterface is readable.
If some error is thrown from the handler, middleware can new a ResponseInterface's implement to upper.
If middleware wants to modify response body, new a ResponseInterface and put modified response body.

* Is there must be a ResponseFactoryInterface?

Secondly,

Another pain point is when writing re-usable middleware or request handlers. In such cases, package authors may need to create and return a response. However, creating discrete instances then ties the package to a specific PSR-7 implementation. If these packages rely on a request factory instead, they can remain agnostic of the PSR-7 implementation.

As a middleware, the promise or interface is returning ResponseInterface.
Before PSR-17, middleware must use a ResponseInterface’s implement.
After PSR-17, middleware must use a ResponseFactoryInterface’s implement.
So, both side relay on some specific implement.

* I want to know what situations PSR-17 is designed for?

--

Robert

[Chuck Burgess]

LGTM overall... PR sent for one tiny grammar fix.

CRB

[Matthew Weier O'Phinney]

On Thu, Jun 7, 2018 at 10:13 PM Robert Lu <robbe...@gmail.com> wrote:
> Could you explain why we need HTTP Factory?
>
> Firstly,
>
> > The former is a significant pain point for PSR-7 middleware, as it can leave the response in an incomplete state. If the stream attached to the response body is not seekable or not writable, there is no way to recover from an error condition in which the body has already been written to.
>
> As a middleware, it has to assume that ResponseInterface is readable.

Middleware generally doesn't need to care if the response is readable,
mainly that it's writable. But that's outside the scope of PSR-17.

> If some error is thrown from the handler, middleware can new a ResponseInterface's implement to upper.

Yes... but this then ties your middleware to a specific PSR-7
implementation, vs the interfaces (more below).

> If middleware wants to modify response body, new a ResponseInterface and put modified response body.

That's very error prone, as you then also need to transfer all
headers, and the status code and reason phrase to the new response as
well.

You could create a new stream to inject as a body, but that ties into
the same problem as above, which I outline more fully below

>
> * Is there must be a ResponseFactoryInterface?

The problem is that when you `new ImplementationOfResponseInterface`,
you are tying your code directly to a specific implementation of
PSR-7, instead of to ANY PSR-7 implementation. The whole point of
having the interfaces is to allow for interoperability and
substitution.

If it's your own application code, you've likely chosen an
implementation, and this is just fine. However:

- If you are writing code to distribute to others as a package, or
- You may want to switch PSR-7 implementations in the future

then you will want to code to the interfaces, and not the implementations.

Composing a _factory_ for creating the instance(s) instead of calling
`new` directly allows you to code to the _abstraction_, and not the
implementation. It also solves other problems, which are best
illustrated with an example.

To illustrate, let's say I want to write middleware to distribute as a
package, and that middleware has a code path where I need to return a
response without delegating to the handler.

Prior to PSR-17, I might have done this:

class MyMiddleware implements MiddlewareInterface
{
private $responsePrototype;

public function __construct(ResponseInterface $responsePrototype)
{
$this->responsePrototype = $responsePrototype;
}

public function process(ServerRequestInterface $request) :
ResponseInterface
{
// do some work
$response = $this->responsePrototype
->withStatus(400)
->withHeader('X-Custom-Header', 'some value')
->withHeader('Content-Type', 'application/json');
$response->getBody()->write(json_encode($data));
return $response
}
}

This seems to work, doesn't it? Why would we need a factory, when we
can just compose a prototype? Since all those `with*()` methods return
a new instance, we're fine, right?

The problem is that there is one element of the response that is not
guaranteed immutable: the body. Because a StreamInterface can be
backed by a PHP resource, we cannot guarantee that something else
doesn't write to that resource. This means if another class composes
the same response prototype, and, during the same request cycle writes
to the response body, we'll end up with additional contents we did not
account for.

There's also a potential concurrency problem if you use an async
application runner such as ReactPHP, IciclePHP, etc: that prototype
will likely be shared across multiple requests, and that request body
will just keep growing.

The point of PSR-17 is to solve these types of problems. When you
consume a factory to generate a new instance of a PSR-7 type, you are
guaranteed a new, unshared instance. The above then becomes this:

class MyMiddleware implements MiddlewareInterface
{
private $responseFactory;

public function __construct(ResponseFactoryInterface $responseFactory)
{
$this->responsePrototype = $responsePrototype;
}

public function process(ServerRequestInterface $request) :
ResponseInterface
{
// do some work
$response = $this->responseFactory->createResponse(400)
->withHeader('X-Custom-Header', 'some value')
->withHeader('Content-Type', 'application/json');
$response->getBody()->write(json_encode($data));
return $response
}
}

You will still use PSR-7 typehints. You'll just also use PSR-17
typehints to compose factories for generating PSR-7 types on-the-fly
when you need them.

> Secondly,
>
> > Another pain point is when writing re-usable middleware or request handlers. In such cases, package authors may need to create and return a response. However, creating discrete instances then ties the package to a specific PSR-7 implementation. If these packages rely on a request factory instead, they can remain agnostic of the PSR-7 implementation.
>
> As a middleware, the promise or interface is returning ResponseInterface.
> Before PSR-17, middleware must use a ResponseInterface’s implement.
> After PSR-17, middleware must use a ResponseFactoryInterface’s implement.
> So, both side relay on some specific implement.
>
> * I want to know what situations PSR-17 is designed for?

The difference is between this:

$response = new \Zend\Diactoros\Response();

and this:

$response = $this->responseFactory->createResponse();

The first is tying to a specific implementation of PSR-7. The second
keeps your code agnostic of which PSR-7 (or even PSR-17!)
implementation is used.

You tie to the implementation when you define at the application level
which PSR-17 implementation (and thus PSR-7 implementation) you will
inject into your classes. Your classes themselves, however, remain
agnostic of the implementation. One huge advantage this has is that
you generally then only need to make a change in one location to swap
out implementation for your entire application. It also means that
third-party code you incorporate can consume whichever implementation
you use, as that code will only depend and typehint on the PSR-7
and/or PSR-17 interfaces.

> On 6/8/18 02:26, Matthew Weier O'Phinney wrote:
>
> As of today, we formally begin the REVIEW phase of the proposed PSR-17
> (HTTP Message Factories) specification. The proposed specification is
> in the fig-standards repository at the following locations:
>
> - Specification:
> https://github.com/php-fig/fig-standards/blob/4af55ad2989ff47fb37b5012d61a81c6adca1bfb/proposed/http-factory/http-factory.md
>
> - Metadocument:
> https://github.com/php-fig/fig-standards/blob/4af55ad2989ff47fb37b5012d61a81c6adca1bfb/proposed/http-factory/http-factory-meta.md
>
> During the Review phase, acceptable changes to the specification
> include wording, typographical fixes, and clarifications. If you feel
> major changes are necessary, please bring your arguments to the list
> under this thread. As Sponsor of the specification, I will be final
> arbiter on what changes we will include. If any major changes are
> considered, we will return to the Draft phase.
>
> The Review period will end no sooner than 21 June 2018 at 11:59pm. At
> that time, if the working group can demonstrate two viable trial
> implementations, and no need for major changes, I will call for an
> Acceptance Vote.
>

[Larry Garfield]

Review as I go:

1. Spec

"HTTP factories MUST implement these interfaces for each object type that is
provided by the package." - Is the intent here that you have to implement
everything or nothing? Is it legit for a package to just implement those it
cares about? I don't quite follow this sentence. What's the intent here?

2.1 RequestFactoryInterface

Should this RECOMMEND composing a UriFactory in order to cast the string to
UriInterface?

2.4 StreamFactoryInterface

Nitpick: The fopen function is referred to in the docblock as "fopen" and as
"fopen()". Please pick one format or the other. I'd say "fopen()".

More feedback to come once I've actually tried using it. :-)

--Larry Garfield