lostisland / go-sawyer

MIT License
34 stars 7 forks source link

Response hypermedia for single objects and arrays #15

Open technoweenie opened 10 years ago

technoweenie commented 10 years ago

Sawyer assumes API requests return a single object. It then merges that object's hypermedia relations (HAL or similar) with the response's hypermedia (pulled from Link headers). This presents a problem for API responses that return an array of items:

[
  {"id": 1, "name": "bob", "url": "/users/1"},
  {"id": 1, "name": "fred", "url": "/users/2"}
]

How should we handle that? My proposal is that we don't try to merge an object's hypermedia into the response's hypermedia if the resource is an Array. If you need to access the relations of one of the objects in the array, call Rels() on that individual object.

owenthereal commented 10 years ago

How about using reflection to fill relations for each object? I've done some initial works here. The changes are part of https://github.com/lostisland/go-sawyer/pull/14 (undone).

What I don't quite follow is why we're caching the relations to the response. Because for the array case, the data structure Relations (map[string]Hyperlink) is not sufficient. Shouldn't caching on the object be enough?

technoweenie commented 10 years ago

What I don't quite follow is why we're caching the relations to the response. Because for the array case, the data structure Relations (map[string]Hyperlink) is not sufficient. Shouldn't caching on the object be enough?

For a singular resource, caching on the response is sufficient. Accessing "/users/1" will store all of the user's relations on the response. When we access the relations of the "/users/1" URL, it includes any link headers and the user's relations all together.

user, res := req.Get(&User{})
res.Rels.Rel("repository", ...)

You're right though, this doesn't work for collections. That's why I'm proposing that we don't even bother trying to parse the collection. If we need to access the relations of an object, then we do it explicitly.

users, res := req.Get(&[]User{})
res.Rels.Rel("next", ...) // links to page 2 of the collection

for _, u := range users {
  hypermedia.Rel(u, "repository", ...)
}

Maybe this is too confusing though? It might be nice to have a single way to access the relations of any object.

users, res := req.Get(&[]User{})
hypermedia.Rel(res, "next", ...)
hypermedia.Rel(users[0], "repository", ...)

The relations cache could also be completely separate from the response cache. If you get a collection of users, it could save the relations cache for each user. I like that idea. Though, I don't know what the cache interface for that would look like.

users, res := req.Get(&[]User{}) // fills relations cache for every returned user
hypermedia.LookupRel("/users/1", "repository", ...) // accesses relations cache
hypermedia.Rel(users[0], "repository", ...) // accesses relations on struct itself
owenthereal commented 10 years ago

What do you think of this:

users := []User{}
res := req.Get(&users)

for _, u := range users {
  u.Rel("repository", ...)
}

We do a bit more work in req.Get to fill the relations for each user if it's a slice. Most of the code of HyperFieldRelations can be reused.

technoweenie commented 10 years ago

u.Rel("repository", ...)

Don't we have to jump through hoops to get that to work? Go's embedded structs don't allow the kind of inheritance we're used to in Ruby. Functions like hypermedia.Rel() seem more like regular go style.

owenthereal commented 10 years ago

Don't we have to jump through hoops to get that to work?

Yes, you're right that things aren't easy comparing to Ruby. But it's possible. We already embed a hypermedia struct to each object. In req.Get, we could create the relations, fill them with reflection and inject them back to the object. The interface is more compact and reflects more on what we're modelling: a hypermedia resource has many relations that lead to other hypermedia resource.

But hypermedia.Rel() definitely performs a bit better since it doesn't have the injection part. Could you elaborate more why hypermedia.Rel() is more a regular go style? Are you saying it's like a service method?

As a side note though, do we need to support method call like user.Rel("follwing_url")? Because we could already get a relation "statically" from a struct field of a resource, e.g., user.FollowingURL. Except for the support of the hal convention, using this method seems unnecessary. Maybe we don't need to unify them.

technoweenie commented 10 years ago

Could you elaborate more why hypermedia.Rel() is more a regular go style? Are you saying like a service method?

It just feels like we're fighting against Go. It's like the difference between foo.split("/") and strings.Split(foo, "/"). Even something as simple as injecting relations back into the resource would require us to introduce a core embedded struct to hold that variable. I think my own examples above are incomplete though.

rels := hypermedia.LookupRels("/users/1")
rels.Rel("repository", ...)
rels.Rel("keys", ...)

As a side note though, do we need to support method call like xxx.Rel("follwing_url")? Because we could already get it "statically" using the method from a resource, e.g., user.FollowingURL. Except for the support of the hal convention, using this method seems unnecessary.

We don't need to. However, we are thinking of moving to HAL in a future API media type change. Supporting Rel() means clients won't have to change code when that happens. It doesn't matter if an object's relations are defined in an *_url property or HAL.

owenthereal commented 10 years ago

It's like the difference between foo.split("/") and strings.Split(foo, "/").

Ah, got ya!

However, we are thinking of moving to HAL in a future API media type change.

Totally forgot you mentioned it. Thanks for putting me back on track with the road map. It's a benefit to work with someone who knows what'll happen under the cover :smile_cat:.

Let's try out hypermedia.Rel() then!

One more question, for line rels := hypermedia.LookupRels("/users/1"), would it be a good idea to look up rels for a resource directly? Say:

users := []User{}
req.Get(&users)

rels := hypermedia.LookupRels(users[0])
rels.Rel("repository", ...)
rels.Rel("keys", ...)
technoweenie commented 10 years ago

I'm thinking rels := hypermedia.LookupRels("/users/1") would be specifically for looking up the relations in a cache first. If you have the object handy, you should definitely use that instead.

owenthereal commented 10 years ago

:cool: I'm more clear now. Thanks for the explanation.