Skip to content

Commit b35fd2e

Browse files
jxblumjxblum
committed
Fix test failures related to Joda Time and ThreeTenBackport type mappings.
Refactor and cleanup codebase. Resolves spring-projectsgh-1170.
1 parent 5960892 commit b35fd2e

12 files changed

+200
-198
lines changed

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/AbstractCassandraConverter.java

-1
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@
1515
*/
1616
package org.springframework.data.cassandra.core.convert;
1717

18-
1918
import java.util.Collections;
2019

2120
import org.springframework.beans.factory.InitializingBean;

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/CassandraConverter.java

+1
Original file line numberDiff line numberDiff line change
@@ -128,4 +128,5 @@ default Object convertToColumnType(Object value, TypeInformation<?> typeInformat
128128
* @param entity must not be {@literal null}.
129129
*/
130130
void write(Object source, Object sink, CassandraPersistentEntity<?> entity);
131+
131132
}

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/CassandraConverters.java

+4-1
Original file line numberDiff line numberDiff line change
@@ -92,7 +92,10 @@ public enum RowToDateConverter implements Converter<Row, Date> {
9292

9393
@Override
9494
public Date convert(Row row) {
95-
return Date.from(row.getInstant(0));
95+
96+
Instant instant = row.getInstant(0);
97+
98+
return instant != null ? Date.from(instant) : null;
9699
}
97100
}
98101

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/CassandraCustomConversions.java

+4-17
Original file line numberDiff line numberDiff line change
@@ -64,31 +64,18 @@ public CassandraCustomConversions(List<?> converters) {
6464

6565
/**
6666
* Cassandra-specific extension to {@link org.springframework.data.convert.CustomConversions.ConverterConfiguration}.
67-
* This extension avoids {@link Converter} registrations that enforce date mapping to {@link Date} from JSR-310, Joda
68-
* Time and ThreeTenBackport.
67+
* This extension avoids {@link Converter} registrations that enforce date mapping to {@link Date} from JSR-310.
6968
*/
7069
static class CassandraConverterConfiguration extends ConverterConfiguration {
7170

72-
public CassandraConverterConfiguration(StoreConversions storeConversions, List<?> userConverters) {
71+
CassandraConverterConfiguration(StoreConversions storeConversions, List<?> userConverters) {
7372
super(storeConversions, userConverters, getConverterFilter());
7473
}
7574

7675
static Predicate<ConvertiblePair> getConverterFilter() {
7776

78-
return convertiblePair -> {
79-
80-
if (sourceMatches(convertiblePair, "org.joda.time") || sourceMatches(convertiblePair, "org.threeten.bp")
81-
|| Jsr310Converters.supports(convertiblePair.getSourceType())
82-
&& Date.class.isAssignableFrom(convertiblePair.getTargetType())) {
83-
return false;
84-
}
85-
86-
return true;
87-
};
88-
}
89-
90-
private static boolean sourceMatches(ConvertiblePair convertiblePair, String packagePrefix) {
91-
return convertiblePair.getSourceType().getName().startsWith(packagePrefix);
77+
return convertiblePair -> !(Jsr310Converters.supports(convertiblePair.getSourceType())
78+
&& Date.class.isAssignableFrom(convertiblePair.getTargetType()));
9279
}
9380
}
9481
}

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/CassandraJsr310Converters.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -55,7 +55,7 @@ private CassandraJsr310Converters() {}
5555
converters.add(LocalTimeToMillisOfDayConverter.INSTANCE);
5656

5757
converters.add(DateToInstantConverter.INSTANCE);
58-
converters.add(LocalDateToInstantConverter.INSTANCE);
58+
converters.add(LocalDateTimeToInstantConverter.INSTANCE);
5959

6060
return converters;
6161
}
@@ -117,7 +117,7 @@ public Instant convert(Date source) {
117117
* @since 3.0
118118
*/
119119
@WritingConverter
120-
enum LocalDateToInstantConverter implements Converter<LocalDateTime, Instant> {
120+
enum LocalDateTimeToInstantConverter implements Converter<LocalDateTime, Instant> {
121121

122122
INSTANCE;
123123

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/DefaultColumnTypeResolver.java

+10-16
Original file line numberDiff line numberDiff line change
@@ -174,16 +174,11 @@ public CassandraColumnType resolve(TypeInformation<?> typeInformation) {
174174

175175
private CassandraColumnType resolve(TypeInformation<?> typeInformation, FrozenIndicator frozen) {
176176

177-
return getCustomWriteTarget(typeInformation).map(it -> {
178-
return createCassandraTypeDescriptor(tryResolve(it), ClassTypeInformation.from(it));
179-
}).orElseGet(() -> {
180-
181-
if (typeInformation.getType().isEnum()) {
182-
return ColumnType.create(String.class, DataTypes.TEXT);
183-
}
184-
185-
return createCassandraTypeDescriptor(typeInformation, frozen);
186-
});
177+
return getCustomWriteTarget(typeInformation)
178+
.map(it -> createCassandraTypeDescriptor(tryResolve(it), ClassTypeInformation.from(it)))
179+
.orElseGet(() -> typeInformation.getType().isEnum()
180+
? ColumnType.create(String.class, DataTypes.TEXT)
181+
: createCassandraTypeDescriptor(typeInformation, frozen));
187182
}
188183

189184
private Optional<Class<?>> getCustomWriteTarget(TypeInformation<?> typeInformation) {
@@ -203,9 +198,9 @@ private DataType tryResolve(Class<?> type) {
203198

204199
try {
205200
return getCodecRegistry().codecFor(type).getCqlType();
206-
} catch (CodecNotFoundException e) {
201+
} catch (CodecNotFoundException cause) {
207202
if (log.isDebugEnabled()) {
208-
log.debug(String.format("Cannot resolve Codec for %s", type.getName()), e);
203+
log.debug(String.format("Cannot resolve Codec for %s", type.getName()), cause);
209204
}
210205
return null;
211206
}
@@ -435,11 +430,10 @@ private CassandraColumnType createCassandraTypeDescriptor(TypeInformation<?> typ
435430
}
436431

437432
DataType dataType = tryResolve(typeInformation.getType());
438-
if (dataType == null) {
439-
return new UnresolvableCassandraType(typeInformation);
440-
}
441433

442-
return new DefaultCassandraColumnType(typeInformation, dataType);
434+
return dataType == null
435+
? new UnresolvableCassandraType(typeInformation)
436+
: new DefaultCassandraColumnType(typeInformation, dataType);
443437
}
444438

445439
private DataType getRequiredDataType(CassandraType annotation, int typeIndex) {

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/core/convert/MappingCassandraConverter.java

+60-54
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,19 @@
3535
import org.springframework.core.convert.ConversionService;
3636
import org.springframework.core.convert.support.DefaultConversionService;
3737
import org.springframework.dao.InvalidDataAccessApiUsageException;
38-
import org.springframework.data.cassandra.core.mapping.*;
38+
import org.springframework.data.cassandra.core.mapping.BasicCassandraPersistentEntity;
39+
import org.springframework.data.cassandra.core.mapping.BasicMapId;
40+
import org.springframework.data.cassandra.core.mapping.CassandraMappingContext;
41+
import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity;
42+
import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty;
43+
import org.springframework.data.cassandra.core.mapping.Column;
44+
import org.springframework.data.cassandra.core.mapping.Element;
45+
import org.springframework.data.cassandra.core.mapping.Embedded;
3946
import org.springframework.data.cassandra.core.mapping.Embedded.OnEmpty;
47+
import org.springframework.data.cassandra.core.mapping.EmbeddedEntityOperations;
48+
import org.springframework.data.cassandra.core.mapping.MapId;
49+
import org.springframework.data.cassandra.core.mapping.MapIdentifiable;
50+
import org.springframework.data.cassandra.core.mapping.UserTypeResolver;
4051
import org.springframework.data.mapping.MappingException;
4152
import org.springframework.data.mapping.PersistentPropertyAccessor;
4253
import org.springframework.data.mapping.PreferredConstructor;
@@ -100,19 +111,7 @@ public class MappingCassandraConverter extends AbstractCassandraConverter
100111
* Create a new {@link MappingCassandraConverter} with a {@link CassandraMappingContext}.
101112
*/
102113
public MappingCassandraConverter() {
103-
104-
super(newConversionService());
105-
106-
CassandraCustomConversions conversions = new CassandraCustomConversions(Collections.emptyList());
107-
108-
this.mappingContext = newDefaultMappingContext(conversions);
109-
this.codecRegistry = mappingContext.getCodecRegistry();
110-
this.spELContext = new SpELContext(RowReaderPropertyAccessor.INSTANCE);
111-
this.cassandraTypeResolver = new DefaultColumnTypeResolver(mappingContext,
112-
userTypeName -> getUserTypeResolver().resolveType(userTypeName), this::getCodecRegistry,
113-
this::getCustomConversions);
114-
this.setCustomConversions(conversions);
115-
this.embeddedEntityOperations = new EmbeddedEntityOperations(mappingContext);
114+
this(newDefaultMappingContext(new CassandraCustomConversions(Collections.emptyList())));
116115
}
117116

118117
/**
@@ -126,32 +125,35 @@ public MappingCassandraConverter(CassandraMappingContext mappingContext) {
126125

127126
Assert.notNull(mappingContext, "CassandraMappingContext must not be null");
128127

128+
UserTypeResolver userTypeResolver = userTypeName -> getUserTypeResolver().resolveType(userTypeName);
129+
129130
this.mappingContext = mappingContext;
130-
this.codecRegistry = mappingContext.getCodecRegistry();
131-
this.spELContext = new SpELContext(RowReaderPropertyAccessor.INSTANCE);
132-
this.cassandraTypeResolver = new DefaultColumnTypeResolver(mappingContext,
133-
userTypeName -> getUserTypeResolver().resolveType(userTypeName), this::getCodecRegistry,
134-
this::getCustomConversions);
131+
this.setCodecRegistry(mappingContext.getCodecRegistry());
135132
this.setCustomConversions(mappingContext.getCustomConversions());
133+
this.cassandraTypeResolver = new DefaultColumnTypeResolver(mappingContext, userTypeResolver,
134+
this::getCodecRegistry, this::getCustomConversions);
136135
this.embeddedEntityOperations = new EmbeddedEntityOperations(mappingContext);
136+
this.spELContext = new SpELContext(RowReaderPropertyAccessor.INSTANCE);
137137
}
138138

139139
/**
140-
* Creates a new {@link ConversionContext} given {@link ObjectPath}.
140+
* Construct a new instance of the {@literal default} Spring {@link ConversionService}.
141141
*
142-
* @param path the current {@link ObjectPath}, must not be {@literal null}.
143-
* @return the {@link ConversionContext}.
142+
* @return a new instance of the {@literal default} Spring {@link ConversionService}.
143+
* @see org.springframework.core.convert.ConversionService
144144
*/
145-
protected ConversionContext getConversionContext() {
146-
147-
return new ConversionContext(this::doReadRow, this::doReadTupleValue, this::doReadUdtValue,
148-
this::readCollectionOrArray, this::readMap, this::getPotentiallyConvertedSimpleRead);
149-
}
150-
151145
private static ConversionService newConversionService() {
152146
return new DefaultConversionService();
153147
}
154148

149+
/**
150+
* Constructs a new instance of a {@link MappingContext} for Cassandra.
151+
*
152+
* @param conversions {@link CassandraCustomConversions} object encapsulating complex type conversion logic.
153+
* @return a new {@link CassandraMappingContext}.
154+
* @see org.springframework.data.cassandra.core.mapping.CassandraMappingContext
155+
* @see org.springframework.data.mapping.context.MappingContext
156+
*/
155157
private static CassandraMappingContext newDefaultMappingContext(CassandraCustomConversions conversions) {
156158

157159
CassandraMappingContext mappingContext = new CassandraMappingContext();
@@ -162,6 +164,19 @@ private static CassandraMappingContext newDefaultMappingContext(CassandraCustomC
162164
return mappingContext;
163165
}
164166

167+
/**
168+
* Constructs a new instance of {@link ConversionContext} with various converters to convert different Cassandra
169+
* value types.
170+
*
171+
* @return the {@link ConversionContext}.
172+
* @see ConversionContext
173+
*/
174+
protected ConversionContext getConversionContext() {
175+
176+
return new ConversionContext(this::doReadRow, this::doReadTupleValue, this::doReadUdtValue,
177+
this::readCollectionOrArray, this::readMap, this::getPotentiallyConvertedSimpleRead);
178+
}
179+
165180
/* (non-Javadoc)
166181
* @see org.springframework.context.ApplicationContextAware#setApplicationContext(org.springframework.context.ApplicationContext)
167182
*/
@@ -203,12 +218,7 @@ public void setCodecRegistry(CodecRegistry codecRegistry) {
203218
*/
204219
@Override
205220
public CodecRegistry getCodecRegistry() {
206-
207-
if (this.codecRegistry == null) {
208-
return mappingContext.getCodecRegistry();
209-
}
210-
211-
return this.codecRegistry;
221+
return this.codecRegistry != null ? this.codecRegistry : getMappingContext().getCodecRegistry();
212222
}
213223

214224
/**
@@ -230,12 +240,7 @@ public void setUserTypeResolver(UserTypeResolver userTypeResolver) {
230240
* @since 3.0
231241
*/
232242
public UserTypeResolver getUserTypeResolver() {
233-
234-
if (this.userTypeResolver == null) {
235-
return this.mappingContext.getUserTypeResolver();
236-
}
237-
238-
return userTypeResolver;
243+
return this.userTypeResolver != null ? this.userTypeResolver : getMappingContext().getUserTypeResolver();
239244
}
240245

241246
/* (non-Javadoc)
@@ -368,8 +373,8 @@ protected <S> S doReadEntity(ConversionContext context, CassandraValueProvider v
368373
return (S) getConversionService().convert(valueProvider.getSource(), rawType);
369374
}
370375

371-
CassandraPersistentEntity<S> entity = (CassandraPersistentEntity<S>) getMappingContext()
372-
.getPersistentEntity(typeHint);
376+
CassandraPersistentEntity<S> entity =
377+
(CassandraPersistentEntity<S>) getMappingContext().getPersistentEntity(typeHint);
373378

374379
if (entity == null) {
375380
throw new MappingException(
@@ -394,8 +399,8 @@ private static Class<?> getRawSourceType(CassandraValueProvider valueProvider) {
394399
return UdtValue.class;
395400
}
396401

397-
throw new InvalidDataAccessApiUsageException(
398-
"Unsupported source type: " + ClassUtils.getDescriptiveType(valueProvider.getSource()));
402+
throw new InvalidDataAccessApiUsageException(String.format("Unsupported source type: %s",
403+
ClassUtils.getDescriptiveType(valueProvider.getSource())));
399404
}
400405

401406
private <S> S doReadEntity(ConversionContext context, CassandraValueProvider valueProvider,
@@ -406,8 +411,8 @@ private <S> S doReadEntity(ConversionContext context, CassandraValueProvider val
406411

407412
if (persistenceConstructor != null && persistenceConstructor.hasParameters()) {
408413
SpELExpressionEvaluator evaluator = new DefaultSpELExpressionEvaluator(valueProvider.getSource(), spELContext);
409-
ParameterValueProvider<CassandraPersistentProperty> parameterValueProvider = newParameterValueProvider(context,
410-
entity, valueProvider);
414+
ParameterValueProvider<CassandraPersistentProperty> parameterValueProvider =
415+
newParameterValueProvider(context, entity, valueProvider);
411416
provider = new ConverterAwareSpELExpressionParameterValueProvider(evaluator, getConversionService(),
412417
parameterValueProvider, context);
413418
} else {
@@ -745,10 +750,7 @@ public Object getId(Object object, CassandraPersistentEntity<?> entity) {
745750

746751
/**
747752
* Check custom conversions for type override or fall back to
748-
* {@link #determineTargetType(CassandraPersistentProperty)}
749-
*
750-
* @param property
751-
* @return
753+
* {@link ColumnTypeResolver#resolve(CassandraPersistentProperty)}.
752754
*/
753755
private Class<?> getTargetType(CassandraPersistentProperty property) {
754756
return getCustomConversions().getCustomWriteTarget(property.getType())
@@ -793,17 +795,20 @@ private Object getWriteValue(@Nullable Object value, ColumnType columnType) {
793795

794796
if (getCustomConversions().hasCustomWriteTarget(value.getClass(), requestedTargetType)) {
795797

796-
Class<?> resolvedTargetType = getCustomConversions().getCustomWriteTarget(value.getClass(), requestedTargetType)
798+
Class<?> resolvedTargetType = getCustomConversions()
799+
.getCustomWriteTarget(value.getClass(), requestedTargetType)
797800
.orElse(requestedTargetType);
798801

799802
return getConversionService().convert(value, resolvedTargetType);
800803
}
801804

802805
if (getCustomConversions().hasCustomWriteTarget(value.getClass())) {
803806

804-
Class<?> resolvedTargetType = getCustomConversions().getCustomWriteTarget(value.getClass())
805-
.orElseThrow(() -> new IllegalStateException(String
806-
.format("Unable to determined custom write target for value type [%s]", value.getClass().getName())));
807+
Class<?> resolvedTargetType = getCustomConversions()
808+
.getCustomWriteTarget(value.getClass())
809+
.orElseThrow(() -> new IllegalStateException(
810+
String.format("Unable to determined custom write target for value type [%s]",
811+
value.getClass().getName())));
807812

808813
return getConversionService().convert(value, resolvedTargetType);
809814
}
@@ -987,6 +992,7 @@ private Object getReadValue(ConversionContext context, CassandraValueProvider va
987992
}
988993

989994
Object value = valueProvider.getPropertyValue(property);
995+
990996
return value == null ? null : context.convert(value, property.getTypeInformation());
991997
}
992998

0 commit comments

Comments
 (0)