From 172e9fc897953c5f7162fd98d2997e3c197f3eb7 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Tue, 14 Mar 2017 18:04:08 +0100 Subject: [PATCH] DATACASS-413 - Convert collection elements to property type when reading collections. We now consider the element type and property component type when reading Cassandra collections (sets and lists). Inspecting every collection element regarding its source and target type allows a fine grained conversion of elements to the target type. Previously, we just applied conversion if custom converters were registered. This change also considers the declared collection type of the property. --- .../convert/MappingCassandraConverter.java | 118 +++++++++++++----- .../cassandra/domain/AllPossibleTypes.java | 2 + ...RepositoryReturnTypesIntegrationTests.java | 2 +- .../CassandraTypeMappingIntegrationTest.java | 24 ++++ 4 files changed, 111 insertions(+), 35 deletions(-) diff --git a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/convert/MappingCassandraConverter.java b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/convert/MappingCassandraConverter.java index 53e948bd5..e4705f492 100644 --- a/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/convert/MappingCassandraConverter.java +++ b/spring-data-cassandra/src/main/java/org/springframework/data/cassandra/convert/MappingCassandraConverter.java @@ -23,6 +23,7 @@ import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; +import java.util.List; import java.util.Map.Entry; import org.slf4j.Logger; @@ -343,8 +344,7 @@ public class MappingCassandraConverter extends AbstractCassandraConverter Object value = getWriteValue(property, accessor); if (log.isDebugEnabled()) { - log.debug("doWithProperties Property.type {}, Property.value {}", - property.getType().getName(), value); + log.debug("doWithProperties Property.type {}, Property.value {}", property.getType().getName(), value); } if (property.isCompositePrimaryKey()) { @@ -352,8 +352,8 @@ public class MappingCassandraConverter extends AbstractCassandraConverter log.debug("Property is a compositeKey"); } - writeInsertFromWrapper(getConvertingAccessor(value, property.getCompositePrimaryKeyEntity()), - insert, property.getCompositePrimaryKeyEntity()); + writeInsertFromWrapper(getConvertingAccessor(value, property.getCompositePrimaryKeyEntity()), insert, + property.getCompositePrimaryKeyEntity()); return; } @@ -435,8 +435,8 @@ public class MappingCassandraConverter extends AbstractCassandraConverter log.debug("Adding udt.value [{}] - [{}]", property.getColumnName().toCql(), value); } - TypeCodec typeCodec = CodecRegistry.DEFAULT_INSTANCE.codecFor( - getMappingContext().getDataType(property)); + TypeCodec typeCodec = CodecRegistry.DEFAULT_INSTANCE + .codecFor(getMappingContext().getDataType(property)); udtValue.set(property.getColumnName().toCql(), value, typeCodec); } @@ -477,8 +477,8 @@ public class MappingCassandraConverter extends AbstractCassandraConverter Class targetType = getTargetType(idProperty); if (getConversionService().canConvert(id.getClass(), targetType)) { - return Collections.singleton(QueryBuilder.eq(idProperty.getColumnName().toCql(), - getPotentiallyConvertedSimpleValue(id, targetType))); + return Collections.singleton( + QueryBuilder.eq(idProperty.getColumnName().toCql(), getPotentiallyConvertedSimpleValue(id, targetType))); } return Collections.singleton(QueryBuilder.eq(idProperty.getColumnName().toCql(), id)); @@ -527,9 +527,8 @@ public class MappingCassandraConverter extends AbstractCassandraConverter for (Entry entry : id.entrySet()) { CassandraPersistentProperty persistentProperty = entity.getPersistentProperty(entry.getKey()); - Assert.notNull(persistentProperty, - String.format("MapId contains references [%s] that is an unknown property of [%s]", - entry.getKey(), entity.getName())); + Assert.notNull(persistentProperty, String.format( + "MapId contains references [%s] that is an unknown property of [%s]", entry.getKey(), entity.getName())); clauses.add(QueryBuilder.eq(persistentProperty.getColumnName().toCql(), getWriteValue(entry.getValue(), persistentProperty.getTypeInformation()))); @@ -549,7 +548,7 @@ public class MappingCassandraConverter extends AbstractCassandraConverter Assert.isTrue(entity.getType().isAssignableFrom(object.getClass()), String.format("Given instance of type [%s] is not of compatible expected type [%s]", - object.getClass().getName(), entity.getType().getName())); + object.getClass().getName(), entity.getType().getName())); if (object instanceof MapIdentifiable) { return ((MapIdentifiable) object).getMapId(); @@ -757,6 +756,32 @@ public class MappingCassandraConverter extends AbstractCassandraConverter return value; } + /** + * Checks whether we have a custom conversion for the given simple object. Converts the given value if so, applies + * {@link Enum} handling or returns the value as is. + * + * @param value + * @param target must not be {@literal null}. + * @return + */ + @SuppressWarnings({ "rawtypes", "unchecked" }) + private Object getPotentiallyConvertedSimpleRead(Object value, Class target) { + + if (value == null || target == null || target.isAssignableFrom(value.getClass())) { + return value; + } + + if (conversions.hasCustomReadTarget(value.getClass(), target)) { + return conversionService.convert(value, target); + } + + if (Enum.class.isAssignableFrom(target)) { + return Enum.valueOf((Class) target, value.toString()); + } + + return conversionService.convert(value, target); + } + private Class getCollectionType(TypeInformation type) { if (type.getType().isInterface()) { @@ -811,28 +836,60 @@ public class MappingCassandraConverter extends AbstractCassandraConverter } } + if (property.isCollectionLike() && obj instanceof Collection) { + return readCollectionOrArray(property.getTypeInformation(), (Collection) obj); + } + CassandraPersistentEntity persistentEntity = getMappingContext().getPersistentEntity(property.getActualType()); + if (persistentEntity != null && persistentEntity.isUserDefinedType() && obj instanceof UDTValue) { + return readEntityFromUdt(persistentEntity, (UDTValue) obj); + } + + return getPotentiallyConvertedSimpleRead(obj, property.getType()); + } + + /** + * Reads the given {@link Collection} into a collection of the given {@link TypeInformation}. + * + * @param targetType must not be {@literal null}. + * @param sourceValue must not be {@literal null}. + * @param path must not be {@literal null}. + * @return the converted {@link Collection} or array, will never be {@literal null}. + */ + @SuppressWarnings({ "rawtypes", "unchecked" }) + private Object readCollectionOrArray(TypeInformation targetType, Collection sourceValue) { + + Assert.notNull(targetType, "Target type must not be null!"); + + Class collectionType = targetType.getType(); + + TypeInformation componentType = targetType.getComponentType(); + Class rawComponentType = componentType == null ? null : componentType.getType(); + + collectionType = Collection.class.isAssignableFrom(collectionType) ? collectionType : List.class; + Collection items = targetType.getType().isArray() ? new ArrayList() + : CollectionFactory.createCollection(collectionType, rawComponentType, sourceValue.size()); + + if (sourceValue.isEmpty()) { + return getPotentiallyConvertedSimpleRead(items, collectionType); + } + + CassandraPersistentEntity persistentEntity = getMappingContext().getPersistentEntity(componentType); + if (persistentEntity != null && persistentEntity.isUserDefinedType()) { - if (property.isCollectionLike() && obj instanceof Collection) { - Collection original = (Collection) obj; + for (Object udtValue : sourceValue) { + items.add(readEntityFromUdt(persistentEntity, (UDTValue) udtValue)); + } - Collection converted = CollectionFactory.createCollection(property.getType(), original.size()); - - for (Object element : original) { - if (element instanceof UDTValue) { - converted.add(readEntityFromUdt(persistentEntity, (UDTValue) element)); - } - } - - return converted; - } else if (obj instanceof UDTValue) { - return readEntityFromUdt(persistentEntity, (UDTValue) obj); + } else { + for (Object item : sourceValue) { + items.add(getPotentiallyConvertedSimpleRead(item, rawComponentType)); } } - return obj; + return getPotentiallyConvertedSimpleRead(items, targetType.getType()); } private TypeCodec getCodec(CassandraPersistentProperty property) { @@ -864,14 +921,7 @@ public class MappingCassandraConverter extends AbstractCassandraConverter */ @Override public Object getPropertyValue(CassandraPersistentProperty property) { - - Object readValue = getReadValue(parent, property); - - if(readValue == null || property.getType().isAssignableFrom(readValue.getClass())){ - return readValue; - } - - return conversionService.convert(readValue, property.getType()); + return getReadValue(parent, property); } } } diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/AllPossibleTypes.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/AllPossibleTypes.java index 6cd7f84f9..e8f2f6149 100644 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/AllPossibleTypes.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/domain/AllPossibleTypes.java @@ -88,6 +88,8 @@ public class AllPossibleTypes { private Map mapOfString; private Condition anEnum; + private Set setOfEnum; + private List listOfEnum; // supported by conversion java.time.LocalDate localDate; diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/isolated/RepositoryReturnTypesIntegrationTests.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/isolated/RepositoryReturnTypesIntegrationTests.java index 987c4d15c..79750e1f2 100755 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/isolated/RepositoryReturnTypesIntegrationTests.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/repository/isolated/RepositoryReturnTypesIntegrationTests.java @@ -262,7 +262,7 @@ public class RepositoryReturnTypesIntegrationTests extends AbstractSpringDataEmb allPossibleTypesRepository.save(entity); Map result = allPossibleTypesRepository.findEntityAsMapById(entity.getId()); - assertThat(result).hasSize(41); + assertThat(result).hasSize(43); assertThat(result.get("primitiveinteger")).isEqualTo((Object) Integer.valueOf(123)); assertThat(result.get("biginteger")).isEqualTo((Object) BigInteger.ONE); } diff --git a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/test/integration/mapping/types/CassandraTypeMappingIntegrationTest.java b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/test/integration/mapping/types/CassandraTypeMappingIntegrationTest.java index e7f716e5f..008ceb3cd 100755 --- a/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/test/integration/mapping/types/CassandraTypeMappingIntegrationTest.java +++ b/spring-data-cassandra/src/test/java/org/springframework/data/cassandra/test/integration/mapping/types/CassandraTypeMappingIntegrationTest.java @@ -402,6 +402,30 @@ public class CassandraTypeMappingIntegrationTest extends AbstractKeyspaceCreatin assertThat(loaded.getAnEnum()).isEqualTo(entity.getAnEnum()); } + @Test // DATACASS-280 + public void shouldReadAndWriteListOfEnum() { + + AllPossibleTypes entity = new AllPossibleTypes("1"); + entity.setListOfEnum(Collections.singletonList(Condition.MINT)); + + operations.insert(entity); + AllPossibleTypes loaded = operations.selectOneById(entity.getId(), AllPossibleTypes.class); + + assertThat(loaded.getListOfEnum()).contains(Condition.MINT); + } + + @Test // DATACASS-280 + public void shouldReadAndWriteSetOfEnum() { + + AllPossibleTypes entity = new AllPossibleTypes("1"); + entity.setSetOfEnum(Collections.singleton(Condition.MINT)); + + operations.insert(entity); + AllPossibleTypes loaded = operations.selectOneById(entity.getId(), AllPossibleTypes.class); + + assertThat(loaded.getSetOfEnum()).contains(Condition.MINT); + } + @Test // DATACASS-271 public void shouldReadAndWriteTime() {