talios / halbuilder

Java based builder for the Hal specification http://stateless.co/hal_specification.html
19 stars 4 forks source link

Problem with links of embedded resources #24

Closed jcassee closed 12 years ago

jcassee commented 12 years ago

An embedded resource has a 'rel' attribute that describes the relation between the containing resources and the embedded resource. In halbuilder, such a 'rel' attribute is repeated in the links of the embedded resource itself. I think this is an error.

Do you agree? If so, I will propose a fix (basically, changing List<Resource> resources to Map<String, List<Resource>> resources in BaseResource).

jcassee commented 12 years ago

Example:

{
  "email": "test@example.com",
  ...
  "_links": {
    "self": {
      "href": "http://example.com/user/1"
    },
  },
  "_embedded": {
    "http://rels.example.com/token": {
      "expires": 1335541161111,
      "token": "QERwo9TcNyPvO9vsD1QLQF5dog5Rh4UmGaOYYaaGz56pvDe7CKsAgU2cHNw97WAA",
      "_links": {
        "http://rels.example.com/token self": {
          "href": "http://example.com/user/1/token"
        }
      }
    }
  }
}

Notice how the relation http://rels.example.com/token is also a relation of the embedded token resource.

talios commented 12 years ago

I've been meaning to follow up on this the last few days but always seemed to get sidetracked. From my understanding of the HAL spec, and various discussions on the hal mailing list this isn't actually an error. It's probably more "accurate" and "verbose" if anything.

The problem, as I see it also only really affects the JSON representation, which includes the embedded resources in the _embedded object, and that the resources href is now rendered as a standard link ( it used to be on the _href attribute ).

When adding an embedded resource to the outer with a certain rel, we're actually adding an additional link to the resource with that rel - so that it now has self and token, which gets collapsed into 'self token'.

If the embedded resource already had multiple links/rels against its resource when being added, for example self edit then the embedded resource would have self edit token.

The HalBuilder implementation actually originally tracked the resources is a Map<String,Resource> but after some experimentation that didn't really sit well with me, mostly as the grouping into rel based map is ONLY relevant for the JSON representation - the XML just includes all embedded resources as embedded elements ( this is also what led to the distinction of links/canonical links (aka, collapsed to space separated rels, and individual rels).

It's interesting, I've seen a few JS implementations of HAL that treat it as a straight object model, doing things like resource._embedded.token to look up the embedded token resource, but that seems broken to me in that the rel attribute is defined as a space separated list of reltypes, so the assumption that its a unique key is flawed.

jcassee commented 12 years ago

Let's see if I understand you correctly. You are saying that if a resource is embedded, and the relation between the subject resource and the inner resource is X, then the embedded resource has a relation X with itself too? Because as the blogpost accompanying the HAL spec says:

The key aspect of these embedded resources is that they reset the context of any resource state and links which they contain. i.e. a link inside an embedded resource implicitly relates to that embedded resource and not the parent.

You mention that the embedded resource's href is not (no longer?) found at resource._embedded.X._links.self, but that seems to contradict both the example and the wording of the spec:

Resource objects MUST have a self link (_link.self) which indicates the URI of the embedded resource

Am I perhaps looking at an older version here? http://stateless.co/hal_specification.html

talios commented 12 years ago

I've been thinking back and forth on this the past few days and it's a continuation of a thought (and long thread) back and forth on the HAL list and on #rest on irc.freenode.org. The reference I made to href no longer being around was that the original HAL spec had _href as a first-class special member of a resource, and not implicitly serialized as a link.

You're correct when you say that the links inside the resource relate to the embedded resource and not the outer - for NORMAL links. The link that is that resource however I think is somewhat 'special' as its not an outward link, its describing the resource itself, which when embedded - does have additional reltypes associated with it.

I'm wondering if part of the problem is also that all of the Javascript based APIs/examples I've seen all seem to want to use an object path such as _link.self to find the link, just as you've mentioned. In my view, this is actually a fundamentally broken approach as a rel is defined as "a space separated list of reltypes" which means that "self foo" is quite valid.

From the looks of it, it's only really the JSON representation that is really faulting here as it uses a map/object keys by rel to contain the resources as well as the _link array, whereas the XML just has inline <resource rel=foo/> elements. I wonder if just adding a LinkType enum such as LinkType.(LINK|RESOURCE) to the Link would make the implementation cleaner - when adding a resource we add a LinkType.RESOURCE link for the rel, and it's only the JSON representation that needs to filter out the resource links.

Thoughts?

jcassee commented 12 years ago

Thanks for a very interesting discussion here. This comment is a bit long, I hope you will bear with me. First, let's get this out of the way:

[...] a rel is defined as "a space separated list of reltypes" which means that "self foo" is quite valid.

Yes, you are right, my example was wrong. Do you use something like JSONPath or jsonselect to select links?


This is perhaps the central point of what you wrote:

The link that is that resource however I think is somewhat 'special' as its not an outward link, its describing the resource itself, which when embedded - does have additional reltypes associated with it.

I do not know anything about the history of HAL, but this is not how I read the specs. I cannot reconcile it with the example in it. (Note how the order relation is not repeated in the embedded resource.)

In other words, the _embedded object key already contains the relation(s), why repeat it in the embedded resource itself?

Also, consider this example:

{
  "name": "John",
  ...
  "_links": {
    "self": {
      "href": "http://example.com/person/1"
    },
  },
  "_embedded": {
    "http://rels.example.com/barber": {
      "name": "Richard",
      "_links": {
        "http://rels.example.com/barber self": {
          "href": "http://example.com/person/2"
        }
      }
    }
  }
}

It is clear that John's barber is Richard. But does Richard shave himself, or not?


In your previous reply you said:

The HalBuilder implementation actually originally tracked the resources is a Map but after some experimentation that didn't really sit well with me, mostly as the grouping into rel based map is ONLY relevant for the JSON representation - the XML just includes all embedded resources as embedded elements [...]

I think you need the relation between the subject resource and the embedded resource, separate from the relations of the embedded resource itself, even for XML. Again, in the XML example from the HAL spec, the embedded resource with rel="order" has itself an embedded resource with rel="customer", but there is no rel="order" relation repeated inside the embedded resource itself. (Sorry, convoluted sentence, I hope you get my meaning.)

And will not the XML representation of an embedded resource with multiple relations still have multiple space-separated relations in the rel attribute?

I do see how a Map<String, List<Resource>> implementation may not be optimal. It feels more like a Map<Resource, List<String>> structure, in that the subject resource embeds a number of resources related in one or more ways. (I am not sure this is the best implementation in practice, though.)


Finally:

From the looks of it, it's only really the JSON representation that is really faulting here as it uses a map/object keys by rel to contain the resources as well as the _link array, whereas the XML just has inline elements.

Can you give me an example that shows the problem? As you might have inferred, I do not see the issue yet.

talios commented 12 years ago

Hi there, sorry for not getting back to you sooner on his topic - the past week can been a rather busy one with a conference and project work.

I tried writing a variation of the desired patch introducing a new LinkType enum as a way of trying to filter the various links which may, or may not be wanted in the various portions of a representation ( without resorting to switching to a Map ).

Unfortunately, I came up with the same problem I originally had when working on this, which is what led me to move away from using a Map which I originally was.

The problem presents itself when you start looking at round-tripping representations, which is important as Halbuilder is designed as both a server and client.

If you have a JSON representation such as:

{
  "_href": "http://yyyy",
  "name": "Test",
  "_external":
    "barber": {
      "name": "Other Test",
      "_links": {
        "self musician": {
          "href": "http://xxxx/xxxx"
        }
      }
    }
  }
}

Here, we have a resource with one embedded resource with the rel layout as you propose, the _links object is keyed by "barber" only, excluding "self" and any other other reltype's associated with the resource. When this is parsed and then rendered as XML you'd have:

<resource href="http://yyyy">
  <name>Test</name>
  <resource href="http://xxxx/xxxx" rel="barber musician">
    <name>Other Test</name>
  </resource>
</resource>

If however, this XML is then parsed and rendered back to JSON, we no longer have a way of determining which reltype's should be in the _external object and which ones should be in the _links object.

The problem I think is that in Mike's example HAL documents, they all assume/show a rel as being a single reltype, and not as a space delimited set of reltypes as per the specification. From what I can see, the only workable solution, is that the self reltype of a given rel is redacted from the XML rel attribute, and the JSON _embedded objects key, the JSON resources _links key would still include the self rel, as that reltype relates to the link and not the resource itself.

talios commented 12 years ago

You also asked how I select the links, whether I used anything like json path? I'm not what the client side our $work app uses as I've not looked too closely to what our web guys have been doing, but on the Halbuilder side thats just using BaseResource#getLinksByRel which iterates the resource and any embedded resource finding links by a reltype, which is matched via a splitting rel attribute on whitespace.

jcassee commented 12 years ago

Great explanation, I understand the issue now. But should not the rendered XML look like this:

<resource href="http://yyyy">
  <name>Test</name>
  <resource href="http://xxxx/xxxx" rel="barber">
    <link href="http://xxxx/xxxx" rel="musician" />
    <name>Other Test</name>
  </resource>
</resource>
talios commented 12 years ago

Resolved on 2.0 branch.

jcassee commented 12 years ago

Thanks, I'm looking forward to the 2.0 branch.

talios commented 12 years ago

On 19/07/12 3:40 AM, Joost Cassee wrote:

Thanks, I'm looking forward to the 2.0 branch.

Any feedback you might have would be great.

I posted to my tumblr blog about some future plans as well:

http://theoryinpractise.tumblr.com/post/27537768009/halbuilder-2-0-planning-modular-builds-and-language