Skip to content

Commit 2683d43

Browse files
mikereichemikereiche
authored
Backport fix for Pageable in String Query. (#1260)
Backport from #1155. Closes #1259.
1 parent 9604193 commit 2683d43

File tree

6 files changed

+128
-46
lines changed

6 files changed

+128
-46
lines changed

‎src/main/java/org/springframework/data/couchbase/core/query/StringQuery.java

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
*/
1616
package org.springframework.data.couchbase.core.query;
1717

18+
import java.util.Locale;
19+
1820
import org.springframework.data.couchbase.core.ReactiveCouchbaseTemplate;
21+
import org.springframework.data.couchbase.core.support.TemplateUtils;
1922

2023
import com.couchbase.client.java.json.JsonArray;
2124
import com.couchbase.client.java.json.JsonValue;
@@ -55,6 +58,11 @@ private void appendInlineN1qlStatement(final StringBuilder sb) {
5558
public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String collection, Class domainClass,
5659
Class resultClass, boolean isCount, String[] distinctFields) {
5760
final StringBuilder statement = new StringBuilder();
61+
boolean makeCount = isCount && inlineN1qlQuery != null
62+
&& !inlineN1qlQuery.toLowerCase(Locale.ROOT).contains("count(");
63+
if (makeCount) {
64+
statement.append("SELECT COUNT(*) AS " + TemplateUtils.SELECT_COUNT + " FROM (");
65+
}
5866
appendInlineN1qlStatement(statement); // apply the string statement
5967
// To use generated parameters for literals
6068
// we need to figure out if we must use positional or named parameters
@@ -68,9 +76,13 @@ public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String coll
6876
paramIndexPtr = new int[] { -1 };
6977
}
7078
appendWhere(statement, paramIndexPtr, template.getConverter()); // criteria on this Query - should be empty for
71-
// StringQuery
72-
appendSort(statement);
73-
appendSkipAndLimit(statement);
79+
if (!isCount) {
80+
appendSort(statement);
81+
appendSkipAndLimit(statement);
82+
}
83+
if (makeCount) {
84+
statement.append(") predicate_query");
85+
}
7486
return statement.toString();
7587
}
7688

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

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@
2222
import static org.springframework.data.couchbase.core.query.QueryCriteria.where;
2323

2424
import java.util.Iterator;
25+
import java.util.Optional;
2526

2627
import org.springframework.core.convert.converter.Converter;
2728
import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
2829
import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
2930
import org.springframework.data.couchbase.core.query.N1QLExpression;
3031
import org.springframework.data.couchbase.core.query.Query;
3132
import org.springframework.data.couchbase.core.query.QueryCriteria;
33+
import org.springframework.data.domain.Pageable;
3234
import org.springframework.data.domain.Sort;
3335
import org.springframework.data.mapping.PersistentPropertyPath;
3436
import org.springframework.data.mapping.context.MappingContext;
@@ -64,6 +66,17 @@ public N1qlQueryCreator(final PartTree tree, final ParameterAccessor accessor, f
6466
this.bucketName = bucketName;
6567
}
6668

69+
@Override
70+
public Query createQuery() {
71+
Query q = this.createQuery((Optional.of(this.accessor).map(ParameterAccessor::getSort).orElse(Sort.unsorted())));
72+
Pageable pageable = accessor.getPageable();
73+
if (pageable.isPaged()) {
74+
q.skip(pageable.getOffset());
75+
q.limit(pageable.getPageSize());
76+
}
77+
return q;
78+
}
79+
6780
@Override
6881
protected QueryCriteria create(final Part part, final Iterator<Object> iterator) {
6982
PersistentPropertyPath<CouchbasePersistentProperty> path = context.getPersistentPropertyPath(part.getProperty());

‎src/main/java/org/springframework/data/couchbase/repository/query/StringBasedN1qlQueryParser.java

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,25 +22,19 @@
2222

2323
import java.util.ArrayList;
2424
import java.util.Collection;
25-
import java.util.Collections;
2625
import java.util.HashSet;
2726
import java.util.List;
2827
import java.util.regex.Matcher;
2928
import java.util.regex.Pattern;
3029

3130
import org.slf4j.Logger;
3231
import org.slf4j.LoggerFactory;
33-
import org.springframework.core.convert.support.DefaultConversionService;
34-
import org.springframework.core.convert.support.GenericConversionService;
35-
import org.springframework.data.convert.CustomConversions;
3632
import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
37-
import org.springframework.data.couchbase.core.convert.CouchbaseCustomConversions;
3833
import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
3934
import org.springframework.data.couchbase.core.query.N1QLExpression;
4035
import org.springframework.data.couchbase.repository.Query;
4136
import org.springframework.data.couchbase.repository.query.support.N1qlUtils;
4237
import org.springframework.data.domain.Pageable;
43-
import org.springframework.data.domain.Sort;
4438
import org.springframework.data.mapping.PersistentEntity;
4539
import org.springframework.data.mapping.PropertyHandler;
4640
import org.springframework.data.repository.query.Parameter;
@@ -494,16 +488,7 @@ private N1QLExpression getExpression(ParameterAccessor accessor, Object[] runtim
494488
runtimeParameters);
495489
N1QLExpression parsedStatement = x(this.doParse(parser, evaluationContext, isCountQuery));
496490

497-
Sort sort = accessor.getSort();
498-
if (sort.isSorted()) {
499-
N1QLExpression[] cbSorts = N1qlUtils.createSort(sort);
500-
parsedStatement = parsedStatement.orderBy(cbSorts);
501-
}
502-
if (queryMethod.isPageQuery()) {
503-
Pageable pageable = accessor.getPageable();
504-
Assert.notNull(pageable, "Pageable must not be null!");
505-
parsedStatement = parsedStatement.limit(pageable.getPageSize()).offset(Math.toIntExact(pageable.getOffset()));
506-
} else if (queryMethod.isSliceQuery()) {
491+
if (queryMethod.isSliceQuery()) {
507492
Pageable pageable = accessor.getPageable();
508493
Assert.notNull(pageable, "Pageable must not be null!");
509494
parsedStatement = parsedStatement.limit(pageable.getPageSize() + 1).offset(Math.toIntExact(pageable.getOffset()));
@@ -517,6 +502,12 @@ private static Object[] getParameters(ParameterAccessor accessor) {
517502
for (Object o : accessor) {
518503
params.add(o);
519504
}
505+
if (accessor.getPageable() != null) {
506+
params.add(accessor.getPageable());
507+
}
508+
if (accessor.getSort() != null) {
509+
params.add(accessor.getSort());
510+
}
520511
return params.toArray();
521512
}
522513
}

‎src/test/java/org/springframework/data/couchbase/domain/Airport.java

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@
1919
import org.springframework.data.annotation.CreatedBy;
2020
import org.springframework.data.annotation.Id;
2121
import org.springframework.data.annotation.PersistenceConstructor;
22-
import org.springframework.data.annotation.Version;
2322
import org.springframework.data.annotation.TypeAlias;
23+
import org.springframework.data.annotation.Version;
2424
import org.springframework.data.couchbase.core.mapping.Document;
2525

2626
/**
@@ -42,6 +42,7 @@ public class Airport extends ComparableEntity {
4242

4343
@CreatedBy private String createdBy;
4444

45+
private String[] organizations = new String[] { "123", "456", "789" };
4546

4647
@PersistenceConstructor
4748
public Airport(String id, String iata, String icao) {
@@ -78,6 +79,7 @@ public Airport clearVersion() {
7879
version = Long.valueOf(0);
7980
return this;
8081
}
82+
8183
public String getCreatedBy() {
8284
return createdBy;
8385
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
@Repository
4444
public interface AirportRepository extends CouchbaseRepository<Airport, String> {
4545

46+
@Query("#{#n1ql.selectEntity} WHERE ARRAY_CONTAINS(organizations, $1) AND #{#n1ql.filter}")
47+
Page<Airport> findAllByOrganizationIdsContains(String id, Pageable pageable);
48+
4649
@Override
4750
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
4851
List<Airport> findAll();
@@ -97,4 +100,18 @@ Long countFancyExpression(@Param("projectIds") List<String> projectIds, @Param("
97100
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
98101
Optional<Airport> findByIdAndIata(String id, String iata);
99102

103+
@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE anything = 'count(*)'") // looks like count query, but is not
104+
Long countBad();
105+
106+
@Query("SELECT count(*) FROM `#{#n1ql.bucket}`")
107+
Long countGood();
108+
109+
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
110+
@Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata != $1")
111+
Page<Airport> getAllByIataNot(String iata, Pageable pageable);
112+
113+
@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE #{#n1ql.filter} " + " #{#projectIds != null ? 'AND blah IN $1' : ''} "
114+
+ " #{#planIds != null ? 'AND blahblah IN $2' : ''} " + " #{#active != null ? 'AND false = $3' : ''} ")
115+
Long countOne();
116+
100117
}

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

Lines changed: 73 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,30 @@
1616

1717
package org.springframework.data.couchbase.repository;
1818

19-
import com.couchbase.client.core.error.CouchbaseException;
20-
import com.couchbase.client.core.error.IndexExistsException;
21-
import com.couchbase.client.java.query.QueryScanConsistency;
19+
import static com.couchbase.client.java.query.QueryScanConsistency.NOT_BOUNDED;
20+
import static com.couchbase.client.java.query.QueryScanConsistency.REQUEST_PLUS;
21+
import static java.util.Arrays.asList;
22+
import static org.assertj.core.api.Assertions.assertThat;
23+
import static org.junit.jupiter.api.Assertions.assertEquals;
24+
import static org.junit.jupiter.api.Assertions.assertFalse;
25+
import static org.junit.jupiter.api.Assertions.assertNotNull;
26+
import static org.junit.jupiter.api.Assertions.assertNull;
27+
import static org.junit.jupiter.api.Assertions.assertThrows;
28+
import static org.junit.jupiter.api.Assertions.assertTrue;
29+
import static org.springframework.data.couchbase.config.BeanNames.COUCHBASE_TEMPLATE;
30+
31+
import java.lang.reflect.Method;
32+
import java.util.ArrayList;
33+
import java.util.Arrays;
34+
import java.util.List;
35+
import java.util.Locale;
36+
import java.util.Optional;
37+
import java.util.concurrent.Callable;
38+
import java.util.concurrent.ExecutorService;
39+
import java.util.concurrent.Executors;
40+
import java.util.concurrent.Future;
41+
import java.util.stream.Collectors;
42+
2243
import org.junit.jupiter.api.BeforeEach;
2344
import org.junit.jupiter.api.Test;
2445
import org.springframework.beans.factory.annotation.Autowired;
@@ -31,12 +52,22 @@
3152
import org.springframework.data.auditing.DateTimeProvider;
3253
import org.springframework.data.couchbase.CouchbaseClientFactory;
3354
import org.springframework.data.couchbase.config.AbstractCouchbaseConfiguration;
55+
import org.springframework.data.couchbase.core.CouchbaseQueryExecutionException;
3456
import org.springframework.data.couchbase.core.CouchbaseTemplate;
3557
import org.springframework.data.couchbase.core.RemoveResult;
3658
import org.springframework.data.couchbase.core.query.N1QLExpression;
3759
import org.springframework.data.couchbase.core.query.Query;
3860
import org.springframework.data.couchbase.core.query.QueryCriteria;
39-
import org.springframework.data.couchbase.domain.*;
61+
import org.springframework.data.couchbase.domain.Address;
62+
import org.springframework.data.couchbase.domain.Airport;
63+
import org.springframework.data.couchbase.domain.AirportDefaultConsistencyRepository;
64+
import org.springframework.data.couchbase.domain.AirportRepository;
65+
import org.springframework.data.couchbase.domain.Iata;
66+
import org.springframework.data.couchbase.domain.NaiveAuditorAware;
67+
import org.springframework.data.couchbase.domain.Person;
68+
import org.springframework.data.couchbase.domain.PersonRepository;
69+
import org.springframework.data.couchbase.domain.User;
70+
import org.springframework.data.couchbase.domain.UserRepository;
4071
import org.springframework.data.couchbase.domain.time.AuditingDateTimeProvider;
4172
import org.springframework.data.couchbase.repository.auditing.EnableCouchbaseAuditing;
4273
import org.springframework.data.couchbase.repository.config.EnableCouchbaseRepositories;
@@ -49,28 +80,14 @@
4980
import org.springframework.data.domain.Page;
5081
import org.springframework.data.domain.PageRequest;
5182
import org.springframework.data.domain.Pageable;
83+
import org.springframework.data.domain.Sort;
5284
import org.springframework.data.projection.SpelAwareProxyProjectionFactory;
5385
import org.springframework.data.repository.core.support.DefaultRepositoryMetadata;
5486
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
5587

56-
import java.lang.reflect.Method;
57-
import java.util.ArrayList;
58-
import java.util.Arrays;
59-
import java.util.List;
60-
import java.util.Locale;
61-
import java.util.Optional;
62-
import java.util.concurrent.Callable;
63-
import java.util.concurrent.ExecutorService;
64-
import java.util.concurrent.Executors;
65-
import java.util.concurrent.Future;
66-
import java.util.stream.Collectors;
67-
68-
import static com.couchbase.client.java.query.QueryScanConsistency.NOT_BOUNDED;
69-
import static com.couchbase.client.java.query.QueryScanConsistency.REQUEST_PLUS;
70-
import static java.util.Arrays.asList;
71-
import static org.assertj.core.api.Assertions.assertThat;
72-
import static org.junit.jupiter.api.Assertions.*;
73-
import static org.springframework.data.couchbase.config.BeanNames.COUCHBASE_TEMPLATE;
88+
import com.couchbase.client.core.error.CouchbaseException;
89+
import com.couchbase.client.core.error.IndexExistsException;
90+
import com.couchbase.client.java.query.QueryScanConsistency;
7491

7592
/**
7693
* Repository tests
@@ -88,8 +105,7 @@ public class CouchbaseRepositoryQueryIntegrationTests extends ClusterAwareIntegr
88105

89106
@Autowired AirportRepository airportRepository;
90107

91-
@Autowired
92-
AirportDefaultConsistencyRepository airportDefaultConsistencyRepository;
108+
@Autowired AirportDefaultConsistencyRepository airportDefaultConsistencyRepository;
93109

94110
@Autowired UserRepository userRepository;
95111

@@ -195,6 +211,19 @@ void findBySimpleProperty() {
195211
}
196212
}
197213

214+
@Test
215+
void findWithPageRequest() {
216+
Airport vie = null;
217+
try {
218+
vie = new Airport("airports::vie", "vie", "low6");
219+
vie = airportRepository.save(vie);
220+
// The exception occurred while building the query. This query will not find anything on execution.
221+
Page<Airport> airports = airportRepository.findAllByOrganizationIdsContains("123", PageRequest.of(0, 1));
222+
} finally {
223+
// airportRepository.delete(vie);
224+
}
225+
}
226+
198227
@Test
199228
public void saveNotBounded() {
200229
// save() followed by query with NOT_BOUNDED will result in not finding the document
@@ -234,7 +263,8 @@ public void saveNotBoundedRequestPlus() {
234263
ApplicationContext ac = new AnnotationConfigApplicationContext(ConfigRequestPlus.class);
235264
// the Config class has been modified, these need to be loaded again
236265
CouchbaseTemplate couchbaseTemplateRP = (CouchbaseTemplate) ac.getBean(COUCHBASE_TEMPLATE);
237-
AirportDefaultConsistencyRepository airportRepositoryRP = (AirportDefaultConsistencyRepository) ac.getBean("airportDefaultConsistencyRepository");
266+
AirportDefaultConsistencyRepository airportRepositoryRP = (AirportDefaultConsistencyRepository) ac
267+
.getBean("airportDefaultConsistencyRepository");
238268

239269
// save() followed by query with NOT_BOUNDED will result in not finding the document
240270
Airport vie = new Airport("airports::vie", "vie", "low9");
@@ -272,7 +302,8 @@ public void saveNotBoundedRequestPlusWithDefaultRepository() {
272302
ApplicationContext ac = new AnnotationConfigApplicationContext(ConfigRequestPlus.class);
273303
// the Config class has been modified, these need to be loaded again
274304
CouchbaseTemplate couchbaseTemplateRP = (CouchbaseTemplate) ac.getBean(COUCHBASE_TEMPLATE);
275-
AirportDefaultConsistencyRepository airportRepositoryRP = (AirportDefaultConsistencyRepository) ac.getBean("airportDefaultConsistencyRepository");
305+
AirportDefaultConsistencyRepository airportRepositoryRP = (AirportDefaultConsistencyRepository) ac
306+
.getBean("airportDefaultConsistencyRepository");
276307

277308
List<Airport> sizeBeforeTest = airportRepositoryRP.findAll();
278309
assertEquals(0, sizeBeforeTest.size());
@@ -357,6 +388,7 @@ public void testStreamQuery() {
357388
void count() {
358389
String[] iatas = { "JFK", "IAD", "SFO", "SJC", "SEA", "LAX", "PHX" };
359390

391+
airportRepository.countOne();
360392
try {
361393

362394
airportRepository.saveAll(
@@ -366,6 +398,11 @@ void count() {
366398
Long count = airportRepository.countFancyExpression(asList("JFK"), asList("jfk"), false);
367399
assertEquals(1, count);
368400

401+
Pageable sPageable = PageRequest.of(0, 2).withSort(Sort.by("iata"));
402+
Page<Airport> sPage = airportRepository.getAllByIataNot("JFK", sPageable);
403+
assertEquals(iatas.length - 1, sPage.getTotalElements());
404+
assertEquals(sPageable.getPageSize(), sPage.getContent().size());
405+
369406
Pageable pageable = PageRequest.of(0, 2);
370407
Page<Airport> aPage = airportRepository.findAllByIataNot("JFK", pageable);
371408
assertEquals(iatas.length - 1, aPage.getTotalElements());
@@ -392,6 +429,16 @@ void count() {
392429
}
393430
}
394431

432+
@Test
433+
void badCount() {
434+
assertThrows(CouchbaseQueryExecutionException.class, () -> airportRepository.countBad());
435+
}
436+
437+
@Test
438+
void goodCount() {
439+
airportRepository.countGood();
440+
}
441+
395442
@Test
396443
void threadSafeParametersTest() throws Exception {
397444
String[] iatas = { "JFK", "IAD", "SFO", "SJC", "SEA", "LAX", "PHX" };

0 commit comments

Comments
 (0)