Making PSR-7 StreamInterface easier to work with

[Woody Gilk]

In light of recent discussions regarding response objects and body not
being immutable, as summarized quite well by Andrew Carter [1], I am
wondering what could be done to make it easier to use the withBody()
method.

[1]: http://andrewcarteruk.github.io/programming/2016/05/22/psr-7-is-not-immutable.html

Right now, it is extremely difficult to create a new stream without
referring to a specific implementation and so we often end up doing
something like this:

$body = $response->getBody();
$body->write($content);

When really, what we should be doing is something like:

$body = get_stream_from_string($content);
$response = $response->withBody($body);

The former breaks the immutability of the response, which leads to the
suggestion that it cannot be trusted. The latter preserves
immutability by replacing the body content entirely, which is what we
_should_ be doing but have a very hard time doing because
StreamInterface is mutable and a resource wrapper.

Rasmus posted a possible factory interface [2] in relation to PSR-7
middleware that could be a basis for a psr/http-factory package. The
factory defines methods to create all types of PSR-7 objects and
definitely help resolve the middleware debate, or at least provide a
better foundation for Anthony's argument against having
ResponseInterface part of the signature. (Though I still don't agree
that dropping $response from the signature is the right thing to do.)

[2]: https://bitbucket.org/mindplaydk/psr-middleware/src/5688dc2f42d5b9eefcacf2fbc39f140b50a14112/psr-7-factory/FactoryInterface.php

Thoughts?
--
Woody Gilk
http://about.me/shadowhand

[Derrick Nelson]

On Sunday, May 22, 2016 at 11:42:33 AM UTC-4, Woody Gilk wrote:

Thoughts?

public function withBody(StreamInterface $body);

public function getBody(): ReadOnlyStreamInterface;

[Tomasz Darmetko]

Idea of the factory sound great to me, but I'm terrified by the number of arguments this methods take eg.

$factory->createServerRequest([], [], null, null, 'php://input', [], [], [], null, '1.0');

IMHO for the sanity of people reading code like that we should limit createRequest, createServerRequest and createResponse to creation of only empty objects or change list of arguments of this methods to an array so that one could do:

$factory->createServerRequest(['protocol' => '1.0']);

[Woody Gilk]

Tomasz,

I agree completely on that point. The method arguments should be as
minimal as possible and not try to fill every part of the
request/response.

--
Woody Gilk
http://about.me/shadowhand

> --
> 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/b6507bb9-abcb-475c-9b5a-ec17c8aaf41f%40googlegroups.com.
>
> For more options, visit https://groups.google.com/d/optout.

[Woody Gilk]

I'm not sure what you mean, Derrick. 

[Derrick Nelson]

On Monday, May 23, 2016 at 10:54:26 AM UTC-4, Woody Gilk wrote:

I'm not sure what you mean, Derrick. 

Sorry, I was being lazy.  What I meant was that your bad practice example:

On Sunday, May 22, 2016 at 11:42:33 AM UTC-4, Woody Gilk wrote:

$body = $response->getBody(); 
$body->write($content);

...could have been prevented by splitting StreamInterface into 2 interfaces and tweaking MessageInterface:

interface ReadOnlyStreamInterface { /* everything except write() */ }

interface StreamInterface extends ReadOnlyStreamInterface { /* write() */ }

interface MessageInterface

{

    public function withBody(StreamInterface $body); // accepts instances of both types

    public function getBody(): ReadOnlyStreamInterface; // only exposes a read-only instance externally

   /* ... */

}

This would have left the message free to accept either type of stream, while restricting it from exposing a stream with content mutators.  How that would actually work is an implementation detail, but the obvious approach would be for implementations to wrap incoming writable streams in read-only proxies when necessary.  This isn't immutability, since the stream object's internal state could still mutate (offset); however, it would have solved the problem everyone seems most concerned with when it comes to the mutability of streams, which is modification of the message body after the message has been constructed.

[Woody Gilk]

On Mon, May 23, 2016 at 5:23 PM, Derrick Nelson <drrc...@gmail.com> wrote:
> ...could have been prevented by splitting StreamInterface into 2 interfaces
> and tweaking MessageInterface:

I agree that might have been ideal, but making a modification to PSR-7
would be difficult at this point. Having a new PSR-7 for factories
would be better.

[Derrick Nelson]

On Tuesday, May 24, 2016 at 10:13:13 AM UTC-4, Woody Gilk wrote:

I agree that might have been ideal, but making a modification to PSR-7
would be difficult at this point. Having a new PSR-7 for factories
would be better.

They solve different problems, so I don't think either can be better than the other.  The proposed factory interface can decouple stream implementations, but does nothing to address mutating message bodies, and vice versa for my suggestion.  You brought up both problems in your original post, but offered a solution that only addressed one of them.  My mistake if I misunderstood the intent here.

[Woody Gilk]

On Tue, May 24, 2016 at 10:50 AM, Derrick Nelson <drrc...@gmail.com> wrote:
> The proposed factory interface can decouple stream implementations, but
> does nothing to address mutating message bodies, and vice versa for my
> suggestion.

That's true, but I am not sure we can or need to solve the mutation
problem. In my mind, it's one of those "feature or bug" scenarios.
Having a factory would at least allow developers to say "you should
create a new response body here, instead of modifying it".

And to that end, I have just opened
https://github.com/php-fig/fig-standards/pull/759 to propose a set of
factory interfaces.

[Matthew Weier O'Phinney]

On Mon, May 23, 2016 at 5:23 PM, Derrick Nelson <drrc...@gmail.com> wrote:

> On Monday, May 23, 2016 at 10:54:26 AM UTC-4, Woody Gilk wrote:
>>
>> I'm not sure what you mean, Derrick.
>
>
> Sorry, I was being lazy. What I meant was that your bad practice example:
>
> On Sunday, May 22, 2016 at 11:42:33 AM UTC-4, Woody Gilk wrote:
>>
>> $body = $response->getBody();
>> $body->write($content);
>
>
> ...could have been prevented by splitting StreamInterface into 2 interfaces
> and tweaking MessageInterface:
>
> interface ReadOnlyStreamInterface { /* everything except write() */ }
> interface StreamInterface extends ReadOnlyStreamInterface { /* write() */ }
>
> interface MessageInterface
> {
> public function withBody(StreamInterface $body); // accepts instances of
> both types
> public function getBody(): ReadOnlyStreamInterface; // only exposes a
> read-only instance externally
> /* ... */
> }
>
> This would have left the message free to accept either type of stream, while
> restricting it from exposing a stream with content mutators.

You missed the discussion in the metadocument where we talk about this, then. :)

The reason we have no immutable stream interfaces is because it's
impossible to lock down the underlying stream resources, which are the
most common way to implement the functionality. Since this is an
insurmountable problem (it lies in the PHP engine itself), we had to
concede and allow mutability in streams. You can mark a stream as
immutable (we have an isWritable() method defined in the interface),
but that is no guarantee.

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