Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added PSR-7 Interfaces brief explanations and Basic Usage Guide #79

Merged
merged 4 commits into from Aug 29, 2019

Conversation

gabidj
Copy link
Contributor

@gabidj gabidj commented Mar 24, 2017

Added some examples for basic PSR-7 interfaces usage

Copy link
Contributor

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, I think this will be a welcome addition.

I'm unsure if the PSR7-Interfaces.md file is strictly necessary; it feels like we could direct developers to the specification, which details each interface, and the methods. That said, having a tabular format is relatively useful for easy lookup of arguments and expected return values. I'm curious if others could weigh in with their opinions.

Second, as noted, I'm reluctant to use Diactoros for the examples, as that could be construed as recommendation of that specific implementation. I think it would suffice to indicate that instantiation will vary based on implementation, and just indicate which interface each of $request, $response, etc. refer to.

| Method Name | Description | Notes |
|------------------------------------| ----------- | ----- |
| `getProtocolVersion()` | Gets HTTP protocol version | 1.0 or 1.1 |
| `withProtocolVersion($version)` | Sets HTTP protocol version | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be careful about using the term "sets" when discussing the various with*() methods. They return a new instance that incorporates the data you provide, while leaving the original instance unchanged. This is more clear in the request/response-specific sections below, but the verbiage for these general message methods needs to be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if I write "Sets***" then noting that "***" means returning a new instance with given parameter instead of actually modifying the original HTTP Message (or Request or Response) ?

| `hasHeader($name)` | Checks if HTTP Header with given name exists | |
| `getHeader($name)` | Retrieves a array with the values for a single header | |
| `getHeaderLine($name)` | Retrieves a comma-separated string of the values for a single header | |
| `withHeader($name, $value)` | Sets a HTTP Header | If header already exists, value will be overwritten |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Returns a new message instance that includes the provided header; if the header existed in the original instance, replaces the header value from the original message with the value provided when creating the new instance."

| `getHeader($name)` | Retrieves a array with the values for a single header | |
| `getHeaderLine($name)` | Retrieves a comma-separated string of the values for a single header | |
| `withHeader($name, $value)` | Sets a HTTP Header | If header already exists, value will be overwritten |
| `withAddedHeader($name, $value)` | Appends value to given header | If header already exists value will be appended, if not a new header will be created |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do similar here as with the withHeader() (start with Returns a new message ...).

| `getHeaderLine($name)` | Retrieves a comma-separated string of the values for a single header | |
| `withHeader($name, $value)` | Sets a HTTP Header | If header already exists, value will be overwritten |
| `withAddedHeader($name, $value)` | Appends value to given header | If header already exists value will be appended, if not a new header will be created |
| `withoutHeader($name)` | Removes HTTP Header with given name| |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Returns a new message instance that does not include the specified HTTP header."

| `getUserInfo()` | Retrieve the user information component of the URI. | |
| `getHost()` | Retrieve the host component of the URI. | |
| `getPort()` | Retrieve the port component of the URI. | |
| `getPath()` | etrieve the path component of the URI. | |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing R in Retreive.

> When using `ServerRequestInterface`, both `RequestInterface` and `Psr\Http\Message\MessageInterface` methods are considered.


Enough with the talking, let's put things in practice.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Omit this line.

#####Examples (using Zend Diactoros)

Zend Diactoros is an implementation for PSR-7 interfaces. It will be used to illustrate these examples.
Installation guide for Zend Diactoros: [Zend Diactoros Documentation - Installation](https://zendframework.github.io/zend-diactoros/install/)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we should use Diactoros here, as that sounds like an endorsement of a specific implementation. I'd change this to indicate that instantiation of messages will vary based on implementation, and leave it at that. The rest of the examples will work fine, and should work the same across implementations.

```php
$body = $response->getBody();
$body->rewind(); // or $body->seek(0);
$bodyText = $body->getContends();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/getContends/getContents/

corrected PSR-7 methods explanations
corrected typos
removed concrete usage of diactoros
removed unnecessary text
removed zend-diactoros autoload
added assumption of $request and $response
@gabidj
Copy link
Contributor Author

gabidj commented Mar 28, 2017

@weierophinney , I have just corrected them with a more technical approach, I made the changes you suggested. Thank you for pointing out the inaccuracies. Please check if other issues exist.

@gabidj gabidj changed the title Added PSR-7 Interfaces Usage guide using Zend Diactoros Added PSR-7 Interfaces Usage guide Apr 3, 2017
@gabidj gabidj changed the title Added PSR-7 Interfaces Usage guide Added PSR-7 Interfaces brief explanations and Basic Usage Guide Apr 3, 2017
@gabidj
Copy link
Contributor Author

gabidj commented Apr 11, 2017

@weierophinney - Suggested changes were made.

Copy link
Contributor

@weierophinney weierophinney left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚢

@weierophinney weierophinney merged commit efd67d1 into php-fig:master Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants