Skip to content

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

Merged

Conversation

herder
Copy link
Contributor

@herder herder commented Jul 1, 2021

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.

Closes #1858

  • You have read the Spring Data contribution guidelines.
  • There is a ticket in the bug tracker for the project in our issue tracker.
  • You use the code formatters provided here and have them applied to your changes. Don’t submit any formatting related changes.
  • You submit test cases (unit or integration tests) that back your changes.
  • You added yourself as author in the headers of the classes you touched. Amend the date range in the Apache license header if needed. For new types, add the license header (copy from another file and set the current year only).

@herder herder force-pushed the support_collection_query_params branch 2 times, most recently from a582f69 to 07598d5 Compare July 1, 2021 12:51
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jul 1, 2021
@herder herder force-pushed the support_collection_query_params branch 4 times, most recently from 903f3d4 to c8bef69 Compare July 1, 2021 13:51
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.
@herder herder force-pushed the support_collection_query_params branch from c8bef69 to 3b59581 Compare July 1, 2021 13:54
Copy link
Collaborator

@sothawo sothawo left a 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

Comment on lines 69 to 71
StringBuilder sb = new StringBuilder("[");
sb.append(collectionParam.stream().map(o -> "\"" + convert(o) + "\"").collect(Collectors.joining(",")));
sb.append("]");
Copy link
Collaborator

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.

Copy link
Contributor Author

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.

Copy link
Contributor Author

@herder herder Jul 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in a026df0 , @sothawo

@herder herder force-pushed the support_collection_query_params branch from 8b154df to a026df0 Compare July 3, 2021 08:04
@sothawo sothawo merged commit 6f84a1c into spring-projects:main Jul 3, 2021
sothawo pushed a commit that referenced this pull request Jul 3, 2021
Original Pull Request #1856
Closes #1858

(cherry picked from commit 6f84a1c)
(cherry picked from commit 254948d)
(cherry picked from commit 979c164)
sothawo pushed a commit that referenced this pull request Jul 3, 2021
Original Pull Request #1856
Closes #1858

(cherry picked from commit 6f84a1c)
(cherry picked from commit 254948d)
sothawo pushed a commit that referenced this pull request Jul 3, 2021
Original Pull Request #1856
Closes #1858

(cherry picked from commit 6f84a1c)
@herder herder deleted the support_collection_query_params branch July 4, 2021 06:22
sothawo pushed a commit that referenced this pull request Jul 6, 2021
Original Pull Request #1856
Closes #1858

(cherry picked from commit 6f84a1c)
(cherry picked from commit 254948d)
(cherry picked from commit 979c164)
(cherry picked from commit d6cfc20)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Collection parameters for @Query-annotated methods get escaped wrongly
3 participants