Before releasing 1.5.0

[Nyholm]

Before we release the next version, I would like to discuss one thing. The discussion started here between @Zegnat, @dbu and myself. A short summery is:

$s = \Nyholm\Psr7\Stream::create('Hello');
$s->write(' W');
echo "getContents(): \n";
echo $s->getContents();
echo "\n\n";
echo "toString(): \n";
echo $s->__toString();

In 1.4.0:

getContents(): 

toString(): 
Hello W

In master:

getContents(): 
llo

toString(): 
 Wllo

---

If we dont use ->write()

$s = \Nyholm\Psr7\Stream::create('Hello');

echo "getContents(): \n";
echo $s->getContents();
echo "\n\n";
echo "toString(): \n";
echo $s->__toString();

In 1.4.0 (last release):

getContents(): 

toString(): 
Hello

In master:

getContents(): 
Hello

toString(): 
Hello

---

Yes the PSR-7 getContents() method is confusing and should never be used. But the issue Im most concerned with is if this is a BC break or not. We have never specified where the read pointer should be after you run Stream::create() and we have always said that you never should assume anything. Ie, when you get a stream, you place the pointer yourself so you know where it is.

This functionality has also never been tested (until now). The reason for making the change is because we have got so many confused users and complaints about this... so we are rather pragmatic than 100% strict/correct.

[mahagr]

This is super tricky to me as write() should by default write to the end of the stream, but you kind of assume that getContents() would return the whole content unless you have moved the internal pointer. So the methods contradict each other...

Has anyone tested how stream_get_contents() works? As it looks to be the inspiration to the PSR-7 method.

[Zegnat]

Has anyone tested how stream_get_contents() works? As it looks to be the inspiration to the PSR-7 method.

stream_get_contents() does exactly what this library does for StreamInterface->getContents(): it gets everything from the cursor to the end of the stream. If you have written to a stream resource you must first rewind() to get the full contents.

But this is pretty consistent between all PSR-7 implementations as far as I know. What isn’t consistent is where the cursor ends up when a stream is created. This is also the part that PSR-7 (nor PSR-17, the factories) defines and there is no right answer other than “do not use StreamInterface->getContents() if you have not taken control of the cursor position.” It is also why the recommendation to use StreamInterface->__toString(), which is specifically defined to take all of the stream contents, exists.

When nyholm/psr7 creates a stream (Stream::create($value)) it follows the effect of opening a new stream and writing $value to it. Because it writes to it, the cursor ends up at the end. Other implementation instead choose to simulate opening a stream that already contains $value, putting the cursor at the start. There is no right or wrong here, there is only user confusion.

My comment from the PR still stands as my thoughts on the matter:

My argument would be that Stream::create() is part of the nyholm/psr7 API, even if not defined by PSR. The output of Stream::create('Hello') can no longer be chained with ->write(' world!') without the result being different. This to me means we have broken the BC promise with PR #177.

If only the PSR methods are seen as included in the promise (or in the “public API” as semver says), it can easily be argued that the promise specifically does not mention the pointer. This would just be a patch or minor release to bring us inline with another part of the ecosystem. But if our own static factory methods are part of the promise my hardliner stance would be that a major version increase is required per strict semver.

But do note that I personally am not stuck to semver at all. But if this library wants to follow semver in the strictest sense, then I do not see how this is anything but a major bump.

[Nyholm]

then I do not see how this is anything but a major bump.

Or a revert of this change.