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

Add an entity.id #33

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

Conversation

gxxcastillo
Copy link

Following up on pull request, #31

@@ -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?

@ericelliott
Copy link
Contributor

I reiterate: 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.

@gxxcastillo
Copy link
Author

That is a valid point. Originally, I had intended for it to be entity.name as apposed to entity.id so the intent wasn't necessarily to serve the same function as an html id (although I agree that its good to borrow/learn from html when possible), but rather to serve as a property name for the entity (much like the way properties inherently have names due to the fact that properties is an object). This is lost, however, with entities since entities is an array.

At this time, I can not think of a good way of drawing a parallel between my proposal and an html id attribute.

@ericelliott
Copy link
Contributor

Yeah, we're chasing our tails here. What you call it doesn't change the fact that you're still selecting something from entities using a property name. Call it class, call it rel, call it id... none of that changes that basic fact.

As I mentioned in our earlier thread, you're trying to impose application-specific semantics on the media type. Do we need a special type of entity called avatar that must be unique? Or does your application need a specific type of entity that must be unique?

I still think you're after something you don't really need, but I think the closest analogy in HTML to what you want would be select by tagname. The problem is, in HTML, those tag names are predefined (except that in HTML5, you can create custom elements that descend from existing elements). What would a set of pre-defined element names be? What should we expect for properties? Seems pretty arbitrary unless you go either really general like HTML (everything is a div these days), or really specific like microformats (so many people have so many opinions of what should and shouldn't be in a microformat).

I think what we really might need that would satisfy your craving is something like HTML5's custom elements. Custom entities with names and rules that you can define. Maybe that could be done by allowing users to specify rules for classes:

"classes": {
   "avatar": {
    "unique": true,
    "scope": "parent" | "document"
   }
}

Or, instead of class, maybe this could be a role attribute? Or expand the type attribute so that it can be applied to entities?

Whatever the case, I think your idea of what id or name should mean is confusing and arbitrary unless you also include the story of how you wanted to allow only one avatar per parent entity.

@ericelliott
Copy link
Contributor

That said, I still think that id is a good idea, and we should definitely include it.

@ericelliott
Copy link
Contributor

"classes": {
   "avatar": {
    "unique": true,
    "scope": "parent" | "document"
   }
}

Even this strikes me as a little silly. If something must be unique, enforce that on the server and send only one. Is it the client's job to verify that your server is working properly, or is that the responsibility of your test suite? IMO, sending data like this just complicates the payload unnecessarily.

I strongly believe what you really need to do is let it be an array, iterate through that array, render everything it contains (which will be a single entity if your server is working correctly), and call it a day.

Ideally, hypermedia works best when the client knows as little as possible about the rules of your application. Let the server do that work for you. Overly smart clients = broken hypermedia.

@apsoto
Copy link

apsoto commented Feb 25, 2014

Throwing in my 2 cents and repeating what I've mentioned in other threads. Selecting by class and rel seem sufficient so far in my experience. In my opinion selecting by class AND rel should provide the uniqueness necessary (class=image, rel=profile-image), it's either an array of one, or not.

I'll grant my experience is limited to one API. I haven't had the use cases @uglymunky is experiencing, or the breadth of implementations that @kevinswiber probably has.

@ericelliott
Copy link
Contributor

Imagine this: you have a mobile client that renders a thumbnail avatar. Ship it to prod.

Two weeks later, the product manager is like, "oh, this image should be 2x larger in this other view state." So you have to suddenly have 2 avatar images, one that's a thumbnail, and one that's more like a profile view. But your client treats avatar like an id and there can be only one. Now you have to change the structure of the JSON payload, and the CSS, and the JavaScript.

Vs, you started out rendering the whole array (of one image). Now your json payload sends two images with "rel": "avatar". You add some css to handle the new profile avatar. Done.

This is kinda the whole point of hypermedia.

@gxxcastillo
Copy link
Author

While our team hasn't set our architecture in stone on this wrt representing this in Siren (I'd love to hear others thoughts on this so long as we can relate it back to the current thread). We currently would have one entity, "avatar". So it would be user.avatar and there is a standard template for requesting image sizes that looks something like:

// Assuming user.avatar.id == 3456 and user.avatar.format == 'jpg'
site.org/img/w[image_width]h[image_height]/3456.jpg // We have certain predefined sizes that our servers pre-render

I think last time our team visited this problem we had decided that we should use the links array to present the different options but I don't think we ever actually implemented that. So our client currently cheats and builds the urls itself.

Just to re-iterate, we have not had any use cases come up yet where an entity would have more than one sub-entity of the same "identifier". Usually they would either get different identifiers (e.g., user.avatar, user.coverImage, user.verificationImage) and share a class (e.g., 'image'). In the case where there are multiple "avatars"; not like in your situation but in the situation where a user can literally have 5 avatars at a time, then avatars would be a collection: user.avatars and each might have properties identifying it as default or favorite or whatever.

Collections (e.g., users.avatars) are the only use case I've come across where it hasn't made any sense for us to have a unique identifier since we would use an index value instead. (I don't have any experience working on any other API so please take with a grain of salt )

For clarification, this is how we represent collections (I think I read this in a google groups thread):

// user entity:
{
    properties: {
        id: 123
        , age: 99
        , email: '[email protected]'
    }
    , entities: [
        {
            name: 'avatars'   // we'll be migrating to calling these "id" per this pull-request 
            , class: ['collection']
            , entities: [
                {
                    properties: {
                        id: 2345
                        , isDefault: false
                        , format: 'jpg'
                    }
                }
                , {
                    properties: {
                        id: 3345
                        , isDefault: true
                        , format: 'jpg'
                    }
                }
                , {
                    properties: {
                        id: 4477
                        , isDefault: false
                        , format: 'jpg'
                    }
                }
            ]
        }
    ]
} 

@gxxcastillo
Copy link
Author

wrt my previous comment about using the links array to represent the various possible image sizes, might be a good use-case for having "templates" in Siren, but I'll leave that for another day in another thread.

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

Successfully merging this pull request may close these issues.

4 participants