-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Support collection parameters in @Query methods #1856
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
Support collection parameters in @Query methods #1856
Conversation
a582f69
to
07598d5
Compare
903f3d4
to
c8bef69
Compare
This adds a test and fix so that collection parameters are supported and quoted correctly, at least for `Collection<String>`. Previously the `StringQueryUtil` class would just call `.toString()` on the parameter and escape the `"` characters in the result, which would cause a syntax error in the query string. This changes so that the class checks if the parameter is a `Collection` and if so starts a Json array and adds elements to it by calling `convert()` on each element.
c8bef69
to
3b59581
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot for this PR. This looks good, I just added a comment/question on this in the code
StringBuilder sb = new StringBuilder("["); | ||
sb.append(collectionParam.stream().map(o -> "\"" + convert(o) + "\"").collect(Collectors.joining(","))); | ||
sb.append("]"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This works, but what do you think about
StringBuilder sb = new StringBuilder("[");
sb.append(collectionParam.stream().map(o -> {
if (o instanceof String) {
String s = (String) o;
return '"' + convert(o) + '"';
} else {
return convert(o);
}
}).collect(Collectors.joining(",")));
sb.append(']');
Might be better to only wrap Strings in quotes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that makes sense. I'll add a test and incorporate your change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
8b154df
to
a026df0
Compare
This adds a test and fix so that collection parameters are supported and quoted correctly, at least for
Collection<String>
.Previously the
StringQueryUtil
class would just call.toString()
on the parameter and escape the"
characters in the result, which would cause a syntax error in the query string.This changes so that the class checks if the parameter is a
Collection
and if so starts a Json array and adds elements to it by callingconvert()
on each element.Closes #1858