spring-projects / spring-hateoas

Spring HATEOAS - Library to support implementing representations for hyper-text driven REST web services.
https://spring.io/projects/spring-hateoas
Apache License 2.0
1.03k stars 476 forks source link

HAL: the rel for an embedded collection should be singular (the item-resource-rel) #339

Open meyertee opened 9 years ago

meyertee commented 9 years ago

I think the implementation of the HalEmbeddedBuilder is outdated/wrong in selecting a plural collection link-rel when embedding multiple resources. In my understanding of the spec, linking to items in a collection resource (e.g. "/items") should be regarded as multiple links with the rel "item" (like multiple <a href="/items/1" rel="item">), serialized into an array. The rel should remain "item".

The collection-link-rel "items" should only be used if actually linking to the collection resource (i.e. "/items").

Otherwise it becomes impossible to distinguish between the link to a collection resource and a link to a single resource.

For example:

/items

{
  "_links": {
    "self": {href: "/items"}
  },
  "_embedded": {
    "item": [
      {
        "_links": {
          "self": {href: "/items/1"}
        }
      }
    ]
  }
}

/items/1

{
  "_links": {
    "self": {href: "/items/1"}
    "items": {href: "/items"}
  },
  "id": "1"
}

The targets for "items" and "item" are distinctly different, so they should have distinct rels.

The HAL spec itself is confusing in that regard - the published spec still holds an example using the orders-rel, but it has been corrected on Mike Kelly's website where it now shows the ea:order-rel. It has also been discussed on the mailing list.

odrotbohm commented 9 years ago

The key point here is that the HAL spec doesn't say anything about the rels to be used in _embedded. I also don't agree that the pluralization of the _embedded rel should be derived from what the contained value's self link points to. In your example _embedded contains a preview of a collection resource, so I'd argue using a collection rel is suitable here.

That said, HalEmbeddedBuilder by default does not prefer collection rels at all. By default it uses the item rel if a single item is added for that rel and expands to collection rels if further items with the very same rel is added (auto expansion to a collection). If you start with a collection value (even if it only contains a single item), we use the collection rel. HalEmbeddedBuilder can be forced to always use collection rels by default using the flag in the constructor.

You can even gain more control about the rel used by wrapping the objects handed to the HalEmbeddedWrapper in an EmbeddedWrapper which in turn can be obtained from an EmbeddedWrappers instance.

meyertee commented 9 years ago

I agree that the spec doesn't say anything about rels; that's actually why I think the automatism in the HalEmbeddedBuilder is doing too much - it shouldn't decide for me when to use a collection rel, and when to use an item rel based on the amount of items.

In your example _embedded contains a preview of a collection resource, so I'd argue using a collection rel is suitable here.

That's where I see things differently - even a preview of a collection resource would be a resource, i.e. something with a self link. In my understanding _links & _embedded should be interchangeable.

Maybe it's clearer with another example - let's say a thing has items.
I can choose to link to the collection resource:

{
  "_links": {
    "self": {href: "/thing/123"}
    "items": {href: "/thing/123/items"}
  }
}

Or I can choose to link to the individual items (which is the equivalent of the embedded collection pattern):

{
  "_links": {
    "self": {href: "/thing/123"}
    "item": [
        {href: "/thing/123/items/1"}
        {href: "/thing/123/items/2"}
        {href: "/thing/123/items/3"}
    ]
  }
}

I could even do both; the point being they're two different things.

Don't get me wrong, I'm not trying to tell anybody when to use plural or singular, I'm just trying to illustrate that the automatism in the HalEmbeddedBuilder uses the collection-rel even when I think item-rel would be correct. You can see the item-rel being used for embedded collections in most of the recent examples on hal-discuss, there seems to consent on that, but it's no rule.

With your help I found the spot where I can customize the rel, i.e. use the EmbeddedWrapper and set an explicit rel on it, thanks! I still think it would be better if it were changed as it's hard to find the workaround and as per above seems incorrect to me.

vivin commented 9 years ago

As far as I understand, the rel defines the semantics of the link (or the resource at that link). If that resource is embedded, then the same rel can be used to refer to that resource as well.

It doesn't seem like HAL mandates what rels should look like based on cardinality. It only says that links and embedded resources use Link Relations as defined in RFC 5988.

The examples in the official draft seem to use the plural for a list of items. Personally, I agree with you that it should probably be singular, since the rel is saying that each individual link/embedded-resource has the same semantics. I use a plural for the rel is if the entire embedded resource is itself some kind of "collection" resource, or if the link refers to a resource that represents a "collection" resource.

I think the thing that matters is that the builder should probably autogenerate a rel with consistent semantics: you shouldn't end up with a situation where you have the same rel describing an embedded list of individual resources and an embedded "collection" resource; the same applies to links as well.

My solution has been to specify my own rels (like you have done) - they are curie'd as well and point to documentation.

drdamour commented 1 year ago

it def should be item. each think in the collection is an item, not an items. hal just collects the things with the same rel as an array, thats in the spec. it should only be items if the resource was a collection of items. and yes the rel should match between links and embedded.

fwiw items is not in the IANA link registry, unlike item which is