Skip to content

A few clarifications added #10

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.

Sign up for GitHub

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

Open
davidan1981 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

A few clarifications added #10

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6ecead0
Clarifications added.
Mar 9, 2016
72f2eb9
Use the term payload instead of JSON payload.
Mar 9, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 11 additions & 6 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,14 +244,13 @@ a single `widget` (`a1b2c3`). What this then implicates is that you may not,
in general, create a `sprocket` at the root `sprocket` endpoint. The root `sprocket`
endpoint may list all of the current context's `sprocket`s, or similar, but in
general should not accept creation requests. In concrete examples:
- `POST /sprockets` is invalid.
- `POST /sprockets` is invalid unless `widget` is specified in the payload.
Copy link

@bendemaree bendemaree Mar 27, 2016

Choose a reason for hiding this comment

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

Was initially hesitant to accept this but I think it's alright. If this is done in practice maybe we can settle on a request format that guides the implementation more.

- `POST /widgets/a1b2c3/sprockets` will create a `sprocket` that may be
returned as a property within `/widgets/a1b2c3`.
- `GET /widgets/a1b2c3/sprockets` will return all the `sprocket`s that are
in the collection for the `a1b2c3` widget.
- `GET /sprockets/d4f5g6` will return a specific `sprocket`.
- `GET /sprockets` may return all `sprocket`s, regardless of `widget`
collection.
- `GET /sprockets` may return all `sprocket`s in the current context.

This nested endpoint relationship avoids having to submit the parent ID as
part of the request body. For example, again assuming a `sprocket` as a child
Expand Down Expand Up @@ -297,7 +296,7 @@ The following mapping guides the verbs to some equivalent wording.
| PUT | Replace | Replace the specified resource |
| DELETE | Remove | Delete the specified resource |
| OPTIONS | Define | View the schema and available actions for the resource or collection |
| HEAD | Describe | Return all of the information about a resource without returning the response itself |
| HEAD | Describe | Return all of the information about a resource or list of resources without returning the response body itself |

The action taken on the representation will be contextual to the media type
being worked on and its current state. Here's an example of how HTTP verbs
Expand All @@ -309,13 +308,14 @@ can map to different actions depending on context.
| `GET` | `/widgets` | List widgets |
| `POST` | `/widgets/3rDrZRbdeSJJeR4jQSWAjK` | Invalid |
| `GET` | `/widgets/3rDrZRbdeSJJeR4jQSWAjK` | Get a single widget |
| `PATCH` | `/widgets/3rDrZRbdeSJJeR4jQSWAjK` | Update a single widget |
Copy link

@bendemaree bendemaree Mar 27, 2016

Choose a reason for hiding this comment

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

Nit: would you be so kind as to re-align the earlier |s?


### Headers

The most basic header that API services must support, and that clients
should provide, is the `Content-Type` header. In general, the client will
supply this header with the value `application/json` to specify that the
request body is JSON. However, API services must not require this header and
request body is JSON. Multipart requests may use other appropriate values in this header. However, API services must not require this header and
Copy link

@bendemaree bendemaree Mar 27, 2016

Choose a reason for hiding this comment

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

Can we change it to be "Multipart requests, for example, may use [...]"? I don't think this was disallowed as it was before, so since it's a clarification by example it's more clear to phrase it as such, IMO.

must default to this (`application/json`) media type if unspecified.

This creates an implication that the API service must be particularly resilient
Expand Down Expand Up @@ -391,6 +391,7 @@ this:
- For the widget in the list, there are additional `_meta` properties for that
specific resource, as well as some actual data properties
(`color` and `make`).
- The `_meta` section in each resource contains properties generated by the API service. They cannot be created or updated by the client. They are not to be confused with data properties of the same name.
Copy link

@bendemaree bendemaree Mar 27, 2016

Choose a reason for hiding this comment

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

Thanks for adding this. I've been meaning to note something similar. I was going to approach it thinking about that section as computed properties:

"_meta properties can be thought of as "computed" properties about the resource: avoid using these properties for data that the client could not itself compute. They are best used if that computation would be expensive for the client but is cheap for the server."

If you can find a nice way to work this into your statement, that would be ideal I think. Yours also addresses some properties that don't fit this (UUID, timestamps, etc.) that I think are valid as well for the reasons you list (read-only, service-managed properties).

- A status is included. All responses must include status information as a way
to display warnings for a successful request. Further information is available
in the [error handling](#error-handling) section.
Expand All @@ -399,7 +400,7 @@ use whatever casing style is best and transform the output to camelCase.

There are also two very general patterns to notice:

- No values are used as keys.
- No values are used as keys (unless a nested property has an object as the value).
Copy link

@bendemaree bendemaree Mar 27, 2016

Choose a reason for hiding this comment

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

Let's talk about this in person. At this point don't want to add this and I know it's happening in practice. 😉 I think we can use some HAL-style resource embedding to work around this nicely.

- The response is an object despite the fact that it is returning a listing.
The listing is nested within the containing object.

Expand All @@ -411,6 +412,8 @@ and then compressed into base57. A concrete reference for this process is
using the [`shortuuid`](https://github.com/stochastic-technologies/shortuuid)
library.

For a legacy resource, a UUID may be constructed using a unique prefix and the existing non-UUID identifier(s).

A resource's ID must be returned in the `id` property in the `_meta` section
of the response.

Expand Down Expand Up @@ -511,6 +514,7 @@ Some of the most common codes and a possible reason include:
- `201 Created` when a new resource is created
- `204 No Content` when a resource is deleted
- `400 Bad Request` when a required value is missing
- `401 Unauthorized` when a request is made without any authorization
Copy link

@bendemaree bendemaree Mar 27, 2016

Choose a reason for hiding this comment

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

We have 401 and 403 switched semantically, which is clearer now that you have this added. 😀 401 should be un_authenticated_ and 403 should be un_authorized_ despite HTTP saying that 401 is Unauthorized...one justification for having it this way is that 401 is what triggers browsers to prompt for HTTP Basic.

- `403 Forbidden` when not logged in
- `404 Not Found` when a resource is not found
- `500 Internal Server Error` when a server failure occurs
Expand Down Expand Up @@ -572,6 +576,7 @@ following people:
- [brechin](https://github.com/brechin)
- [ctharings](https://github.com/ctharings)
- [daveisadork](https://github.com/daveisadork)
- [davidan42](https://github.com/davidan42)
- [jselby](https://github.com/jselby)
- [kristenwomack](https://github.com/kristenwomack)
- [nwswanson](https://github.com/nwswanson)
Expand Down