Skip to content

Commit 77a12f1

Browse files
mp911demp911de
committed
Revise simple query and update value conversion.
We now defer simple value conversion in the converter to ensure we're converting all complex values first. We also apply conversion if the value isn't assignable to the requested target type. We removed double-conversion from part-tree queries to avoid duplicate conversion. Closes #1384
1 parent b793c94 commit 77a12f1

File tree

12 files changed

+120
-158
lines changed

12 files changed

+120
-158
lines changed

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

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -500,6 +500,16 @@ public UnresolvableCassandraType(TypeInformation<?> type, ColumnType... paramete
500500
public DataType getDataType() {
501501
throw new MappingException(String.format("Cannot resolve DataType for %s", getType().getName()));
502502
}
503+
504+
@Override
505+
public boolean isTupleType() {
506+
return false;
507+
}
508+
509+
@Override
510+
public boolean isUserDefinedType() {
511+
return false;
512+
}
503513
}
504514

505515
}

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

Lines changed: 29 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -36,30 +36,15 @@
3636
import org.springframework.core.convert.ConversionService;
3737
import org.springframework.core.convert.support.DefaultConversionService;
3838
import org.springframework.dao.InvalidDataAccessApiUsageException;
39-
import org.springframework.data.cassandra.core.mapping.BasicCassandraPersistentEntity;
40-
import org.springframework.data.cassandra.core.mapping.BasicMapId;
41-
import org.springframework.data.cassandra.core.mapping.CassandraMappingContext;
42-
import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity;
43-
import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty;
44-
import org.springframework.data.cassandra.core.mapping.Column;
45-
import org.springframework.data.cassandra.core.mapping.Element;
46-
import org.springframework.data.cassandra.core.mapping.Embedded;
39+
import org.springframework.data.cassandra.core.mapping.*;
4740
import org.springframework.data.cassandra.core.mapping.Embedded.OnEmpty;
48-
import org.springframework.data.cassandra.core.mapping.EmbeddedEntityOperations;
49-
import org.springframework.data.cassandra.core.mapping.MapId;
50-
import org.springframework.data.cassandra.core.mapping.MapIdentifiable;
51-
import org.springframework.data.cassandra.core.mapping.PersistentPropertyTranslator;
52-
import org.springframework.data.cassandra.core.mapping.UserTypeResolver;
5341
import org.springframework.data.convert.CustomConversions;
54-
import org.springframework.data.mapping.AccessOptions;
5542
import org.springframework.data.mapping.InstanceCreatorMetadata;
5643
import org.springframework.data.mapping.MappingException;
5744
import org.springframework.data.mapping.Parameter;
5845
import org.springframework.data.mapping.PersistentEntity;
5946
import org.springframework.data.mapping.PersistentProperty;
6047
import org.springframework.data.mapping.PersistentPropertyAccessor;
61-
import org.springframework.data.mapping.PersistentPropertyPath;
62-
import org.springframework.data.mapping.PersistentPropertyPathAccessor;
6348
import org.springframework.data.mapping.context.MappingContext;
6449
import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
6550
import org.springframework.data.mapping.model.DefaultSpELExpressionEvaluator;
@@ -347,8 +332,7 @@ private <R> R doReadProjection(ConversionContext context, CassandraValueProvider
347332
CassandraValueProvider valueProviderToUse = new TranslatingCassandraValueProvider(propertyTranslator,
348333
valueProvider);
349334

350-
InstanceCreatorMetadata<CassandraPersistentProperty> persistenceCreator = mappedEntity
351-
.getInstanceCreatorMetadata();
335+
InstanceCreatorMetadata<CassandraPersistentProperty> persistenceCreator = mappedEntity.getInstanceCreatorMetadata();
352336

353337
ParameterValueProvider<CassandraPersistentProperty> provider;
354338
if (persistenceCreator != null && persistenceCreator.hasParameters()) {
@@ -922,10 +906,6 @@ private Object getWriteValue(@Nullable Object value, ColumnType columnType) {
922906
return getConversionService().convert(value, resolvedTargetType);
923907
}
924908

925-
if (getCustomConversions().isSimpleType(value.getClass())) {
926-
return getPotentiallyConvertedSimpleValue(value, requestedTargetType);
927-
}
928-
929909
if (value instanceof Collection) {
930910
return writeCollectionInternal((Collection<Object>) value, columnType);
931911
}
@@ -938,29 +918,41 @@ private Object getWriteValue(@Nullable Object value, ColumnType columnType) {
938918
TypeInformation<?> actualType = type.getRequiredActualType();
939919
BasicCassandraPersistentEntity<?> entity = getMappingContext().getPersistentEntity(actualType.getType());
940920

941-
if (entity != null && columnType instanceof CassandraColumnType) {
921+
if (columnType instanceof CassandraColumnType cassandraType) {
942922

943-
CassandraColumnType cassandraType = (CassandraColumnType) columnType;
923+
if (cassandraType.isTupleType()) {
944924

945-
if (entity.isTupleType() && cassandraType.isTupleType()) {
925+
if (entity != null && entity.isTupleType()) {
946926

947-
TupleValue tupleValue = ((TupleType) cassandraType.getDataType()).newValue();
948-
949-
write(value, tupleValue, entity);
927+
TupleValue tupleValue = ((TupleType) cassandraType.getDataType()).newValue();
928+
write(value, tupleValue, entity);
929+
return tupleValue;
930+
}
950931

951-
return tupleValue;
932+
if (value instanceof TupleValue) {
933+
return value;
934+
}
952935
}
953936

954-
if (entity.isUserDefinedType() && cassandraType.isUserDefinedType()) {
937+
if (cassandraType.isUserDefinedType()) {
955938

956-
UdtValue udtValue = ((UserDefinedType) cassandraType.getDataType()).newValue();
939+
if (entity != null && entity.isUserDefinedType()) {
957940

958-
write(value, udtValue, entity);
941+
UdtValue udtValue = ((UserDefinedType) cassandraType.getDataType()).newValue();
942+
write(value, udtValue, entity);
943+
return udtValue;
944+
}
959945

960-
return udtValue;
946+
if (value instanceof UdtValue) {
947+
return value;
948+
}
961949
}
962950
}
963951

952+
if (getCustomConversions().isSimpleType(value.getClass())) {
953+
return getPotentiallyConvertedSimpleValue(value, requestedTargetType);
954+
}
955+
964956
return value;
965957
}
966958

@@ -1017,6 +1009,10 @@ && getConversionService().canConvert(value.getClass(), requestedTargetType)) {
10171009
return ((Enum<?>) value).name();
10181010
}
10191011

1012+
if (requestedTargetType != null && !ClassUtils.isAssignableValue(requestedTargetType, value)) {
1013+
return getConversionService().convert(value, requestedTargetType);
1014+
}
1015+
10201016
return value;
10211017
}
10221018

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractCassandraQuery.java‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -47,18 +47,6 @@ public abstract class AbstractCassandraQuery extends CassandraRepositoryQuerySup
4747

4848
private final CassandraOperations operations;
4949

50-
51-
private static CassandraConverter toConverter(CassandraOperations operations) {
52-
53-
Assert.notNull(operations, "CassandraOperations must not be null");
54-
55-
return operations.getConverter();
56-
}
57-
58-
private static CassandraMappingContext toMappingContext(CassandraOperations operations) {
59-
return toConverter(operations).getMappingContext();
60-
}
61-
6250
/**
6351
* Create a new {@link AbstractCassandraQuery} from the given {@link CassandraQueryMethod} and
6452
* {@link CassandraOperations}.
@@ -87,11 +75,9 @@ protected CassandraOperations getOperations() {
8775
@Override
8876
public Object execute(Object[] parameters) {
8977

90-
CassandraParameterAccessor parameterAccessor = new ConvertingParameterAccessor(toConverter(getOperations()),
91-
new CassandraParametersParameterAccessor(getQueryMethod(), parameters));
92-
78+
CassandraParameterAccessor parameterAccessor = new CassandraParametersParameterAccessor(getQueryMethod(),
79+
parameters);
9380
ResultProcessor resultProcessor = getQueryMethod().getResultProcessor().withDynamicProjection(parameterAccessor);
94-
9581
Statement<?> statement = createQuery(parameterAccessor);
9682

9783
CassandraQueryExecution queryExecution = getExecution(parameterAccessor,
@@ -183,4 +169,15 @@ private CassandraQueryExecution getExecutionToWrap(CassandraParameterAccessor pa
183169
* @since 2.2
184170
*/
185171
protected abstract boolean isModifyingQuery();
172+
173+
private static CassandraConverter toConverter(CassandraOperations operations) {
174+
175+
Assert.notNull(operations, "CassandraOperations must not be null");
176+
177+
return operations.getConverter();
178+
}
179+
180+
private static CassandraMappingContext toMappingContext(CassandraOperations operations) {
181+
return toConverter(operations).getMappingContext();
182+
}
186183
}

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/AbstractReactiveCassandraQuery.java‎

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -81,14 +81,8 @@ public Object execute(Object[] parameters) {
8181

8282
private Publisher<Object> executeLater(ReactiveCassandraParameterAccessor parameterAccessor) {
8383

84-
CassandraParameterAccessor convertingParameterAccessor = new ConvertingParameterAccessor(
85-
getRequiredConverter(getReactiveCassandraOperations()), parameterAccessor);
86-
87-
Mono<SimpleStatement> statement = createQuery(convertingParameterAccessor);
88-
89-
ResultProcessor resultProcessor = getQueryMethod().getResultProcessor()
90-
.withDynamicProjection(convertingParameterAccessor);
91-
84+
Mono<SimpleStatement> statement = createQuery(parameterAccessor);
85+
ResultProcessor resultProcessor = getQueryMethod().getResultProcessor().withDynamicProjection(parameterAccessor);
9286
ReactiveCassandraQueryExecution queryExecution = getExecution(parameterAccessor, new ResultProcessingConverter(
9387
resultProcessor, getRequiredMappingContext(getReactiveCassandraOperations()), getEntityInstantiators()));
9488

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/CassandraQueryCreator.java‎

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@
3030
import org.springframework.data.cassandra.core.query.CriteriaDefinition;
3131
import org.springframework.data.cassandra.core.query.Filter;
3232
import org.springframework.data.cassandra.core.query.Query;
33-
import org.springframework.data.cassandra.repository.query.ConvertingParameterAccessor.PotentiallyConvertingIterator;
3433
import org.springframework.data.domain.Range;
3534
import org.springframework.data.domain.Sort;
3635
import org.springframework.data.mapping.PersistentPropertyPath;
@@ -104,8 +103,7 @@ protected Filter create(Part part, Iterator<Object> iterator) {
104103

105104
Assert.state(property != null && path.toDotPath() != null, "Leaf property must not be null");
106105

107-
Object filterOrCriteria = from(part, property, Criteria.where(path.toDotPath()),
108-
(PotentiallyConvertingIterator) iterator);
106+
Object filterOrCriteria = from(part, property, Criteria.where(path.toDotPath()), iterator);
109107

110108
if (filterOrCriteria instanceof CriteriaDefinition) {
111109
return Filter.from((CriteriaDefinition) filterOrCriteria);
@@ -151,38 +149,37 @@ protected Query complete(Filter criteria, Sort sort) {
151149
/**
152150
* Returns a {@link Filter} or {@link CriteriaDefinition} object representing the criterion for a {@link Part}.
153151
*/
154-
private Object from(Part part, CassandraPersistentProperty property, Criteria where,
155-
PotentiallyConvertingIterator parameters) {
152+
private Object from(Part part, CassandraPersistentProperty property, Criteria where, Iterator<Object> parameters) {
156153

157154
Type type = part.getType();
158155

159156
switch (type) {
160157
case AFTER:
161158
case GREATER_THAN:
162-
return where.gt(parameters.nextConverted(property));
159+
return where.gt(parameters.next());
163160
case GREATER_THAN_EQUAL:
164-
return where.gte(parameters.nextConverted(property));
161+
return where.gte(parameters.next());
165162
case BEFORE:
166163
case LESS_THAN:
167-
return where.lt(parameters.nextConverted(property));
164+
return where.lt(parameters.next());
168165
case LESS_THAN_EQUAL:
169-
return where.lte(parameters.nextConverted(property));
166+
return where.lte(parameters.next());
170167
case BETWEEN:
171168
return computeBetweenPart(where, parameters);
172169
case IN:
173-
return where.in(nextAsArray(property, parameters));
170+
return where.in(nextAsArray(parameters));
174171
case LIKE:
175172
case STARTING_WITH:
176173
case ENDING_WITH:
177-
return where.like(like(type, parameters.nextConverted(property)));
174+
return where.like(like(type, parameters.next()));
178175
case CONTAINING:
179-
return containing(where, property, parameters.nextConverted(property));
176+
return containing(where, property, parameters.next());
180177
case TRUE:
181178
return where.is(true);
182179
case FALSE:
183180
return where.is(false);
184181
case SIMPLE_PROPERTY:
185-
return where.is(parameters.nextConverted(property));
182+
return where.is(parameters.next());
186183
default:
187184
throw new InvalidDataAccessApiUsageException(
188185
String.format("Unsupported keyword [%s] in part [%s]", type, part));
@@ -193,7 +190,7 @@ private Object from(Part part, CassandraPersistentProperty property, Criteria wh
193190
* Compute a {@link Type#BETWEEN} {@link Part}.
194191
* <p>
195192
* In case the first {@literal value} is actually a {@link Range} the lower and upper bounds of the {@link Range} are
196-
* used according to their {@link Range.Bound#isInclusive() inclusion} definition. Otherwise the {@literal value} is
193+
* used according to their {@link Range.Bound#isInclusive() inclusion} definition. Otherwise, the {@literal value} is
197194
* used for greater than and {@link Iterator#next() parameters.next()} as less than criterions.
198195
*
199196
* @param where must not be {@literal null}.
@@ -260,9 +257,9 @@ private Object like(Type type, Object value) {
260257
throw new IllegalArgumentException(String.format("Part Type [%s] not supported with like queries", type));
261258
}
262259

263-
private Object[] nextAsArray(CassandraPersistentProperty property, PotentiallyConvertingIterator iterator) {
260+
private Object[] nextAsArray(Iterator<Object> iterator) {
264261

265-
Object next = iterator.nextConverted(property);
262+
Object next = iterator.next();
266263

267264
if (next instanceof Collection) {
268265
return ((Collection<?>) next).toArray();

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ConvertingParameterAccessor.java‎

Lines changed: 4 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,10 @@
1515
*/
1616
package org.springframework.data.cassandra.repository.query;
1717

18-
import java.util.Collection;
1918
import java.util.Iterator;
2019

2120
import org.springframework.data.cassandra.core.convert.CassandraConverter;
2221
import org.springframework.data.cassandra.core.cql.QueryOptions;
23-
import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty;
2422
import org.springframework.data.cassandra.core.mapping.CassandraType;
2523
import org.springframework.data.domain.Pageable;
2624
import org.springframework.data.domain.Range;
@@ -74,7 +72,7 @@ public Class<?> findDynamicProjection() {
7472

7573
@Override
7674
public Object getBindableValue(int index) {
77-
return potentiallyConvert(index, this.delegate.getBindableValue(index), null);
75+
return potentiallyConvert(index, this.delegate.getBindableValue(index));
7876
}
7977

8078
@Override
@@ -114,7 +112,7 @@ public Object[] getValues() {
114112

115113
@SuppressWarnings("unchecked")
116114
@Nullable
117-
Object potentiallyConvert(int index, @Nullable Object bindableValue, @Nullable CassandraPersistentProperty property) {
115+
Object potentiallyConvert(int index, @Nullable Object bindableValue) {
118116

119117
if (bindableValue == null) {
120118
return null;
@@ -130,11 +128,6 @@ Object potentiallyConvert(int index, @Nullable Object bindableValue, @Nullable C
130128
this.converter.convertToColumnType(bindableValue, converter.getColumnTypeResolver().resolve(cassandraType));
131129
}
132130

133-
if (property != null && ((property.isCollectionLike() && bindableValue instanceof Collection)
134-
|| (!property.isCollectionLike() && !(bindableValue instanceof Collection)))) {
135-
return this.converter.convertToColumnType(bindableValue, converter.getColumnTypeResolver().resolve(property));
136-
}
137-
138131
return this.converter.convertToColumnType(bindableValue, converter.getColumnTypeResolver().resolve(bindableValue));
139132
}
140133

@@ -143,7 +136,7 @@ Object potentiallyConvert(int index, @Nullable Object bindableValue, @Nullable C
143136
*
144137
* @author Mark Paluch
145138
*/
146-
private class ConvertingIterator implements PotentiallyConvertingIterator {
139+
private class ConvertingIterator implements Iterator<Object> {
147140

148141
private final Iterator<Object> delegate;
149142

@@ -164,34 +157,12 @@ public boolean hasNext() {
164157

165158
@Nullable
166159
public Object next() {
167-
return potentiallyConvert(this.index++, this.delegate.next(), null);
160+
return potentiallyConvert(this.index++, this.delegate.next());
168161
}
169162

170163
public void remove() {
171164
this.delegate.remove();
172165
}
173166

174-
@Nullable
175-
@Override
176-
public Object nextConverted(CassandraPersistentProperty property) {
177-
return potentiallyConvert(this.index++, this.delegate.next(), property);
178-
}
179-
}
180-
181-
/**
182-
* Custom {@link Iterator} that adds a method to access elements in a converted manner.
183-
*
184-
* @author Mark Paluch
185-
*/
186-
interface PotentiallyConvertingIterator extends Iterator<Object> {
187-
188-
/**
189-
* Returns the next element and pass in type information for potential conversion.
190-
*
191-
* @return the converted object, may be {@literal null}.
192-
*/
193-
@Nullable
194-
Object nextConverted(CassandraPersistentProperty property);
195-
196167
}
197168
}

‎spring-data-cassandra/src/main/java/org/springframework/data/cassandra/repository/query/ReactiveStringBasedCassandraQuery.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -124,11 +124,12 @@ protected StringBasedQuery getStringBasedQuery() {
124124
public Mono<SimpleStatement> createQuery(CassandraParameterAccessor parameterAccessor) {
125125

126126
StringBasedQuery query = getStringBasedQuery();
127-
127+
ConvertingParameterAccessor parameterAccessorToUse = new ConvertingParameterAccessor(
128+
getReactiveCassandraOperations().getConverter(), parameterAccessor);
128129
Mono<SpELExpressionEvaluator> spelEvaluator = getSpelEvaluatorFor(query.getExpressionDependencies(),
129-
parameterAccessor);
130+
parameterAccessorToUse);
130131

131-
return spelEvaluator.map(it -> getQueryStatementCreator().select(query, parameterAccessor, it));
132+
return spelEvaluator.map(it -> getQueryStatementCreator().select(query, parameterAccessorToUse, it));
132133
}
133134

134135
@Override

0 commit comments

Comments
 (0)