DATACOUCH-603 Do not cast query parameters in N1qlQueryCreator.

For IN and NOT_IN - they can take varargs, an array or a JsonArray
Don't cast query criteria parameters, let parameter accessor handle that.
Fixed conversion of query criteria values to parameters
Cleaned up QueryCriteria

Co-authored-by: mikereiche <[email protected]>

Author: mikereiche

src/main/java/org/springframework/data/couchbase/core/query/Query.java

@@ -26,6 +26,7 @@
 import com.couchbase.client.java.query.QueryOptions;
 import com.couchbase.client.java.query.QueryScanConsistency;
 import org.springframework.data.couchbase.core.ReactiveCouchbaseTemplate;
+import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity;
 import org.springframework.data.couchbase.repository.query.StringBasedN1qlQueryParser;
 import org.springframework.data.couchbase.repository.support.MappingCouchbaseEntityInformation;
@@ -189,7 +190,7 @@ public void appendSort(final StringBuilder sb) {
 		sb.deleteCharAt(sb.length() - 1);
 	}
 
-	public void appendWhere(final StringBuilder sb, int[] paramIndexPtr) {
+	public void appendWhere(final StringBuilder sb, int[] paramIndexPtr, CouchbaseConverter converter) {
 		if (!criteria.isEmpty()) {
 			appendWhereOrAnd(sb);
 			boolean first = true;
@@ -199,16 +200,11 @@ public void appendWhere(final StringBuilder sb, int[] paramIndexPtr) {
 				} else {
 					sb.append(" AND ");
 				}
-				sb.append(c.export(paramIndexPtr));
+				sb.append(c.export(paramIndexPtr, parameters, converter));
 			}
 		}
 	}
 
-	public void appendCriteria(StringBuilder sb, QueryCriteria criteria) {
-		appendWhereOrAnd(sb);
-		sb.append(criteria.export());
-	}
-
 	public void appendWhereString(StringBuilder sb, String whereString) {
 		appendWhereOrAnd(sb);
 		sb.append(whereString);
@@ -257,9 +253,9 @@ private static boolean notQuoted(int start, int end, String querySoFar) {
 		return true; // is not quoted
 	}
 
-	public String export() {
+	public String export(int[]... paramIndexPtrHolder) { // used only by tests
 		StringBuilder sb = new StringBuilder();
-		appendWhere(sb, null);
+		appendWhere(sb, paramIndexPtrHolder.length > 0 ? paramIndexPtrHolder[0] : null, null);
 		appendSort(sb);
 		appendSkipAndLimit(sb);
 		return sb.toString();
@@ -270,7 +266,7 @@ public String toN1qlSelectString(ReactiveCouchbaseTemplate template, Class domai
 		final StringBuilder statement = new StringBuilder();
 		appendString(statement, n1ql.selectEntity); // select ...
 		appendWhereString(statement, n1ql.filter); // typeKey = typeValue
-		appendWhere(statement, new int[] { 0 }); // criteria on this Query
+		appendWhere(statement, new int[] { 0 }, template.getConverter()); // criteria on this Query
 		appendSort(statement);
 		appendSkipAndLimit(statement);
 		return statement.toString();
@@ -281,7 +277,7 @@ public String toN1qlRemoveString(ReactiveCouchbaseTemplate template, Class domai
 		final StringBuilder statement = new StringBuilder();
 		appendString(statement, n1ql.delete); // delete ...
 		appendWhereString(statement, n1ql.filter); // typeKey = typeValue
-		appendWhere(statement, null); // criteria on this Query
+		appendWhere(statement, null, template.getConverter()); // criteria on this Query
 		appendString(statement, n1ql.returning);
 		return statement.toString();
 	}

src/main/java/org/springframework/data/couchbase/core/query/QueryCriteria.java

@@ -20,6 +20,11 @@
 import java.util.LinkedList;
 import java.util.List;
 
+import com.couchbase.client.core.error.InvalidArgumentException;
+import com.couchbase.client.java.json.JsonArray;
+import com.couchbase.client.java.json.JsonObject;
+import com.couchbase.client.java.json.JsonValue;
+import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
 import org.springframework.lang.Nullable;
 
 /**
@@ -60,6 +65,10 @@ public class QueryCriteria implements QueryCriteriaDefinition {
 		this.format = format;
 	}
 
+	Object[] getValue() {
+		return value;
+	}
+
 	/**
 	 * Static factory method to create a Criteria using the provided key.
 	 */
@@ -68,8 +77,8 @@ public static QueryCriteria where(String key) {
 	}
 
 	private static QueryCriteria wrap(QueryCriteria criteria) {
-		QueryCriteria qc = new QueryCriteria(new LinkedList<QueryCriteria>(), criteria.key, criteria.value, null,
-				criteria.operator, criteria.format);
+		QueryCriteria qc = new QueryCriteria(new LinkedList<>(), criteria.key, criteria.value, null, criteria.operator,
+				criteria.format);
 		return qc;
 	}
 
@@ -167,7 +176,7 @@ public QueryCriteria containing(@Nullable Object o) {
 	public QueryCriteria notContaining(@Nullable Object o) {
 		value = new QueryCriteria[] { wrap(containing(o)) };
 		operator = "NOT";
-		format = format = "not( %3$s )";
+		format = "not( %3$s )";
 		return this;
 	}
 
@@ -196,7 +205,7 @@ public QueryCriteria isNotNull() {
 		operator = "IS_NOT_NULL";
 		value = null;
 		format = "%1$s is not null";
-		return (QueryCriteria) this;
+		return this;
 	}
 
 	public QueryCriteria isMissing() {
@@ -210,7 +219,7 @@ public QueryCriteria isNotMissing() {
 		operator = "IS_NOT_MiSSING";
 		value = null;
 		format = "%1$s is not missing";
-		return (QueryCriteria) this;
+		return this;
 	}
 
 	public QueryCriteria isValued() {
@@ -224,68 +233,74 @@ public QueryCriteria isNotValued() {
 		operator = "IS_NOT_VALUED";
 		value = null;
 		format = "%1$s is not valued";
-		return (QueryCriteria) this;
+		return this;
 	}
 
 	public QueryCriteria within(@Nullable Object o) {
 		operator = "WITHIN";
 		value = new Object[] { o };
-		format = "%1$s within $3$s";
-		return (QueryCriteria) this;
+		format = "%1$s within %3$s";
+		return this;
 	}
 
 	public QueryCriteria between(@Nullable Object o1, @Nullable Object o2) {
 		operator = "BETWEEN";
 		value = new Object[] { o1, o2 };
 		format = "%1$s between %3$s and %4$s";
-		return (QueryCriteria) this;
+		return this;
 	}
 
 	public QueryCriteria in(@Nullable Object... o) {
 		operator = "IN";
-		value = o;
-		StringBuilder sb = new StringBuilder("%1$s in ( [ ");
-		for (int i = 1; i <= value.length; i++) { // format indices start at 1
-			if (i > 1)
-				sb.append(", ");
-			sb.append("%" + (i + 2) + "$s"); // the first is fieldName, second is operator, args start at 3
+		format = "%1$s in ( %3$s )";
+		// IN takes a single argument that is a list
+		if (o.length > 0) {
+			if (o[0] instanceof JsonArray || o[0] instanceof List || o[0] instanceof Object[]) {
+				if (o.length != 1) {
+					throw new RuntimeException("IN cannot take multiple lists");
+				}
+				value = o;
+			} else {
+				value = new Object[1];
+				value[0] = o; // JsonArray.from(o);
+			}
 		}
-		format = sb.append(" ] )").toString();
-		return (QueryCriteria) this;
+		return this;
 	}
 
 	public QueryCriteria notIn(@Nullable Object... o) {
 		value = new QueryCriteria[] { wrap(in(o)) };
 		operator = "NOT";
-		format = format = "not( %3$s )"; // field = 1$, operator = 2$, value=$3, $4, ...
-		return (QueryCriteria) this;
+		format = "not( %3$s )"; // field = 1$, operator = 2$, value=$3, $4, ...
+		return this;
 	}
 
 	public QueryCriteria TRUE() { // true/false are reserved, use TRUE/FALSE
 		value = null;
 		operator = null;
-		format = format = "%1$s"; // field = 1$, operator = 2$, value=$3, $4, ...
-		return (QueryCriteria) this;
+		format = "%1$s"; // field = 1$, operator = 2$, value=$3, $4, ...
+		return this;
 	}
 
 	public QueryCriteria FALSE() {
 		value = new QueryCriteria[] { wrap(TRUE()) };
 		operator = "not";
-		format = format = "not( %3$s )";
-		return (QueryCriteria) this;
+		format = "not( %3$s )";
+		return this;
 	}
 
 	/**
-	 * This exports the query criteria into a string to be appended to the beginning of an N1QL statement
+	 * This exports the query criteria chain into a string to be appended to the beginning of an N1QL statement
 	 *
-	 * @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters
-	 *                      There may already be positional parameters in the beginning of the statement,
-	 *                      so it may not always start at 1.  If it has the value -1, the query is using
-	 *                      named parameters. If the pointer is null, the query is not using parameters.
+	 * @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters There may
+	 *          already be positional parameters in the beginning of the statement, so it may not always start at 1. If it
+	 *          has the value -1, the query is using named parameters. If the pointer is null, the query is not using
+	 *          parameters.
+	 * @param parameters - parameters of the query. If operands are parameterized, their values are added to parameters
 	 * @return string containing part of N1QL query
 	 */
 	@Override
-	public String export(int[] paramIndexPtr) {
+	public String export(int[] paramIndexPtr, JsonValue parameters, CouchbaseConverter converter) {
 		StringBuilder output = new StringBuilder();
 		boolean first = true;
 		for (QueryCriteria c : this.criteriaChain) {
@@ -298,7 +313,7 @@ public String export(int[] paramIndexPtr) {
 			} else {
 				first = false;
 			}
-			c.exportSingle(output, paramIndexPtr);
+			c.exportSingle(output, paramIndexPtr, parameters, converter);
 		}
 
 		return output.toString();
@@ -310,22 +325,34 @@ public String export(int[] paramIndexPtr) {
 	 * @return string containing part of N1QL query
 	 */
 	@Override
-	public String export() {
-		return export(null);
+	public String export() { // used only by tests
+		return export(null, null, null);
 
 	}
 
-	private StringBuilder exportSingle(StringBuilder sb, int[] paramIndexPtr) {
+	/**
+	 * Appends the query criteria to a StringBuilder which will be appended to a N1QL statement
+	 *
+	 * @param sb - the string builder
+	 * @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters There may
+	 *          already be positional parameters in the beginning of the statement, so it may not always start at 1. If it
+	 *          has the value -1, the query is using named parameters. If the pointer is null, the query is not using
+	 *          parameters.
+	 * @param parameters - parameters of the query. If operands are parameterized, their values are added to parameters
+	 * @return string containing part of N1QL query
+	 */
+	private StringBuilder exportSingle(StringBuilder sb, int[] paramIndexPtr, JsonValue parameters,
+			CouchbaseConverter converter) {
 		String fieldName = maybeQuote(key);
 		int valueLen = value == null ? 0 : value.length;
 		Object[] v = new Object[valueLen + 2];
 		v[0] = fieldName;
 		v[1] = operator;
 		for (int i = 0; i < valueLen; i++) {
 			if (value[i] instanceof QueryCriteria) {
-				v[i + 2] = "(" + ((QueryCriteria) value[i]).export(paramIndexPtr) + ")";
+				v[i + 2] = "(" + ((QueryCriteria) value[i]).export(paramIndexPtr, parameters, converter) + ")";
 			} else {
-				v[i + 2] = maybeWrapValue(key, value[i], paramIndexPtr);
+				v[i + 2] = maybeWrapValue(key, value[i], paramIndexPtr, parameters, converter);
 			}
 		}
 
@@ -340,24 +367,84 @@ private StringBuilder exportSingle(StringBuilder sb, int[] paramIndexPtr) {
 		return sb;
 	}
 
-	private String maybeWrapValue(String key, Object value, int[] paramIndexPtr) {
+	/**
+	 * Possibly convert an operand to a positional or named parameter
+	 *
+	 * @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters There may
+	 *          already be positional parameters in the beginning of the statement, so it may not always start at 1. If it
+	 *          has the value -1, the query is using named parameters. If the pointer is null, the query is not using
+	 *          parameters.
+	 * @param parameters - parameters of the query. If operands are parameterized, their values are added to parameters
+	 * @return string containing part of N1QL query
+	 */
+	private String maybeWrapValue(String key, Object value, int[] paramIndexPtr, JsonValue parameters,
+			CouchbaseConverter converter) {
 		if (paramIndexPtr != null) {
 			if (paramIndexPtr[0] >= 0) {
+				JsonArray params = (JsonArray) parameters;
+				// from StringBasedN1qlQueryParser.getPositionalPlaceholderValues()
+				try {
+					params.add(convert(converter, value));
+				} catch (InvalidArgumentException iae) {
+					if (value instanceof Object[]) {
+						addAsArray(params, value, converter);
+					} else {
+						throw iae;
+					}
+				}
 				return "$" + (++paramIndexPtr[0]); // these are generated in order
 			} else {
+				JsonObject params = (JsonObject) parameters;
+				// from StringBasedN1qlQueryParser.getNamedPlaceholderValues()
+				try {
+					params.put(key, convert(converter, value));
+				} catch (InvalidArgumentException iae) {
+					if (value instanceof Object[]) {
+						params.put(key, JsonArray.from((Object[]) value));
+					} else {
+						throw iae;
+					}
+				}
 				return "$" + key;
 			}
 		}
 
+		// Did not convert to a parameter. Add quotes or whatever it might need.
+
 		if (value instanceof String) {
 			return "\"" + value + "\"";
 		} else if (value == null) {
 			return "null";
+		} else if (value instanceof Object[]) { // convert array into sequence of comma-separated values
+			StringBuffer l = new StringBuffer();
+			l.append("[");
+			Object[] array = (Object[]) value;
+			for (int i = 0; i < array.length; i++) {
+				if (i > 0) {
+					l.append(",");
+				}
+				l.append(maybeWrapValue(null, array[i], null, null, converter));
+			}
+			l.append("]");
+			return l.toString();
 		} else {
 			return value.toString();
 		}
 	}
 
+	private static Object convert(CouchbaseConverter converter, Object value) {
+		return converter != null ? converter.convertForWriteIfNeeded(value) : value;
+	}
+
+	private void addAsArray(JsonArray posValues, Object o, CouchbaseConverter converter) {
+		Object[] array = (Object[]) o;
+		JsonArray ja = JsonValue.ja();
+		for (Object e : array) {
+			ja.add(String.valueOf(convert(converter, e)));
+		}
+		posValues.add(ja);
+	}
+
 	private String maybeQuote(String value) {
 		if (value == null || (value.startsWith("\"") && value.endsWith("\""))) {
 			return value;

src/main/java/org/springframework/data/couchbase/core/query/QueryCriteriaDefinition.java

@@ -15,6 +15,9 @@
  */
 package org.springframework.data.couchbase.core.query;
 
+import com.couchbase.client.java.json.JsonValue;
+import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
+
 /**
  * @author Oliver Gierke
  * @author Christoph Strobl
@@ -25,13 +28,15 @@ public interface QueryCriteriaDefinition {
 	/**
 	 * This exports the query criteria into a string to be appended to the beginning of an N1QL statement
 	 *
-	 * @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters
-	 *                      There may already be positional parameters in the beginning of the statement,
-	 *                      so it may not always start at 1.  If it has the value -1, the query is using
-	 *                      named parameters. If the pointer is null, the query is not using parameters.
+	 * @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters There may
+	 *          already be positional parameters in the beginning of the statement, so it may not always start at 1. If it
+	 *          has the value -1, the query is using named parameters. If the pointer is null, the query is not using
+	 *          parameters.
+	 * @param parameters - query parameters. Criteria values that are converted to arguments are added to parameters
+	 * @param converter - converter to use for converting criteria values
 	 * @return string containing part of N1QL query
 	 */
-	String export(int[] paramIndexPtr);
+	String export(int[] paramIndexPtr, JsonValue parameters, CouchbaseConverter converter);
 
 	/**
 	 * Export the query criteria to a string without using positional or named parameters.