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.

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

Add an entity.id #33

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

Add an entity.id #33

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
71f65d7
[fix #11] Add semantics for specifying client-side validation
Apr 9, 2013
de9f368
fix anchor link?
Apr 9, 2013
75284ef
Merge remote-tracking branch 'upstream/master'
Feb 18, 2014
5a6081a
add an optional `entity.id` attribute
Feb 25, 2014
fc571f2
add an id to the examples
Feb 25, 2014
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
7 changes: 6 additions & 1 deletion README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,14 @@ The media type for JSON Siren is `application/vnd.siren+json`.
"status": "pending"
},
"entities": [
{
{
"id": "items",
"class": [ "items", "collection" ],
"rel": [ "http://x.io/rels/order-items" ],
"href": "http://api.x.io/orders/42/items"
},
{
"id": "customerInfo",
"class": [ "info", "customer" ],
"rel": [ "http://x.io/rels/customer" ],
"properties": {
Expand Down Expand Up @@ -113,6 +115,9 @@ Sub-entities can be expressed as either an embedded link or an embedded represen

A sub-entity that's an embedded link may contain the following:

#####`id`
Identifies the sub-entity relative to the current parent entity. If an `id` is provided it MUST be unique to the parent entity. Optional.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unique to the parent entity, or unique to the returned document? The latter would allow for a more HTML-like behavior. The former sounds more like an appropriate use case for class or rel.

Copy link
Author

Choose a reason for hiding this comment

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

The intent is to have it be unique to the parent entity which would provide enough for a local reference to the client.

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand the intent, it just seems to me that you're trying to jam class or rel functionality into id. If you have many things in a document with the same identifier for selection, it seems to me that's a class of potentially many things. Not the unique identifier of one thing. Just because your API serves up only one avatar for each user, that doesn't mean that avatar uniquely identifies one thing. It just means that you happen to have something with an avatar property. Now, if you're selecting "id": "avatar26531-34", that's a good use of an id property. But then it should be perfectly fine to let the scope of the id encompass the entire document, which would also make selecting something particular from the document more convenient for the client.

Am I making any sense?

Copy link
Author

Choose a reason for hiding this comment

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

I get where you're coming from but as pointed out in pull #31, class and rel don't satisfy my use-case as filtering by them returns a set. I also understand that I could filter that set down to get the first/last/3rd or whatever but that also does not address my use case as that does not guarantee what I will get back. I agree with you that both of these "features" are useful parts of a Siren client.

Copy link
Contributor

Choose a reason for hiding this comment

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

My point is that it sounds like you have a special use case which doesn't apply to apis in general. I think it's more important to consider the wider applicability of id. I am really unconvinced that class would not satisfy your use case.

Copy link
Owner

Choose a reason for hiding this comment

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

Man, I really want this PR merged. I think we do need to change the language to say the id value should be unique within the scope of the top-level entity and all its children (as @dilvie suggested).

I want URI-addressable fragments. http://api.example.com/orders/42#customerInfo and such.

Can we compromise on this?

Copy link
Author

Choose a reason for hiding this comment

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

I'm sorry, I'm not following and I don't want to be a blocker.

We're probably talking about two very different and orthogonal use cases. Perhaps your proposal should be a separate pull request and this one can be closed out since what I'm talking about is probably better described as a "name" instead of an "id"?

Backbone.Siren relies on sub-enitties having a name and with a couple of applications using Siren + Backbone.Siren in production I must be missing something because I don't see how you can have a client without entities having a name. It could be that id's make sense from a server side implementation, whereas I'm looking at this more from a consumer point of view? I dunno :P, @kevinswiber maybe we can chat offline at some point and you can help me out. Eventually, I'll either see the light and realize that names aren't useful or I'll figure out how to better communicate their effectiveness at which point I can open a new pull request.

Copy link

Choose a reason for hiding this comment

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

@kevinswiber You mean an api request with a fragment? Does that mean the response is just the customerInfo sub-entity?

On the client side I believe I'm with @dilvie that class works well enough but a concrete example would help me tremendously. In the mean time, if you have the id as a property of the entity couldn't you select all sub-entities and then filter on the id property?

I've never been a fan of the '#ID' style of selection. I prefer to select items generically and filter on known properties. Sort of like jquery style where all selection results are an array regardless if there are 0 or more results.

Copy link
Owner

Choose a reason for hiding this comment

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

@apsoto @uglymunky

Dealing with the fragment is a client-side concern; it doesn't even make it back to the server in the HTTP request. What it would allow is linking to a sub-entity within the context of its parent entity.

Take the following URL, as an example: http://stackoverflow.com/questions/1647631/c-state-machine-design/1647679#1647679

The server has no idea that fragment was even included in the link. However, when a response comes back to the client, the client searches for that ID and highlights the StackOverflow answer in context.

This could be helpful for UI-driven apps on top of hypermedia, though I imagine there could be an M2M use case, as well.

This is why it needs to be unique within the entirety of the response--you only get one fragment.

Copy link
Author

Choose a reason for hiding this comment

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

I'm very much with you on the ability for a client to reference directly to a sub-entity but having a hard time wrapping my head around the implementation of a top level unique id.

Things I like about sub-entities are: the decoupling between entities, their re-usability, and the affordance to infinitely nest. It seems a unique id, relative to the root level document would make that all very tricky to implement. What is the closest parallel to sub-entities in HTML?


#####`class`

Describes the nature of an entity's content based on the current representation. Possible values are implementation-dependent and should be documented. MUST be an array of strings. Optional.
Expand Down