-
Notifications
You must be signed in to change notification settings - Fork 471
Add support for additional attributes on HAL Links #230
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
Comments
I don't think we're going to go ahead and add representation specific properties to model types. Be reminded that the model types need to be independent of any representation format so that they can be reused. Spring HATEOAS is not a HAL implementation, and never was intended to be one. We support rendering the representation model in a HAL compliant way. That doesn't mean we're going to add support for every detail of it as this would subvert the design decisions made for the library. |
I guess I'm a bit confused on what exactly the goal of the project is. Hal and Siren are very specific media types, with specific needs. In order to fully support the various media types, i.e. Hal and Siren, the model must reflect the needs to each specific media type. I don't see how this can be handled purely by the rendering layer. It seems that if a REST endpoint will support different media types via the "Accept" header, then the developer will have to build different model representations to fully support the request, i.e. a Hal model and a Siren model. Could you please clarify the goals of this project in regards to fully supporting specific media types? |
Would it be more acceptable to modify the Link class to include a Map<String,String> of attributes and provide a "withAttribute" method to get the extra attributes in? |
Of course. The core goal of the project is to ease the development of hypermedia driven REST services. To achieve this the library provides model types (e.g. That's what we actually do with the If you take a step back and think about it: if you want to support both XML and some JSON format, would you want to duplicate controllers to return an |
I'm totally in agreement with this. The only issue is how to handle the extra attributes on links in the Hal spec. If we do it generically, i.e. via a "withAttribute" method on Link, would this be more acceptable? |
Wouldn't you then write HAL specific controller code again? From this point of view, there's no difference between |
Yes, that's correct, but if we cannot add extra attributes to links, then it's impossible to use the optional attributes from the HAL spec, thus losing some of the benefits of HAL. Here's a use case we're trying to implement. We want the user to be able to select an item from an array of links in a Resource. The title attribute in HAL can be used to add the "display friendly" text that the user will see in a combo box or similar type of component. If seems like we could add optional attributes and that various renderers might ignore ones they don't support. |
My two cents: at least the Link object should support target attributes as described for the link header in the web linking rfc. That rfc is not media type specific, but many hypermedia types are likely to adopt aspects of the web linking rfc, e.g. HAL did this with the title attribute. Link.toString() already gives you a Link header, albeit a restricted one. I don't see why adding the official link-param feature from rfc5988 would subvert the decision to have a mediatype agnostic model. Please consider to open up the Link object for the link-params defined in rfc5988, including link extensions. Link-param names in rfc5988 may be repeated, a multimap or list of LinkParam might be more appropriate than a map. Best regards, Dietrich Schulten ---- Jeff Stano schrieb ----
|
Yes, if we think about these attributes in the context of rfc5988, then it seems like there is not an issue, since this is an official part of the standard. I'll put together the chanes to the Link class and see what we all think. Thanks for all the comments. Jeff Stano |
I created a new issue, #235, that will supplant this issue. I will close it. |
I think the best approach will be to have a method in the Link class called "asHalLink" that will create a new HalLink instance. The HalLink class is new and will have the extra attributes.
This would work also if we decide to support Siren in the future, then it just a matter of having a "asSirenLink" method.
I creating a pull request with changes to support this.
It doesn't feel right adding the extra attributes directly to the Link class, at least not in the manner that I've done. An alternate approach would be to just have a map of extra attributes, but that will be more cumbersome to use and use lose type safety.
The text was updated successfully, but these errors were encountered: