FMWK-352 Update creating secondary index filter (#733)

* Add fixes/updates to create secondary index filter correctly * Add tests annotations to automatically check for indexed bins
aerospike · Apr 24, 2024 · e4ad264 · e4ad264
1 parent 0d491ed
commit e4ad264
Show file tree

Hide file tree

Showing 38 changed files with 761 additions and 269 deletions.
diff --git a/src/main/java/org/springframework/data/aerospike/core/AerospikeTemplate.java b/src/main/java/org/springframework/data/aerospike/core/AerospikeTemplate.java
@@ -38,7 +38,6 @@
 import org.springframework.data.aerospike.index.IndexesCacheRefresher;
 import org.springframework.data.aerospike.mapping.AerospikeMappingContext;
 import org.springframework.data.aerospike.mapping.AerospikePersistentEntity;
-import org.springframework.data.aerospike.mapping.AerospikePersistentProperty;
 import org.springframework.data.aerospike.query.KeyRecordIterator;
 import org.springframework.data.aerospike.query.QueryEngine;
 import org.springframework.data.aerospike.query.cache.IndexRefresher;
@@ -47,7 +46,6 @@
 import org.springframework.data.aerospike.server.version.ServerVersionSupport;
 import org.springframework.data.aerospike.util.Utils;
 import org.springframework.data.domain.Sort;
-import org.springframework.data.mapping.PropertyHandler;
 import org.springframework.data.util.StreamUtils;
 import org.springframework.util.Assert;
 
@@ -76,6 +74,7 @@
 import static org.springframework.data.aerospike.core.CoreUtils.operations;
 import static org.springframework.data.aerospike.core.CoreUtils.verifyUnsortedWithOffset;
 import static org.springframework.data.aerospike.core.TemplateUtils.excludeIdQualifier;
+import static org.springframework.data.aerospike.core.TemplateUtils.getBinNamesFromTargetClass;
 import static org.springframework.data.aerospike.core.TemplateUtils.getIdValue;
 import static org.springframework.data.aerospike.query.QualifierUtils.getIdQualifier;
 import static org.springframework.data.aerospike.query.QualifierUtils.queryCriteriaIsNotNull;
@@ -802,7 +801,7 @@ private Key[] getKeys(Collection<?> ids, String setName) {
     private <S> Object getRecordMapToTargetClass(AerospikePersistentEntity<?> entity, Key key, Class<S> targetClass,
                                                  Query query) {
         Record aeroRecord;
-        String[] binNames = getBinNamesFromTargetClass(targetClass);
+        String[] binNames = getBinNamesFromTargetClass(targetClass, mappingContext);
         if (entity.isTouchOnRead()) {
             Assert.state(!entity.hasExpirationProperty(), "Touch on read is not supported for expiration property");
             aeroRecord = getAndTouch(key, entity.getExpiration(), binNames, query);
@@ -813,17 +812,6 @@ private <S> Object getRecordMapToTargetClass(AerospikePersistentEntity<?> entity
         return mapToEntity(key, targetClass, aeroRecord);
     }
 
-    private String[] getBinNamesFromTargetClass(Class<?> targetClass) {
-        AerospikePersistentEntity<?> targetEntity = mappingContext.getRequiredPersistentEntity(targetClass);
-
-        List<String> binNamesList = new ArrayList<>();
-
-        targetEntity.doWithProperties((PropertyHandler<AerospikePersistentProperty>) property
-            -> binNamesList.add(property.getFieldName()));
-
-        return binNamesList.toArray(new String[0]);
-    }
-
     private Policy getPolicyFilterExp(Query query) {
         if (queryCriteriaIsNotNull(query)) {
             Policy policy = new Policy(getAerospikeClient().getReadPolicyDefault());
@@ -981,7 +969,7 @@ public <T, S> List<?> findByIdsUsingQuery(Collection<?> ids, Class<T> entityClas
             Class<?> target;
             Record[] aeroRecords;
             if (targetClass != null && targetClass != entityClass) {
-                String[] binNames = getBinNamesFromTargetClass(targetClass);
+                String[] binNames = getBinNamesFromTargetClass(targetClass, mappingContext);
                 aeroRecords = getAerospikeClient().get(policy, keys, binNames);
                 target = targetClass;
             } else {
@@ -1438,7 +1426,7 @@ private <T> Stream<KeyRecord> findRecordsUsingQuery(String setName, Class<T> tar
         KeyRecordIterator recIterator;
 
         if (targetClass != null) {
-            String[] binNames = getBinNamesFromTargetClass(targetClass);
+            String[] binNames = getBinNamesFromTargetClass(targetClass, mappingContext);
             recIterator = queryEngine.select(namespace, setName, binNames, query);
         } else {
             recIterator = queryEngine.select(namespace, setName, query);
@@ -1468,7 +1456,7 @@ private List<KeyRecord> findByIdsWithoutMapping(Collection<?> ids, String setNam
 
             Record[] aeroRecords;
             if (targetClass != null) {
-                String[] binNames = getBinNamesFromTargetClass(targetClass);
+                String[] binNames = getBinNamesFromTargetClass(targetClass, mappingContext);
                 aeroRecords = getAerospikeClient().get(policy, keys, binNames);
             } else {
                 aeroRecords = getAerospikeClient().get(policy, keys);

diff --git a/src/main/java/org/springframework/data/aerospike/core/TemplateUtils.java b/src/main/java/org/springframework/data/aerospike/core/TemplateUtils.java
@@ -1,8 +1,13 @@
 package org.springframework.data.aerospike.core;
 
 import lombok.experimental.UtilityClass;
+import org.springframework.data.aerospike.mapping.AerospikePersistentEntity;
+import org.springframework.data.aerospike.mapping.AerospikePersistentProperty;
+import org.springframework.data.aerospike.mapping.BasicAerospikePersistentEntity;
 import org.springframework.data.aerospike.query.FilterOperation;
 import org.springframework.data.aerospike.query.qualifier.Qualifier;
+import org.springframework.data.mapping.PropertyHandler;
+import org.springframework.data.mapping.context.MappingContext;
 import org.springframework.util.Assert;
 
 import java.util.ArrayList;
@@ -93,4 +98,18 @@ private static Qualifier combineMultipleQualifiers(FilterOperation operation, Qu
             throw new UnsupportedOperationException("Only OR / AND operations are supported");
         }
     }
+
+    public static String[] getBinNamesFromTargetClass(Class<?> targetClass,
+                                                      MappingContext<BasicAerospikePersistentEntity<?>,
+                                                          AerospikePersistentProperty> mappingContext) {
+        AerospikePersistentEntity<?> targetEntity = mappingContext.getRequiredPersistentEntity(targetClass);
+
+        List<String> binNamesList = new ArrayList<>();
+
+        targetEntity.doWithProperties((PropertyHandler<AerospikePersistentProperty>) property
+            -> binNamesList.add(property.getFieldName()));
+
+        return binNamesList.toArray(new String[0]);
+    }
+
 }
diff --git a/src/main/java/org/springframework/data/aerospike/query/FilterExpressionsBuilder.java b/src/main/java/org/springframework/data/aerospike/query/FilterExpressionsBuilder.java
@@ -20,30 +20,31 @@
 import org.springframework.data.aerospike.query.qualifier.Qualifier;
 import org.springframework.data.aerospike.repository.query.Query;
 
+import static org.springframework.data.aerospike.query.FilterOperation.dualFilterOperations;
 import static org.springframework.data.aerospike.query.QualifierUtils.queryCriteriaIsNotNull;
 
 public class FilterExpressionsBuilder {
 
     public Expression build(Query query) {
         Qualifier qualifier = queryCriteriaIsNotNull(query) ? query.getCriteriaObject() : null;
-        if (qualifier != null && excludeIrrelevantFilters(qualifier)) {
-            return Exp.build(qualifier.toFilterExp());
+        if (qualifier != null && requiresFilterExp(qualifier)) {
+            return Exp.build(qualifier.getFilterExp());
         }
         return null;
     }
 
     /**
-     * The filter allows only qualifiers without sIndexFilter and those with the dualFilterOperation that require both
+     * FilterExp is built only for a qualifier without sIndexFilter or for dualFilterOperation that requires both
      * sIndexFilter and FilterExpression. The filter is irrelevant for AND operation (nested qualifiers)
      */
-    private boolean excludeIrrelevantFilters(Qualifier qualifier) {
-        if (!qualifier.queryAsFilter()) {
+    private boolean requiresFilterExp(Qualifier qualifier) {
+        if (!qualifier.hasSecIndexFilter()) {
             return true;
-        } else if (qualifier.queryAsFilter() && FilterOperation.dualFilterOperations.contains(qualifier.getOperation())) {
-            qualifier.setQueryAsFilter(false); // clear the flag in case if the same Qualifier is going to be reused
+        } else if (qualifier.hasSecIndexFilter() && dualFilterOperations.contains(qualifier.getOperation())) {
+            qualifier.setHasSecIndexFilter(false); // clear the flag in case if the same Qualifier is going to be reused
             return true;
         } else {
-            qualifier.setQueryAsFilter(false); // clear the flag in case if the same Qualifier is going to be reused
+            qualifier.setHasSecIndexFilter(false); // clear the flag in case if the same Qualifier is going to be reused
             return false;
         }
     }