-
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
sothawo
left a comment
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
StringQueryUtilclass 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
Collectionand if so starts a Json array and adds elements to it by callingconvert()on each element.Closes #1858