Skip to content

Commit

Permalink
update repository queries validation to only support Lists with compa…
Browse files Browse the repository at this point in the history
…rison operators
  • Loading branch information
agrgr committed May 12, 2024
1 parent 998ae47 commit 9ceaba3
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 26 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -116,8 +116,8 @@ protected CriteriaDefinition create(Part part, Iterator<Object> iterator) {
}

private CriteriaDefinition create(Part part, AerospikePersistentProperty property, Iterator<?> parameters) {
List<Object> queryParameters = getQueryParameters(parameters);
FilterOperation filterOperation = getFilterOperation(part.getType());
List<Object> queryParameters = getQueryParameters(parameters, filterOperation);
IAerospikeQueryCreator queryCreator = getQueryCreator(part, property, queryParameters, filterOperation);

queryCreator.validate();
Expand Down Expand Up @@ -193,9 +193,9 @@ private FilterOperation getFilterOperation(Part.Type type) {
};
}

private List<Object> getQueryParameters(Iterator<?> parametersIterator) {
private List<Object> getQueryParameters(Iterator<?> parametersIterator, FilterOperation filterOperation) {
List<Object> params = new ArrayList<>();
parametersIterator.forEachRemaining(param -> params.add(convertIfNecessary(param, converter)));
parametersIterator.forEachRemaining(param -> params.add(convertIfNecessary(param, converter, filterOperation)));
// null parameters are not allowed, instead AerospikeNullQueryCriteria.NULL_PARAM should be used
return params.stream().filter(Objects::nonNull).collect(Collectors.toList());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -144,8 +144,9 @@ protected static void setQbValuesForMapByKey(QualifierBuilder qb, Object key, Ob
qb.setValue(Value.get(key)); // contains key
}

protected static Object convertIfNecessary(Object obj, MappingAerospikeConverter converter) {
if (obj == null || obj instanceof AerospikeQueryCriterion || obj instanceof AerospikeNullQueryCriterion) {
protected static Object convertIfNecessary(Object obj, MappingAerospikeConverter converter,
FilterOperation filterOperation) {
if (typeDoesNotRequireConversion(obj, filterOperation)) {
return obj;
}

Expand All @@ -154,6 +155,20 @@ protected static Object convertIfNecessary(Object obj, MappingAerospikeConverter
return converter.toWritableValue(obj, valueType);
}

private static boolean typeDoesNotRequireConversion(Object obj, FilterOperation filterOperation) {
return obj == null
|| obj instanceof AerospikeQueryCriterion
|| obj instanceof AerospikeNullQueryCriterion
|| (isComparisonOperation(filterOperation) && obj instanceof Collection<?>);
}

private static boolean isComparisonOperation(FilterOperation filterOperation) {
return switch (filterOperation) {
case EQ, NOTEQ, LT, LTEQ, GT, GTEQ, BETWEEN: yield true;
default: yield false;
};
}

protected static Value getValueOfQueryParameter(Object queryParameter) {
return Value.get(convertNullParameter(queryParameter));
}
Expand Down Expand Up @@ -204,7 +219,7 @@ protected static void validateTypes(MappingAerospikeConverter converter, Class<?
List<Object> queryParameters, FilterOperation op, String queryPartDescription,
String... alternativeTypes) {
// Checking versus Number rather than strict type to be able to compare, e.g., integer to a long
if (isAssignable(Number.class, propertyType) && isAssignableValue(Number.class, queryParameters.get(0))) {
if (propertyTypeAndFirstParamAssignableToNumber(propertyType, queryParameters)) {
propertyType = Number.class;
}

Expand All @@ -226,6 +241,14 @@ protected static void validateTypes(MappingAerospikeConverter converter, Class<?
}
}

private static boolean propertyTypeAndFirstParamAssignableToNumber(Class<?> propertyType,
List<Object> queryParameters) {
return queryParameters != null
&& queryParameters.size() > 0
&& isAssignable(Number.class, propertyType)
&& isAssignableValue(Number.class, queryParameters.get(0));
}

protected static void validateQueryIsNull(List<Object> queryParameters, String queryPartDescription) {
// Number of arguments is not zero
if (!queryParameters.isEmpty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,14 @@ private void validateCollectionQueryComparison(List<Object> queryParameters, Str
}

if (queryParameters.get(0) instanceof Collection) {
validateTypes(converter, Collection.class, queryParameters, filterOperation, queryPartDescription);
validateTypes(converter, Collection.class, queryParameters, this.filterOperation, queryPartDescription);
} else {
throw new IllegalArgumentException(queryPartDescription + ": invalid argument type, expecting Collection");
}

if (queryParameters.stream().anyMatch(param -> !(param instanceof List<?>))) {
throw new IllegalArgumentException(queryPartDescription + ": only Lists can be compared");
}
}

private void validateCollectionQueryBetween(List<Object> queryParameters, String queryPartDescription) {
Expand All @@ -112,6 +116,10 @@ private void validateCollectionQueryBetween(List<Object> queryParameters, String
throw new IllegalArgumentException(queryPartDescription + ": invalid arguments type, expecting both " +
"to be of the same class");
}

if (queryParameters.stream().anyMatch(param -> !(param instanceof List<?>))) {
throw new IllegalArgumentException(queryPartDescription + ": only Lists can be compared");
}
}

private void validateCollectionContainingTypes(PropertyPath property, List<Object> queryParameters,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import java.util.Set;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatThrownBy;

/**
* Tests for the "Is greater than or equal" repository query. Keywords: GreaterThanEqual, IsGreaterThanEqual.
Expand All @@ -25,15 +26,16 @@ void findByNestedSimplePropertyGreaterThan_String() {
}

@Test
void findByCollectionGreaterThanOrEqual_Set() {
void findByCollectionGreaterThanOrEqual_Set_NegativeTest() {
if (serverVersionSupport.isFindByCDTSupported()) {
Set<Integer> setToCompareWith = Set.of(0, 1, 2, 3, 4);
dave.setIntSet(setToCompareWith);
repository.save(dave);
assertThat(dave.getIntSet()).isEqualTo(setToCompareWith);

List<Person> persons = repository.findByIntSetGreaterThanEqual(setToCompareWith);
assertThat(persons).contains(dave);
// only Lists can be compared because they maintain ordering
assertThatThrownBy(() -> negativeTestsRepository.findByIntSetGreaterThanEqual(setToCompareWith))
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Person.intSet GTEQ: only Lists can be compared");
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ void findByNestedSimplePropertyIn_String() {
.containsExactlyInAnyOrder(dave, carter);
}

@Test
void findByCollectionIn() {
if (serverVersionSupport.isFindByCDTSupported()) {
dave.setInts(List.of(1, 2, 3, 4));
repository.save(dave);

List<Person> result = repository.findByIntsIn(List.of(List.of(0, 1, 2, 3, 4, 5, 6, 7),
List.of(1, 2, 3), List.of(1, 2, 3, 4)));
assertThat(result).contains(dave);
}
}

@Test
void findByNestedCollectionIn() {
if (serverVersionSupport.isFindByCDTSupported()) {
Expand All @@ -52,6 +64,18 @@ void findByNestedCollectionIn() {
}
}

@Test
void findByMapIn() {
if (serverVersionSupport.isFindByCDTSupported()) {
dave.setIntMap(Map.of("1", 2, "3", 4));
repository.save(dave);

List<Person> result = repository.findByIntMapIn(List.of(Map.of("0", 1, "2", 3, "4", 5, "6", 7),
Map.of("1", 2, "3", 4567), Map.of("1", 2, "3", 4)));
assertThat(result).contains(dave);
}
}

@Test
void findByNestedMapIn() {
if (serverVersionSupport.isFindByCDTSupported()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.Collection;
import java.util.List;
import java.util.Map;
import java.util.Set;

/**
* This repository acts as a storage for invalid method names used for testing. For actual repository see
Expand Down Expand Up @@ -239,6 +240,12 @@ List<P> findByIntMapContaining(CriteriaDefinition.AerospikeQueryCriterion criter
*/
List<P> findByIntsGreaterThan(int number);


/**
* Only Lists can be compared
*/
List<P> findByIntSetGreaterThanEqual(Set<Integer> set);

/**
* Invalid number of arguments: expecting at least one
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -320,18 +320,18 @@ List<P> findByAgeOrLastNameLikeAndFirstNameLike(QueryParam age, QueryParam lastN
Page<P> findByAddressIn(List<Address> address, Pageable page);

/**
* Find all entities that satisfy the condition "have strings the same as the given argument" (find by collection)
* Find all entities that satisfy the condition "have strings the same as the given argument" (find by List)
*
* @param list List to compare strings with
*/
List<P> findByStringsEquals(List<String> list);

/**
* Find all entities that satisfy the condition "have strings the same as the given argument" (find by collection)
* Find all entities that satisfy the condition "have strings the same as the given argument" (find by List)
*
* @param collection Collection to compare strings with
*/
List<P> findByStrings(Collection<String> collection);
List<P> findByStrings(List<String> collection);

/**
* Find all entities with existing strings list not equal to the given argument
Expand Down Expand Up @@ -362,23 +362,13 @@ List<P> findByAgeOrLastNameLikeAndFirstNameLike(QueryParam age, QueryParam lastN

/**
* Find all entities that satisfy the condition "have strings set with more elements or with a corresponding element
* higher in ordering than in the given argument" (find by collection).
* higher in ordering than in the given argument" (find by List).
* <p>
* <a href="https://docs.aerospike.com/server/guide/data-types/cdt-ordering#list">Information about ordering</a>
*
* @param collection - Collection to compare with
*/
List<P> findByIntSetGreaterThanEqual(Collection<Integer> collection);

/**
* Find all entities that satisfy the condition "have strings set with more elements or with a corresponding element
* higher in ordering than in the given argument" (find by collection).
* <p>
* <a href="https://docs.aerospike.com/server/guide/data-types/cdt-ordering#list">Information about ordering</a>
*
* @param collection - Collection to compare with
*/
List<P> findByIntsGreaterThanEqual(Collection<Integer> collection);
List<P> findByIntsGreaterThanEqual(List<Integer> collection);

/**
* Find all entities containing the given map element (key or value depending on the given criterion)
Expand Down Expand Up @@ -516,6 +506,14 @@ List<P> findByAgeOrLastNameLikeAndFirstNameLike(QueryParam age, QueryParam lastN
*/
List<P> findByIntMapBetween(Map<String, Integer> from, Map<String, Integer> to);

/**
* Find all entities that satisfy the condition "have intMap equal to one of the values in the given list"
* (find by Map)
*
* @param list - list of possible values
*/
List<P> findByIntMapIn(List<Map<String, Integer>> list);

/**
* Find all entities that satisfy the condition "have a bestFriend who has a friend with address apartment value in
* the range between the given integers (deeply nested)"
Expand Down Expand Up @@ -1171,6 +1169,14 @@ List<P> findByFriendStringMapNotContaining(AerospikeQueryCriterion criterion,
*/
List<P> findByIntsContaining(int integer);

/**
* Find all entities that satisfy the condition "have ints equal to one of the values in the given list"
* (find by List)
*
* @param list - list of possible values
*/
List<P> findByIntsIn(List<List<Integer>> list);

/**
* Find all entities that satisfy the condition "have the array which contains the given integer"
* <p>
Expand Down

0 comments on commit 9ceaba3

Please sign in to comment.