-
-
Notifications
You must be signed in to change notification settings - Fork 315
Split collections example (and rename items to elements) #464
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's better now with pagination things apart.
There are still confusing things, which were already there from #448, so we can either decide to go and merge this one and work on the points I raised in this review later or we can handle them now.
<preamble> | ||
Here are all of the links that apply to this instance, | ||
including those that are referenced by using the "thing" | ||
schema for the individual items. The "self" links for |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that the example is simpler, I realize that the "thing" schema is not presented in the example. This makes one wonder where the rel="self"
links with a contextPointer
pointing to array items come from. An omission?
EDIT: I finally realized that it appears to be defined several examples above (in section 9.2, and we're in 9.5). If it's really meant as is, I guess that at least a reference is needed.
is a good example of why it is best to only construct links upon request. | ||
There is no need for having as many functionally identical "collection" | ||
links as there are items in a collection page on every collection | ||
representation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, but I don't quite understand this paragraph :)
(Arguably it was already there, but I did not review the first version of this section, so commenting here.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which part? It makes several points:
- That we need to add the following link to our "thing" schema
- That such a link would appear for each item in the collection representation
- That such an abundance of very unlikely to be used links is unwise to construct automatically for both computational and memory reasons
- That having all of those links wouldn't be very useful even if you did construct them
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which part? It makes several points:
I think the last two items make good points but could be better expressed in the paragraph. In other words, it takes a bit of extrapolation from the text to guess "this". (It's just a wording issue.)
"href": "/things", | ||
"targetSchema": {"$ref": "thing-collection#"}, | ||
"submissionSchema": {"$ref": "#"} | ||
}]]> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it'd be clearer if the "thing" schema would be presented completely in the beginning along with (or maybe inlined in) the "thing-collection" schema.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm trying to introduce small steps at a time as repeating the whole hyper-schema every time would make the spec gigantic. And I think the individual points are best done in separate sections and not with a single giant example at the top and then a lot of discussion.
I'm really not sure what to do here. You and I have very different opinions on how examples should work. In the absence of other clear feedback I'd like to try things my way and see what we get from broader review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BTW that is not intended as criticism of your view of examples, just an observation that we approach it very differently and I do not know how to handle it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, here it's not really about different point of views about examples presentation. Rather, it's about the "thing" schema being presented 3 sections above this one without any reference (see my comment above). I don't think we can assume that the reader would remember previous examples or think about looking around (as I had to do myself as a reviewer).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do agree that there need to be better references.
individual "thing" schema as there is no individual "thing" to fetch. | ||
And the "tag:rel.example.com,2017:thing" link in the entry point | ||
resource does not indicate how to create a "thing". The "self" link | ||
requires the "id" to already exist, but it is assigned by the server. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Which "self" link are you talking about? I suspect this is the one in "thing" schema (which has a requirement on "id"), but it's said above that we cannot use it. On the other hand, if it's the one in "thing-collection" schema's, unless I'm missing something, this one does not a requirement on "id".
In fact, I'd use the "thing-collection" schema's "self" link as a natural place to put the submissionSchema
to create new collection item.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #421 for more information about why "self" is probably inappropriate for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #421 for more information about why "self" is probably inappropriate for this.
I see no reference to submissionSchema
in this issue so this looks unrelated to me. This also applies the the comment below on the cref.
Maybe I missed something, would you mind elaborating a bit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's actually in one of the crefs in this text that I had pasted into the issue (I'd forgotten that).
Note also that POST-ing to a collection page that will not contain
the created item also seems weird. While retrieving the collection
from a query parameter-less URI will still retrieve a page, that's
a limit imposed by the server. POST-ing to such a URI and disregarding
the "default" values for the parameters seems correct. This is another
reason to not automatically write default values into the query.
So if "self" is something like things?offset=10&limit=10
and you POST to that to create a new "thing", if that new "thing" does not end up on that page (from the 10th to 20th "thing") then you've done something kind of strange. You created a resource via a collection that does not actually contain the resource (there is no difference between query string parameters and path components- a change in either means a different URI and therefore a different resource).
has a "collection" link and is therefore an item, and notice that | ||
its "collection" link produces the same(-ish?) URI, but that seems | ||
overly complicated. And gets more so once pagination and filtering | ||
are introduced. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is complicated. This gets back to my suggestion of using the "self" link on the "thing-collection" schema to hold submissionSchema
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, as noted in #421 the point of this paragraph is to highlight that none of the options work very well. It's not remotely clear to me what would. The "self" link does not work, in my opinion, as noted in #421.
Since you suggested that merging this would be OK, I'm going to resolve the conflicts and merge. The remaining discussion points seem to be:
|
In the collection example, it's (probably) more confusing to have the actual collection array named the same as a JSON Schema keyword than it is to have them be asymmetric.
Do a basic collection/item example, and then add pagination as yet another example.
After merging this, I would like to discuss the "self" link thing in #421 as this is part of that problem (I will probably need to explain that connection a bit better, but we'll do that in the issue) For the example arrangement, it would be great if you could start a new issue and maybe give an overall proposal of where you think examples need to be, or need some sort of reference, or need to include more of previous examples? |
See #478. |
This pulls the pagination stuff out into a follow-up example. There is also a commit that just changes "items" to "elements" if we decide we don't like the split.
This is in response to feedback from @dlax and @philsturgeon on the existing example.