Fix regression in IN operator. (#1309)

The regression occurred in changes for https://issues.couchbase.com/browse/MB-26606.

Closes #1308.

Author: mikereiche

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

@@ -24,6 +24,7 @@
 import java.util.LinkedList;
 import java.util.List;
 
+import com.couchbase.client.core.error.CouchbaseException;
 import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
 import org.springframework.lang.Nullable;
 import org.springframework.util.CollectionUtils;
@@ -281,30 +282,34 @@ public QueryCriteria between(@Nullable Object o1, @Nullable Object o2) {
 
 	public QueryCriteria in(@Nullable Object... o) {
 		operator = "IN";
-		format = "%1$s in ( ";
+		format = "%1$s in %3$s";
+		value = new Object[1];
 		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");
 				}
 				if (o[0] instanceof Object[]) {
-					value = (Object[]) o[0];
+					value[0] = o[0];
 				} else if (o[0] instanceof JsonArray) {
 					JsonArray ja = ((JsonArray) o[0]);
-					value = ja.toList().toArray();
+					value[0] = ja.toList().toArray();
 				} else if (o[0] instanceof List) {
 					List l = ((List) o[0]);
-					value = l.toArray();
+					value[0] = l.toArray();
 				}
 			} else {
-				value = o;
-			}
-			for (int i = 0; i < value.length; i++) {
-				if (i > 0)
-					format = format + ", ";
-				format = format + "%" + (i + 3) + "$s";
+				// N1qlQueryCreatorTests.queryParametersArray()
+				// Query expected = (new Query()).addCriteria(where(i("firstname")).in("Oliver", "Charles"));
+				if (o instanceof Object[]) {
+					value[0] = o;
+				} else {
+					// see QueryCriteriaTests.testNestedNotIn() - if arg to notIn is not cast to Object
+					// notIn((Object) new String[] { "Alabama", "Florida" }));
+					throw new CouchbaseException("unhandled parameters "+o);
+				}
 			}
-			format = format + " )";
+
 		}
 		return this;
 	}

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

@@ -84,9 +84,9 @@ void testNestedOrCriteria() {
 	@Test
 	void testNestedNotIn() {
 		QueryCriteria c = where(i("name")).is("Bubba").or(where(i("age")).gt(12).or(i("country")).is("Austria"))
-				.and(where(i("state")).notIn(new String[] { "Alabama", "Florida" }));
+				.and(where(i("state")).notIn((Object) new String[] { "Alabama", "Florida" }));
 		JsonArray parameters = JsonArray.create();
-		assertEquals("`name` = $1 or (`age` > $2 or `country` = $3) and (not( (`state` in ( $4, $5 )) ))",
+		assertEquals("`name` = $1 or (`age` > $2 or `country` = $3) and (not( (`state` in $4) ))",
 				c.export(new int[1], parameters, null));
 	}
 
@@ -225,22 +225,22 @@ void testBetween() {
 	@Test
 	void testIn() {
 		String[] args = new String[] { "gump", "davis" };
-		QueryCriteria c = where(i("name")).in((Object) args);
-		assertEquals("`name` in ( \"gump\", \"davis\" )", c.export());
+		QueryCriteria c = where(i("name")).in((Object) args); // the first arg is an array
+		assertEquals("`name` in [\"gump\",\"davis\"]", c.export());
 		JsonArray parameters = JsonArray.create();
-		assertEquals("`name` in ( $1, $2 )", c.export(new int[1], parameters, null));
-		assertEquals(arrayToString(args), parameters.toString());
+		assertEquals("`name` in $1", c.export(new int[1], parameters, null));
+		assertEquals(arrayToString(args), parameters.get(0).toString());
 	}
 
 	@Test
 	void testNotIn() {
 		String[] args = new String[] { "gump", "davis" };
-		QueryCriteria c = where(i("name")).notIn((Object) args);
-		assertEquals("not( (`name` in ( \"gump\", \"davis\" )) )", c.export());
+		QueryCriteria c = where(i("name")).notIn((Object) args); // the first arg is an array
+		assertEquals("not( (`name` in [\"gump\",\"davis\"]) )", c.export());
 		// this tests creating parameters from the args.
 		JsonArray parameters = JsonArray.create();
-		assertEquals("not( (`name` in ( $1, $2 )) )", c.export(new int[1], parameters, null));
-		assertEquals(arrayToString(args), parameters.toString());
+		assertEquals("not( (`name` in $1) )", c.export(new int[1], parameters, null));
+		assertEquals(arrayToString(args), parameters.get(0).toString());
 	}
 
 	@Test

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

@@ -43,6 +43,7 @@
 import org.springframework.stereotype.Repository;
 
 import com.couchbase.client.java.analytics.AnalyticsScanConsistency;
+import com.couchbase.client.java.json.JsonArray;
 import com.couchbase.client.java.query.QueryScanConsistency;
 
 /**
@@ -60,6 +61,10 @@
 // @ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
 public interface AirportRepository extends CouchbaseRepository<Airport, String>, DynamicProxyable<AirportRepository> {
 
+	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
+	List<Airport> findByIataInAndIcaoIn(java.util.Collection<String> size, java.util.Collection<String> color,
+			Pageable pageable);
+
 	@Override
 	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
 	List<Airport> findAll();
@@ -81,7 +86,10 @@ public interface AirportRepository extends CouchbaseRepository<Airport, String>,
 	Airport findByIataIn(java.util.Collection<Iata> iatas);
 
 	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
-	Airport findByIataIn(Iata[] iata);
+	Airport findByIataIn(Iata... iatas);
+
+	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
+	Airport findByIataIn(JsonArray iatas);
 
 	// NOT_BOUNDED to test ScanConsistency
 	// @ScanConsistency(query = QueryScanConsistency.NOT_BOUNDED)

src/test/java/org/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java

@@ -36,6 +36,7 @@
 import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.LinkedList;
 import java.util.List;
 import java.util.Locale;
 import java.util.Optional;
@@ -236,6 +237,30 @@ void findByInjection() {
 
 	}
 
+	@Test
+	void issue1304CollectionParameter() {
+		Airport vie = null;
+		try {
+			vie = new Airport("airports::vie", "vie", "low5");
+			airportRepository.save(vie);
+			java.util.Collection<String> iatas = new LinkedList<String>();
+			iatas.add(vie.getIata());
+			java.util.Collection<String> icaos = new LinkedList<String>();
+			icaos.add(vie.getIcao());
+			icaos.add("blue");
+			PageRequest pageable = PageRequest.of( 0, 1, Sort.by("iata"));
+			List<Airport>airports = airportRepository.findByIataInAndIcaoIn(iatas, icaos, pageable);
+			assertEquals(1, airports.size());
+
+			List<Airport>airports2 = airportRepository.findByIataInAndIcaoIn(iatas, icaos, pageable);
+			assertEquals(1, airports2.size());
+
+		} finally {
+			airportRepository.delete(vie);
+		}
+
+	}
+
 	@Test
 	void findBySimpleProperty() {
 		Airport vie = null;
@@ -368,9 +393,11 @@ void findByTypeAlias() {
 	@Test
 	void findByEnum() {
 		Airport vie = null;
+		Airport zzz = null;
 		try {
 			vie = new Airport("airports::vie", "vie", "loww");
 			vie = airportRepository.save(vie);
+			zzz = airportRepository.save(vie.withId("airports::zzz").withIata("zzz"));
 			Airport airport2 = airportRepository.findByIata(Iata.vie);
 			assertNotNull(airport2, "should have found " + vie);
 			assertEquals(airport2.getId(), vie.getId());
@@ -385,8 +412,19 @@ void findByEnum() {
 			assertNotNull(airport4, "should have found " + vie);
 			assertEquals(airport4.getId(), vie.getId());
 
+			Airport airport5 = airportRepository.findByIataIn(Iata.vie, Iata.xxx);
+			assertNotNull(airport5, "should have found " + vie);
+			assertEquals(airport5.getId(), vie.getId());
+
+			JsonArray iatasJson = JsonArray.ja();
+			iatasJson.add(Iata.vie.toString());
+			iatasJson.add(Iata.xxx.toString());
+			Airport airport6 = airportRepository.findByIataIn(iatasJson);
+			assertNotNull(airport6, "should have found " + vie);
+			assertEquals(airport6.getId(), vie.getId());
 		} finally {
 			airportRepository.delete(vie);
+			airportRepository.delete(zzz);
 		}
 	}
 

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

@@ -109,7 +109,7 @@ void queryParametersArray() throws Exception {
 		Query query = creator.createQuery();
 
 		// Query expected = (new Query()).addCriteria(where("firstname").in("Oliver", "Charles"));
-		assertEquals(" WHERE `firstname` in ( $1, $2 )", query.export(new int[1]));
+		assertEquals(" WHERE `firstname` in $1", query.export(new int[1]));
 		JsonObject expectedOptions = JsonObject.create();
 		expected.buildQueryOptions(null, null).build().injectParams(expectedOptions);
 		JsonObject actualOptions = JsonObject.create();
@@ -132,7 +132,7 @@ void queryParametersJsonArray() throws Exception {
 		Query query = creator.createQuery();
 
 		Query expected = (new Query()).addCriteria(where(i("firstname")).in("Oliver", "Charles"));
-		assertEquals(" WHERE `firstname` in ( $1, $2 )", query.export(new int[1]));
+		assertEquals(" WHERE `firstname` in $1", query.export(new int[1]));
 		JsonObject expectedOptions = JsonObject.create();
 		expected.buildQueryOptions(null, null).build().injectParams(expectedOptions);
 		JsonObject actualOptions = JsonObject.create();
@@ -156,7 +156,7 @@ void queryParametersList() throws Exception {
 
 		Query expected = (new Query()).addCriteria(where(i("firstname")).in("Oliver", "Charles"));
 
-		assertEquals(" WHERE `firstname` in ( $1, $2 )", query.export(new int[1]));
+		assertEquals(" WHERE `firstname` in $1", query.export(new int[1]));
 		JsonObject expectedOptions = JsonObject.create();
 		expected.buildQueryOptions(null, null).build().injectParams(expectedOptions);
 		JsonObject actualOptions = JsonObject.create();