Skip to content

issue #288: (Approach #2) Allow links on a particular rel to be displayed as an array even if there is only one link #291

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

Closed

Conversation

vivin
Copy link

@vivin vivin commented Feb 7, 2015

Allows links under a single rel to be represented as a multiple (i.e., serialized to an array) even if there is only one.

This approach is a little better than the other one. Here, I created a new annotation that you can attach to a resource class. This annotation will contain a list of rels that will always be serialized as multiple links (i.e., in a JSON array) regardless of cardinality.

I did have to add a propertly to the Link class, which is just the class of the owning resource. The setter is package-private and so no one else can mess with it. The value of the field is set automatically when links are added.

I have also added a test for this case.

Allows links under a single rel to be represented as a multiple (i.e., serialized to an array) even if there is only one.
@odrotbohm
Copy link
Member

What is "the other one" you're referring to. Link must neither know about ResourceSupport nor should it be made mutable. The annotation lookup looks rather hacky, too. Can we just extend the configuration options to HalHandlerInstantiator and tweak the rendering accordingly?

@odrotbohm odrotbohm self-assigned this Feb 10, 2015
@vivin
Copy link
Author

vivin commented Feb 10, 2015

The other one had the setting on the Link itself which only applied to HAL. Yes, I agree this is not as clean as I would like.

I did try to do it via HalHandlerInstantiator. However, I ran into a few issues. The list of links is serialized by OptionalListJackson2Serializer. The idea is that we need to know if that particular rel on that particular resource needs to be forced into a multiple-link representation. In most cases it is not desirable that the rel be forced into an array representation across all resources. For example, I have a collection resource where I do want the rel to be forced into an array representation regardless of cardinality. But in another representation, I know that the resource described by the same rel will only ever have one instance and so the object representation is fine. So what I'm looking for is a way to force an array representation on specific resource-rel combinations.

I like the approach you suggested with the configuration options. I'm assuming that we would pass in the class of the resource and the rels that need to be forced into an array representation? Perhaps in some sort of map? I'll look into that option a little bit more. I think there would still be the problem of lacking context inside the OptionalListJackson2Serializer. How would we know that we need to force the list of links to be serialized into an array? We don't know which resource the link belongs to.

@vivin
Copy link
Author

vivin commented Feb 12, 2015

@olivergierke Do you know of a way how we can keep track of the resource that is also being serialized while its links are being serialized?

@vivin
Copy link
Author

vivin commented Feb 13, 2015

@olivergierke I've been looking into this to try and figure out how I can convey information to the list serializer regarding the owning resource. Do you have a suggestion as to how this can be accomplished? This is why I added the owningResource property. But if you know of a better way, please let me know! I'd like to get a fix for this as soon as possible because this is a real problem with the way the links are being serialized in HAL+JSON.

@vivin
Copy link
Author

vivin commented Feb 13, 2015

@olivergierke Another question (sorry for so many!). After thinking about this a bit more, I think I will take an approach where we always force certain rels to be serialized into an array. That is, instead of doing it based on the resource it belongs to, we do it regardless. So wherever the rel appears, it will always be serialized into a list of links. From an API-design perspective, I think this is what I was going for anyway (with regard to consistency).

My question is regarding configuration. Would you prefer that this be configured using an annotation (kind of like how we are doing it right now with @EnableHyperMediaSupport), or would you rather this information be inside a bean definition of some sort?

@vivin
Copy link
Author

vivin commented Feb 13, 2015

Closing this pull request. Please see #295.

@vivin vivin closed this Feb 13, 2015
gregturn added a commit that referenced this pull request May 22, 2017
Provide the means to render a single link entry as a JSON Array.

Original pull-request: #295
Related issues: #291
gregturn added a commit that referenced this pull request Sep 11, 2017
Provide the means to render a single link entry as a JSON Array.

Original pull-request: #295
Related issues: #291
odrotbohm pushed a commit that referenced this pull request Oct 13, 2017
…ng options.

We now expose HalConfiguration to be defined as Spring bean in user configuration to control certain aspects of the HAL rendering. Initially we allow to control whether links shall always be rendered as collection. If no user-provided bean is available, we register a default instance.

Original pull request: #295
Related issues: #291
odrotbohm added a commit that referenced this pull request Oct 13, 2017
Moved RenderSingleLinks enum into HalConfiguration. Simplified HalConfiguration setup by moving the default into the class. The lookup of a user-provided HalConfiguration is now handled on the bean name level to avoid premature initialization of a potentially defined bean. Formatting.

Original pull request: #295
Related issues: #291
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants