Skip to content

DATACOUCH-603 - Do not cast query parameters in N1qlQueryCreator. #263

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import com.couchbase.client.java.query.QueryOptions;
import com.couchbase.client.java.query.QueryScanConsistency;
import org.springframework.data.couchbase.core.ReactiveCouchbaseTemplate;
import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
import org.springframework.data.couchbase.core.mapping.CouchbasePersistentEntity;
import org.springframework.data.couchbase.repository.query.StringBasedN1qlQueryParser;
import org.springframework.data.couchbase.repository.support.MappingCouchbaseEntityInformation;
Expand Down Expand Up @@ -189,7 +190,7 @@ public void appendSort(final StringBuilder sb) {
sb.deleteCharAt(sb.length() - 1);
}

public void appendWhere(final StringBuilder sb, int[] paramIndexPtr) {
public void appendWhere(final StringBuilder sb, int[] paramIndexPtr, CouchbaseConverter converter) {
if (!criteria.isEmpty()) {
appendWhereOrAnd(sb);
boolean first = true;
Expand All @@ -199,16 +200,11 @@ public void appendWhere(final StringBuilder sb, int[] paramIndexPtr) {
} else {
sb.append(" AND ");
}
sb.append(c.export(paramIndexPtr));
sb.append(c.export(paramIndexPtr, parameters, converter));
}
}
}

public void appendCriteria(StringBuilder sb, QueryCriteria criteria) {
appendWhereOrAnd(sb);
sb.append(criteria.export());
}

public void appendWhereString(StringBuilder sb, String whereString) {
appendWhereOrAnd(sb);
sb.append(whereString);
Expand Down Expand Up @@ -257,9 +253,9 @@ private static boolean notQuoted(int start, int end, String querySoFar) {
return true; // is not quoted
}

public String export() {
public String export(int[]... paramIndexPtrHolder) { // used only by tests
StringBuilder sb = new StringBuilder();
appendWhere(sb, null);
appendWhere(sb, paramIndexPtrHolder.length > 0 ? paramIndexPtrHolder[0] : null, null);
appendSort(sb);
appendSkipAndLimit(sb);
return sb.toString();
Expand All @@ -270,7 +266,7 @@ public String toN1qlSelectString(ReactiveCouchbaseTemplate template, Class domai
final StringBuilder statement = new StringBuilder();
appendString(statement, n1ql.selectEntity); // select ...
appendWhereString(statement, n1ql.filter); // typeKey = typeValue
appendWhere(statement, new int[] { 0 }); // criteria on this Query
appendWhere(statement, new int[] { 0 }, template.getConverter()); // criteria on this Query
appendSort(statement);
appendSkipAndLimit(statement);
return statement.toString();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@
import java.util.LinkedList;
import java.util.List;

import com.couchbase.client.core.error.InvalidArgumentException;
import com.couchbase.client.java.json.JsonArray;
import com.couchbase.client.java.json.JsonObject;
import com.couchbase.client.java.json.JsonValue;
import org.springframework.data.couchbase.core.convert.CouchbaseConverter;
import org.springframework.lang.Nullable;

/**
Expand Down Expand Up @@ -60,6 +65,10 @@ public class QueryCriteria implements QueryCriteriaDefinition {
this.format = format;
}

Object[] getValue() {
return value;
}

/**
* Static factory method to create a Criteria using the provided key.
*/
Expand All @@ -68,8 +77,8 @@ public static QueryCriteria where(String key) {
}

private static QueryCriteria wrap(QueryCriteria criteria) {
QueryCriteria qc = new QueryCriteria(new LinkedList<QueryCriteria>(), criteria.key, criteria.value, null,
criteria.operator, criteria.format);
QueryCriteria qc = new QueryCriteria(new LinkedList<>(), criteria.key, criteria.value, null, criteria.operator,
criteria.format);
return qc;
}

Expand Down Expand Up @@ -167,7 +176,7 @@ public QueryCriteria containing(@Nullable Object o) {
public QueryCriteria notContaining(@Nullable Object o) {
value = new QueryCriteria[] { wrap(containing(o)) };
operator = "NOT";
format = format = "not( %3$s )";
format = "not( %3$s )";
return this;
}

Expand Down Expand Up @@ -196,7 +205,7 @@ public QueryCriteria isNotNull() {
operator = "IS_NOT_NULL";
value = null;
format = "%1$s is not null";
return (QueryCriteria) this;
return this;
}

public QueryCriteria isMissing() {
Expand All @@ -210,7 +219,7 @@ public QueryCriteria isNotMissing() {
operator = "IS_NOT_MiSSING";
value = null;
format = "%1$s is not missing";
return (QueryCriteria) this;
return this;
}

public QueryCriteria isValued() {
Expand All @@ -224,68 +233,74 @@ public QueryCriteria isNotValued() {
operator = "IS_NOT_VALUED";
value = null;
format = "%1$s is not valued";
return (QueryCriteria) this;
return this;
}

public QueryCriteria within(@Nullable Object o) {
operator = "WITHIN";
value = new Object[] { o };
format = "%1$s within $3$s";
return (QueryCriteria) this;
format = "%1$s within %3$s";
return this;
}

public QueryCriteria between(@Nullable Object o1, @Nullable Object o2) {
operator = "BETWEEN";
value = new Object[] { o1, o2 };
format = "%1$s between %3$s and %4$s";
return (QueryCriteria) this;
return this;
}

public QueryCriteria in(@Nullable Object... o) {
operator = "IN";
value = o;
StringBuilder sb = new StringBuilder("%1$s in ( [ ");
for (int i = 1; i <= value.length; i++) { // format indices start at 1
if (i > 1)
sb.append(", ");
sb.append("%" + (i + 2) + "$s"); // the first is fieldName, second is operator, args start at 3
format = "%1$s in ( %3$s )";
// IN takes a single argument that is a list
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");
}
value = o;
} else {
value = new Object[1];
value[0] = o; // JsonArray.from(o);
}
}
format = sb.append(" ] )").toString();
return (QueryCriteria) this;
return this;
}

public QueryCriteria notIn(@Nullable Object... o) {
value = new QueryCriteria[] { wrap(in(o)) };
operator = "NOT";
format = format = "not( %3$s )"; // field = 1$, operator = 2$, value=$3, $4, ...
return (QueryCriteria) this;
format = "not( %3$s )"; // field = 1$, operator = 2$, value=$3, $4, ...
return this;
}

public QueryCriteria TRUE() { // true/false are reserved, use TRUE/FALSE
value = null;
operator = null;
format = format = "%1$s"; // field = 1$, operator = 2$, value=$3, $4, ...
return (QueryCriteria) this;
format = "%1$s"; // field = 1$, operator = 2$, value=$3, $4, ...
return this;
}

public QueryCriteria FALSE() {
value = new QueryCriteria[] { wrap(TRUE()) };
operator = "not";
format = format = "not( %3$s )";
return (QueryCriteria) this;
format = "not( %3$s )";
return this;
}

/**
* This exports the query criteria into a string to be appended to the beginning of an N1QL statement
* This exports the query criteria chain into a string to be appended to the beginning of an N1QL statement
*
* @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters
* There may already be positional parameters in the beginning of the statement,
* so it may not always start at 1. If it has the value -1, the query is using
* named parameters. If the pointer is null, the query is not using parameters.
* @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters There may
* already be positional parameters in the beginning of the statement, so it may not always start at 1. If it
* has the value -1, the query is using named parameters. If the pointer is null, the query is not using
* parameters.
* @param parameters - parameters of the query. If operands are parameterized, their values are added to parameters
* @return string containing part of N1QL query
*/
@Override
public String export(int[] paramIndexPtr) {
public String export(int[] paramIndexPtr, JsonValue parameters, CouchbaseConverter converter) {
StringBuilder output = new StringBuilder();
boolean first = true;
for (QueryCriteria c : this.criteriaChain) {
Expand All @@ -298,7 +313,7 @@ public String export(int[] paramIndexPtr) {
} else {
first = false;
}
c.exportSingle(output, paramIndexPtr);
c.exportSingle(output, paramIndexPtr, parameters, converter);
}

return output.toString();
Expand All @@ -310,22 +325,34 @@ public String export(int[] paramIndexPtr) {
* @return string containing part of N1QL query
*/
@Override
public String export() {
return export(null);
public String export() { // used only by tests
return export(null, null, null);

}

private StringBuilder exportSingle(StringBuilder sb, int[] paramIndexPtr) {
/**
* Appends the query criteria to a StringBuilder which will be appended to a N1QL statement
*
* @param sb - the string builder
* @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters There may
* already be positional parameters in the beginning of the statement, so it may not always start at 1. If it
* has the value -1, the query is using named parameters. If the pointer is null, the query is not using
* parameters.
* @param parameters - parameters of the query. If operands are parameterized, their values are added to parameters
* @return string containing part of N1QL query
*/
private StringBuilder exportSingle(StringBuilder sb, int[] paramIndexPtr, JsonValue parameters,
CouchbaseConverter converter) {
String fieldName = maybeQuote(key);
int valueLen = value == null ? 0 : value.length;
Object[] v = new Object[valueLen + 2];
v[0] = fieldName;
v[1] = operator;
for (int i = 0; i < valueLen; i++) {
if (value[i] instanceof QueryCriteria) {
v[i + 2] = "(" + ((QueryCriteria) value[i]).export(paramIndexPtr) + ")";
v[i + 2] = "(" + ((QueryCriteria) value[i]).export(paramIndexPtr, parameters, converter) + ")";
} else {
v[i + 2] = maybeWrapValue(key, value[i], paramIndexPtr);
v[i + 2] = maybeWrapValue(key, value[i], paramIndexPtr, parameters, converter);
}
}

Expand All @@ -340,24 +367,84 @@ private StringBuilder exportSingle(StringBuilder sb, int[] paramIndexPtr) {
return sb;
}

private String maybeWrapValue(String key, Object value, int[] paramIndexPtr) {
/**
* Possibly convert an operand to a positional or named parameter
*
* @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters There may
* already be positional parameters in the beginning of the statement, so it may not always start at 1. If it
* has the value -1, the query is using named parameters. If the pointer is null, the query is not using
* parameters.
* @param parameters - parameters of the query. If operands are parameterized, their values are added to parameters
* @return string containing part of N1QL query
*/
private String maybeWrapValue(String key, Object value, int[] paramIndexPtr, JsonValue parameters,
CouchbaseConverter converter) {
if (paramIndexPtr != null) {
if (paramIndexPtr[0] >= 0) {
JsonArray params = (JsonArray) parameters;
// from StringBasedN1qlQueryParser.getPositionalPlaceholderValues()
try {
params.add(convert(converter, value));
} catch (InvalidArgumentException iae) {
if (value instanceof Object[]) {
addAsArray(params, value, converter);
} else {
throw iae;
}
}
return "$" + (++paramIndexPtr[0]); // these are generated in order
} else {
JsonObject params = (JsonObject) parameters;
// from StringBasedN1qlQueryParser.getNamedPlaceholderValues()
try {
params.put(key, convert(converter, value));
} catch (InvalidArgumentException iae) {
if (value instanceof Object[]) {
params.put(key, JsonArray.from((Object[]) value));
} else {
throw iae;
}
}
return "$" + key;
}
}

// Did not convert to a parameter. Add quotes or whatever it might need.

if (value instanceof String) {
return "\"" + value + "\"";
} else if (value == null) {
return "null";
} else if (value instanceof Object[]) { // convert array into sequence of comma-separated values
StringBuffer l = new StringBuffer();
l.append("[");
Object[] array = (Object[]) value;
for (int i = 0; i < array.length; i++) {
if (i > 0) {
l.append(",");
}
l.append(maybeWrapValue(null, array[i], null, null, converter));
}
l.append("]");
return l.toString();
} else {
return value.toString();
}
}

private static Object convert(CouchbaseConverter converter, Object value) {
return converter != null ? converter.convertForWriteIfNeeded(value) : value;
}

private void addAsArray(JsonArray posValues, Object o, CouchbaseConverter converter) {
Object[] array = (Object[]) o;
JsonArray ja = JsonValue.ja();
for (Object e : array) {
ja.add(String.valueOf(convert(converter, e)));
}
posValues.add(ja);
}

private String maybeQuote(String value) {
if (value == null || (value.startsWith("\"") && value.endsWith("\""))) {
return value;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@
*/
package org.springframework.data.couchbase.core.query;

import com.couchbase.client.java.json.JsonValue;
import org.springframework.data.couchbase.core.convert.CouchbaseConverter;

/**
* @author Oliver Gierke
* @author Christoph Strobl
Expand All @@ -25,13 +28,15 @@ public interface QueryCriteriaDefinition {
/**
* This exports the query criteria into a string to be appended to the beginning of an N1QL statement
*
* @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters
* There may already be positional parameters in the beginning of the statement,
* so it may not always start at 1. If it has the value -1, the query is using
* named parameters. If the pointer is null, the query is not using parameters.
* @param paramIndexPtr - this is a reference to the parameter index to be used for positional parameters There may
* already be positional parameters in the beginning of the statement, so it may not always start at 1. If it
* has the value -1, the query is using named parameters. If the pointer is null, the query is not using
* parameters.
* @param parameters - query parameters. Criteria values that are converted to arguments are added to parameters
* @param converter - converter to use for converting criteria values
* @return string containing part of N1QL query
*/
String export(int[] paramIndexPtr);
String export(int[] paramIndexPtr, JsonValue parameters, CouchbaseConverter converter);

/**
* Export the query criteria to a string without using positional or named parameters.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ public String toN1qlSelectString(ReactiveCouchbaseTemplate template, Class domai
} else { // named parameters or no parameters, no index required
paramIndexPtr = new int[] { -1 };
}
appendWhere(statement, paramIndexPtr); // criteria on this Query - should be empty for StringQuery
appendWhere(statement, paramIndexPtr, template.getConverter()); // criteria on this Query - should be empty for
// StringQuery
appendSort(statement);
appendSkipAndLimit(statement);
return statement.toString();
Expand Down
Loading