FMWK-426 Validate mandatory parameters in custom queries (#742)

* validate mandatory parameters in custom queries
aerospike · May 19, 2024 · caa1311 · caa1311
1 parent 2658de3
commit caa1311
Show file tree

Hide file tree

Showing 4 changed files with 67 additions and 23 deletions.
diff --git a/src/main/java/org/springframework/data/aerospike/query/qualifier/BaseQualifierBuilder.java b/src/main/java/org/springframework/data/aerospike/query/qualifier/BaseQualifierBuilder.java
@@ -1,5 +1,6 @@
 package org.springframework.data.aerospike.query.qualifier;
 
+import com.aerospike.client.Value;
 import org.springframework.data.aerospike.query.FilterOperation;
 
 import java.util.Collections;
@@ -23,6 +24,9 @@ public FilterOperation getFilterOperation() {
         return (FilterOperation) map.get(OPERATION);
     }
 
+    /**
+     * Set filter operation. Mandatory parameter.
+     */
     public T setFilterOperation(FilterOperation filterOperation) {
         map.put(OPERATION, filterOperation);
         return (T) this;
@@ -40,6 +44,10 @@ public boolean hasValue() {
         return map.get(VALUE) != null;
     }
 
+    public Value getValue() {
+        return (Value) map.get(VALUE);
+    }
+
     public boolean hasSecondValue() {
         return map.get(SECOND_VALUE) != null;
     }

diff --git a/...ain/java/org/springframework/data/aerospike/query/qualifier/MetadataQualifierBuilder.java b/...ain/java/org/springframework/data/aerospike/query/qualifier/MetadataQualifierBuilder.java
@@ -20,6 +20,9 @@ public CriteriaDefinition.AerospikeMetadata getMetadataField() {
         return (CriteriaDefinition.AerospikeMetadata) map.get(METADATA_FIELD);
     }
 
+    /**
+     * Set metadata field. Mandatory parameter for metadata query.
+     */
     public MetadataQualifierBuilder setMetadataField(CriteriaDefinition.AerospikeMetadata metadataField) {
         this.map.put(METADATA_FIELD, metadataField);
         return this;
@@ -38,6 +41,9 @@ public Object getValueAsObj() {
         return this.map.get(VALUE);
     }
 
+    /**
+     * Set value as Object. Mandatory parameter for metadata query.
+     */
     public MetadataQualifierBuilder setValueAsObj(Object object) {
         this.map.put(VALUE, object);
         return this;
@@ -52,32 +58,42 @@ public MetadataQualifierBuilder setSecondValueAsObj(Object object) {
         return this;
     }
 
-    @SuppressWarnings("unchecked")
     @Override
     protected void validate() {
-        // metadata query
+        // metadata query validation
         if (this.getMetadataField() != null) {
             if (this.getBinName() == null) {
-                FilterOperation operation = this.getFilterOperation();
-                switch (operation) {
-                    case EQ, NOTEQ, LT, LTEQ, GT, GTEQ -> Assert.isTrue(getValueAsObj() instanceof Long,
-                        operation.name() + ": value1 is expected to be set as Long");
-                    case BETWEEN -> {
-                        Assert.isTrue(getValueAsObj() instanceof Long,
-                            "BETWEEN: value1 is expected to be set as Long");
-                        Assert.isTrue(getSecondValueAsObj() instanceof Long,
-                            "BETWEEN: value2 is expected to be set as Long");
-                    }
-                    case NOT_IN, IN -> Assert.isTrue(getValueAsObj() instanceof Collection
-                            && !((Collection<Object>) getValueAsObj()).isEmpty()
-                            && ((Collection<Object>) getValueAsObj()).toArray()[0] instanceof Long,
-                        operation.name() + ": value1 is expected to be a non-empty Collection<Long>");
-                    default -> throw new IllegalArgumentException("Operation " + operation + " cannot be applied to " +
-                        "metadataField");
+                if (this.getValueAsObj() != null) {
+                    validateValueAsObj();
+                } else {
+                    throw new IllegalArgumentException("Expecting valueAsObj parameter to be provided");
                 }
             } else {
-                throw new IllegalArgumentException("Either a field or a metadataField must be set, not both");
+                throw new IllegalArgumentException("Unexpected parameter: bin name (unnecessary for metadata query)");
             }
+        } else {
+            throw new IllegalArgumentException("Expecting metadataField parameter to be provided");
+        }
+    }
+
+    private void validateValueAsObj() {
+        FilterOperation operation = this.getFilterOperation();
+        switch (operation) {
+            case EQ, NOTEQ, LT, LTEQ, GT, GTEQ -> Assert.isTrue(getValueAsObj() instanceof Long,
+                operation.name() + ": value1 is expected to be set as Long");
+            case BETWEEN -> {
+                Assert.isTrue(getValueAsObj() instanceof Long,
+                    "BETWEEN: value1 is expected to be set as Long");
+                Assert.isTrue(getSecondValueAsObj() instanceof Long,
+                    "BETWEEN: value2 is expected to be set as Long");
+            }
+            case NOT_IN, IN -> //noinspection unchecked
+                Assert.isTrue(getValueAsObj() instanceof Collection
+                        && !((Collection<Object>) getValueAsObj()).isEmpty()
+                        && ((Collection<Object>) getValueAsObj()).toArray()[0] instanceof Long,
+                    operation.name() + ": value1 is expected to be a non-empty Collection<Long>");
+            default -> throw new IllegalArgumentException("Operation " + operation + " cannot be applied to " +
+                "metadataField");
         }
     }
 }
diff --git a/src/main/java/org/springframework/data/aerospike/query/qualifier/QualifierBuilder.java b/src/main/java/org/springframework/data/aerospike/query/qualifier/QualifierBuilder.java
@@ -3,7 +3,9 @@
 import com.aerospike.client.Value;
 import com.aerospike.client.command.ParticleType;
 import com.aerospike.client.exp.Exp;
+import org.springframework.data.aerospike.query.FilterOperation;
 import org.springframework.data.aerospike.server.version.ServerVersionSupport;
+import org.springframework.util.StringUtils;
 
 import java.util.List;
 
@@ -20,23 +22,23 @@ public QualifierBuilder setIgnoreCase(boolean ignoreCase) {
     }
 
     /**
-     * Set bin name.
+     * Set bin name. Mandatory parameter for bin query.
      */
     public QualifierBuilder setBinName(String field) {
         this.map.put(BIN_NAME, field);
         return this;
     }
 
     /**
-     * Set bin name.
+     * Set bin type.
      */
     public QualifierBuilder setBinType(Exp.Type type) {
         this.map.put(BIN_TYPE, type);
         return this;
     }
 
     /**
-     * Set full path from bin name to required element
+     * Set full path from bin name to required element.
      */
     public QualifierBuilder setDotPath(List<String> dotPath) {
         this.map.put(DOT_PATH, dotPath);
@@ -82,7 +84,8 @@ public QualifierBuilder setNestedKey(Value key) {
     }
 
     /**
-     * Set value.
+     * Set value. Mandatory parameter for bin query for all operations except {@link FilterOperation#IS_NOT_NULL} and
+     * {@link FilterOperation#IS_NULL}.
      * <p>
      * Use one of the Value get() methods ({@link Value#get(int)}, {@link Value#get(String)} etc.) to firstly read the
      * value into a {@link Value} object.
@@ -119,4 +122,20 @@ public QualifierBuilder setServerVersionSupport(ServerVersionSupport serverVersi
         this.map.put(SERVER_VERSION_SUPPORT, serverVersionSupport);
         return this;
     }
+
+    protected void validate() {
+        if (!StringUtils.hasText(this.getBinName())) {
+            throw new IllegalArgumentException("Expecting bin name parameter to be provided");
+        }
+
+        if (this.getFilterOperation() == null) {
+            throw new IllegalArgumentException("Expecting filter operation parameter to be provided");
+        }
+
+        if (this.getValue() == null
+            && this.getFilterOperation() != FilterOperation.IS_NULL
+            && this.getFilterOperation() != FilterOperation.IS_NOT_NULL) {
+            throw new IllegalArgumentException("Expecting value parameter to be provided");
+        }
+    }
 }
diff --git a/...framework/data/aerospike/repository/query/blocking/noindex/findBy/CustomQueriesTests.java b/...framework/data/aerospike/repository/query/blocking/noindex/findBy/CustomQueriesTests.java
@@ -190,6 +190,7 @@ void findPersonsByQueryMustBeValid() {
         assertThatThrownBy(() -> repository.findUsingQuery(new Query(Qualifier.metadataBuilder()
             .setMetadataField(SINCE_UPDATE_TIME)
             .setFilterOperation(FilterOperation.BETWEEN)
+            .setValueAsObj("value")
             .setSecondValueAsObj(1L)
             .build())))
             .isInstanceOf(IllegalArgumentException.class)