Added support for multipart stream created from uri streams #27
Conversation
…seekable streams is not fetched using string casting. Added testSupportURIResources phpUnit test validating the multipart stream created from an uri resource. Changed composer dev requirement zendframework/zend-diactoros for guzzlehttp/psr7 due to the fact that zend-diactoros DiactorosStreamFactory's createStream function runs a rewind which throws an exception if stream is not seekable. Removed dependency on Zend\Diactoros\Stream in unit tests.
tests/FunctionTest.php
Outdated
| @@ -15,7 +15,7 @@ public function testSupportStreams() | |||
| $body = 'stream contents'; | |||
|
|
|||
| $builder = new MultipartStreamBuilder(); | |||
| $builder->addResource('foobar', $this->createStream($body)); | |||
There was a problem hiding this comment.
This tests make sure we support a Stream. Please do not change this.
| @@ -95,7 +95,14 @@ public function build() | |||
| $this->getHeaders($data['headers'])."\r\n"; | |||
|
|
|||
| // Convert the stream to string | |||
| $streams .= (string) $data['contents']; | |||
| /* @var $contentStream StreamInterface */ | |||
composer.json
Outdated
| @@ -19,7 +19,7 @@ | |||
| "require-dev": { | |||
| "phpunit/phpunit": "^4.8 || ^5.4", | |||
| "php-http/message": "^1.0", | |||
| "zendframework/zend-diactoros": "^1.3.5" | |||
| "guzzlehttp/psr7": "^1.3.1" | |||
|
Done. The uri test will fail until your PR has been merged. |
|
Thank you. I'll see if we can come to a conclusion quickly on the PRs on php-http/message. |
|
No problem, happy to pitch in. |
|
Refactored some variable names in the test to mirror what has been used in the other tests. |
src/MultipartStreamBuilder.php
Outdated
| /* @var $contentStream StreamInterface */ | ||
| $contentStream = $data['contents']; | ||
| if ($contentStream->isSeekable()) { | ||
| $streams .= (string) $data['contents']; |
There was a problem hiding this comment.
Even though the output it the same. I think I would like to change this line to:
$streams .= $contentStream->_toString();It will increase readability.
There was a problem hiding this comment.
Great to hear you plan to merge the PR next week. I agree with your comment about readability and have made the change from string casting to explicitly calling __toString().
…explicitly calling __toString() for clarity as suggested by @Nyholm
|
It works with message 1.5.0 Thank you. |
Added support for multipart uri streams by ensuring content from non-seekable streams is not fetched using string casting.
Added testSupportURIResources phpUnit test validating the multipart stream created from an uri resource.
Changed composer dev requirement zendframework/zend-diactoros for guzzlehttp/psr7 due to the fact that zend-diactoros DiactorosStreamFactory's createStream function runs a rewind which throws an exception if stream is not seekable.
Removed dependency on Zend\Diactoros\Stream in unit tests.