Fix ArrayIndexOutOfBoundsException on String queries that use paging. (#1197)
Closes #1155. Co-authored-by: mikereiche <michael.reiche@couchbase.com>
This commit is contained in:
@@ -136,7 +136,7 @@ public class Query {
|
||||
}
|
||||
this.limit = pageable.getPageSize();
|
||||
this.skip = pageable.getOffset();
|
||||
if(!this.sort.equals(pageable.getSort()))
|
||||
if (!this.sort.equals(pageable.getSort()))
|
||||
this.sort.and(pageable.getSort());
|
||||
return this;
|
||||
}
|
||||
@@ -176,7 +176,7 @@ public class Query {
|
||||
return this;
|
||||
}
|
||||
|
||||
public Query withoutSort(){
|
||||
public Query withoutSort() {
|
||||
this.sort = Sort.unsorted();
|
||||
return this;
|
||||
}
|
||||
@@ -277,11 +277,6 @@ public class Query {
|
||||
return sb.toString();
|
||||
}
|
||||
|
||||
public String toN1qlSelectString(ReactiveCouchbaseTemplate template, String collectionName, Class domainClass,
|
||||
boolean isCount) {
|
||||
return toN1qlSelectString(template, collectionName, domainClass, null, isCount, null);
|
||||
}
|
||||
|
||||
public String toN1qlSelectString(ReactiveCouchbaseTemplate template, Class domainClass, boolean isCount) {
|
||||
return toN1qlSelectString(template, null, domainClass, null, isCount, null);
|
||||
}
|
||||
@@ -294,7 +289,7 @@ public class Query {
|
||||
appendString(statement, n1ql.selectEntity); // select ...
|
||||
appendWhereString(statement, n1ql.filter); // typeKey = typeValue
|
||||
appendWhere(statement, new int[] { 0 }, template.getConverter()); // criteria on this Query
|
||||
if(!isCount){
|
||||
if (!isCount) {
|
||||
appendSort(statement);
|
||||
appendSkipAndLimit(statement);
|
||||
}
|
||||
|
||||
@@ -19,6 +19,9 @@ import org.springframework.data.couchbase.core.ReactiveCouchbaseTemplate;
|
||||
|
||||
import com.couchbase.client.java.json.JsonArray;
|
||||
import com.couchbase.client.java.json.JsonValue;
|
||||
import org.springframework.data.couchbase.core.support.TemplateUtils;
|
||||
|
||||
import java.util.Locale;
|
||||
|
||||
/**
|
||||
* Query created from the string in @Query annotation in the repository interface.
|
||||
@@ -35,7 +38,7 @@ import com.couchbase.client.java.json.JsonValue;
|
||||
*/
|
||||
public class StringQuery extends Query {
|
||||
|
||||
private String inlineN1qlQuery;
|
||||
private final String inlineN1qlQuery;
|
||||
|
||||
public StringQuery(String n1qlString) {
|
||||
inlineN1qlQuery = n1qlString;
|
||||
@@ -55,6 +58,11 @@ public class StringQuery extends Query {
|
||||
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 class StringQuery extends Query {
|
||||
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();
|
||||
}
|
||||
|
||||
|
||||
@@ -16,8 +16,6 @@
|
||||
package org.springframework.data.couchbase.repository.query;
|
||||
|
||||
import static org.springframework.data.couchbase.core.query.N1QLExpression.i;
|
||||
import static org.springframework.data.couchbase.core.query.N1QLExpression.meta;
|
||||
import static org.springframework.data.couchbase.core.query.N1QLExpression.path;
|
||||
import static org.springframework.data.couchbase.core.query.N1QLExpression.x;
|
||||
import static org.springframework.data.couchbase.core.support.TemplateUtils.SELECT_CAS;
|
||||
import static org.springframework.data.couchbase.core.support.TemplateUtils.SELECT_ID;
|
||||
@@ -31,14 +29,12 @@ import java.util.regex.Pattern;
|
||||
|
||||
import org.slf4j.Logger;
|
||||
import org.slf4j.LoggerFactory;
|
||||
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.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.PersistentPropertyPath;
|
||||
import org.springframework.data.mapping.PropertyHandler;
|
||||
@@ -197,7 +193,7 @@ public class StringBasedN1qlQueryParser {
|
||||
TypeInformation resultClass/*, String path*/) {
|
||||
|
||||
PersistentEntity persistentEntity = couchbaseConverter.getMappingContext().getPersistentEntity(resultClass);
|
||||
//CouchbasePersistentProperty property = path.getLeafProperty();
|
||||
// CouchbasePersistentProperty property = path.getLeafProperty();
|
||||
persistentEntity.doWithProperties(new PropertyHandler<CouchbasePersistentProperty>() {
|
||||
@Override
|
||||
public void doWithPersistentProperty(final CouchbasePersistentProperty prop) {
|
||||
@@ -207,7 +203,8 @@ public class StringBasedN1qlQueryParser {
|
||||
if (prop.isVersionProperty()) {
|
||||
return;
|
||||
}
|
||||
PersistentPropertyPath<CouchbasePersistentProperty> path = couchbaseConverter.getMappingContext().getPersistentPropertyPath(prop.getName(), resultClass.getType());
|
||||
PersistentPropertyPath<CouchbasePersistentProperty> path = couchbaseConverter.getMappingContext()
|
||||
.getPersistentPropertyPath(prop.getName(), resultClass.getType());
|
||||
|
||||
// The current limitation is that only top-level properties can be projected
|
||||
// This traversing of nested data structures would need to replicate the processing done by
|
||||
@@ -495,16 +492,7 @@ public class StringBasedN1qlQueryParser {
|
||||
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()));
|
||||
@@ -518,6 +506,8 @@ public class StringBasedN1qlQueryParser {
|
||||
for (Object o : accessor) {
|
||||
params.add(o);
|
||||
}
|
||||
params.add(accessor.getPageable());
|
||||
params.add(accessor.getSort());
|
||||
return params.toArray();
|
||||
}
|
||||
}
|
||||
|
||||
@@ -19,6 +19,7 @@ import static org.springframework.data.couchbase.core.query.N1QLExpression.x;
|
||||
import static org.springframework.data.couchbase.core.query.QueryCriteria.where;
|
||||
|
||||
import java.util.Iterator;
|
||||
import java.util.Optional;
|
||||
|
||||
import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
|
||||
import org.springframework.data.couchbase.core.mapping.CouchbasePersistentProperty;
|
||||
@@ -26,6 +27,7 @@ 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.core.query.StringQuery;
|
||||
import org.springframework.data.domain.Pageable;
|
||||
import org.springframework.data.domain.Sort;
|
||||
import org.springframework.data.mapping.Alias;
|
||||
import org.springframework.data.mapping.PersistentPropertyPath;
|
||||
@@ -116,6 +118,17 @@ public class StringN1qlQueryCreator extends AbstractQueryCreator<Query, QueryCri
|
||||
return from(part, property, where(x(path.toDotPath())), iterator);
|
||||
}
|
||||
|
||||
@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 and(final Part part, final QueryCriteria base, final Iterator<Object> iterator) {
|
||||
if (base == null) {
|
||||
|
||||
@@ -114,18 +114,26 @@ public interface AirportRepository extends CouchbaseRepository<Airport, String>,
|
||||
Long countFancyExpression(@Param("projectIds") List<String> projectIds, @Param("planIds") List<String> planIds,
|
||||
@Param("active") Boolean active);
|
||||
|
||||
@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE 0 = 1" )
|
||||
@Query("SELECT 1 FROM `#{#n1ql.bucket}` WHERE anything = 'count(*)'") // looks like count query, but is not
|
||||
Long countBad();
|
||||
|
||||
@Query("SELECT count(*) FROM `#{#n1ql.bucket}`" )
|
||||
@Query("SELECT count(*) FROM `#{#n1ql.bucket}`")
|
||||
Long countGood();
|
||||
|
||||
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
|
||||
Page<Airport> findAllByIataNot(String iata, Pageable pageable);
|
||||
|
||||
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
|
||||
@Query("#{#n1ql.selectEntity} WHERE #{#n1ql.filter} AND iata != $1")
|
||||
Page<Airport> getAllByIataNot(String iata, Pageable pageable);
|
||||
|
||||
@ScanConsistency(query = QueryScanConsistency.REQUEST_PLUS)
|
||||
Optional<Airport> findByIdAndIata(String id, String iata);
|
||||
|
||||
@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();
|
||||
|
||||
@Retention(RetentionPolicy.RUNTIME)
|
||||
@Target({ ElementType.METHOD, ElementType.TYPE })
|
||||
// @Meta
|
||||
|
||||
@@ -39,7 +39,6 @@ import java.util.concurrent.Executors;
|
||||
import java.util.concurrent.Future;
|
||||
import java.util.stream.Collectors;
|
||||
|
||||
import com.couchbase.client.java.kv.UpsertOptions;
|
||||
import org.junit.jupiter.api.BeforeEach;
|
||||
import org.junit.jupiter.api.Test;
|
||||
import org.springframework.beans.factory.annotation.Autowired;
|
||||
@@ -89,6 +88,7 @@ import com.couchbase.client.java.env.ClusterEnvironment;
|
||||
import com.couchbase.client.java.json.JsonArray;
|
||||
import com.couchbase.client.java.kv.GetResult;
|
||||
import com.couchbase.client.java.kv.MutationState;
|
||||
import com.couchbase.client.java.kv.UpsertOptions;
|
||||
import com.couchbase.client.java.query.QueryOptions;
|
||||
import com.couchbase.client.java.query.QueryScanConsistency;
|
||||
|
||||
@@ -187,10 +187,9 @@ public class CouchbaseRepositoryQueryIntegrationTests extends ClusterAwareIntegr
|
||||
assertEquals(person.getSalutation(), result.contentAsObject().get("prefix"));
|
||||
Person person2 = personRepository.findById(person.getId().toString()).get();
|
||||
assertEquals(person.getSalutation(), person2.getSalutation());
|
||||
// needs fix from datacouch_1184
|
||||
//List<Person> persons3 = personRepository.findBySalutation("Mrs");
|
||||
//assertEquals(1, persons3.size());
|
||||
//assertEquals(person.getSalutation(), persons3.get(0).getSalutation());
|
||||
List<Person> persons3 = personRepository.findBySalutation("Mrs");
|
||||
assertEquals(1, persons3.size());
|
||||
assertEquals(person.getSalutation(), persons3.get(0).getSalutation());
|
||||
} finally {
|
||||
personRepository.deleteById(person.getId().toString());
|
||||
}
|
||||
@@ -442,6 +441,7 @@ public class CouchbaseRepositoryQueryIntegrationTests extends ClusterAwareIntegr
|
||||
void count() {
|
||||
String[] iatas = { "JFK", "IAD", "SFO", "SJC", "SEA", "LAX", "PHX" };
|
||||
|
||||
airportRepository.countOne();
|
||||
try {
|
||||
airportRepository.saveAll(
|
||||
Arrays.stream(iatas).map((iata) -> new Airport("airports::" + iata, iata, iata.toLowerCase(Locale.ROOT)))
|
||||
@@ -450,6 +450,11 @@ public class CouchbaseRepositoryQueryIntegrationTests extends ClusterAwareIntegr
|
||||
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).withSort(Sort.by("iata"));
|
||||
Page<Airport> aPage = airportRepository.findAllByIataNot("JFK", pageable);
|
||||
assertEquals(iatas.length - 1, aPage.getTotalElements());
|
||||
@@ -476,13 +481,13 @@ public class CouchbaseRepositoryQueryIntegrationTests extends ClusterAwareIntegr
|
||||
}
|
||||
}
|
||||
|
||||
@Test
|
||||
void badCount(){
|
||||
@Test
|
||||
void badCount() {
|
||||
assertThrows(CouchbaseQueryExecutionException.class, () -> airportRepository.countBad());
|
||||
}
|
||||
|
||||
@Test
|
||||
void goodCount(){
|
||||
@Test
|
||||
void goodCount() {
|
||||
airportRepository.countGood();
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user