FMWK-438 Add tests and javadocs for comparing Collections (#740)

* Add tests and javadocs for comparing Collection
* Add correct validation of types of Map query arguments

src/main/java/org/springframework/data/aerospike/repository/query/AerospikeQueryCreator.java

@@ -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();
@@ -142,10 +142,12 @@ private IAerospikeQueryCreator getQueryCreator(Part part, AerospikePersistentPro
             }
         } else if (property.isMap()) {
             if (part.getProperty().hasNext()) { // a POJO field
-                queryCreator = new MapQueryCreator(part, property, fieldName, queryParameters, filterOperation,
-                    converter, versionSupport, true);
+                PropertyPath nestedProperty = getNestedPropertyPath(part.getProperty());
+                queryCreator = new MapQueryCreator(part, nestedProperty, property, fieldName, queryParameters,
+                    filterOperation, converter, versionSupport, true);
             } else {
-                queryCreator = new MapQueryCreator(part, property, fieldName, queryParameters, filterOperation,
+                queryCreator = new MapQueryCreator(part, part.getProperty(), property, fieldName, queryParameters,
+                    filterOperation,
                     converter, versionSupport, false);
             }
         } else {
@@ -193,7 +195,7 @@ 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)));
         // null parameters are not allowed, instead AerospikeNullQueryCriteria.NULL_PARAM should be used

src/main/java/org/springframework/data/aerospike/repository/query/AerospikeQueryCreatorUtils.java

@@ -145,7 +145,7 @@ protected static void setQbValuesForMapByKey(QualifierBuilder qb, Object key, Ob
     }
 
     protected static Object convertIfNecessary(Object obj, MappingAerospikeConverter converter) {
-        if (obj == null || obj instanceof AerospikeQueryCriterion || obj instanceof AerospikeNullQueryCriterion) {
+        if (typeDoesNotRequireConversion(obj)) {
             return obj;
         }
 
@@ -154,6 +154,12 @@ protected static Object convertIfNecessary(Object obj, MappingAerospikeConverter
         return converter.toWritableValue(obj, valueType);
     }
 
+    private static boolean typeDoesNotRequireConversion(Object obj) {
+        return obj == null
+            || obj instanceof AerospikeQueryCriterion
+            || obj instanceof AerospikeNullQueryCriterion;
+    }
+
     protected static Value getValueOfQueryParameter(Object queryParameter) {
         return Value.get(convertNullParameter(queryParameter));
     }
@@ -196,15 +202,15 @@ protected static void validateTypes(MappingAerospikeConverter converter, Propert
 
     protected static void validateTypes(MappingAerospikeConverter converter, PropertyPath propertyPath,
                                         List<Object> queryParameters, FilterOperation op, String queryPartDescription) {
-        validateTypes(converter, propertyPath.getTypeInformation()
-            .getType(), queryParameters, op, queryPartDescription);
+        validateTypes(converter, propertyPath.getTypeInformation().getType(), queryParameters, op,
+            queryPartDescription);
     }
 
     protected static void validateTypes(MappingAerospikeConverter converter, Class<?> propertyType,
                                         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;
         }
 
@@ -226,6 +232,13 @@ protected static void validateTypes(MappingAerospikeConverter converter, Class<?
         }
     }
 
+    private static boolean propertyTypeAndFirstParamAssignableToNumber(Class<?> propertyType,
+                                                                       List<Object> queryParameters) {
+        return !queryParameters.isEmpty()
+            && 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()) {

src/main/java/org/springframework/data/aerospike/repository/query/CollectionQueryCreator.java

@@ -87,7 +87,7 @@ 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");
         }

src/main/java/org/springframework/data/aerospike/repository/query/MapQueryCreator.java

@@ -9,6 +9,7 @@
 import org.springframework.data.aerospike.query.qualifier.QualifierBuilder;
 import org.springframework.data.aerospike.repository.query.CriteriaDefinition.AerospikeQueryCriterion;
 import org.springframework.data.aerospike.server.version.ServerVersionSupport;
+import org.springframework.data.mapping.PropertyPath;
 import org.springframework.data.repository.query.parser.Part;
 import org.springframework.data.util.TypeInformation;
 
@@ -24,17 +25,19 @@ public class MapQueryCreator implements IAerospikeQueryCreator {
 
     private final Part part;
     private final AerospikePersistentProperty property;
+    private final PropertyPath propertyPath;
     private final String fieldName;
     private final List<Object> queryParameters;
     private final FilterOperation filterOperation;
     private final MappingAerospikeConverter converter;
     private final ServerVersionSupport versionSupport;
     private final boolean isNested;
 
-    public MapQueryCreator(Part part, AerospikePersistentProperty property, String fieldName,
+    public MapQueryCreator(Part part, PropertyPath propertyPath, AerospikePersistentProperty property, String fieldName,
                            List<Object> queryParameters, FilterOperation filterOperation,
                            MappingAerospikeConverter converter, ServerVersionSupport versionSupport, boolean isNested) {
         this.part = part;
+        this.propertyPath = propertyPath;
         this.property = property;
         this.fieldName = fieldName;
         this.queryParameters = queryParameters;
@@ -46,13 +49,26 @@ public MapQueryCreator(Part part, AerospikePersistentProperty property, String f
 
     @Override
     public void validate() {
-        String queryPartDescription = String.join(" ", part.getProperty().toString(), filterOperation.toString());
+        String queryPartDescription = String.join(" ", propertyPath.toString(), filterOperation.toString());
         switch (filterOperation) {
+            // Types of query arguments for CONTAINING and NOT_CONTAINING are validated within the method
             case CONTAINING, NOT_CONTAINING -> validateMapQueryContaining(queryPartDescription);
-            case EQ, NOTEQ, GT, GTEQ, LT, LTEQ -> validateMapQueryComparison(queryPartDescription);
-            case BETWEEN -> validateMapQueryBetween(queryPartDescription);
-            case IN, NOT_IN -> validateQueryIn(queryParameters, queryPartDescription);
-            case IS_NOT_NULL, IS_NULL -> validateQueryIsNull(queryParameters, queryPartDescription);
+            case EQ, NOTEQ, GT, GTEQ, LT, LTEQ -> {
+                validateMapQueryComparison(queryPartDescription);
+                validateTypes(converter, propertyPath, filterOperation, queryParameters);
+            }
+            case BETWEEN -> {
+                validateMapQueryBetween(queryPartDescription);
+                validateTypes(converter, propertyPath, filterOperation, queryParameters);
+            }
+            case IN, NOT_IN -> {
+                validateQueryIn(queryParameters, queryPartDescription);
+                validateTypes(converter, propertyPath, filterOperation, queryParameters);
+            }
+            case IS_NOT_NULL, IS_NULL -> {
+                validateQueryIsNull(queryParameters, queryPartDescription);
+                validateTypes(converter, propertyPath, filterOperation, queryParameters);
+            }
             default -> throw new UnsupportedOperationException("Unsupported operation: " + queryPartDescription);
         }
     }

src/test/java/org/springframework/data/aerospike/repository/query/blocking/noindex/findBy/BetweenTests.java

@@ -6,8 +6,12 @@
 import org.springframework.data.aerospike.sample.Person;
 import org.springframework.data.aerospike.util.TestUtils;
 
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
+import java.util.PriorityQueue;
+import java.util.Set;
+import java.util.TreeSet;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
@@ -55,6 +59,47 @@ void findByNestedSimplePropertyBetween_String() {
             .containsExactly(dave);
     }
 
+    /**
+     * Collections are converted to Lists when saved to AerospikeDB.
+     * <p>
+     * Argument of type Collection meant to be compared with a List in DB also gets converted to a List.
+     * <p>
+     * In this test we are providing a SortedSet and a PriorityQueue which preserve the order of elements,
+     * such Collections can be consistently compared to a List saved in DB.
+     */
+    @Test
+    void findByCollectionBetween_SortedSet() {
+        if (serverVersionSupport.isFindByCDTSupported()) {
+            Set<Integer> davesIntSet = Set.of(1);
+            dave.setIntSet(davesIntSet);
+            repository.save(dave);
+            assertThat(dave.getIntSet()).isEqualTo(davesIntSet);
+
+            Set<Integer> setToCompareWith = new TreeSet<>(Set.of(3, 1, 2, 4, 0)); // gets sorted using natural order
+            List<Person> persons = repository.findByIntSetBetween(Set.of(0), setToCompareWith);
+            assertThat(persons).doesNotContain(dave);
+
+            Set<Integer> setToCompareWith2 = new TreeSet<>(Comparator.reverseOrder());
+            setToCompareWith2.addAll(Set.of(3, 1, 2, 4, 0)); // gets sorted using Comparator: 4, 3, 2, 1, 0
+            List<Person> persons2 = repository.findByIntSetBetween(Set.of(0), setToCompareWith2);
+            assertThat(persons2).contains(dave);
+
+            List<Integer> listToCompareWith = List.of(0, 4, 3, 1, 2); // the insertion order is preserved
+            List<Person> persons3 = repository.findByIntSetBetween(Set.of(0), listToCompareWith);
+            assertThat(persons3).doesNotContain(dave);
+
+            // gets sorted using natural order
+            PriorityQueue<Integer> queueToCompareWith = new PriorityQueue<>(Set.of(3, 1, 2, 4, 0));
+            List<Person> persons4 = repository.findByIntSetBetween(Set.of(0), queueToCompareWith);
+            assertThat(persons4).doesNotContain(dave);
+
+            PriorityQueue<Integer> queueToCompareWith2 = new PriorityQueue<>(Comparator.reverseOrder());
+            queueToCompareWith2.addAll(Set.of(3, 1, 2, 4, 0)); // gets sorted using Comparator: 4, 3, 2, 1, 0
+            List<Person> persons5 = repository.findByIntSetBetween(Set.of(0), queueToCompareWith2);
+            assertThat(persons5).contains(dave);
+        }
+    }
+
     @Test
     void findByCollectionBetween_IntegerList() {
         List<Integer> list1 = List.of(100, 200, 300);

src/test/java/org/springframework/data/aerospike/repository/query/blocking/noindex/findBy/GreaterThanOrEqualTests.java

@@ -6,9 +6,12 @@
 import org.springframework.data.aerospike.sample.Person;
 import org.springframework.data.aerospike.util.TestUtils;
 
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
+import java.util.PriorityQueue;
 import java.util.Set;
+import java.util.TreeSet;
 
 import static org.assertj.core.api.Assertions.assertThat;
 
@@ -24,8 +27,19 @@ void findByNestedSimplePropertyGreaterThan_String() {
         assertThat(repository.findByAddressZipCodeGreaterThanEqual(zipCode)).containsExactly(carter);
     }
 
+    /**
+     * Collections are converted to Lists when saved to AerospikeDB.
+     * <p>
+     * Argument of type Collection meant to be compared with a List in DB also gets converted to a List.
+     * <p>
+     * In this test we are providing an unordered Collection (Set) which means that the order of elements in a resulting
+     * List cannot be guaranteed.
+     * <p>
+     * Comparing with an unordered Collection works only for equality (EQ/NOTEQ) operations and not for
+     * LT/LTEQ/GT/GTEQ/BETWEEN.
+     */
     @Test
-    void findByCollectionGreaterThanOrEqual_Set() {
+    void findByCollectionGreaterThanOrEqual_UnorderedSet_Equals_List() {
         if (serverVersionSupport.isFindByCDTSupported()) {
             Set<Integer> setToCompareWith = Set.of(0, 1, 2, 3, 4);
             dave.setIntSet(setToCompareWith);
@@ -37,6 +51,42 @@ void findByCollectionGreaterThanOrEqual_Set() {
         }
     }
 
+    /**
+     * Collections are converted to Lists when saved to AerospikeDB.
+     * <p>
+     * Argument of type Collection meant to be compared with a List in DB also gets converted to a List.
+     * <p>
+     * In this test we are providing a SortedSet and a PriorityQueue which preserve the order of elements, such
+     * Collections can be consistently compared to a List saved in DB.
+     */
+    @Test
+    void findByCollectionGreaterThanOrEqual_SortedSet() {
+        if (serverVersionSupport.isFindByCDTSupported()) {
+            Set<Integer> davesIntSet = Set.of(1);
+            dave.setIntSet(davesIntSet);
+            repository.save(dave);
+            assertThat(dave.getIntSet()).isEqualTo(davesIntSet);
+
+            Set<Integer> setToCompareWith = new TreeSet<>(Set.of(0, 1, 2, 3, 4)); // natural order
+            List<Person> persons = repository.findByIntSetGreaterThanEqual(setToCompareWith);
+            assertThat(persons).contains(dave);
+
+            Set<Integer> setToCompareWith2 = new TreeSet<>(Comparator.comparingInt(Integer::intValue));
+            setToCompareWith2.addAll(Set.of(3, 1, 2, 4, 0)); // gets sorted using Comparator
+            List<Person> persons2 = repository.findByIntSetGreaterThanEqual(setToCompareWith2);
+            assertThat(persons2).contains(dave);
+
+            List<Integer> listToCompareWith = List.of(3, 1, 2, 0, 4); // the insertion order is preserved
+            List<Person> persons3 = repository.findByIntSetGreaterThanEqual(listToCompareWith);
+            assertThat(persons3).doesNotContain(dave);
+
+            // gets sorted using natural order
+            PriorityQueue<Integer> queueToCompareWith = new PriorityQueue<>(Set.of(3, 1, 2, 4, 0));
+            List<Person> persons4 = repository.findByIntSetGreaterThanEqual(queueToCompareWith);
+            assertThat(persons4).contains(dave);
+        }
+    }
+
     @Test
     void findByCollectionGreaterThanOrEqual_List() {
         if (serverVersionSupport.isFindByCDTSupported()) {

src/test/java/org/springframework/data/aerospike/repository/query/blocking/noindex/findBy/GreaterThanTests.java

@@ -11,8 +11,12 @@
 import org.springframework.data.domain.Slice;
 import org.springframework.data.domain.Sort;
 
+import java.util.Comparator;
 import java.util.List;
 import java.util.Map;
+import java.util.PriorityQueue;
+import java.util.Set;
+import java.util.TreeSet;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
@@ -134,6 +138,42 @@ void findByNestedSimplePropertyGreaterThan_Integer() {
         TestUtils.setFriendsToNull(repository, alicia, dave, carter, leroi);
     }
 
+    /**
+     * Collections are converted to Lists when saved to AerospikeDB.
+     * <p>
+     * Argument of type Collection meant to be compared with a List in DB also gets converted to a List.
+     * <p>
+     * In this test we are providing a SortedSet and a PriorityQueue which preserve the order of elements, such
+     * Collections can be consistently compared to a List saved in DB.
+     */
+    @Test
+    void findByCollectionGreaterThan_SortedSet() {
+        if (serverVersionSupport.isFindByCDTSupported()) {
+            Set<Integer> davesIntSet = Set.of(1);
+            dave.setIntSet(davesIntSet);
+            repository.save(dave);
+            assertThat(dave.getIntSet()).isEqualTo(davesIntSet);
+
+            Set<Integer> setToCompareWith = new TreeSet<>(Set.of(0, 1, 2, 3, 4)); // natural order
+            List<Person> persons = repository.findByIntSetGreaterThan(setToCompareWith);
+            assertThat(persons).contains(dave);
+
+            Set<Integer> setToCompareWith2 = new TreeSet<>(Comparator.comparingInt(Integer::intValue));
+            setToCompareWith2.addAll(Set.of(3, 1, 2, 4, 0)); // gets sorted using Comparator
+            List<Person> persons2 = repository.findByIntSetGreaterThan(setToCompareWith2);
+            assertThat(persons2).contains(dave);
+
+            List<Integer> listToCompareWith = List.of(3, 1, 2, 0, 4); // the insertion order is preserved
+            List<Person> persons3 = repository.findByIntSetGreaterThan(listToCompareWith);
+            assertThat(persons3).doesNotContain(dave);
+
+            // gets sorted using natural order
+            PriorityQueue<Integer> queueToCompareWith = new PriorityQueue<>(Set.of(3, 1, 2, 4, 0));
+            List<Person> persons4 = repository.findByIntSetGreaterThan(queueToCompareWith);
+            assertThat(persons4).contains(dave);
+        }
+    }
+
     @Test
     void findByCollectionGreaterThan() {
         List<Integer> listToCompare1 = List.of(100, 200, 300, 400);

src/test/java/org/springframework/data/aerospike/repository/query/blocking/noindex/findBy/InTests.java

@@ -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()) {
@@ -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()) {