Skip to content

Commit f0880da

Browse files
mikereichemikereiche
authored
Fix ArrayIndexOutOfBoundsException on String queries that use paging. (#1197)
Closes #1155. Co-authored-by: mikereiche <[email protected]>
1 parent dffd203 commit f0880da

File tree

6 files changed

+62
-39
lines changed

6 files changed

+62
-39
lines changed

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

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -136,7 +136,7 @@ public Query with(final Pageable pageable) {
136136
}
137137
this.limit = pageable.getPageSize();
138138
this.skip = pageable.getOffset();
139-
if(!this.sort.equals(pageable.getSort()))
139+
if (!this.sort.equals(pageable.getSort()))
140140
this.sort.and(pageable.getSort());
141141
return this;
142142
}
@@ -176,7 +176,7 @@ public Query with(final Sort sort) {
176176
return this;
177177
}
178178

179-
public Query withoutSort(){
179+
public Query withoutSort() {
180180
this.sort = Sort.unsorted();
181181
return this;
182182
}
@@ -277,11 +277,6 @@ public String export(int[]... paramIndexPtrHolder) { // used only by tests
277277
return sb.toString();
278278
}
279279

280-
public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String collectionName, Class domainClass,
281-
boolean isCount) {
282-
return toN1qlSelectString(template, collectionName, domainClass, null, isCount, null);
283-
}
284-
285280
public String toN1qlSelectString(ReactiveCouchbaseTemplate template, Class domainClass, boolean isCount) {
286281
return toN1qlSelectString(template, null, domainClass, null, isCount, null);
287282
}
@@ -294,7 +289,7 @@ public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String coll
294289
appendString(statement, n1ql.selectEntity); // select ...
295290
appendWhereString(statement, n1ql.filter); // typeKey = typeValue
296291
appendWhere(statement, new int[] { 0 }, template.getConverter()); // criteria on this Query
297-
if(!isCount){
292+
if (!isCount) {
298293
appendSort(statement);
299294
appendSkipAndLimit(statement);
300295
}

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

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,9 @@
1919

2020
import com.couchbase.client.java.json.JsonArray;
2121
import com.couchbase.client.java.json.JsonValue;
22+
import org.springframework.data.couchbase.core.support.TemplateUtils;
23+
24+
import java.util.Locale;
2225

2326
/**
2427
* Query created from the string in @Query annotation in the repository interface.
@@ -35,7 +38,7 @@
3538
*/
3639
public class StringQuery extends Query {
3740

38-
private String inlineN1qlQuery;
41+
private final String inlineN1qlQuery;
3942

4043
public StringQuery(String n1qlString) {
4144
inlineN1qlQuery = n1qlString;
@@ -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/StringBasedN1qlQueryParser.java

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
package org.springframework.data.couchbase.repository.query;
1717

1818
import static org.springframework.data.couchbase.core.query.N1QLExpression.i;
19-
import static org.springframework.data.couchbase.core.query.N1QLExpression.meta;
20-
import static org.springframework.data.couchbase.core.query.N1QLExpression.path;
2119
import static org.springframework.data.couchbase.core.query.N1QLExpression.x;
2220
import static org.springframework.data.couchbase.core.support.TemplateUtils.SELECT_CAS;
2321
import static org.springframework.data.couchbase.core.support.TemplateUtils.SELECT_ID;
@@ -31,14 +29,12 @@
3129

3230
import org.slf4j.Logger;
3331
import org.slf4j.LoggerFactory;
34-
import org.springframework.core.convert.converter.Converter;
3532
import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
3633
import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
3734
import org.springframework.data.couchbase.core.query.N1QLExpression;
3835
import org.springframework.data.couchbase.repository.Query;
3936
import org.springframework.data.couchbase.repository.query.support.N1qlUtils;
4037
import org.springframework.data.domain.Pageable;
41-
import org.springframework.data.domain.Sort;
4238
import org.springframework.data.mapping.PersistentEntity;
4339
import org.springframework.data.mapping.PersistentPropertyPath;
4440
import org.springframework.data.mapping.PropertyHandler;
@@ -197,7 +193,7 @@ private void getProjectedFieldsInternal(String bucketName, CouchbasePersistentPr
197193
TypeInformation resultClass/*, String path*/) {
198194

199195
PersistentEntity persistentEntity = couchbaseConverter.getMappingContext().getPersistentEntity(resultClass);
200-
//CouchbasePersistentProperty property = path.getLeafProperty();
196+
// CouchbasePersistentProperty property = path.getLeafProperty();
201197
persistentEntity.doWithProperties(new PropertyHandler<CouchbasePersistentProperty>() {
202198
@Override
203199
public void doWithPersistentProperty(final CouchbasePersistentProperty prop) {
@@ -207,7 +203,8 @@ public void doWithPersistentProperty(final CouchbasePersistentProperty prop) {
207203
if (prop.isVersionProperty()) {
208204
return;
209205
}
210-
PersistentPropertyPath<CouchbasePersistentProperty> path = couchbaseConverter.getMappingContext().getPersistentPropertyPath(prop.getName(), resultClass.getType());
206+
PersistentPropertyPath<CouchbasePersistentProperty> path = couchbaseConverter.getMappingContext()
207+
.getPersistentPropertyPath(prop.getName(), resultClass.getType());
211208

212209
// The current limitation is that only top-level properties can be projected
213210
// This traversing of nested data structures would need to replicate the processing done by
@@ -495,16 +492,7 @@ private N1QLExpression getExpression(ParameterAccessor accessor, Object[] runtim
495492
runtimeParameters);
496493
N1QLExpression parsedStatement = x(this.doParse(parser, evaluationContext, isCountQuery));
497494

498-
Sort sort = accessor.getSort();
499-
if (sort.isSorted()) {
500-
N1QLExpression[] cbSorts = N1qlUtils.createSort(sort);
501-
parsedStatement = parsedStatement.orderBy(cbSorts);
502-
}
503-
if (queryMethod.isPageQuery()) {
504-
Pageable pageable = accessor.getPageable();
505-
Assert.notNull(pageable, "Pageable must not be null!");
506-
parsedStatement = parsedStatement.limit(pageable.getPageSize()).offset(Math.toIntExact(pageable.getOffset()));
507-
} else if (queryMethod.isSliceQuery()) {
495+
if (queryMethod.isSliceQuery()) {
508496
Pageable pageable = accessor.getPageable();
509497
Assert.notNull(pageable, "Pageable must not be null!");
510498
parsedStatement = parsedStatement.limit(pageable.getPageSize() + 1).offset(Math.toIntExact(pageable.getOffset()));
@@ -518,6 +506,8 @@ private static Object[] getParameters(ParameterAccessor accessor) {
518506
for (Object o : accessor) {
519507
params.add(o);
520508
}
509+
params.add(accessor.getPageable());
510+
params.add(accessor.getSort());
521511
return params.toArray();
522512
}
523513
}

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

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

2121
import java.util.Iterator;
22+
import java.util.Optional;
2223

2324
import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
2425
import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
2526
import org.springframework.data.couchbase.core.query.N1QLExpression;
2627
import org.springframework.data.couchbase.core.query.Query;
2728
import org.springframework.data.couchbase.core.query.QueryCriteria;
2829
import org.springframework.data.couchbase.core.query.StringQuery;
30+
import org.springframework.data.domain.Pageable;
2931
import org.springframework.data.domain.Sort;
3032
import org.springframework.data.mapping.Alias;
3133
import org.springframework.data.mapping.PersistentPropertyPath;
@@ -116,6 +118,17 @@ protected QueryCriteria create(final Part part, final Iterator<Object> iterator)
116118
return from(part, property, where(x(path.toDotPath())), iterator);
117119
}
118120

121+
@Override
122+
public Query createQuery() {
123+
Query q = this.createQuery((Optional.of(this.accessor).map(ParameterAccessor::getSort).orElse(Sort.unsorted())));
124+
Pageable pageable = accessor.getPageable();
125+
if (pageable.isPaged()) {
126+
q.skip(pageable.getOffset());
127+
q.limit(pageable.getPageSize());
128+
}
129+
return q;
130+
}
131+
119132
@Override
120133
protected QueryCriteria and(final Part part, final QueryCriteria base, final Iterator<Object> iterator) {
121134
if (base == null) {

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

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -114,18 +114,26 @@ public interface AirportRepository extends CouchbaseRepository<Airport, String>,
114114
Long countFancyExpression(@Param("projectIds") List<String> projectIds, @Param("planIds") List<String> planIds,
115115
@Param("active") Boolean active);
116116

117-
@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE 0 = 1" )
117+
@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE anything = 'count(*)'") // looks like count query, but is not
118118
Long countBad();
119119

120-
@Query("SELECT count(*) FROM `#{#n1ql.bucket}`" )
120+
@Query("SELECT count(*) FROM `#{#n1ql.bucket}`")
121121
Long countGood();
122122

123123
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
124124
Page<Airport> findAllByIataNot(String iata, Pageable pageable);
125125

126+
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
127+
@Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata != $1")
128+
Page<Airport> getAllByIataNot(String iata, Pageable pageable);
129+
126130
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
127131
Optional<Airport> findByIdAndIata(String id, String iata);
128132

133+
@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE #{#n1ql.filter} " + " #{#projectIds != null ? 'AND blah IN $1' : ''} "
134+
+ " #{#planIds != null ? 'AND blahblah IN $2' : ''} " + " #{#active != null ? 'AND false = $3' : ''} ")
135+
Long countOne();
136+
129137
@Retention(RetentionPolicy.RUNTIME)
130138
@Target({ ElementType.METHOD, ElementType.TYPE })
131139
// @Meta

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

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@
3939
import java.util.concurrent.Future;
4040
import java.util.stream.Collectors;
4141

42-
import com.couchbase.client.java.kv.UpsertOptions;
4342
import org.junit.jupiter.api.BeforeEach;
4443
import org.junit.jupiter.api.Test;
4544
import org.springframework.beans.factory.annotation.Autowired;
@@ -89,6 +88,7 @@
8988
import com.couchbase.client.java.json.JsonArray;
9089
import com.couchbase.client.java.kv.GetResult;
9190
import com.couchbase.client.java.kv.MutationState;
91+
import com.couchbase.client.java.kv.UpsertOptions;
9292
import com.couchbase.client.java.query.QueryOptions;
9393
import com.couchbase.client.java.query.QueryScanConsistency;
9494

@@ -187,10 +187,9 @@ void annotatedFieldFindName() {
187187
assertEquals(person.getSalutation(), result.contentAsObject().get("prefix"));
188188
Person person2 = personRepository.findById(person.getId().toString()).get();
189189
assertEquals(person.getSalutation(), person2.getSalutation());
190-
// needs fix from datacouch_1184
191-
//List<Person> persons3 = personRepository.findBySalutation("Mrs");
192-
//assertEquals(1, persons3.size());
193-
//assertEquals(person.getSalutation(), persons3.get(0).getSalutation());
190+
List<Person> persons3 = personRepository.findBySalutation("Mrs");
191+
assertEquals(1, persons3.size());
192+
assertEquals(person.getSalutation(), persons3.get(0).getSalutation());
194193
} finally {
195194
personRepository.deleteById(person.getId().toString());
196195
}
@@ -442,6 +441,7 @@ public void testExpiryAnnotation() {
442441
void count() {
443442
String[] iatas = { "JFK", "IAD", "SFO", "SJC", "SEA", "LAX", "PHX" };
444443

444+
airportRepository.countOne();
445445
try {
446446
airportRepository.saveAll(
447447
Arrays.stream(iatas).map((iata) -> new Airport("airports::" + iata, iata, iata.toLowerCase(Locale.ROOT)))
@@ -450,6 +450,11 @@ void count() {
450450
Long count = airportRepository.countFancyExpression(asList("JFK"), asList("jfk"), false);
451451
assertEquals(1, count);
452452

453+
Pageable sPageable = PageRequest.of(0, 2).withSort(Sort.by("iata"));
454+
Page<Airport> sPage = airportRepository.getAllByIataNot("JFK", sPageable);
455+
assertEquals(iatas.length - 1, sPage.getTotalElements());
456+
assertEquals(sPageable.getPageSize(), sPage.getContent().size());
457+
453458
Pageable pageable = PageRequest.of(0, 2).withSort(Sort.by("iata"));
454459
Page<Airport> aPage = airportRepository.findAllByIataNot("JFK", pageable);
455460
assertEquals(iatas.length - 1, aPage.getTotalElements());
@@ -476,13 +481,13 @@ void count() {
476481
}
477482
}
478483

479-
@Test
480-
void badCount(){
484+
@Test
485+
void badCount() {
481486
assertThrows(CouchbaseQueryExecutionException.class, () -> airportRepository.countBad());
482487
}
483488

484-
@Test
485-
void goodCount(){
489+
@Test
490+
void goodCount() {
486491
airportRepository.countGood();
487492
}
488493

0 commit comments

Comments
 (0)