spring-projects · mikereiche · Oct 21, 2021 · Oct 21, 2021
diff --git a/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java b/src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java
@@ -15,7 +15,10 @@
  */
 package org.springframework.data.couchbase.core.query;
 
+import java.util.Locale;
+
 import org.springframework.data.couchbase.core.ReactiveCouchbaseTemplate;
+import org.springframework.data.couchbase.core.support.TemplateUtils;
 
 import com.couchbase.client.java.json.JsonArray;
 import com.couchbase.client.java.json.JsonValue;
@@ -55,6 +58,11 @@ private void appendInlineN1qlStatement(final StringBuilder sb) {
 	public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String collection, Class domainClass,
 			Class resultClass, boolean isCount, String[] distinctFields) {
 		final StringBuilder statement = new StringBuilder();
+		boolean makeCount = isCount && inlineN1qlQuery != null
+				&& !inlineN1qlQuery.toLowerCase(Locale.ROOT).contains("count(");
+		if (makeCount) {
+			statement.append("SELECT COUNT(*) AS " + TemplateUtils.SELECT_COUNT + " FROM (");
+		}
 		appendInlineN1qlStatement(statement); // apply the string statement
 		// To use generated parameters for literals
 		// we need to figure out if we must use positional or named parameters
@@ -68,9 +76,13 @@ public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String coll
 			paramIndexPtr = new int[] { -1 };
 		}
 		appendWhere(statement, paramIndexPtr, template.getConverter()); // criteria on this Query - should be empty for
-																																		// StringQuery
-		appendSort(statement);
-		appendSkipAndLimit(statement);
+		if (!isCount) {
+			appendSort(statement);
+			appendSkipAndLimit(statement);
+		}
+		if (makeCount) {
+			statement.append(") predicate_query");
+		}
 		return statement.toString();
 	}
 

diff --git a/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java b/src/main/java/org/springframework/data/couchbase/repository/query/N1qlQueryCreator.java
@@ -22,13 +22,15 @@
 import static org.springframework.data.couchbase.core.query.QueryCriteria.where;
 
 import java.util.Iterator;
+import java.util.Optional;
 
 import org.springframework.core.convert.converter.Converter;
 import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
 import org.springframework.data.couchbase.core.query.N1QLExpression;
 import org.springframework.data.couchbase.core.query.Query;
 import org.springframework.data.couchbase.core.query.QueryCriteria;
+import org.springframework.data.domain.Pageable;
 import org.springframework.data.domain.Sort;
 import org.springframework.data.mapping.PersistentPropertyPath;
 import org.springframework.data.mapping.context.MappingContext;
@@ -64,6 +66,17 @@ public N1qlQueryCreator(final PartTree tree, final ParameterAccessor accessor, f
 		this.bucketName = bucketName;
 	}
 
+	@Override
+	public Query createQuery() {
+		Query q = this.createQuery((Optional.of(this.accessor).map(ParameterAccessor::getSort).orElse(Sort.unsorted())));
+		Pageable pageable = accessor.getPageable();
+		if (pageable.isPaged()) {
+			q.skip(pageable.getOffset());
+			q.limit(pageable.getPageSize());
+		}
+		return q;
+	}
+
 	@Override
 	protected QueryCriteria create(final Part part, final Iterator<Object> iterator) {
 		PersistentPropertyPath<CouchbasePersistentProperty> path = context.getPersistentPropertyPath(part.getProperty());

diff --git a/.../java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java b/.../java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java
@@ -22,25 +22,19 @@
 
 import java.util.ArrayList;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.springframework.core.convert.support.DefaultConversionService;
-import org.springframework.core.convert.support.GenericConversionService;
-import org.springframework.data.convert.CustomConversions;
 import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
-import org.springframework.data.couchbase.core.convert.CouchbaseCustomConversions;
 import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
 import org.springframework.data.couchbase.core.query.N1QLExpression;
 import org.springframework.data.couchbase.repository.Query;
 import org.springframework.data.couchbase.repository.query.support.N1qlUtils;
 import org.springframework.data.domain.Pageable;
-import org.springframework.data.domain.Sort;
 import org.springframework.data.mapping.PersistentEntity;
 import org.springframework.data.mapping.PropertyHandler;
 import org.springframework.data.repository.query.Parameter;
@@ -494,16 +488,7 @@ private N1QLExpression getExpression(ParameterAccessor accessor, Object[] runtim
 				runtimeParameters);
 		N1QLExpression parsedStatement = x(this.doParse(parser, evaluationContext, isCountQuery));
 
-		Sort sort = accessor.getSort();
-		if (sort.isSorted()) {
-			N1QLExpression[] cbSorts = N1qlUtils.createSort(sort);
-			parsedStatement = parsedStatement.orderBy(cbSorts);
-		}
-		if (queryMethod.isPageQuery()) {
-			Pageable pageable = accessor.getPageable();
-			Assert.notNull(pageable, "Pageable must not be null!");
-			parsedStatement = parsedStatement.limit(pageable.getPageSize()).offset(Math.toIntExact(pageable.getOffset()));
-		} else if (queryMethod.isSliceQuery()) {
+		if (queryMethod.isSliceQuery()) {
 			Pageable pageable = accessor.getPageable();
 			Assert.notNull(pageable, "Pageable must not be null!");
 			parsedStatement = parsedStatement.limit(pageable.getPageSize() + 1).offset(Math.toIntExact(pageable.getOffset()));
@@ -517,6 +502,12 @@ private static Object[] getParameters(ParameterAccessor accessor) {
 		for (Object o : accessor) {
 			params.add(o);
 		}
+		if (accessor.getPageable() != null) {
+			params.add(accessor.getPageable());
+		}
+		if (accessor.getSort() != null) {
+			params.add(accessor.getSort());
+		}
 		return params.toArray();
 	}
 }
diff --git a/src/test/java/org/springframework/data/couchbase/domain/Airport.java b/src/test/java/org/springframework/data/couchbase/domain/Airport.java
@@ -19,8 +19,8 @@
 import org.springframework.data.annotation.CreatedBy;
 import org.springframework.data.annotation.Id;
 import org.springframework.data.annotation.PersistenceConstructor;
-import org.springframework.data.annotation.Version;
 import org.springframework.data.annotation.TypeAlias;
+import org.springframework.data.annotation.Version;
 import org.springframework.data.couchbase.core.mapping.Document;
 
 /**
@@ -42,6 +42,7 @@ public class Airport extends ComparableEntity {
 
 	@CreatedBy private String createdBy;
 
+	private String[] organizations = new String[] { "123", "456", "789" };
 
 	@PersistenceConstructor
 	public Airport(String id, String iata, String icao) {
@@ -78,6 +79,7 @@ public Airport clearVersion() {
 		version = Long.valueOf(0);
 		return this;
 	}
+
 	public String getCreatedBy() {
 		return createdBy;
 	}

diff --git a/src/test/java/org/springframework/data/couchbase/domain/AirportRepository.java b/src/test/java/org/springframework/data/couchbase/domain/AirportRepository.java
@@ -43,6 +43,9 @@
 @Repository
 public interface AirportRepository extends CouchbaseRepository<Airport, String> {
 
+	@Query("#{#n1ql.selectEntity} WHERE ARRAY_CONTAINS(organizations, $1) AND #{#n1ql.filter}")
+	Page<Airport> findAllByOrganizationIdsContains(String id, Pageable pageable);
+
 	@Override
 	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
 	List<Airport> findAll();
@@ -97,4 +100,18 @@ Long countFancyExpression(@Param("projectIds") List<String> projectIds, @Param("
 	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
 	Optional<Airport> findByIdAndIata(String id, String iata);
 
+	@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE anything = 'count(*)'") // looks like count query, but is not
+	Long countBad();
+
+	@Query("SELECT count(*) FROM `#{#n1ql.bucket}`")
+	Long countGood();
+
+	@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
+	@Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata != $1")
+	Page<Airport> getAllByIataNot(String iata, Pageable pageable);
+
+	@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE #{#n1ql.filter} " + " #{#projectIds != null ? 'AND blah IN $1' : ''} "
+			+ " #{#planIds != null ? 'AND blahblah IN $2' : ''} " + " #{#active != null ? 'AND false = $3' : ''} ")
+	Long countOne();
+
 }
diff --git a/...g/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java b/...g/springframework/data/couchbase/repository/CouchbaseRepositoryQueryIntegrationTests.java
@@ -16,9 +16,30 @@
 
 package org.springframework.data.couchbase.repository;
 
-import com.couchbase.client.core.error.CouchbaseException;
-import com.couchbase.client.core.error.IndexExistsException;
-import com.couchbase.client.java.query.QueryScanConsistency;
+import static com.couchbase.client.java.query.QueryScanConsistency.NOT_BOUNDED;
+import static com.couchbase.client.java.query.QueryScanConsistency.REQUEST_PLUS;
+import static java.util.Arrays.asList;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNull;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.springframework.data.couchbase.config.BeanNames.COUCHBASE_TEMPLATE;
+
+import java.lang.reflect.Method;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+import java.util.Locale;
+import java.util.Optional;
+import java.util.concurrent.Callable;
+import java.util.concurrent.ExecutorService;
+import java.util.concurrent.Executors;
+import java.util.concurrent.Future;
+import java.util.stream.Collectors;
+
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
 import org.springframework.beans.factory.annotation.Autowired;
@@ -31,12 +52,22 @@
 import org.springframework.data.auditing.DateTimeProvider;
 import org.springframework.data.couchbase.CouchbaseClientFactory;
 import org.springframework.data.couchbase.config.AbstractCouchbaseConfiguration;
+import org.springframework.data.couchbase.core.CouchbaseQueryExecutionException;
 import org.springframework.data.couchbase.core.CouchbaseTemplate;
 import org.springframework.data.couchbase.core.RemoveResult;
 import org.springframework.data.couchbase.core.query.N1QLExpression;
 import org.springframework.data.couchbase.core.query.Query;
 import org.springframework.data.couchbase.core.query.QueryCriteria;
-import org.springframework.data.couchbase.domain.*;
+import org.springframework.data.couchbase.domain.Address;
+import org.springframework.data.couchbase.domain.Airport;
+import org.springframework.data.couchbase.domain.AirportDefaultConsistencyRepository;
+import org.springframework.data.couchbase.domain.AirportRepository;
+import org.springframework.data.couchbase.domain.Iata;
+import org.springframework.data.couchbase.domain.NaiveAuditorAware;
+import org.springframework.data.couchbase.domain.Person;
+import org.springframework.data.couchbase.domain.PersonRepository;
+import org.springframework.data.couchbase.domain.User;
+import org.springframework.data.couchbase.domain.UserRepository;
 import org.springframework.data.couchbase.domain.time.AuditingDateTimeProvider;
 import org.springframework.data.couchbase.repository.auditing.EnableCouchbaseAuditing;
 import org.springframework.data.couchbase.repository.config.EnableCouchbaseRepositories;
@@ -49,28 +80,14 @@
 import org.springframework.data.domain.Page;
 import org.springframework.data.domain.PageRequest;
 import org.springframework.data.domain.Pageable;
+import org.springframework.data.domain.Sort;
 import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
 import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
 import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
 
-import java.lang.reflect.Method;
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-import java.util.Locale;
-import java.util.Optional;
-import java.util.concurrent.Callable;
-import java.util.concurrent.ExecutorService;
-import java.util.concurrent.Executors;
-import java.util.concurrent.Future;
-import java.util.stream.Collectors;
-
-import static com.couchbase.client.java.query.QueryScanConsistency.NOT_BOUNDED;
-import static com.couchbase.client.java.query.QueryScanConsistency.REQUEST_PLUS;
-import static java.util.Arrays.asList;
-import static org.assertj.core.api.Assertions.assertThat;
-import static org.junit.jupiter.api.Assertions.*;
-import static org.springframework.data.couchbase.config.BeanNames.COUCHBASE_TEMPLATE;
+import com.couchbase.client.core.error.CouchbaseException;
+import com.couchbase.client.core.error.IndexExistsException;
+import com.couchbase.client.java.query.QueryScanConsistency;
 
 /**
  * Repository tests
@@ -88,8 +105,7 @@ public class CouchbaseRepositoryQueryIntegrationTests extends ClusterAwareIntegr
 
 	@Autowired AirportRepository airportRepository;
 
-	@Autowired
-	AirportDefaultConsistencyRepository airportDefaultConsistencyRepository;
+	@Autowired AirportDefaultConsistencyRepository airportDefaultConsistencyRepository;
 
 	@Autowired UserRepository userRepository;
 
@@ -195,6 +211,19 @@ void findBySimpleProperty() {
 		}
 	}
 
+	@Test
+	void findWithPageRequest() {
+		Airport vie = null;
+		try {
+			vie = new Airport("airports::vie", "vie", "low6");
+			vie = airportRepository.save(vie);
+			// The exception occurred while building the query. This query will not find anything on execution.
+			Page<Airport> airports = airportRepository.findAllByOrganizationIdsContains("123", PageRequest.of(0, 1));
+		} finally {
+			// airportRepository.delete(vie);
+		}
+	}
+
 	@Test
 	public void saveNotBounded() {
 		// save() followed by query with NOT_BOUNDED will result in not finding the document
@@ -234,7 +263,8 @@ public void saveNotBoundedRequestPlus() {
 		ApplicationContext ac = new AnnotationConfigApplicationContext(ConfigRequestPlus.class);
 		// the Config class has been modified, these need to be loaded again
 		CouchbaseTemplate couchbaseTemplateRP = (CouchbaseTemplate) ac.getBean(COUCHBASE_TEMPLATE);
-		AirportDefaultConsistencyRepository airportRepositoryRP = (AirportDefaultConsistencyRepository) ac.getBean("airportDefaultConsistencyRepository");
+		AirportDefaultConsistencyRepository airportRepositoryRP = (AirportDefaultConsistencyRepository) ac
+				.getBean("airportDefaultConsistencyRepository");
 
 		// save() followed by query with NOT_BOUNDED will result in not finding the document
 		Airport vie = new Airport("airports::vie", "vie", "low9");
@@ -272,7 +302,8 @@ public void saveNotBoundedRequestPlusWithDefaultRepository() {
 		ApplicationContext ac = new AnnotationConfigApplicationContext(ConfigRequestPlus.class);
 		// the Config class has been modified, these need to be loaded again
 		CouchbaseTemplate couchbaseTemplateRP = (CouchbaseTemplate) ac.getBean(COUCHBASE_TEMPLATE);
-		AirportDefaultConsistencyRepository airportRepositoryRP = (AirportDefaultConsistencyRepository) ac.getBean("airportDefaultConsistencyRepository");
+		AirportDefaultConsistencyRepository airportRepositoryRP = (AirportDefaultConsistencyRepository) ac
+				.getBean("airportDefaultConsistencyRepository");
 
 		List<Airport> sizeBeforeTest = airportRepositoryRP.findAll();
 		assertEquals(0, sizeBeforeTest.size());
@@ -357,6 +388,7 @@ public void testStreamQuery() {
 	void count() {
 		String[] iatas = { "JFK", "IAD", "SFO", "SJC", "SEA", "LAX", "PHX" };
 
+		airportRepository.countOne();
 		try {
 
 			airportRepository.saveAll(
@@ -366,6 +398,11 @@ void count() {
 			Long count = airportRepository.countFancyExpression(asList("JFK"), asList("jfk"), false);
 			assertEquals(1, count);
 
+			Pageable sPageable = PageRequest.of(0, 2).withSort(Sort.by("iata"));
+			Page<Airport> sPage = airportRepository.getAllByIataNot("JFK", sPageable);
+			assertEquals(iatas.length - 1, sPage.getTotalElements());
+			assertEquals(sPageable.getPageSize(), sPage.getContent().size());
+
 			Pageable pageable = PageRequest.of(0, 2);
 			Page<Airport> aPage = airportRepository.findAllByIataNot("JFK", pageable);
 			assertEquals(iatas.length - 1, aPage.getTotalElements());
@@ -392,6 +429,16 @@ void count() {
 		}
 	}
 
+	@Test
+	void badCount() {
+		assertThrows(CouchbaseQueryExecutionException.class, () -> airportRepository.countBad());
+	}
+
+	@Test
+	void goodCount() {
+		airportRepository.countGood();
+	}
+
 	@Test
 	void threadSafeParametersTest() throws Exception {
 		String[] iatas = { "JFK", "IAD", "SFO", "SJC", "SEA", "LAX", "PHX" };