Skip to content

ResourceSupport might benefit from having a method that returns a list of links that have the same rel. #318

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
vivin opened this issue Mar 16, 2015 · 17 comments

Comments

@vivin
Copy link

vivin commented Mar 16, 2015

Currently there is a method called getLink(String rel) that returns a link with the given rel. However, it is possible to have multiple links that share the same rel. Therefore, I think it might be beneficial to have a method like so:

public List<Link> getLinks(String rel);

Which will return a list of links that have the given rel.

vivin added a commit to vivin/spring-hateoas that referenced this issue Mar 16, 2015
Adds a method to `ResourceSupport` that returns a list of links with the given rel.
@chrylis
Copy link

chrylis commented Mar 21, 2015

#157

@vivin
Copy link
Author

vivin commented Mar 21, 2015

Ahh thanks! I did not see the original issue. @olivergierke thoughts about this?

@chrylis
Copy link

chrylis commented Mar 22, 2015

That's not a 100% duplicate, but I think we're looking at the same issue. Essentially, the current getLink (1) returns a single value and (2) doesn't play nicely with the map-navigation syntax offered by Groovy and *EL. What I'd prefer to see in place of your signature is

Map<String, Collection<Link>> getLinks();

That way, something like myobj.links['picture'].href would work.

@vivin
Copy link
Author

vivin commented Mar 22, 2015

I think changing the signature of the existing one would be a breaking change, unfortunately. The new method I suggested would sort of do what you want, except instead of working with a map directly, you're asking the ResourceSupport class to filter out links with the rel you want. The performance would be lacking since it would be O(n) since you have to traverse the entire list. I think that could be mitigated if the internal representation is modified from List<T> into Map<K, V>.

@chrylis
Copy link

chrylis commented Mar 23, 2015

It absolutely would be a breaking change, but we're still at major version 0. (I note additionally that HAL seems to specify that link rels are unique per entity, which means that the current list-based implementation is also problematic in permitting duplicates.) Changing the internal representation is what I recommended a while back, but I haven't heard any comments.

@dschulten
Copy link
Contributor

Hi,

although I find it problematic, too, to use duplicates, HAL specifically
allows them in section 4.1.1 where it says that the value of the rel may be
an array of link objects. It uses this feature explicitly for curies.

Best,
Dietrich

Am 23. März 2015 04:24:11 schrieb Christopher Smith [email protected]:

It absolutely would be a breaking change, but we're still at major version
0. (I note additionally that HAL seems to specify that link rels are unique
per entity, which means that the current list-based implementation is also
problematic in permitting duplicates.) Changing the internal representation
is what I recommended a while back, but I haven't heard any comments.


Reply to this email directly or view it on GitHub:
#318 (comment)

@chrylis
Copy link

chrylis commented Mar 23, 2015

Aha. Well, in that case, something with the semantics of a multimap are probably best.

@vivin
Copy link
Author

vivin commented Mar 23, 2015

Yes, a rel can represent multiple links and HAL does allow that. Internally a Map<String, List<Link>> would make much more sense, I think, but I haven't heard any comments either about this issue (and some others as well).

@odrotbohm
Copy link
Member

What if we just added List<Link> getLinks(String rel)?

@chrylis
Copy link

chrylis commented May 21, 2015

That would be an improvement, but it's still awkward in contexts (Groovy, SpEL) that support map-style syntax.

@odrotbohm
Copy link
Member

foo.getLinks(…).href?

@chrylis
Copy link

chrylis commented May 21, 2015

Works okay if you're looking for links for a specific rel (though still more cumbersome than foo.links.picture[0].href, since getLinks isn't a property accessor), but there's no simple way to iterate over rels.

@gregturn
Copy link
Contributor

Your saying to offer a keys() and values() pair of ops, similar to a Map.

@chrylis
Copy link

chrylis commented May 21, 2015

@gregturn I've actually suggested just making the links a Map<String,Link> (or Map<String,Collection<Link>>. The interface makes a lot more sense to me.

@vivin
Copy link
Author

vivin commented May 21, 2015

@olivergierke List<Link> getLinks(String rel) would work. To address @chrylis concerns, perhaps a method Map<String, List<Link>> getLinks() would work.

@gregturn
Copy link
Contributor

Related to #542

gregturn added a commit that referenced this issue Mar 26, 2017
Introduces `ResourceSupport.getLinks(String rel)` to allow returning ALL links related to a give rel.

Resolves #542,#318,#319
gregturn added a commit that referenced this issue Mar 26, 2017
Introduces `ResourceSupport.getLinks(String rel)` to allow returning ALL links related to a give rel.

Resolves #542,#318,#319
gregturn added a commit that referenced this issue Mar 26, 2017
Introduces `ResourceSupport.getLinks(String rel)` to allow returning ALL links related to a give rel.

Resolves #542,#318,#319,#157
@gregturn gregturn assigned gregturn and unassigned odrotbohm Mar 26, 2017
gregturn added a commit that referenced this issue Mar 26, 2017
Introduces `ResourceSupport.getLinks(String rel)` to allow returning ALL links related to a give rel.

Resolves #542,#318,#319,#157
@gregturn
Copy link
Contributor

Resolved via 0e8b0e1

@gregturn gregturn added this to the 0.24 milestone Mar 26, 2017
odrotbohm pushed a commit that referenced this issue Mar 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants