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

issues with rel=empty #169

Open
geemus opened this issue Oct 7, 2014 · 40 comments
Open

issues with rel=empty #169

geemus opened this issue Oct 7, 2014 · 40 comments

Comments

@geemus
Copy link
Member

geemus commented Oct 7, 2014

I realized today that we are probably using empty a bit innacurately. We use it to mean 202, when really (arguably) it would better describe 204. In fact, I suspect we might really want another value, something like asynchronous to better call out this kind of case. Thoughts?

@geemus
Copy link
Member Author

geemus commented Oct 7, 2014

Perhaps we should leave the existing one for backwards-compatibility, but add no-content and asynchronous as new statuses that we could begin to recommend/migrate to?

Or perhaps this just illustrates that we are stretching the meaning of rel a bit too much and we should consider using a different attribute/mechanism here?

@brandur @rmcafee thoughts?

@brandur
Copy link
Member

brandur commented Oct 8, 2014

@geemus +1 man. I personally feel like we're pushing rel way too far here; it's really designed to specify the relationship of a particular link to the current entity, rather than specify any particular semantics about the response (i.e. rel=create makes sense, but rel=empty doesn't).

I almost feel like something like no-content should be a first-class attribute on a link, like no-content: true because it's not really coupled to whether the call is asynchronous or what kind of relation to the target it has. A link is equally likely to be a create rel that responds with no content, or a destroy rel that responds with no content.

I think that we could even punt on asynchronous for now because I'm not sure that this is something that consuming clients need to care about too much. Rather, support libraries would have to assume that something like rel=create could have a target status of 200, 201, or 202.

@geemus
Copy link
Member Author

geemus commented Oct 8, 2014

@brandur yeah, rel create as one of those makes sense. The difficulty comes in generating examples, ie what should a curl create example show as the response code? Similarly, what should arbitrary actions show? Maybe we should go back toward just using the defined rels (instance/instances/etc) and lean more heavily on targetSchema? I guess I'm not exactly sure how to define an empty schema either though. Perhaps we should just be explicit in a way akin to excon, ie on different actions we could enumerate expected response codes. 200 is perhaps then just implied, but you can override with other possibilities. I guess if you have a list though you return to the problem of which to choose for usage in examples though.

@rmcafee
Copy link

rmcafee commented Oct 8, 2014

I like @brandur 's idea. "no-content: true" sounds pretty practical.

@geemus beat me to the question of what status code is generated in the examples from the schema. Maybe it's okay to imply a certain status code as long as we can override it in a way that is as practical as using 'no-content'.

@brandur
Copy link
Member

brandur commented Oct 9, 2014

@geemus Ah, I see! +1 on an Excon-style method to specify response status codes for links. I think we should always make sure to keep it optional (and have consuming libraries be generous about what they allow if no status is specified) because it's not really part of any particular spec, but I feel like this would be useful in a number of different situations including this one.

lean more heavily on targetSchema? I guess I'm not exactly sure how to define an empty schema either though.

An empty schema is easy, but it corresponds to a "{}" rather than a "". We may need something like 'no-content' in addition to that.

@geemus
Copy link
Member Author

geemus commented Oct 9, 2014

Maybe empty is simply the absence of a target schema then?

@geemus
Copy link
Member Author

geemus commented Oct 9, 2014

The other option might be "targetSchema": null. But I suspect that is quite, quite invalid.

@brandur
Copy link
Member

brandur commented Oct 14, 2014

The other option might be "targetSchema": null. But I suspect that is
quite, quite invalid.

@geemus Ah, interesting. I'm not sure I could speak to validity one way or
the other. One though I had is that it might imply a JSON response
containing just a null value, but I don't think that's valid JSON anyway.
Our Ruby-written tools would have to start being careful around use of
#[] vs. #key?.

On Thu, Oct 9, 2014 at 12:54 PM, Wesley Beary notifications@github.com
wrote:

The other option might be "targetSchema": null. But I suspect that is
quite, quite invalid.


Reply to this email directly or view it on GitHub
#169 (comment).

@geemus
Copy link
Member Author

geemus commented Oct 20, 2014

Good point. I think we are trying to add too much meaning to things that already have meaning. Separating out status (and probably eventually headers) seems more like what we need. Unfortunately I don't see much precedent for us there, so we may have to trail blaze a bit.

@ernesto-jimenez
Copy link
Contributor

@brandur @geemus I think the right option would be "targetSchema": { "type": "null" }

I sent a Pull Request for schematics today to rely on schema and targetSchema rather than rel: interagent/schematic#27

It actually aligns schematic's library more with documentation generated by prmd.

You can find some details here ernesto-jimenez/schematic@3b605c5

@brandur
Copy link
Member

brandur commented Nov 11, 2014

@ernesto-jimenez Ah, good call! And OMG, +1 million that Schematic pull request! Thanks!

With regards to "targetSchema": { "type": "null" }, I acknowledge that I'm getting into some pretty fine semantic differences that probably don't practically matter, but wouldn't that technically describe null rather than ""?

@ernesto-jimenez
Copy link
Contributor

@brandur relying on "targetSchema": null seems trickier to me since some JSON libraries might require workarounds about targetSchema being null and not undefined.

e.g: In Schematic unmarshals content into structs and the TargetSchema property would be nil when the property is null and when it is undefined.

If we don't rely on "targetSchema": null, using {"type": "null"} seems like the most sensible option among the 7 supported types: array, object, string, integer, number, boolean, null.

@brandur
Copy link
Member

brandur commented Nov 21, 2014

@ernesto-jimenez Ah right. I just meant to say that null and the empty string are two different things.

What about something like {"type":"string","maxLength":0} or {"type":"string","pattern":"^$"}?

@ernesto-jimenez
Copy link
Contributor

@brandur, I would interpret that as replying with an empty string serialized in JSON. e.g:

HTTP/1.1 200 OK

""

Rather than:

HTTP/1.1 200 OK


I understand that "type": "null" really means null, but if we are going to bend the semantics of the schema types, null makes much more sense to me in this case than an empty string.

@brandur
Copy link
Member

brandur commented Nov 22, 2014

@ernesto-jimenez That's what it is though isn't it? The response body is always a string, even if its Content-Length is zero.

@ernesto-jimenez
Copy link
Contributor

@brandur In my opinion, when I define a targetSchema, I'm defining the schema of the returned JSON. In your examples, that's equivalent to `""withContent-Length`` 2.

If the target schema were {"type":"string","maxLength":1} a valid response would be "a" with Content-Length 3.

In my opinion, I would be more surprised by having to define an empty response with a type string + conditions than with a type null. I understand we are twisting semantics in both cases, but I personally find the string+condition more strange.

If we are willing to assume "" is the same as Content-Lenght zero, why not assuming that about null and use a simpler type that requires no conditions?

@ernesto-jimenez
Copy link
Contributor

@brandur btw, I just sent an email to the spec authors in case they want to comment about this and asking what's the status of the spec, since we are obviously discussing a hacky workaround and it would be better to have a nicer solution in the spec.

@brandur
Copy link
Member

brandur commented Nov 25, 2014

@ernesto-jimenez Ah, when I said "" I actually just meant to use that to represent the empty string (i.e. non-null but blank, content-length 0).

Let us know what you find out from the spec authors though!

@geraintluff
Copy link

Hi people. :) Sorry for the slow (and long) response.

I'm afraid I'm missing some context about the kind of situation for which you're returning 206 (which seems to be the issue in question). It might not be relevant, but could someone quickly fill me in?

The JSON (Hyper-)Schema standard as it stands doesn't describe HTTP status codes, HTTP headers, et cetera. It describes documents (and their hypermedia environment) when they are provided, so it doesn't really have anything to say when you don't have a document at all.

In the absence of such status-code or header information, I'd expect clients to be on their toes, ready to handle any reasonable response. As your rather excellent API design manifesto describes, the set of expected successful response codes is pretty limited for a given HTTP verb, so much of the dynamic behaviour will be error-handling. (Although I did notice when reading your design guide that 204 seemed to be missing - I think some advice on when to use it would be a good addition to that document.)

We could (or you could, and push it back - the standard is deliberately extensible) add an extension to the Link Description Objects that listed expected JSON responses. However, describing all the known error states is possibly not appropriate for every interaction - plus clients should at least keep breathing after encountering a 500 response, which isn't exactly something you should be documenting. I guess what I'm saying is I'd expected there to always be some off-piste error handling - you should always be prepared to get 401s as your session suddenly expires, you log out in another browser tab, etc.

Something like {"type":"string","maxLength":0} as @brandur suggested would unfortunately not work. Having a schema implies a JSON value as response - and the empty-string JSON value is serialised as "", not a blank response.

Another angle could be to think about mediaType - after all, what's the Content-Type of a 204 response? "mediaType": "" seems a bit strange (syntax is odd), but something along those lines could indicate that no content was expected at all.

The one that looks best to me, though is @ernesto-jimenez's suggestion {"type": "null"}. This implies you'll return the JSON text null (with a content-ful response code, e.g. 200). However, if your clients are expected to be aware of general HTTP semantics, then they should be able to accept 206 instead without skipping a beat - you're not really subverting their expectations, because HTTP semantics mean they should be ready for a 206 whenever it makes sense, especially when they weren't about to really look at the document anyway.


TL;DR: the best suggestion from my POV is {"type":"null"}. Clients will expect the document null, but should be completely unruffled when you give them a 206 instead, because HTTP semantics say you can.

@ernesto-jimenez
Copy link
Contributor

@geraintluff thanks for your reply!

Here's some context about when this would be important:

Generating libraries

schematic generates a Go library for an API based on the JSON Schema.

Some API's return empty responses in certain endpoints (e.g: when deleting a resource).

However, when generating the code to access those endpoints, we are missing being able to specify in JSON Schema that those endpoints never return a resource.

Without that information, we would always define methods like these in those situations:

// resource would allways be empty, so returning it is just noise
func (s *Service) DeleteResource(id string) resource, error

My work around was to using {"type":"null"} in the JSON Schema to define the methods this way

// no resource returned by the method, since the endpoint doesn't return a resource
func (s *Service) DeleteResource(id string) error

Generating documentation

prmd generates documentation for an API based on the JSON Schema.

The documentation includes examples of API request and responses using curl. However, some API's return empty responses in certain endpoints (e.g: when deleting a resource).

We need a way in JSON schema to define those endpoints would respond with an empty body and render the appropriate curl response.


TL;DR: the issue is when generating code or documentation, since we need to specify in JSON-Schema that link would not return any resource in order to generate the appropriate code/docs.

@geemus
Copy link
Member Author

geemus commented Dec 1, 2014

@geraintluff thanks for weighing in and glad you like the guide! A lot of the questions posed here have to do with those cases which do not match with the guide (for better or worse). ie non-greenfield stuff and/or exceptional cases. We tried to design the guide such that it did not have such exceptions (because it is more useful when broadly applicable). I still think that in many cases the right solution may be to NOT use a 204, especially when it is just for a single resource where others are non-204, but sometimes this is hard.

At the end of the day, the biggest place this has become apparent is in documentation generation, as @ernesto-jimenez mentions. Although clients should, where possible, be resilient in what they accept/allow (made harder when clients are being generated), having a solid/accurate example is pretty valuable and it seems like being able to generate them from the schema should be sufficient (and it usually is). As you allude to, I suspect we are pushing at the limits of what json-schema specifies, so an extension of some sort may better serve our purposes than trying to overload existing things.

@brandur
Copy link
Member

brandur commented Dec 1, 2014

Also for context: I think that many supporting libraries that bring JSON Schema and HTTP together are going to run into similar problems. For example, we're looking at finding a good path for this to get interagent/committee#58 resolved as well.

Cool! The {"type":"null"} still feels a bit strange to me in that we're using a schema to describe a lack of schema, but since I don't think that it could be ambiguous with any other use cases, +1 to this so that we can standardize on a solution. What do you guys think about just making this official?

@ernesto-jimenez Thanks for helping shepherd this through!

@ernesto-jimenez
Copy link
Contributor

nudging @geraintluff about @brandur's comment back in December :)

@geraintluff
Copy link

What I meant with {"type":"null"} is that the client should be prepared to accept a document null in response. I don't think that this should be exactly equivalent to expecting 204 No Content, but I can't imagine a situation where one would be OK and the other not.

My belief is just that clients should be flexible. If a client is expecting 200 OK with result null, then it should be able to cope with 204 No Content (and a client expecting 204 No Content could reasonably throw away any 2XX response it gets).

So yeah, I don't see an issue here.

@michaelsauter
Copy link
Contributor

Sorry if this is a dumb idea - but ... is there a way to override the response status code currently? If not - would it be possible to add this to the schema, and then we could imply an empty response from it?

E.g.

links:
  -
    href: "/foo"  
    http_status: 204

In which case an empty targetSchema is implicit.

@geemus
Copy link
Member Author

geemus commented Sep 15, 2015

There isn't a direct means within json schema to specify anything about status codes (or headers) for that matter. I think it would be really helpful if this were not the case (and json-schema is extensible). I've had a sense that trying to define ways to specify this stuff more formally would be really valuable, but I keep not-quite managing to get it done. By leaning on a small set of codes as best-practice we are able to mostly avoid the issue, but obviously there end up being some outliers (such as this).

@waterlink
Copy link
Contributor

This problem hit me. I was really expecting rel: empty result in 204 No content. But it is not. Taking into account everything that is said before, I can propose the following solution:

rel: empty         # 202 Accepted (backwards compatibility, Maybe(deprecated))
rel: no-content    # 204 No content
rel: asynchronous  # 202 Accepted (encouraged to use)

I am up for creating a PR for that.

WDYT @geemus ?

@michaelsauter
Copy link
Contributor

@waterlink I like the proposal, would be great to have control like this over the status code.

@geemus
Copy link
Member Author

geemus commented Dec 21, 2015

Sounds reasonable. That said, I wonder if we should stop burying this inside rel (ie maybe we should just add some attribute that lists the expected status code or codes). What do you think?

@waterlink
Copy link
Contributor

status:
- 201
- 404

Something along these lines?

@waterlink
Copy link
Contributor

That is almost the same thing, that @michaelsauter proposed in his comment: #169 (comment)

@geemus
Copy link
Member Author

geemus commented Dec 21, 2015

Yeah, probably. My concern just becomes, how do you know which value makes sense to use in examples? Maybe just by convention (the first one) or maybe some other attribute to make it explicit. What do you think?

@waterlink
Copy link
Contributor

We can live it as simple as possible and just pick the first one.

Other way would be trying to guess which one is successful, e.g.:

  • first find 2xx and pick first,
  • otherwise find 3xx and pick first,
  • otherwise pick first from the whole list.

Other way would be to be explicit about it. If you provide more than one value, then you have to specify which would be used for an example:

http_status: 200
# Ok, example will show `200 OK`
http_status:
- 200
- 201
- 404
# Schema validation will fail, requiring you to provide `default_http_status`
http_status:
- 200
- 201
- 404
default_http_status: 201
# Ok, example will show `201 Created`

Or, make http_status more comprehensive object in last case:

http_status:
  items:
  - 200
  - 201
  - 404
  default: 201
# Ok, example will show `201 Created`

@geemus WDYT? Which would you prefer?

@michaelsauter
Copy link
Contributor

I think convention (first one) is good enough. All other suggestions are more verbose/complicated, and do not offer more flexibility ...

@geemus
Copy link
Member Author

geemus commented Dec 22, 2015

Yeah, I think an array, assuming the first one is default, would be good enough. Thanks for discussing!

@agonzalezro
Copy link

Hi!

We are facing a problem that seems kinda related with this one. We wanted to return a 406 status code and and response_example associated. The problem is that to return a 406 we need to set our own head here:

<%= response_example['head'] %>

But if we do that we always end up in this line:

<%= response_example['body'] %>
and it will not do any kind of JSON.pretty_generate(.

Are we approaching this the proper way? Or is there a better way to do it? Sorry if it doesn't exactly belongs here, but it looked like.

@ernesto-jimenez
Copy link
Contributor

@agonzalezro unfortunately, JSON Hyper Schema only pertains to the JSON representation of REST sources and the links where to fetch or act on those resources.

It's missing any information regarding the HTTP transport, such as Authentication or HTTP Status codes.

prmd uses the rel attribute in links to imply certain transport conventions, but that's arbitrary rather than specified by the JSON Hyper Schema spec.

@vcastellm
Copy link

thanks for the response @ernesto-jimenez, then is this #169 (comment) something that it's planned to be implemented in prmd?

@ernesto-jimenez
Copy link
Contributor

@Victorcoder, I don't maintain prmd, so they should be the ones to answer that question.

What I know is that the JSON Hyper-Schema spec doesn't cover that and has not been updated since 2013. Even if it's implemented, other JSON Hyper Schema tools will not support it.

Depending on what you want to accomplish: generate docs for an existing API, generate docs and clients, define a new API...

It might be enough to contribute/wait for some support from prmd to extend the support. It might not :)

If you are working on designing a new API, you can also take a look at JSON-API, which does specify the transport. It's missing a way to define resources' and API schemas, but it can be extended, so you could leverage JSON Schema.

Also, on Jan 1st Swagger contributed their Swagger 2.0 spec to the Open API Initiative as a starting point to standardise a spec under the umbrella of the Linux Foundation. I'm unaware whether they have a commitment to make the OAS (Open API Specification) backwards compatible.

Finally, you can also check what Google did with their API Discovery Service to see how they defines their APIs with custom JSON format which leverages JSON Schema. They use id to generate docs and clients.

@geemus
Copy link
Member Author

geemus commented Feb 23, 2016

Agreed, json-schema doesn't fully cover things. There are definitely some other formats that have varying levels of coverage/support as @ernesto-jimenez suggests. It's a pretty rapidly evolving/improving ecosystem, so definitely worth checking out whats going on and choosing the one that is the best fit. The changes to support status/headers we proposed in those comments would be our own extensions to json-schema, so it would just work in prmd (that may or may not be sufficient for you, depending on what you are up to).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants