-
Notifications
You must be signed in to change notification settings - Fork 29
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
FMWK-438 Add tests and javadocs for comparing Collections #740
Conversation
if (queryParameters.stream().anyMatch(param -> !(param instanceof List<?>))) { | ||
throw new IllegalArgumentException(queryPartDescription + ": only Lists can be compared"); | ||
} |
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.
Consider extracting to a util method and reusing it.
private static boolean propertyTypeAndFirstParamAssignableToNumber(Class<?> propertyType, | ||
List<Object> queryParameters) { | ||
return queryParameters != null | ||
&& queryParameters.size() > 0 |
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.
Use !queryParameters.isEmpty()
@agrgr As discussed, removing the support for collections comparison other than list (e.g. sets) is problematic, it introduces a breaking change and it also removes a useful feature. We will talk about it tomorrow (and cover additional sets comparison scenarios such as GT, LE etc...) |
Changing the PR as discussed. |
PriorityQueue<Integer> queueToCompareWith = new PriorityQueue<>(Set.of(3, 1, 2, 4, 0)); | ||
List<Person> persons4 = repository.findByIntSetGreaterThan(queueToCompareWith); | ||
assertThat(persons4).contains(dave); | ||
|
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.
Redundant new line
PriorityQueue<Integer> queueToCompareWith = new PriorityQueue<>(Set.of(3, 1, 2, 4, 0)); | ||
List<Person> persons4 = repository.findByIntSetGreaterThanEqual(queueToCompareWith); | ||
assertThat(persons4).contains(dave); | ||
|
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.
Redundant new line
Update repository queries validation to only support Lists with comparison operators instead of Collection