From 363c734f0a5439220375c2412693585d1d480b07 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 30 Jun 2023 14:35:01 +0200 Subject: [PATCH] Refine value binding for statements accepting TTL and Timestamp. We now use bind markers to represent the TTL and Timestamp for Insert, Update, and Delete if the StatementFactory uses bind markers. Otherwise, we fall back to inline values. This change prevents re-preparing statements for statements with different TTL and Timestamp values. Closes #1401 --- .../data/cassandra/core/StatementFactory.java | 114 +++++--- .../cassandra/core/cql/QueryOptionsUtil.java | 254 ++++++++++++++++-- .../cassandra/core/cql/util/Bindings.java | 42 +++ .../core/cql/util/StatementBuilder.java | 44 ++- .../cassandra/core/cql/util/TermFactory.java | 17 ++ .../core/StatementFactoryUnitTests.java | 65 +++++ 6 files changed, 470 insertions(+), 66 deletions(-) create mode 100644 spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/Bindings.java diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/StatementFactory.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/StatementFactory.java index d009a1c96..77df286e6 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/StatementFactory.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/StatementFactory.java @@ -33,6 +33,7 @@ import org.springframework.data.cassandra.core.convert.UpdateMapper; import org.springframework.data.cassandra.core.convert.Where; import org.springframework.data.cassandra.core.cql.QueryOptions; import org.springframework.data.cassandra.core.cql.QueryOptionsUtil; +import org.springframework.data.cassandra.core.cql.QueryOptionsUtil.CqlStatementOptionsAccessor; import org.springframework.data.cassandra.core.cql.WriteOptions; import org.springframework.data.cassandra.core.cql.util.StatementBuilder; import org.springframework.data.cassandra.core.cql.util.TermFactory; @@ -309,9 +310,13 @@ public class StatementFactory { .of(QueryBuilder.insertInto(tableName).valuesByIds(Collections.emptyMap())).bind((statement, factory) -> { Map values = createTerms(insertNulls, object, factory); + CqlStatementOptionsAccessor accessor = factory.ifBoundOrInline( + bindings -> CqlStatementOptionsAccessor.ofInsert(bindings, statement), + () -> CqlStatementOptionsAccessor.ofInsert(statement)); + RegularInsert afterOptions = (RegularInsert) addInsertOptions(accessor, options); - return statement.valuesByIds(values); - }).apply(statement -> (RegularInsert) addWriteOptions(statement, options)); + return afterOptions.valuesByIds(values); + }); builder.transform(statement -> QueryOptionsUtil.addQueryOptions(statement, options)); @@ -373,15 +378,12 @@ public class StatementFactory { Update mappedUpdate = getUpdateMapper().getMappedObject(update, persistentEntity); StatementBuilder builder = update(tableName, mappedUpdate, - filter); + filter, query.getQueryOptions().filter(WriteOptions.class::isInstance).map(WriteOptions.class::cast)); query.getQueryOptions().filter(UpdateOptions.class::isInstance).map(UpdateOptions.class::cast) .map(UpdateOptions::getIfCondition) .ifPresent(criteriaDefinitions -> applyUpdateIfCondition(builder, criteriaDefinitions)); - query.getQueryOptions().filter(WriteOptions.class::isInstance).map(WriteOptions.class::cast) - .ifPresent(writeOptions -> builder.apply(statement -> addWriteOptions(statement, writeOptions))); - query.getQueryOptions().ifPresent( options -> builder.transform(statementBuilder -> QueryOptionsUtil.addQueryOptions(statementBuilder, options))); @@ -435,10 +437,16 @@ public class StatementFactory { where.forEach((cqlIdentifier, o) -> object.remove(cqlIdentifier)); StatementBuilder builder = StatementBuilder - .of(QueryBuilder.update(tableName).set().where()) - .bind((statement, factory) -> ((UpdateWithAssignments) statement).set(toAssignments(object, factory)) - .where(toRelations(where, factory))) - .apply(update -> addWriteOptions(update, options)); + .of(QueryBuilder.update(tableName).set().where()).bind((statement, factory) -> { + + CqlStatementOptionsAccessor accessor = factory.ifBoundOrInline( + bindings -> CqlStatementOptionsAccessor.ofUpdate(bindings, (UpdateStart) statement), + () -> CqlStatementOptionsAccessor.ofUpdate((UpdateStart) statement)); + com.datastax.oss.driver.api.querybuilder.update.Update statementToUse = addUpdateOptions(accessor, options); + + return ((UpdateWithAssignments) statementToUse).set(toAssignments(object, factory)) + .where(toRelations(where, factory)); + }); Optional.of(options).filter(UpdateOptions.class::isInstance).map(UpdateOptions.class::cast) .map(UpdateOptions::getIfCondition) @@ -503,15 +511,13 @@ public class StatementFactory { Filter filter = getQueryMapper().getMappedObject(query, persistentEntity); List columnNames = getQueryMapper().getMappedColumnNames(query.getColumns(), persistentEntity); - StatementBuilder builder = delete(columnNames, tableName, filter); + StatementBuilder builder = delete(columnNames, tableName, filter, + query.getQueryOptions().filter(WriteOptions.class::isInstance).map(WriteOptions.class::cast)); query.getQueryOptions().filter(DeleteOptions.class::isInstance).map(DeleteOptions.class::cast) .map(DeleteOptions::getIfCondition) .ifPresent(criteriaDefinitions -> applyDeleteIfCondition(builder, criteriaDefinitions)); - query.getQueryOptions().filter(WriteOptions.class::isInstance).map(WriteOptions.class::cast) - .ifPresent(writeOptions -> builder.apply(statement -> addWriteOptions(statement, writeOptions))); - query.getQueryOptions() .ifPresent(options -> builder.transform(statement -> QueryOptionsUtil.addQueryOptions(statement, options))); @@ -539,10 +545,21 @@ public class StatementFactory { entityWriter.write(entity, where); StatementBuilder builder = StatementBuilder.of(QueryBuilder.deleteFrom(tableName).where()) - .bind((statement, factory) -> statement.where(toRelations(where, factory))); + .bind((statement, factory) -> { - Optional.of(options).filter(WriteOptions.class::isInstance).map(WriteOptions.class::cast) - .ifPresent(it -> builder.apply(statement -> addWriteOptions(statement, it))); + Delete statementToUse; + if (options instanceof WriteOptions wo) { + + CqlStatementOptionsAccessor accessor = factory.ifBoundOrInline( + bindings -> CqlStatementOptionsAccessor.ofDelete(bindings, (DeleteSelection) statement), + () -> CqlStatementOptionsAccessor.ofDelete((DeleteSelection) statement)); + statementToUse = addDeleteOptions(accessor, wo); + } else { + statementToUse = statement; + } + + return statementToUse.where(toRelations(where, factory)); + }); Optional.of(options).filter(DeleteOptions.class::isInstance).map(DeleteOptions.class::cast) .map(DeleteOptions::getIfCondition) @@ -707,17 +724,28 @@ public class StatementFactory { } private static StatementBuilder update(CqlIdentifier table, - Update mappedUpdate, Filter filter) { + Update mappedUpdate, Filter filter, Optional optionalOptions) { UpdateStart updateStart = QueryBuilder.update(table); return StatementBuilder.of((com.datastax.oss.driver.api.querybuilder.update.Update) updateStart) .bind((statement, factory) -> { + com.datastax.oss.driver.api.querybuilder.update.Update statementToUse; + WriteOptions options = optionalOptions.orElse(null); + if (options != null) { + CqlStatementOptionsAccessor accessor = factory.ifBoundOrInline( + bindings -> CqlStatementOptionsAccessor.ofUpdate(bindings, (UpdateStart) statement), + () -> CqlStatementOptionsAccessor.ofUpdate((UpdateStart) statement)); + statementToUse = addUpdateOptions(accessor, options); + } else { + statementToUse = statement; + } + List assignments = mappedUpdate.getUpdateOperations().stream() .map(assignmentOp -> getAssignment(assignmentOp, factory)).collect(Collectors.toList()); - return (com.datastax.oss.driver.api.querybuilder.update.Update) ((OngoingAssignment) statement) + return (com.datastax.oss.driver.api.querybuilder.update.Update) ((OngoingAssignment) statementToUse) .set(assignments); }).bind((statement, factory) -> { @@ -851,7 +879,8 @@ public class StatementFactory { return Assignment.append(updateOp.toCqlIdentifier(), termFactory.create(updateOp.getValue())); } - private StatementBuilder delete(List columnNames, CqlIdentifier from, Filter filter) { + private StatementBuilder delete(List columnNames, CqlIdentifier from, Filter filter, + Optional optionsOptional) { DeleteSelection select = QueryBuilder.deleteFrom(from); @@ -860,7 +889,19 @@ public class StatementFactory { } return StatementBuilder.of(select.where()).bind((statement, factory) -> { - return statement.where(getRelations(filter, factory)); + + WriteOptions options = optionsOptional.orElse(null); + Delete statementToUse; + if (options != null) { + CqlStatementOptionsAccessor accessor = factory.ifBoundOrInline( + bindings -> CqlStatementOptionsAccessor.ofDelete(bindings, (DeleteSelection) statement), + () -> CqlStatementOptionsAccessor.ofDelete((DeleteSelection) statement)); + statementToUse = addDeleteOptions(accessor, options); + } else { + statementToUse = statement; + } + + return statementToUse.where(getRelations(filter, factory)); }); } @@ -870,21 +911,22 @@ public class StatementFactory { * @param insert {@link Insert} CQL statement, must not be {@literal null}. * @param writeOptions write options (e.g. consistency level) to add to the CQL statement. * @return the given {@link Insert}. - * @see #addWriteOptions(Insert, WriteOptions) * @since 2.1 */ - static Insert addWriteOptions(Insert insert, WriteOptions writeOptions) { + static Insert addInsertOptions(CqlStatementOptionsAccessor insert, WriteOptions writeOptions) { Assert.notNull(insert, "Insert must not be null"); + Insert insertToUse = QueryOptionsUtil.addWriteOptions(insert, writeOptions); + if (writeOptions instanceof InsertOptions insertOptions) { if (insertOptions.isIfNotExists()) { - insert = insert.ifNotExists(); + insertToUse = insertToUse.ifNotExists(); } } - return QueryOptionsUtil.addWriteOptions(insert, writeOptions); + return insertToUse; } /** @@ -898,13 +940,13 @@ public class StatementFactory { * @see QueryOptionsUtil#addWriteOptions(com.datastax.oss.driver.api.querybuilder.update.Update, WriteOptions) * @since 2.1 */ - static com.datastax.oss.driver.api.querybuilder.update.Update addWriteOptions( - com.datastax.oss.driver.api.querybuilder.update.Update update, WriteOptions writeOptions) { + static com.datastax.oss.driver.api.querybuilder.update.Update addUpdateOptions( + CqlStatementOptionsAccessor update, WriteOptions writeOptions) { Assert.notNull(update, "Update must not be null"); - com.datastax.oss.driver.api.querybuilder.update.Update updateToUse = QueryOptionsUtil.addWriteOptions(update, - writeOptions); + com.datastax.oss.driver.api.querybuilder.update.Update updateToUse = (com.datastax.oss.driver.api.querybuilder.update.Update) QueryOptionsUtil + .addWriteOptions(update, writeOptions); if (writeOptions instanceof UpdateOptions updateOptions) { @@ -924,11 +966,11 @@ public class StatementFactory { * @return the given {@link Delete}. * @since 2.1 */ - static Delete addWriteOptions(Delete delete, WriteOptions writeOptions) { + static Delete addDeleteOptions(CqlStatementOptionsAccessor delete, WriteOptions writeOptions) { Assert.notNull(delete, "Delete must not be null"); - Delete deleteToUse = QueryOptionsUtil.addWriteOptions(delete, writeOptions); + Delete deleteToUse = (Delete) QueryOptionsUtil.addWriteOptions(delete, writeOptions); if (writeOptions instanceof DeleteOptions deleteOptions) { @@ -996,15 +1038,13 @@ public class StatementFactory { case CONTAINS: - Assert.state(value != null, - () -> String.format("CONTAINS value for column %s is null", columnName)); + Assert.state(value != null, () -> String.format("CONTAINS value for column %s is null", columnName)); return column.contains(factory.create(value)); case CONTAINS_KEY: - Assert.state(value != null, - () -> String.format("CONTAINS KEY value for column %s is null", columnName)); + Assert.state(value != null, () -> String.format("CONTAINS KEY value for column %s is null", columnName)); return column.containsKey(factory.create(value)); } @@ -1061,8 +1101,8 @@ public class StatementFactory { return column.in(factory.create(value)); } - throw new IllegalArgumentException(String.format("Criteria %s %s %s not supported for IF Conditions", columnName, - predicate.getOperator(), value)); + throw new IllegalArgumentException( + String.format("Criteria %s %s %s not supported for IF Conditions", columnName, predicate.getOperator(), value)); } static List toLiterals(@Nullable Object arrayOrList) { diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/QueryOptionsUtil.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/QueryOptionsUtil.java index 341b90dbb..126ea31f5 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/QueryOptionsUtil.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/QueryOptionsUtil.java @@ -16,13 +16,16 @@ package org.springframework.data.cassandra.core.cql; import java.time.Duration; +import java.util.function.BiFunction; +import org.springframework.data.cassandra.core.cql.util.Bindings; import org.springframework.util.Assert; import com.datastax.oss.driver.api.core.cql.BatchStatement; import com.datastax.oss.driver.api.core.cql.BoundStatement; import com.datastax.oss.driver.api.core.cql.SimpleStatement; import com.datastax.oss.driver.api.core.cql.Statement; +import com.datastax.oss.driver.api.querybuilder.BindMarker; import com.datastax.oss.driver.api.querybuilder.delete.Delete; import com.datastax.oss.driver.api.querybuilder.delete.DeleteSelection; import com.datastax.oss.driver.api.querybuilder.insert.Insert; @@ -85,7 +88,7 @@ public abstract class QueryOptionsUtil { if (queryOptions.getTracing() != null) { // While the following statement is null-safe, avoid setting Statement tracing if the tracing query option - // is null since Statements are immutable and the call creates a new object. Therefore keep the following + // is null since Statements are immutable and the call creates a new object. Therefore keep the following // statement wrapped in the conditional null check to avoid additional garbage and added GC pressure. statementToUse = statementToUse.setTracing(Boolean.TRUE.equals(queryOptions.getTracing())); } @@ -120,17 +123,40 @@ public abstract class QueryOptionsUtil { Insert insertToUse = insert; - if (!writeOptions.getTtl().isNegative()) { - insertToUse = insertToUse.usingTtl(Math.toIntExact(writeOptions.getTtl().getSeconds())); - } - if (writeOptions.getTimestamp() != null) { insertToUse = insertToUse.usingTimestamp(writeOptions.getTimestamp()); } + if (hasTtl(writeOptions.getTtl())) { + insertToUse = insertToUse.usingTtl(Math.toIntExact(writeOptions.getTtl().getSeconds())); + } + return insertToUse; } + /** + * Add common {@link WriteOptions} options to {@link Update} CQL statements. + * + * @param update {@link Update} CQL statement, must not be {@literal null}. + * @param writeOptions write options (e.g. consistency level) to add to the CQL statement. + * @return the given {@link Update}. + */ + public static Update addWriteOptions(Update update, WriteOptions writeOptions) { + + Assert.notNull(update, "Update must not be null"); + Assert.notNull(writeOptions, "WriteOptions must not be null"); + + if (writeOptions.getTimestamp() != null) { + update = (Update) ((UpdateStart) update).usingTimestamp(writeOptions.getTimestamp()); + } + + if (hasTtl(writeOptions.getTtl())) { + update = (Update) ((UpdateStart) update).usingTtl(getTtlSeconds(writeOptions.getTtl())); + } + + return update; + } + /** * Add common {@link WriteOptions} options to {@link Delete} CQL statements. * @@ -152,26 +178,27 @@ public abstract class QueryOptionsUtil { } /** - * Add common {@link WriteOptions} options to {@link Update} CQL statements. + * Add common {@link WriteOptions} options to CQL statements through {@link CqlStatementOptionsAccessor}. * - * @param update {@link Update} CQL statement, must not be {@literal null}. + * @param accessor CQL statement accessor, must not be {@literal null}. * @param writeOptions write options (e.g. consistency level) to add to the CQL statement. - * @return the given {@link Update}. + * @return the resulting statement. + * @since 4.2 */ - public static Update addWriteOptions(Update update, WriteOptions writeOptions) { + public static T addWriteOptions(CqlStatementOptionsAccessor accessor, WriteOptions writeOptions) { - Assert.notNull(update, "Update must not be null"); + Assert.notNull(accessor, "CqlStatementOptionsAccessor must not be null"); Assert.notNull(writeOptions, "WriteOptions must not be null"); - if (hasTtl(writeOptions.getTtl())) { - update = (Update) ((UpdateStart) update).usingTtl(getTtlSeconds(writeOptions.getTtl())); - } - if (writeOptions.getTimestamp() != null) { - update = (Update) ((UpdateStart) update).usingTimestamp(writeOptions.getTimestamp()); + accessor.usingTimestamp(writeOptions.getTimestamp()); } - return update; + if (hasTtl(writeOptions.getTtl())) { + accessor.usingTtl(getTtlSeconds(writeOptions.getTtl())); + } + + return accessor.getStatement(); } private static int getTtlSeconds(Duration ttl) { @@ -182,4 +209,199 @@ public abstract class QueryOptionsUtil { return !ttl.isZero() && !ttl.isNegative(); } + /** + * Wrapper for common options used with CQL statements that are represented in the CQL statement such as TTL and + * timestamp. + * + * @param + * @since 4.2 + */ + public static abstract class CqlStatementOptionsAccessor { + + /** + * Set the timestamp to the underlying statement. + * + * @param timestamp the timestamp value to use. + */ + abstract void usingTimestamp(long timestamp); + + /** + * Set the TTL to the underlying statement. + * + * @param ttl the TTL value to use. + */ + abstract void usingTtl(int ttl); + + /** + * Returns the current statement instance. + * + * @return the current statement instance. + */ + abstract T getStatement(); + + /** + * Creates an accessor variant that captures options through {@link BindMarker} for {@link Insert}. + * + * @param bindings + * @param statement + * @return + */ + public static CqlStatementOptionsAccessor ofInsert(Bindings bindings, Insert statement) { + return new BoundOptionsAccessor<>(bindings, statement, Insert::usingTimestamp, Insert::usingTtl); + } + + /** + * Creates an accessor variant that applies options directly within the CQL statement for {@link Insert}. + * + * @param statement + * @return + */ + public static CqlStatementOptionsAccessor ofInsert(Insert statement) { + return new InlineOptionsAccessor<>(statement, Insert::usingTimestamp, Insert::usingTtl); + } + + /** + * Creates an accessor variant that captures options through {@link BindMarker} for {@link Update}. + * + * @param bindings + * @param statement + * @return + */ + public static CqlStatementOptionsAccessor ofUpdate(Bindings bindings, UpdateStart statement) { + return new BoundOptionsAccessor<>(bindings, statement, UpdateStart::usingTimestamp, UpdateStart::usingTtl); + } + + /** + * Creates an accessor variant that applies options directly within the CQL statement for {@link Update}. + * + * @param statement + * @return + */ + public static CqlStatementOptionsAccessor ofUpdate(UpdateStart statement) { + return new InlineOptionsAccessor<>(statement, UpdateStart::usingTimestamp, UpdateStart::usingTtl); + } + + /** + * Creates an accessor variant that captures options through {@link BindMarker} for {@link Delete}. + * + * @param bindings + * @param statement + * @return + */ + public static CqlStatementOptionsAccessor ofDelete(Bindings bindings, DeleteSelection statement) { + return new BoundOptionsAccessor<>(bindings, statement, DeleteSelection::usingTimestamp, + (deleteSelection, bindMarker) -> deleteSelection); + } + + /** + * Creates an accessor variant that applies options directly within the CQL statement for {@link Delete}. + * + * @param statement + * @return + */ + public static CqlStatementOptionsAccessor ofDelete(DeleteSelection statement) { + return new InlineOptionsAccessor<>(statement, DeleteSelection::usingTimestamp, + (deleteSelection, bindMarker) -> deleteSelection); + } + + } + + /** + * Accessor variant that uses bind markers. + * + * @param + */ + private static class BoundOptionsAccessor extends CqlStatementOptionsAccessor { + + private final Bindings bindings; + private T instance; + + private final BiFunction timestampFunction; + + private final BiFunction ttlFunction; + + private BoundOptionsAccessor(Bindings bindings, T instance, BiFunction timestampFunction, + BiFunction ttlFunction) { + this.bindings = bindings; + this.instance = instance; + this.timestampFunction = timestampFunction; + this.ttlFunction = ttlFunction; + } + + @Override + void usingTimestamp(long timestamp) { + + BindMarker bindMarker = bindings.bind(timestamp); + instance = timestampFunction.apply(instance, bindMarker); + } + + @Override + void usingTtl(int ttl) { + BindMarker bindMarker = bindings.bind(ttl); + instance = ttlFunction.apply(instance, bindMarker); + } + + @Override + T getStatement() { + return instance; + } + + } + + /** + * Accessor variant that uses inline values. + * + * @param + */ + private static class InlineOptionsAccessor extends CqlStatementOptionsAccessor { + + private T instance; + + private final TimestampFunction timestampFunction; + + private final TtlFunction ttlFunction; + + private InlineOptionsAccessor(T instance, TimestampFunction timestampFunction, TtlFunction ttlFunction) { + this.instance = instance; + this.timestampFunction = timestampFunction; + this.ttlFunction = ttlFunction; + } + + @Override + void usingTimestamp(long timestamp) { + instance = timestampFunction.apply(instance, timestamp); + } + + @Override + void usingTtl(int ttl) { + instance = ttlFunction.apply(instance, ttl); + } + + @Override + T getStatement() { + return instance; + } + + /** + * Bi-function accepting a statement and {@code long} timestamp returning the modified statement. + * + * @param + */ + interface TimestampFunction { + + T apply(T statement, long timestamp); + } + + /** + * Bi-function accepting a statement and {@code int} TTL returning the modified statement. + * + * @param + */ + interface TtlFunction { + + T apply(T statement, int ttl); + } + + } + } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/Bindings.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/Bindings.java new file mode 100644 index 000000000..4d6cb5586 --- /dev/null +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/Bindings.java @@ -0,0 +1,42 @@ +/* + * Copyright 2023 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.springframework.data.cassandra.core.cql.util; + +import org.springframework.lang.Nullable; + +import com.datastax.oss.driver.api.querybuilder.BindMarker; + +/** + * Factory for {@link BindMarker} capturing binding {@code value}s. + *

+ * A {@link Bindings} object is typically used with {@link StatementBuilder}. + * + * @author Mark Paluch + * @since 4.2 + */ +@FunctionalInterface +public interface Bindings { + + /** + * Create a {@link BindMarker} for the given {@code value}. Using bindings with positional bind markers must consider + * the usage order within a statement. + * + * @param value the value to bind, can be {@literal null}. + * @return the {@link BindMarker} for the given {@code value}. + */ + BindMarker bind(@Nullable Object value); + +} diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/StatementBuilder.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/StatementBuilder.java index eb6659172..84cb0b5bc 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/StatementBuilder.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/StatementBuilder.java @@ -23,6 +23,7 @@ import java.util.Map; import java.util.Set; import java.util.function.Consumer; import java.util.function.Function; +import java.util.function.Supplier; import java.util.function.UnaryOperator; import org.springframework.lang.NonNull; @@ -32,6 +33,7 @@ import org.springframework.util.Assert; import com.datastax.oss.driver.api.core.cql.SimpleStatement; import com.datastax.oss.driver.api.core.cql.SimpleStatementBuilder; import com.datastax.oss.driver.api.core.type.codec.registry.CodecRegistry; +import com.datastax.oss.driver.api.querybuilder.BindMarker; import com.datastax.oss.driver.api.querybuilder.BuildableQuery; import com.datastax.oss.driver.api.querybuilder.QueryBuilder; import com.datastax.oss.driver.api.querybuilder.term.Term; @@ -76,8 +78,8 @@ public class StatementBuilder { private final List> onBuilt = new ArrayList<>(); /** - * Factory method used to create a new {@link StatementBuilder} with the given {@link BuildableQuery query stub}. - * The stub is used as base for the built query so each query inherits properties of this stub. + * Factory method used to create a new {@link StatementBuilder} with the given {@link BuildableQuery query stub}. The + * stub is used as base for the built query so each query inherits properties of this stub. * * @param query type. * @param stub the {@link BuildableQuery query stub} to use. @@ -95,8 +97,8 @@ public class StatementBuilder { /** * Constructs a new instance of this {@link StatementBuilder} with the given {@link BuildableQuery query stub}. * - * @param statement the {@link BuildableQuery query stub} from which to build - * the {@link com.datastax.oss.driver.api.core.cql.Statement}. + * @param statement the {@link BuildableQuery query stub} from which to build the + * {@link com.datastax.oss.driver.api.core.cql.Statement}. * @see com.datastax.oss.driver.api.querybuilder.BuildableQuery */ private StatementBuilder(S statement) { @@ -229,9 +231,17 @@ public class StatementBuilder { List values = new ArrayList<>(); - TermFactory termFactory = value -> { - values.add(value); - return QueryBuilder.bindMarker(); + TermFactory termFactory = new TermFactory() { + @Override + public BindMarker create(@Nullable Object value) { + values.add(value); + return QueryBuilder.bindMarker(); + } + + @Override + public T ifBoundOrInline(Function bindingFunction, Supplier inlineFunction) { + return bindingFunction.apply(this::create); + } }; for (BuilderRunnable runnable : queryActions) { @@ -249,10 +259,18 @@ public class StatementBuilder { Map values = new LinkedHashMap<>(); - TermFactory termFactory = value -> { - String name = "p" + values.size(); - values.put(name, value); - return QueryBuilder.bindMarker(name); + TermFactory termFactory = new TermFactory() { + @Override + public BindMarker create(@Nullable Object value) { + String name = "p" + values.size(); + values.put(name, value); + return QueryBuilder.bindMarker(name); + } + + @Override + public T ifBoundOrInline(Function bindingFunction, Supplier inlineFunction) { + return bindingFunction.apply(this::create); + } }; for (BuilderRunnable runnable : queryActions) { @@ -316,8 +334,8 @@ public class StatementBuilder { Map terms = new LinkedHashMap<>(); - ((Map) value).forEach((k, v) -> - terms.put(toLiteralTerms(k, codecRegistry), toLiteralTerms(v, codecRegistry))); + ((Map) value) + .forEach((k, v) -> terms.put(toLiteralTerms(k, codecRegistry), toLiteralTerms(v, codecRegistry))); return new MapTerm(terms); } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/TermFactory.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/TermFactory.java index 463b97248..cae4e5dc0 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/TermFactory.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/cql/util/TermFactory.java @@ -15,6 +15,9 @@ */ package org.springframework.data.cassandra.core.cql.util; +import java.util.function.Function; +import java.util.function.Supplier; + import org.springframework.lang.Nullable; import com.datastax.oss.driver.api.querybuilder.term.Term; @@ -49,4 +52,18 @@ public interface TermFactory { default boolean canBindCollection() { return true; } + + /** + * Apply functions depending on whether the term factory uses bind markers or inline values. + * + * @param bindingFunction the binding functions to apply with {@link Bindings} for bind marker capturing. + * @param inlineFunction the function to run otherwise, if the term factory uses inline values. + * @return the outcome of the binding operation. + * @param + * @since 4.2 + */ + default T ifBoundOrInline(Function bindingFunction, Supplier inlineFunction) { + return inlineFunction.get(); + } + } diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/StatementFactoryUnitTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/StatementFactoryUnitTests.java index c153ec133..9d7e0bc70 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/StatementFactoryUnitTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/StatementFactoryUnitTests.java @@ -246,6 +246,38 @@ class StatementFactoryUnitTests { .isEqualTo("DELETE FROM group USING TIMESTAMP 1234 WHERE foo='bar'"); } + @Test // GH-1401 + void deleteByQueryWithOptionsShouldRenderBindMarkers() { + + DeleteOptions options = DeleteOptions.builder().timestamp(1234).build(); + Query query = Query.query(Criteria.where("foo").is("bar")).queryOptions(options); + + StatementBuilder delete = statementFactory.delete(query, + converter.getMappingContext().getRequiredPersistentEntity(Group.class)); + + SimpleStatement statement = delete.build(ParameterHandling.BY_INDEX); + + assertThat(statement.getQuery()).isEqualTo("DELETE FROM group USING TIMESTAMP ? WHERE foo=?"); + assertThat(statement.getPositionalValues()).containsExactly(1234L, "bar"); + } + + @Test // GH-1401 + void deleteByEntityWithOptionsShouldRenderBindMarkers() { + + Person person = new Person(); + person.id = "foo"; + + DeleteOptions options = DeleteOptions.builder().timestamp(1234).build(); + + StatementBuilder delete = statementFactory.delete(person, options, converter, + CqlIdentifier.fromCql("person")); + + SimpleStatement statement = delete.build(ParameterHandling.BY_INDEX); + + assertThat(statement.getQuery()).isEqualTo("DELETE FROM person USING TIMESTAMP ? WHERE id=?"); + assertThat(statement.getPositionalValues()).containsExactly(1234L, "foo"); + } + @Test // DATACASS-708 void deleteShouldApplyQueryOptions() { @@ -294,6 +326,23 @@ class StatementFactoryUnitTests { assertThat(statement.getSerialConsistencyLevel()).isEqualTo(DefaultConsistencyLevel.QUORUM); } + @Test // GH-1401 + void insertWithOptionsShouldRenderBindMarkers() { + + Person person = new Person(); + person.id = "foo"; + + WriteOptions queryOptions = WriteOptions.builder() // + .ttl(10).timestamp(1234).build(); + + StatementBuilder insert = statementFactory.insert(person, queryOptions); + + SimpleStatement statement = insert.build(ParameterHandling.BY_INDEX); + + assertThat(statement.getQuery()).isEqualTo("INSERT INTO person (id) VALUES (?) USING TIMESTAMP ? AND TTL ?"); + assertThat(statement.getPositionalValues()).containsExactly("foo", 1234L, 10); + } + @Test // DATACASS-656 void shouldCreateInsertIfNotExists() { @@ -384,6 +433,22 @@ class StatementFactoryUnitTests { .isEqualTo("UPDATE person USING TIMESTAMP 1234 SET first_name='baz' WHERE foo='bar'"); } + @Test // GH-1401 + void updateWithOptionsShouldRenderBindMarker() { + + WriteOptions options = WriteOptions.builder().ttl(Duration.ofMinutes(1)).timestamp(1234).build(); + Query query = Query.query(Criteria.where("foo").is("bar")).queryOptions(options); + + StatementBuilder update = statementFactory.update(query, + Update.empty().set("firstName", "baz"), personEntity); + + SimpleStatement statement = update.build(ParameterHandling.BY_INDEX); + assertThat(statement.getQuery()) + .isEqualTo("UPDATE person USING TIMESTAMP ? AND TTL ? SET first_name=? WHERE foo=?"); + + assertThat(statement.getPositionalValues()).containsExactly(1234L, 60, "baz", "bar"); + } + @Test // DATACASS-343, DATACASS-712 void shouldCreateSetAtIndexUpdate() {