From 322c2d43996b76901dbb610e94a47c6f1dcf39c5 Mon Sep 17 00:00:00 2001 From: Rasmus Schultz Date: Fri, 11 Jan 2019 09:50:46 +0100 Subject: [PATCH 1/6] add missing requirements for internal file pointers --- accepted/PSR-17-http-factory-meta.md | 22 ++++++++++++++++++++++ accepted/PSR-17-http-factory.md | 8 ++++++++ 2 files changed, 30 insertions(+) diff --git a/accepted/PSR-17-http-factory-meta.md b/accepted/PSR-17-http-factory-meta.md index 97679e1f8..6e94c3bca 100644 --- a/accepted/PSR-17-http-factory-meta.md +++ b/accepted/PSR-17-http-factory-meta.md @@ -268,6 +268,28 @@ the time this proposal was developed allowed a string URI when creating a `UriInterface` implementation they provided. As such, accepting a string is expedient and follows existing semantics. +### 5.7 Why are Streams created with the read/write pointer at the beginning? + +The primary use case of `StreamFactoryInterface::createStream()` and +`StreamFactoryInterface::createStreamFromFile()` is to create streams for the +body of a `ResponseInterface` instance to be emitted as an HTTP response. + +An emitter can make no assumptions about streams (or their internal resource +handles) being repeatable, and therefore can't simply `rewind()` the stream. + +Even in the case where a stream returns `true` for `isSeekable()`, rewinding +the stream could have unacceptable performance implications. Similarly, the +`__toString()` method results in unacceptable memory overhead for large streams. + +Consequently, to efficiently emit the body stream of a `ResponseInterface` +instance as body of an HTTP response, we must be able to make an assumption +about the stream pointer being positioned at the beginning of the stream. + +As for `StreamFactoryInterface::createStreamFromResource()`, the caller is +responsible for the creation of the PHP resource to use as the basis for the +stream, and therefore also assumes responsibility for the read/write pointer +state of the resource and resulting stream. + ## 6. People This PSR was produced by a FIG Working Group with the following members: diff --git a/accepted/PSR-17-http-factory.md b/accepted/PSR-17-http-factory.md index cf7b31c88..6a502c6a1 100644 --- a/accepted/PSR-17-http-factory.md +++ b/accepted/PSR-17-http-factory.md @@ -117,6 +117,9 @@ interface StreamFactoryInterface * Create a new stream from a string. * * The stream SHOULD be created with a temporary resource. + * + * The stream MUST be created with the current position of the file read/write + * pointer at the beginning of the stream. * * @param string $content String content with which to populate the stream. */ @@ -130,6 +133,9 @@ interface StreamFactoryInterface * * The `$filename` MAY be any string supported by `fopen()`. * + * The stream MUST be created with the current position of the file read/write + * pointer at the beginning of the stream. + * * @param string $filename The filename or stream URI to use as basis of stream. * @param string $mode The mode with which to open the underlying filename/stream. * @@ -143,6 +149,8 @@ interface StreamFactoryInterface * * The stream MUST be readable and may be writable. * + * The current read/write position of the given PHP resource MUST NOT be modified. + * * @param resource $resource The PHP resource to use as the basis for the stream. */ public function createStreamFromResource($resource): StreamInterface; From 740fa21a1b7649bb3e6665f31b3febf67c4a20e4 Mon Sep 17 00:00:00 2001 From: Rasmus Schultz Date: Sun, 13 Jan 2019 10:53:09 +0100 Subject: [PATCH 2/6] add errata section --- accepted/PSR-17-http-factory-meta.md | 20 ++++++++++++++++++++ accepted/PSR-17-http-factory.md | 6 +++--- 2 files changed, 23 insertions(+), 3 deletions(-) diff --git a/accepted/PSR-17-http-factory-meta.md b/accepted/PSR-17-http-factory-meta.md index 6e94c3bca..c7f85f5fd 100644 --- a/accepted/PSR-17-http-factory-meta.md +++ b/accepted/PSR-17-http-factory-meta.md @@ -328,3 +328,23 @@ _**Note:** Order descending chronologically._ - [shadowhand Dependency Inversion and PSR-7 Bodies](http://shadowhand.me/dependency-inversion-and-psr-7-bodies/) - [PHP-FIG mailing list thread discussing factories](https://groups.google.com/d/msg/php-fig/G5pgQfQ9fpA/UWeM1gm1CwAJ) - [PHP-FIG mailing list thread feedback on proposal](https://groups.google.com/d/msg/php-fig/piRtB2Z-AZs/8UIwY1RtDgAJ) + +## 9. Errata + +This recommendation initially omitted the following requirements. + +### 9.1 `StreamFactoryInterface::createStream()` + +The state of the created temporary resource was unspecified - per section 5.7, the +requirement was added to position the temporary resource at the beginning of the stream. + +### 9.2 `StreamFactoryInterface::createStreamFromFile()` + +The state of the created temporary resource was unspecified - per section 5.7, the +requirement was added to position the temporary resource at the beginning of the stream. + +### 9.3 `StreamFactoryInterface::createStreamFromResource()` + +The requirements did not specify whether this method might affect the state of the given +resource - per section 5.7, the requirement was added to clarify that this method MUST NOT +modify the position of the given resource. diff --git a/accepted/PSR-17-http-factory.md b/accepted/PSR-17-http-factory.md index 6a502c6a1..8ccf159c6 100644 --- a/accepted/PSR-17-http-factory.md +++ b/accepted/PSR-17-http-factory.md @@ -119,7 +119,7 @@ interface StreamFactoryInterface * The stream SHOULD be created with a temporary resource. * * The stream MUST be created with the current position of the file read/write - * pointer at the beginning of the stream. + * pointer at the beginning of the stream. (Errata 9.1) * * @param string $content String content with which to populate the stream. */ @@ -134,7 +134,7 @@ interface StreamFactoryInterface * The `$filename` MAY be any string supported by `fopen()`. * * The stream MUST be created with the current position of the file read/write - * pointer at the beginning of the stream. + * pointer at the beginning of the stream. (Errata 9.2) * * @param string $filename The filename or stream URI to use as basis of stream. * @param string $mode The mode with which to open the underlying filename/stream. @@ -149,7 +149,7 @@ interface StreamFactoryInterface * * The stream MUST be readable and may be writable. * - * The current read/write position of the given PHP resource MUST NOT be modified. + * The current read/write position of the given PHP resource MUST NOT be modified. (Errata 9.3) * * @param resource $resource The PHP resource to use as the basis for the stream. */ From 983002eb09fb961bccb0c77250e39af498c313be Mon Sep 17 00:00:00 2001 From: Rasmus Schultz Date: Wed, 30 Jan 2019 08:18:57 +0100 Subject: [PATCH 3/6] add missing requirements for body-streams in generated Message instances; per feedback from Oscar Otero here https://groups.google.com/forum/#!topic/php-fig/mv9-MWDVVBM --- accepted/PSR-17-http-factory-meta.md | 6 ++++++ accepted/PSR-17-http-factory.md | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/accepted/PSR-17-http-factory-meta.md b/accepted/PSR-17-http-factory-meta.md index c7f85f5fd..2af114d92 100644 --- a/accepted/PSR-17-http-factory-meta.md +++ b/accepted/PSR-17-http-factory-meta.md @@ -290,6 +290,12 @@ responsible for the creation of the PHP resource to use as the basis for the stream, and therefore also assumes responsibility for the read/write pointer state of the resource and resulting stream. +And finally, the instances created by the `RequestFactoryInterface()`, +`ResponseFactoryInterface()` and `ServerRequestFactoryInterface` factories +must generate instances that return an empty body-stream in read and write +mode, so that client-code can start writing to the body stream without +manually needing to initialize anything else. + ## 6. People This PSR was produced by a FIG Working Group with the following members: diff --git a/accepted/PSR-17-http-factory.md b/accepted/PSR-17-http-factory.md index 8ccf159c6..0d8513325 100644 --- a/accepted/PSR-17-http-factory.md +++ b/accepted/PSR-17-http-factory.md @@ -51,6 +51,8 @@ interface RequestFactoryInterface } ``` +The created instance MUST return an empty body-stream in read and write mode. + ### 2.2 ResponseFactoryInterface Has the ability to create responses. @@ -74,6 +76,8 @@ interface ResponseFactoryInterface } ``` +The created instance MUST return an empty body-stream in read and write mode. + ### 2.3 ServerRequestFactoryInterface Has the ability to create server requests. @@ -102,6 +106,8 @@ interface ServerRequestFactoryInterface } ``` +The created instance MUST return an empty body-stream in read and write mode. + ### 2.4 StreamFactoryInterface Has the ability to create streams for requests and responses. From 79afc6397e1cee611b5c6d57d83d82282b070ec6 Mon Sep 17 00:00:00 2001 From: Rasmus Schultz Date: Wed, 30 Jan 2019 16:24:08 +0100 Subject: [PATCH 4/6] "body stream" -> `StreamInterface` instance --- accepted/PSR-17-http-factory-meta.md | 6 +++--- accepted/PSR-17-http-factory.md | 6 +++--- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/accepted/PSR-17-http-factory-meta.md b/accepted/PSR-17-http-factory-meta.md index 2af114d92..65d6bb9bb 100644 --- a/accepted/PSR-17-http-factory-meta.md +++ b/accepted/PSR-17-http-factory-meta.md @@ -292,9 +292,9 @@ state of the resource and resulting stream. And finally, the instances created by the `RequestFactoryInterface()`, `ResponseFactoryInterface()` and `ServerRequestFactoryInterface` factories -must generate instances that return an empty body-stream in read and write -mode, so that client-code can start writing to the body stream without -manually needing to initialize anything else. +must generate instances that return an empty `StreamInterface` instance in +read and write mode, so that client-code can start writing to the body stream, +without manually needing to initialize anything else. ## 6. People diff --git a/accepted/PSR-17-http-factory.md b/accepted/PSR-17-http-factory.md index 0d8513325..40495eeb3 100644 --- a/accepted/PSR-17-http-factory.md +++ b/accepted/PSR-17-http-factory.md @@ -51,7 +51,7 @@ interface RequestFactoryInterface } ``` -The created instance MUST return an empty body-stream in read and write mode. +The created instance MUST return an empty `StreamInterface` instance in read and write mode. ### 2.2 ResponseFactoryInterface @@ -76,7 +76,7 @@ interface ResponseFactoryInterface } ``` -The created instance MUST return an empty body-stream in read and write mode. +The created instance MUST return an empty `StreamInterface` instance in read and write mode. ### 2.3 ServerRequestFactoryInterface @@ -106,7 +106,7 @@ interface ServerRequestFactoryInterface } ``` -The created instance MUST return an empty body-stream in read and write mode. +The created instance MUST return an empty `StreamInterface` instance in read and write mode. ### 2.4 StreamFactoryInterface From 4d58ada3a32bbd620bca05be1c6ebc5c41d0664f Mon Sep 17 00:00:00 2001 From: Rasmus Schultz Date: Thu, 31 Jan 2019 08:19:58 +0100 Subject: [PATCH 5/6] `Interface()` -> `Interface` --- accepted/PSR-17-http-factory-meta.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/accepted/PSR-17-http-factory-meta.md b/accepted/PSR-17-http-factory-meta.md index 65d6bb9bb..c7d89906d 100644 --- a/accepted/PSR-17-http-factory-meta.md +++ b/accepted/PSR-17-http-factory-meta.md @@ -290,8 +290,8 @@ responsible for the creation of the PHP resource to use as the basis for the stream, and therefore also assumes responsibility for the read/write pointer state of the resource and resulting stream. -And finally, the instances created by the `RequestFactoryInterface()`, -`ResponseFactoryInterface()` and `ServerRequestFactoryInterface` factories +And finally, the instances created by the `RequestFactoryInterface`, +`ResponseFactoryInterface` and `ServerRequestFactoryInterface` factories must generate instances that return an empty `StreamInterface` instance in read and write mode, so that client-code can start writing to the body stream, without manually needing to initialize anything else. From e2ea01a0bd29ed384ba0b1fb210ad80fdd719c3e Mon Sep 17 00:00:00 2001 From: Rasmus Schultz Date: Wed, 20 Feb 2019 08:54:36 +0100 Subject: [PATCH 6/6] `createStreamFromFile()` should behave like `fopen()` --- accepted/PSR-17-http-factory-meta.md | 17 ++++++++++++----- accepted/PSR-17-http-factory.md | 3 ++- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/accepted/PSR-17-http-factory-meta.md b/accepted/PSR-17-http-factory-meta.md index c7d89906d..d952b9a17 100644 --- a/accepted/PSR-17-http-factory-meta.md +++ b/accepted/PSR-17-http-factory-meta.md @@ -268,11 +268,18 @@ the time this proposal was developed allowed a string URI when creating a `UriInterface` implementation they provided. As such, accepting a string is expedient and follows existing semantics. -### 5.7 Why are Streams created with the read/write pointer at the beginning? +### 5.7 Why are Streams created with the specified read/write pointer positions? -The primary use case of `StreamFactoryInterface::createStream()` and -`StreamFactoryInterface::createStreamFromFile()` is to create streams for the -body of a `ResponseInterface` instance to be emitted as an HTTP response. +The `StreamFactoryInterface::createStreamFromFile()` method implementation +typically maps directly to an `fopen()` call and, in terms of the read/write +pointer position, should behave consistently with `fopen()` with regards to +the `$mode` argument, as covered by the +[PHP manual page](http://php.net/manual/en/function.fopen.php). + +The primary use case for `StreamFactoryInterface::createStream()` is to +create streams for the body of a `ResponseInterface` instance to be emitted +as an HTTP response. It can also be used to create an empty stream, which +can subsequently be written to. An emitter can make no assumptions about streams (or their internal resource handles) being repeatable, and therefore can't simply `rewind()` the stream. @@ -347,7 +354,7 @@ requirement was added to position the temporary resource at the beginning of the ### 9.2 `StreamFactoryInterface::createStreamFromFile()` The state of the created temporary resource was unspecified - per section 5.7, the -requirement was added to position the temporary resource at the beginning of the stream. +requirement was added to position the resource consistently with `fopen()`. ### 9.3 `StreamFactoryInterface::createStreamFromResource()` diff --git a/accepted/PSR-17-http-factory.md b/accepted/PSR-17-http-factory.md index 40495eeb3..cb1e00286 100644 --- a/accepted/PSR-17-http-factory.md +++ b/accepted/PSR-17-http-factory.md @@ -140,7 +140,8 @@ interface StreamFactoryInterface * The `$filename` MAY be any string supported by `fopen()`. * * The stream MUST be created with the current position of the file read/write - * pointer at the beginning of the stream. (Errata 9.2) + * pointer positioned consistently with that of `fopen()` with regards to + * the `$mode` flag. (Errata 9.2) * * @param string $filename The filename or stream URI to use as basis of stream. * @param string $mode The mode with which to open the underlying filename/stream.