From 8093392d60430b27b75d6902fd41027f26be24ce Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 26 Nov 2020 14:53:27 +0100 Subject: [PATCH] DATACASS-829 - Consider contextual ColumnType when updating a column. We now consider the contextual ColumnType that applies when using particular update operations within the context of a property reference. Query and Update objects reference properties in their criteria/assignments using operators that are related either to the entire property, the key-, value- or component-type aspect (querying whether map contains a key, updating a list at an index). Previously, we considered the contextual column type only for criteria operators and updating a list with a mapped UDT failed as the column type of the property was used. By using a ColumnTypeTransformer abstraction we resolve the correct contextual column type so that subsequent mapping/conversion operations use the appropriate type hint. --- .../cassandra/core/convert/QueryMapper.java | 136 ++++++++++++++---- .../cassandra/core/convert/UpdateMapper.java | 8 +- .../CassandraTemplateIntegrationTests.java | 37 +++++ .../core/convert/UpdateMapperUnitTests.java | 12 ++ 4 files changed, 162 insertions(+), 31 deletions(-) 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 dd1e4e4b6..4b5147129 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 @@ -16,7 +16,6 @@ package org.springframework.data.cassandra.core.convert; import java.util.ArrayList; -import java.util.Collection; import java.util.Collections; import java.util.HashSet; import java.util.List; @@ -133,7 +132,7 @@ public class QueryMapper { Predicate predicate = criteriaDefinition.getPredicate(); Object value = predicate.getValue(); - ColumnType typeDescriptor = getColumnType(field, value, predicate.getOperator()); + ColumnType typeDescriptor = getColumnType(field, value, ColumnTypeTransformer.of(field, predicate.getOperator())); Object mappedValue = value != null ? getConverter().convertToColumnType(value, typeDescriptor) : null; @@ -347,43 +346,124 @@ public class QueryMapper { .orElseGet(() -> new Field(key)); } - ColumnType getColumnType(Field field, @Nullable Object value, @Nullable CriteriaDefinition.Operator operator) { + ColumnType getColumnType(Field field, @Nullable Object value, ColumnTypeTransformer operator) { - ColumnType typeDescriptor; - if (field.getProperty().isPresent()) { - typeDescriptor = converter.getColumnTypeResolver().resolve(field.getProperty().get()); - } else { + ColumnTypeResolver resolver = converter.getColumnTypeResolver(); - typeDescriptor = converter.getColumnTypeResolver().resolve(value); - } + return field.getProperty().map(it -> operator.transform(resolver.resolve(it), it)).map(ColumnType.class::cast) + .orElseGet(() -> resolver.resolve(value)); + } - if (field.getProperty().isPresent()) { + /** + * Transform a {@link ColumnType} determined from a {@link CassandraPersistentProperty} into a specific + * {@link ColumnType} depending on the actual context. Typically used when querying a collection component type. + */ + enum ColumnTypeTransformer { - CassandraPersistentProperty property = field.getProperty().get(); + /** + * Pass-thru. + */ + AS_IS { - if (property.isCollectionLike()) { - if (operator == CriteriaDefinition.Operators.CONTAINS) { - typeDescriptor = typeDescriptor.getRequiredComponentType(); + @Override + ColumnType transform(ColumnType typeDescriptor, CassandraPersistentProperty property) { + return typeDescriptor; + } + }, + + /** + * Use the collection component type. + */ + COLLECTION_COMPONENT_TYPE { + + @Override + ColumnType transform(ColumnType typeDescriptor, CassandraPersistentProperty property) { + + if (property.isCollectionLike()) { + return typeDescriptor.getRequiredComponentType(); } + + return typeDescriptor; + } + }, + + /** + * Wrap {@link ColumnType} into a list. + */ + ENCLOSING_LIST { + + @Override + ColumnType transform(ColumnType typeDescriptor, CassandraPersistentProperty property) { + return ColumnType.listOf(typeDescriptor); + } + }, + + /** + * Use the map key type. + */ + MAP_KEY_TYPE { + + @Override + ColumnType transform(ColumnType typeDescriptor, CassandraPersistentProperty property) { + + if (property.isMapLike()) { + return typeDescriptor.getRequiredComponentType(); + } + + return typeDescriptor; + } + }, + + /** + * Use the map value type. + */ + MAP_VALUE_TYPE { + + @Override + ColumnType transform(ColumnType typeDescriptor, CassandraPersistentProperty property) { + + if (property.isMapLike()) { + return typeDescriptor.getRequiredMapValueType(); + } + + return typeDescriptor; + } + }; + + /** + * Transform the {@link ColumnType} depending on contextual requirements (update list/map, query map key/value) into + * the specific {@link ColumnType} that matches the collection type requirements. + * + * @param typeDescriptor the type descriptor resolved from {@link CassandraPersistentProperty}. + * @param property the underlying property. + * @return the {@link ColumnType} to use. + */ + abstract ColumnType transform(ColumnType typeDescriptor, CassandraPersistentProperty property); + + /** + * Determine a {@link ColumnTypeTransformer} based on a criteria {@link CriteriaDefinition.Operator}. + * + * @param field the field to query. + * @param operator criteria operator. + * @return + */ + static ColumnTypeTransformer of(Field field, CriteriaDefinition.Operator operator) { + + if (operator == CriteriaDefinition.Operators.CONTAINS) { + return field.getProperty().filter(CassandraPersistentProperty::isMapLike).map(it -> MAP_VALUE_TYPE) + .orElse(COLLECTION_COMPONENT_TYPE); } - if (property.isMapLike()) { - - if (operator == CriteriaDefinition.Operators.CONTAINS_KEY) { - typeDescriptor = typeDescriptor.getRequiredComponentType(); - } - - if (operator == CriteriaDefinition.Operators.CONTAINS) { - typeDescriptor = typeDescriptor.getRequiredMapValueType(); - } + if (operator == CriteriaDefinition.Operators.CONTAINS_KEY) { + return MAP_KEY_TYPE; } - } - if (value instanceof Collection && operator == CriteriaDefinition.Operators.IN) { - typeDescriptor = ColumnType.listOf(typeDescriptor); - } + if (operator == CriteriaDefinition.Operators.IN) { + return ENCLOSING_LIST; + } - return typeDescriptor; + return AS_IS; + } } /** 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 4e3e0c870..f3d41b242 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 @@ -149,7 +149,9 @@ public class UpdateMapper extends QueryMapper { return new SetAtKeyOp(field.getMappedKey(), mappedKey, mappedValue); } - ColumnType descriptor = getColumnType(field, rawValue, null); + ColumnType descriptor = getColumnType(field, rawValue, + updateOp instanceof SetAtIndexOp ? ColumnTypeTransformer.COLLECTION_COMPONENT_TYPE + : ColumnTypeTransformer.AS_IS); if (updateOp instanceof SetAtIndexOp) { @@ -189,7 +191,7 @@ public class UpdateMapper extends QueryMapper { private AssignmentOp getMappedUpdateOperation(Field field, RemoveOp updateOp) { Object value = updateOp.getValue(); - ColumnType descriptor = getColumnType(field, value, null); + ColumnType descriptor = getColumnType(field, value, ColumnTypeTransformer.AS_IS); Object mappedValue = getConverter().convertToColumnType(value, descriptor); return new RemoveOp(field.getMappedKey(), mappedValue); @@ -199,7 +201,7 @@ public class UpdateMapper extends QueryMapper { private AssignmentOp getMappedUpdateOperation(Field field, AddToOp updateOp) { Iterable value = updateOp.getValue(); - ColumnType descriptor = getColumnType(field, value, null); + ColumnType descriptor = getColumnType(field, value, ColumnTypeTransformer.AS_IS); Collection mappedValue = (Collection) getConverter().convertToColumnType(value, descriptor); if (field.getProperty().isPresent()) { 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 07865a3b1..1bbf398e2 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 @@ -38,6 +38,7 @@ import java.util.stream.Stream; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; + import org.springframework.data.annotation.Id; import org.springframework.data.cassandra.CassandraInvalidQueryException; import org.springframework.data.cassandra.core.convert.MappingCassandraConverter; @@ -50,6 +51,7 @@ import org.springframework.data.cassandra.core.mapping.PrimaryKey; import org.springframework.data.cassandra.core.mapping.PrimaryKeyClass; import org.springframework.data.cassandra.core.mapping.PrimaryKeyColumn; import org.springframework.data.cassandra.core.mapping.SimpleUserTypeResolver; +import org.springframework.data.cassandra.core.mapping.Table; import org.springframework.data.cassandra.core.mapping.UserDefinedType; import org.springframework.data.cassandra.core.query.CassandraPageRequest; import org.springframework.data.cassandra.core.query.Columns; @@ -105,6 +107,7 @@ class CassandraTemplateIntegrationTests extends AbstractKeyspaceCreatingIntegrat SchemaTestUtils.potentiallyCreateTableFor(WithPrefixedNullableEmbeddedType.class, template); SchemaTestUtils.createTableAndTypes(OuterWithNullableEmbeddedType.class, template); SchemaTestUtils.createTableAndTypes(OuterWithPrefixedNullableEmbeddedType.class, template); + SchemaTestUtils.createTableAndTypes(WithMappedUdtList.class, template); SchemaTestUtils.truncate(User.class, template); SchemaTestUtils.truncate(UserToken.class, template); SchemaTestUtils.truncate(BookReference.class, template); @@ -114,6 +117,7 @@ class CassandraTemplateIntegrationTests extends AbstractKeyspaceCreatingIntegrat SchemaTestUtils.truncate(WithPrefixedNullableEmbeddedType.class, template); SchemaTestUtils.truncate(OuterWithNullableEmbeddedType.class, template); SchemaTestUtils.truncate(OuterWithPrefixedNullableEmbeddedType.class, template); + SchemaTestUtils.truncate(WithMappedUdtList.class, template); } @Test // DATACASS-343 @@ -734,6 +738,39 @@ class CassandraTemplateIntegrationTests extends AbstractKeyspaceCreatingIntegrat assertThat(target).isEqualTo(entity); } + @Test // DATACASS-829 + void shouldPartiallyUpdateListOfMappedUdt() { + + WithMappedUdtList entity = new WithMappedUdtList(); + entity.id = "id-1"; + entity.mappedUdts = Arrays.asList(new MappedUdt("one"), new MappedUdt("two"), new MappedUdt("three")); + + template.insert(entity); + + Update update = Update.empty().set("mappedUdts").atIndex(1).to(new MappedUdt("replacement")); + + template.update(Query.query(where("id").is("id-1")), update, WithMappedUdtList.class); + + WithMappedUdtList updated = template.selectOne(Query.query(where("id").is("id-1")), WithMappedUdtList.class); + assertThat(updated.getMappedUdts()).extracting(MappedUdt::getName).containsExactly("one", "replacement", "three"); + } + + @Data + @UserDefinedType + static class MappedUdt { + + final String name; + } + + @Data + @Table + static class WithMappedUdtList { + + @Id String id; + + List mappedUdts; + } + @Data static class TimeClass { 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 e67bfb128..cda97c68b 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 @@ -136,6 +136,16 @@ class UpdateMapperUnitTests { assertThat(update).hasToString("list[10] = 'Euro'"); } + @Test // DATACASS-829 + void shouldCreateSetAtUdtIndexUpdate() { + + Update update = updateMapper.getMappedObject( + Update.empty().set("manufacturerList").atIndex(10).to(new Manufacturer("foo")), persistentEntity); + + assertThat(update.getUpdateOperations()).hasSize(1); + assertThat(update).hasToString("manufacturerlist[10] = {name:'foo'}"); + } + @Test // DATACASS-343 void shouldCreateSetAtKeyUpdate() { @@ -380,6 +390,8 @@ class UpdateMapperUnitTests { Map map; Map manufacturers; + List manufacturerList; + MappedTuple tuple; @Column("set_col") Set set;