From e2aef3bc1aad6f7a97055eaffde25ac471e483a7 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Fri, 29 Jan 2021 15:08:21 +0100 Subject: [PATCH] Allow multi-value collection removal and map entry removal by key. We now accept multiple values when removing items from a collection. Additionally, we support now removal by key/keys for map columns. Resolves #1007. --- .../data/cassandra/core/StatementFactory.java | 62 ++++++++++--- .../cassandra/core/convert/QueryMapper.java | 22 +++++ .../cassandra/core/convert/UpdateMapper.java | 26 ++++-- .../data/cassandra/core/query/Update.java | 86 +++++++++++++++++++ .../CassandraTemplateIntegrationTests.java | 34 ++++++++ .../core/StatementFactoryUnitTests.java | 14 +++ .../core/convert/UpdateMapperUnitTests.java | 9 ++ .../data/cassandra/domain/BookReference.java | 3 + .../src/test/resources/logback.xml | 1 - 9 files changed, 233 insertions(+), 24 deletions(-) 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 252ae446c..f9d649b35 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 @@ -24,6 +24,7 @@ import java.util.List; import java.util.Map; import java.util.Optional; import java.util.Set; +import java.util.function.Function; import java.util.stream.Collectors; import org.springframework.data.cassandra.core.convert.CassandraConverter; @@ -800,27 +801,23 @@ public class StatementFactory { } @SuppressWarnings("unchecked") - private static Assignment getAssignment(RemoveOp updateOp, TermFactory termFactory) { + private static Assignment getAssignment(RemoveOp removeOp, TermFactory termFactory) { - if (updateOp.getValue() instanceof Set) { + if (removeOp.getValue() instanceof Set) { - Collection collection = (Collection) updateOp.getValue(); + Collection collection = (Collection) removeOp.getValue(); - Assert.isTrue(collection.size() == 1, "RemoveOp must contain a single set element"); - - return Assignment.removeSetElement(updateOp.toCqlIdentifier(), termFactory.create(collection.iterator().next())); + return new RemoveCollectionElementsAssignment(removeOp.toCqlIdentifier(), termFactory.create(collection)); } - if (updateOp.getValue() instanceof List) { + if (removeOp.getValue() instanceof List) { - Collection collection = (Collection) updateOp.getValue(); + Collection collection = (Collection) removeOp.getValue(); - Assert.isTrue(collection.size() == 1, "RemoveOp must contain a single list element"); - - return Assignment.removeListElement(updateOp.toCqlIdentifier(), termFactory.create(collection.iterator().next())); + return new RemoveCollectionElementsAssignment(removeOp.toCqlIdentifier(), termFactory.create(collection)); } - return Assignment.remove(updateOp.toCqlIdentifier(), termFactory.create(updateOp.getValue())); + return Assignment.remove(removeOp.toCqlIdentifier(), termFactory.create(removeOp.getValue())); } private static Assignment getAssignment(AddToOp updateOp, TermFactory termFactory) { @@ -1045,13 +1042,17 @@ public class StatementFactory { } static List toLiterals(@Nullable Object arrayOrList) { + return toLiterals(arrayOrList, QueryBuilder::literal); + } + + static List toLiterals(@Nullable Object arrayOrList, Function termFactory) { if (arrayOrList instanceof List) { List list = (List) arrayOrList; List literals = new ArrayList<>(list.size()); for (Object o : list) { - literals.add(QueryBuilder.literal(o)); + literals.add(termFactory.apply(o)); } return literals; @@ -1062,7 +1063,7 @@ public class StatementFactory { Object[] array = (Object[]) arrayOrList; List literals = new ArrayList<>(array.length); for (Object o : array) { - literals.add(QueryBuilder.literal(o)); + literals.add(termFactory.apply(o)); } return literals; @@ -1096,4 +1097,37 @@ public class StatementFactory { builder.append(selector); } } + + private static class RemoveCollectionElementsAssignment implements Assignment { + + private final CqlIdentifier columnId; + private final Term value; + + protected RemoveCollectionElementsAssignment(CqlIdentifier columnId, Term value) { + this.columnId = columnId; + this.value = value; + } + + @Override + public void appendTo(StringBuilder builder) { + builder.append(String.format("%1$s=%1$s-%2$s", columnId.asCql(true), buildRightOperand())); + } + + private String buildRightOperand() { + StringBuilder builder = new StringBuilder(); + value.appendTo(builder); + return builder.toString(); + } + + @Override + public boolean isIdempotent() { + return value.isIdempotent(); + } + + public Term getValue() { + return value; + } + + } + } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/QueryMapper.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/QueryMapper.java index 3ef3c7bae..d84dc34b7 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/QueryMapper.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/QueryMapper.java @@ -398,6 +398,17 @@ public class QueryMapper { } }, + /** + * Wrap {@link ColumnType} into a set. + */ + ENCLOSING_SET { + + @Override + ColumnType transform(ColumnType typeDescriptor, CassandraPersistentProperty property) { + return ColumnType.setOf(typeDescriptor); + } + }, + /** * Use the map key type. */ @@ -428,6 +439,17 @@ public class QueryMapper { return typeDescriptor; } + }, + + /** + * Wrap {@link ColumnType} into a set. + */ + ENCLOSING_MAP_KEY_SET { + + @Override + ColumnType transform(ColumnType typeDescriptor, CassandraPersistentProperty property) { + return ColumnType.setOf(MAP_KEY_TYPE.transform(typeDescriptor, property)); + } }; /** diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/UpdateMapper.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/UpdateMapper.java index 39e9476c6..e81b93df4 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/UpdateMapper.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/UpdateMapper.java @@ -15,6 +15,8 @@ */ package org.springframework.data.cassandra.core.convert; +import static org.springframework.data.cassandra.core.query.Update.*; + import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -28,14 +30,7 @@ import java.util.Set; import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity; import org.springframework.data.cassandra.core.query.Filter; import org.springframework.data.cassandra.core.query.Update; -import org.springframework.data.cassandra.core.query.Update.AddToMapOp; -import org.springframework.data.cassandra.core.query.Update.AddToOp; -import org.springframework.data.cassandra.core.query.Update.AssignmentOp; -import org.springframework.data.cassandra.core.query.Update.IncrOp; -import org.springframework.data.cassandra.core.query.Update.RemoveOp; -import org.springframework.data.cassandra.core.query.Update.SetAtIndexOp; -import org.springframework.data.cassandra.core.query.Update.SetAtKeyOp; -import org.springframework.data.cassandra.core.query.Update.SetOp; +import org.springframework.data.cassandra.core.query.Update.*; import org.springframework.data.mapping.PersistentProperty; import org.springframework.data.util.TypeInformation; import org.springframework.util.Assert; @@ -95,7 +90,7 @@ public class UpdateMapper extends QueryMapper { mapped.add(getMappedUpdateOperation(assignmentOp, field)); } - return Update.of(mapped); + return of(mapped); } private AssignmentOp getMappedUpdateOperation(AssignmentOp assignmentOp, Field field) { @@ -192,8 +187,21 @@ public class UpdateMapper extends QueryMapper { Object value = updateOp.getValue(); ColumnType descriptor = getColumnType(field, value, ColumnTypeTransformer.AS_IS); + boolean mapLike = false; + + if (field.getProperty().isPresent() && field.getProperty().get().isMapLike()) { + + descriptor = getColumnType(field, value, value instanceof Collection ? ColumnTypeTransformer.ENCLOSING_MAP_KEY_SET + : ColumnTypeTransformer.MAP_KEY_TYPE); + mapLike = true; + } + Object mappedValue = getConverter().convertToColumnType(value, descriptor); + if (mapLike && !(mappedValue instanceof Collection)) { + mappedValue = Collections.singleton(mappedValue); + } + return new RemoveOp(field.getMappedKey(), mappedValue); } diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/query/Update.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/query/Update.java index eec02d58c..717dfa119 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/query/Update.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/query/Update.java @@ -120,6 +120,17 @@ public class Update { return new DefaultAddToBuilder(ColumnName.from(columnName)); } + /** + * Create a new {@link RemoveFromBuilder} to remove items from a collection for {@code columnName} in a fluent style. + * + * @param columnName must not be {@literal null}. + * @return a new {@link RemoveFromBuilder} to build an remove-from assignment. + * @since 3.1.4 + */ + public RemoveFromBuilder removeFrom(String columnName) { + return new DefaultRemoveFromBuilder(ColumnName.from(columnName)); + } + /** * Remove {@code value} from the collection at {@code columnName}. * @@ -403,6 +414,80 @@ public class Update { } } + /** + * Builder to remove a single element/multiple elements from a collection associated with a {@link ColumnName}. + * + * @author Mark Paluch + * @since 3.1.4 + */ + @SuppressWarnings("unused") + public interface RemoveFromBuilder { + + /** + * Remove all entries matching {@code value} from a set, list or map (map key). + * + * @param value must not be {@literal null}. + * @return a new {@link Update} object containing the merge result of the existing assignments and the current + * assignment. + */ + Update value(Object value); + + /** + * Remove all entries matching {@code values} from a set, list or map (map key). + * + * @param values must not be {@literal null}. + * @return a new {@link Update} object containing the merge result of the existing assignments and the current + * assignment. + */ + default Update values(Object... values) { + + Assert.notNull(values, "Values must not be null"); + + return values(Arrays.asList(values)); + } + + /** + * Remove all entries matching {@code values} from a set, list or map (map key). + * + * @param values must not be {@literal null}. + * @return a new {@link Update} object containing the merge result of the existing assignments and the current + * assignment. + */ + Update values(Iterable values); + + } + + /** + * Default {@link RemoveFromBuilder} implementation. + */ + private class DefaultRemoveFromBuilder implements RemoveFromBuilder { + + private final ColumnName columnName; + + DefaultRemoveFromBuilder(ColumnName columnName) { + this.columnName = columnName; + } + + /* (non-Javadoc) + * @see org.springframework.data.cassandra.core.query.Update.RemoveFromBuilder#mapValue(java.lang.Object) + */ + @Override + public Update value(Object value) { + return add(new RemoveOp(columnName, value)); + } + + /* (non-Javadoc) + * @see org.springframework.data.cassandra.core.query.Update.RemoveFromBuilder#mapValues(java.lang.Iterable) + */ + @Override + public Update values(Iterable values) { + + Assert.notNull(values, "Values must not be null"); + + return add(new RemoveOp(columnName, values)); + } + } + /** * Builder to associate a single value with a collection at a given index at {@link ColumnName}. * @@ -718,4 +803,5 @@ public class Update { return String.format("%s = %s - %s", getColumnName(), getColumnName(), serializeToCqlSafely(getValue())); } } + } diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraTemplateIntegrationTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraTemplateIntegrationTests.java index 59882829f..6f6b36bc0 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraTemplateIntegrationTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/CassandraTemplateIntegrationTests.java @@ -30,8 +30,10 @@ import java.time.ZoneId; import java.util.Arrays; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -573,6 +575,38 @@ class CassandraTemplateIntegrationTests extends AbstractKeyspaceCreatingIntegrat assertThat(loaded.getBookmarks()).isNull(); } + @Test // #1007 + void updateCollection() { + + BookReference bookReference = new BookReference(); + + bookReference.setIsbn("isbn"); + bookReference.setBookmarks(Arrays.asList(1, 2, 3, 4)); + bookReference.setReferences(new LinkedHashSet<>(Arrays.asList("one", "two", "three"))); + + Map credits = new LinkedHashMap<>(); + credits.put("hello", "world"); + credits.put("other", "world"); + credits.put("external", "place"); + + bookReference.setCredits(credits); + + template.insert(bookReference); + + Query query = Query.query(where("isbn").is(bookReference.getIsbn())); + + Update update = Update.empty().removeFrom("bookmarks").values(3, 4).removeFrom("references").values("one", "three") + .removeFrom("credits").values("hello", "other", "place"); + + template.update(query, update, BookReference.class); + + BookReference loaded = template.selectOneById(bookReference.getIsbn(), BookReference.class); + + assertThat(loaded.getBookmarks()).containsOnly(1, 2); + assertThat(loaded.getReferences()).containsOnly("two"); + assertThat(loaded.getCredits()).containsOnlyKeys("external"); + } + @Test // DATACASS-206 void shouldUseSpecifiedColumnNamesForSingleEntityModifyingOperations() { 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 27a70135f..072b61946 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 @@ -359,6 +359,20 @@ class StatementFactoryUnitTests { assertThat(update.build(ParameterHandling.INLINE).getQuery()).isEqualTo("UPDATE person SET map=map+{'foo':'Euro'}"); } + @Test // #1007 + void shouldRemoveFromMap() { + + StatementBuilder update = statementFactory + .update(Query.empty(), Update.empty().removeFrom("map").value("foo"), personEntity); + + assertThat(update.build(ParameterHandling.INLINE).getQuery()).isEqualTo("UPDATE person SET map=map-{'foo'}"); + + update = statementFactory.update(Query.empty(), Update.empty().removeFrom("map").values("foo", "bar"), + personEntity); + + assertThat(update.build(ParameterHandling.INLINE).getQuery()).isEqualTo("UPDATE person SET map=map-{'foo','bar'}"); + } + @Test // DATACASS-343 void shouldPrependAllToList() { diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/convert/UpdateMapperUnitTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/convert/UpdateMapperUnitTests.java index 43a988596..913629da0 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/convert/UpdateMapperUnitTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/core/convert/UpdateMapperUnitTests.java @@ -179,6 +179,15 @@ class UpdateMapperUnitTests { assertThat(update).hasToString("map = map + {'foo':'Euro'}"); } + @Test // #1007 + void shouldRemoveFromMap() { + + Update update = updateMapper.getMappedObject(Update.empty().removeFrom("map").value("foo"), persistentEntity); + + assertThat(update.getUpdateOperations()).hasSize(1); + assertThat(update).hasToString("map = map - {'foo'}"); + } + @Test // DATACASS-487 void shouldAddUdtToMap() { diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/BookReference.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/BookReference.java index 57d07f161..620780522 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/BookReference.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/BookReference.java @@ -18,6 +18,7 @@ package org.springframework.data.cassandra.domain; import lombok.Data; import java.util.List; +import java.util.Map; import java.util.Set; import org.springframework.data.cassandra.core.mapping.PrimaryKey; @@ -27,6 +28,7 @@ import org.springframework.data.cassandra.core.mapping.Table; * Test POJO * * @author David Webb + * @author Mark Paluch */ @Table("bookReference") @Data @@ -37,4 +39,5 @@ public class BookReference { private String title; private Set references; private List bookmarks; + private Map credits; } diff --git a/spring-data-cassandra/src/test/resources/logback.xml b/spring-data-cassandra/src/test/resources/logback.xml index 893e1683a..f746e4e57 100644 --- a/spring-data-cassandra/src/test/resources/logback.xml +++ b/spring-data-cassandra/src/test/resources/logback.xml @@ -8,7 +8,6 @@ -