DATACOUCH-603 Do not cast query parameters in N1qlQueryCreator.

For IN and NOT_IN - they can take varargs, and array or a JsonArray
- don't cast the parameter, let parameter accessor handle that.
- JsonArray.toString() provides the brackets, don't include them in the format
- Object[] array needs explicit conversion, as toString() is not sufficient

Also cleaned up some weirdness in QueryCriteria

Author: mikereiche

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

@@ -20,6 +20,7 @@
 import java.util.LinkedList;
 import java.util.List;
 
+import com.couchbase.client.java.json.JsonArray;
 import org.springframework.lang.Nullable;
 
 /**
@@ -68,8 +69,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 +168,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 +197,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 +211,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,64 +225,73 @@ 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;
+		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 ( [ ");
+		StringBuilder sb = new StringBuilder("%1$s in ( ");
+		boolean notArray = false;
+		// if parameter is an array, then toString() will provide brackets [ ... ]
+		if (!(value.length == 1 && (value[0] instanceof Object[] || value[0] instanceof JsonArray))) {
+			notArray = true;
+			sb.append("[");
+		}
 		for (int i = 1; i <= value.length; i++) { // format indices start at 1
 			if (i > 1)
-				sb.append(", ");
+				sb.append(",");
 			sb.append("%" + (i + 2) + "$s"); // the first is fieldName, second is operator, args start at 3
 		}
-		format = sb.append(" ] )").toString();
-		return (QueryCriteria) this;
+		if (notArray) {
+			sb.append("]");
+		}
+		format = sb.append(" )").toString();
+		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
 	 *
-	 * @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.
 	 * @return string containing part of N1QL query
 	 */
 	@Override
@@ -353,6 +363,18 @@ private String maybeWrapValue(String key, Object value, int[] paramIndexPtr) {
 			return "\"" + value + "\"";
 		} else if (value == null) {
 			return "null";
+		} else if (value instanceof Object[]) {
+			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));
+			}
+			l.append("]");
+			return l.toString();
 		} else {
 			return value.toString();
 		}

src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java

@@ -17,7 +17,7 @@
 
 import static org.springframework.data.couchbase.core.query.QueryCriteria.*;
 
-import java.lang.reflect.Array;
+import java.util.Arrays;
 import java.util.Iterator;
 
 import com.couchbase.client.core.error.InvalidArgumentException;
@@ -141,9 +141,9 @@ private QueryCriteria from(final Part part, final CouchbasePersistentProperty pr
 		case BETWEEN:
 			return criteria.between(parameters.next(), parameters.next());
 		case IN:
-			return criteria.in((Object[]) parameters.next());
+			return criteria.in(parameters.next());
 		case NOT_IN:
-			return criteria.notIn((Object[]) parameters.next());
+			return criteria.notIn(parameters.next());
 		case TRUE:
 			return criteria.TRUE();
 		case FALSE:
@@ -159,15 +159,14 @@ private JsonValue getPositionalPlaceholderValues(ParameterAccessor accessor) {
 		if (queryMethod == null)
 			return posValues;
 		for (Parameter parameter : this.queryMethod.getParameters().getBindableParameters()) {
+			Object param = accessor.getBindableValue(parameter.getIndex());
 			try {
-				posValues.add(converter.convertForWriteIfNeeded(accessor.getBindableValue(parameter.getIndex())));
+				posValues.add(converter.convertForWriteIfNeeded(param));
 			} catch (InvalidArgumentException iae) {
-				Object o = accessor.getBindableValue(parameter.getIndex());
-				if (o instanceof Object[]) {
-					Object[] array = (Object[]) o;
-					for (Object e : array) {
-						posValues.add(converter.convertForWriteIfNeeded(e));
-					}
+				if (param instanceof Object[]) {
+					posValues.add(JsonArray.from(Arrays.asList((Object[]) param)));
+				} else {
+					throw iae;
 				}
 			}
 		}

src/test/java/org/springframework/data/couchbase/core/query/QueryCriteriaTests.java

@@ -70,7 +70,7 @@ void testNestedNotIn() {
 		QueryCriteria c = where("name").is("Bubba").or(where("age").gt(12).or("country").is("Austria")).and(
 				where("state").notIn(new String[] { "Alabama", "Florida" }));
 		assertEquals("`name` = \"Bubba\" or (`age` > 12 or `country` = \"Austria\") and "
-				+ "(not( (`state` in ( [ \"Alabama\", \"Florida\" ] )) ))", c.export());
+				+ "(not( (`state` in ( [\"Alabama\",\"Florida\"] )) ))", c.export());
 	}
 
 	@Test
@@ -205,13 +205,13 @@ void testBetween() {
 	@Test
 	void testIn() {
 		QueryCriteria c = where("name").in(new String[] { "gump", "davis" });
-		assertEquals("`name` in ( [ \"gump\", \"davis\" ] )", c.export());
+		assertEquals("`name` in ( [\"gump\",\"davis\"] )", c.export());
 	}
 
 	@Test
 	void testNotIn() {
 		QueryCriteria c = where("name").notIn(new String[] { "gump", "davis" });
-		assertEquals("not( (`name` in ( [ \"gump\", \"davis\" ] )) )", c.export());
+		assertEquals("not( (`name` in ( [\"gump\",\"davis\"] )) )", c.export());
 	}
 
 	@Test

src/test/java/org/springframework/data/couchbase/domain/UserRepository.java

@@ -18,6 +18,7 @@
 
 import java.util.List;
 
+import com.couchbase.client.java.json.JsonArray;
 import org.springframework.data.couchbase.repository.Query;
 import org.springframework.data.repository.PagingAndSortingRepository;
 import org.springframework.data.repository.query.Param;
@@ -34,6 +35,10 @@ public interface UserRepository extends PagingAndSortingRepository<User, String>
 
 	List<User> findByFirstname(String firstname);
 
+	List<User> findByFirstnameIn(String... firstnames);
+
+	List<User> findByFirstnameIn(JsonArray firstnames);
+
 	List<User> findByFirstnameAndLastname(String firstname, String lastname);
 
 	@Query("#{#n1ql.selectEntity} where #{#n1ql.filter} and firstname = $1 and lastname = $2")

src/test/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreatorTests.java

@@ -20,6 +20,7 @@
 
 import java.lang.reflect.Method;
 
+import com.couchbase.client.java.json.JsonArray;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
@@ -62,6 +63,35 @@ void createsQueryCorrectly() throws Exception {
 		assertEquals(query.export(), " WHERE " + where("firstname").is("Oliver").export());
 	}
 
+	@Test
+	void queryParametersArray() throws Exception {
+		String input = "findByFirstnameIn";
+		PartTree tree = new PartTree(input, User.class);
+		Method method = UserRepository.class.getMethod(input, String[].class);
+
+		N1qlQueryCreator creator = new N1qlQueryCreator(tree,
+				getAccessor(getParameters(method), new Object[] { new String[] { "Oliver", "Charles" } }), null, converter);
+		Query query = creator.createQuery();
+
+		assertEquals(" WHERE " + where("firstname").in("Oliver", "Charles").export(), query.export());
+	}
+
+	@Test
+	void queryParametersJsonArray() throws Exception {
+		String input = "findByFirstnameIn";
+		PartTree tree = new PartTree(input, User.class);
+		Method method = UserRepository.class.getMethod(input, JsonArray.class);
+
+		JsonArray jsonArray = JsonArray.create();
+		jsonArray.add("Oliver");
+		jsonArray.add("Charles");
+		N1qlQueryCreator creator = new N1qlQueryCreator(tree, getAccessor(getParameters(method), jsonArray), null,
+				converter);
+		Query query = creator.createQuery();
+
+		assertEquals(" WHERE " + where("firstname").in(new String[] { "Oliver", "Charles" }).export(), query.export());
+	}
+
 	@Test
 	void createsAndQueryCorrectly() throws Exception {
 		String input = "findByFirstnameAndLastname";
@@ -71,7 +101,7 @@ void createsAndQueryCorrectly() throws Exception {
 				converter);
 		Query query = creator.createQuery();
 
-		assertEquals(query.export(), " WHERE " + where("firstname").is("John").and("lastname").is("Doe").export());
+		assertEquals( " WHERE " + where("firstname").is("John").and("lastname").is("Doe").export(), query.export());
 	}
 
 	private ParameterAccessor getAccessor(Parameters<?, ?> params, Object... values) {