Skip to content

Commit

Permalink
Revise simple query and update value conversion.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@mp911de
mp911de committed May 25, 2023
1 parent f62d172 commit 7e831fd
Show file tree
Hide file tree
Showing 12 changed files with 120 additions and 158 deletions.
10 changes: 10 additions & 0 deletions .../main/java/org/springframework/data/cassandra/core/convert/DefaultColumnTypeResolver.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,16 @@ public UnresolvableCassandraType(TypeInformation<?> type, ColumnType... paramete
public DataType getDataType() {
throw new MappingException(String.format("Cannot resolve DataType for %s", getType().getName()));
}

@Override
public boolean isTupleType() {
return false;
}

@Override
public boolean isUserDefinedType() {
return false;
}
}

}
62 changes: 29 additions & 33 deletions .../main/java/org/springframework/data/cassandra/core/convert/MappingCassandraConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,30 +36,15 @@
import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.support.DefaultConversionService;
import org.springframework.dao.InvalidDataAccessApiUsageException;
import org.springframework.data.cassandra.core.mapping.BasicCassandraPersistentEntity;
import org.springframework.data.cassandra.core.mapping.BasicMapId;
import org.springframework.data.cassandra.core.mapping.CassandraMappingContext;
import org.springframework.data.cassandra.core.mapping.CassandraPersistentEntity;
import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty;
import org.springframework.data.cassandra.core.mapping.Column;
import org.springframework.data.cassandra.core.mapping.Element;
import org.springframework.data.cassandra.core.mapping.Embedded;
import org.springframework.data.cassandra.core.mapping.*;
import org.springframework.data.cassandra.core.mapping.Embedded.OnEmpty;
import org.springframework.data.cassandra.core.mapping.EmbeddedEntityOperations;
import org.springframework.data.cassandra.core.mapping.MapId;
import org.springframework.data.cassandra.core.mapping.MapIdentifiable;
import org.springframework.data.cassandra.core.mapping.PersistentPropertyTranslator;
import org.springframework.data.cassandra.core.mapping.UserTypeResolver;
import org.springframework.data.convert.CustomConversions;
import org.springframework.data.mapping.AccessOptions;
import org.springframework.data.mapping.InstanceCreatorMetadata;
import org.springframework.data.mapping.MappingException;
import org.springframework.data.mapping.Parameter;
import org.springframework.data.mapping.PersistentEntity;
import org.springframework.data.mapping.PersistentProperty;
import org.springframework.data.mapping.PersistentPropertyAccessor;
import org.springframework.data.mapping.PersistentPropertyPath;
import org.springframework.data.mapping.PersistentPropertyPathAccessor;
import org.springframework.data.mapping.context.MappingContext;
import org.springframework.data.mapping.model.ConvertingPropertyAccessor;
import org.springframework.data.mapping.model.DefaultSpELExpressionEvaluator;
Expand Down Expand Up @@ -347,8 +332,7 @@ private <R> R doReadProjection(ConversionContext context, CassandraValueProvider
CassandraValueProvider valueProviderToUse = new TranslatingCassandraValueProvider(propertyTranslator,
valueProvider);

InstanceCreatorMetadata<CassandraPersistentProperty> persistenceCreator = mappedEntity
.getInstanceCreatorMetadata();
InstanceCreatorMetadata<CassandraPersistentProperty> persistenceCreator = mappedEntity.getInstanceCreatorMetadata();

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

if (getCustomConversions().isSimpleType(value.getClass())) {
return getPotentiallyConvertedSimpleValue(value, requestedTargetType);
}

if (value instanceof Collection) {
return writeCollectionInternal((Collection<Object>) value, columnType);
}
Expand All @@ -938,29 +918,41 @@ private Object getWriteValue(@Nullable Object value, ColumnType columnType) {
TypeInformation<?> actualType = type.getRequiredActualType();
BasicCassandraPersistentEntity<?> entity = getMappingContext().getPersistentEntity(actualType.getType());

if (entity != null && columnType instanceof CassandraColumnType) {
if (columnType instanceof CassandraColumnType cassandraType) {

CassandraColumnType cassandraType = (CassandraColumnType) columnType;
if (cassandraType.isTupleType()) {

if (entity.isTupleType() && cassandraType.isTupleType()) {
if (entity != null && entity.isTupleType()) {

TupleValue tupleValue = ((TupleType) cassandraType.getDataType()).newValue();

write(value, tupleValue, entity);
TupleValue tupleValue = ((TupleType) cassandraType.getDataType()).newValue();
write(value, tupleValue, entity);
return tupleValue;
}

return tupleValue;
if (value instanceof TupleValue) {
return value;
}
}

if (entity.isUserDefinedType() && cassandraType.isUserDefinedType()) {
if (cassandraType.isUserDefinedType()) {

UdtValue udtValue = ((UserDefinedType) cassandraType.getDataType()).newValue();
if (entity != null && entity.isUserDefinedType()) {

write(value, udtValue, entity);
UdtValue udtValue = ((UserDefinedType) cassandraType.getDataType()).newValue();
write(value, udtValue, entity);
return udtValue;
}

return udtValue;
if (value instanceof UdtValue) {
return value;
}
}
}

if (getCustomConversions().isSimpleType(value.getClass())) {
return getPotentiallyConvertedSimpleValue(value, requestedTargetType);
}

return value;
}

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

if (requestedTargetType != null && !ClassUtils.isAssignableValue(requestedTargetType, value)) {
return getConversionService().convert(value, requestedTargetType);
}

return value;
}

Expand Down
29 changes: 13 additions & 16 deletions ...main/java/org/springframework/data/cassandra/repository/query/AbstractCassandraQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,18 +47,6 @@ public abstract class AbstractCassandraQuery extends CassandraRepositoryQuerySup

private final CassandraOperations operations;


private static CassandraConverter toConverter(CassandraOperations operations) {

Assert.notNull(operations, "CassandraOperations must not be null");

return operations.getConverter();
}

private static CassandraMappingContext toMappingContext(CassandraOperations operations) {
return toConverter(operations).getMappingContext();
}

/**
* Create a new {@link AbstractCassandraQuery} from the given {@link CassandraQueryMethod} and
* {@link CassandraOperations}.
Expand Down Expand Up @@ -87,11 +75,9 @@ protected CassandraOperations getOperations() {
@Override
public Object execute(Object[] parameters) {

CassandraParameterAccessor parameterAccessor = new ConvertingParameterAccessor(toConverter(getOperations()),
new CassandraParametersParameterAccessor(getQueryMethod(), parameters));

CassandraParameterAccessor parameterAccessor = new CassandraParametersParameterAccessor(getQueryMethod(),
parameters);
ResultProcessor resultProcessor = getQueryMethod().getResultProcessor().withDynamicProjection(parameterAccessor);

Statement<?> statement = createQuery(parameterAccessor);

CassandraQueryExecution queryExecution = getExecution(parameterAccessor,
Expand Down Expand Up @@ -183,4 +169,15 @@ private CassandraQueryExecution getExecutionToWrap(CassandraParameterAccessor pa
* @since 2.2
*/
protected abstract boolean isModifyingQuery();

private static CassandraConverter toConverter(CassandraOperations operations) {

Assert.notNull(operations, "CassandraOperations must not be null");

return operations.getConverter();
}

private static CassandraMappingContext toMappingContext(CassandraOperations operations) {
return toConverter(operations).getMappingContext();
}
}
10 changes: 2 additions & 8 deletions ...a/org/springframework/data/cassandra/repository/query/AbstractReactiveCassandraQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -81,14 +81,8 @@ public Object execute(Object[] parameters) {

private Publisher<Object> executeLater(ReactiveCassandraParameterAccessor parameterAccessor) {

CassandraParameterAccessor convertingParameterAccessor = new ConvertingParameterAccessor(
getRequiredConverter(getReactiveCassandraOperations()), parameterAccessor);

Mono<SimpleStatement> statement = createQuery(convertingParameterAccessor);

ResultProcessor resultProcessor = getQueryMethod().getResultProcessor()
.withDynamicProjection(convertingParameterAccessor);

Mono<SimpleStatement> statement = createQuery(parameterAccessor);
ResultProcessor resultProcessor = getQueryMethod().getResultProcessor().withDynamicProjection(parameterAccessor);
ReactiveCassandraQueryExecution queryExecution = getExecution(parameterAccessor, new ResultProcessingConverter(
resultProcessor, getRequiredMappingContext(getReactiveCassandraOperations()), getEntityInstantiators()));

Expand Down
29 changes: 13 additions & 16 deletions .../main/java/org/springframework/data/cassandra/repository/query/CassandraQueryCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@
import org.springframework.data.cassandra.core.query.CriteriaDefinition;
import org.springframework.data.cassandra.core.query.Filter;
import org.springframework.data.cassandra.core.query.Query;
import org.springframework.data.cassandra.repository.query.ConvertingParameterAccessor.PotentiallyConvertingIterator;
import org.springframework.data.domain.Range;
import org.springframework.data.domain.Sort;
import org.springframework.data.mapping.PersistentPropertyPath;
Expand Down Expand Up @@ -104,8 +103,7 @@ protected Filter create(Part part, Iterator<Object> iterator) {

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

Object filterOrCriteria = from(part, property, Criteria.where(path.toDotPath()),
(PotentiallyConvertingIterator) iterator);
Object filterOrCriteria = from(part, property, Criteria.where(path.toDotPath()), iterator);

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

Type type = part.getType();

switch (type) {
case AFTER:
case GREATER_THAN:
return where.gt(parameters.nextConverted(property));
return where.gt(parameters.next());
case GREATER_THAN_EQUAL:
return where.gte(parameters.nextConverted(property));
return where.gte(parameters.next());
case BEFORE:
case LESS_THAN:
return where.lt(parameters.nextConverted(property));
return where.lt(parameters.next());
case LESS_THAN_EQUAL:
return where.lte(parameters.nextConverted(property));
return where.lte(parameters.next());
case BETWEEN:
return computeBetweenPart(where, parameters);
case IN:
return where.in(nextAsArray(property, parameters));
return where.in(nextAsArray(parameters));
case LIKE:
case STARTING_WITH:
case ENDING_WITH:
return where.like(like(type, parameters.nextConverted(property)));
return where.like(like(type, parameters.next()));
case CONTAINING:
return containing(where, property, parameters.nextConverted(property));
return containing(where, property, parameters.next());
case TRUE:
return where.is(true);
case FALSE:
return where.is(false);
case SIMPLE_PROPERTY:
return where.is(parameters.nextConverted(property));
return where.is(parameters.next());
default:
throw new InvalidDataAccessApiUsageException(
String.format("Unsupported keyword [%s] in part [%s]", type, part));
Expand All @@ -193,7 +190,7 @@ private Object from(Part part, CassandraPersistentProperty property, Criteria wh
* Compute a {@link Type#BETWEEN} {@link Part}.
* <p>
* In case the first {@literal value} is actually a {@link Range} the lower and upper bounds of the {@link Range} are
* used according to their {@link Range.Bound#isInclusive() inclusion} definition. Otherwise the {@literal value} is
* used according to their {@link Range.Bound#isInclusive() inclusion} definition. Otherwise, the {@literal value} is
* used for greater than and {@link Iterator#next() parameters.next()} as less than criterions.
*
* @param where must not be {@literal null}.
Expand Down Expand Up @@ -260,9 +257,9 @@ private Object like(Type type, Object value) {
throw new IllegalArgumentException(String.format("Part Type [%s] not supported with like queries", type));
}

private Object[] nextAsArray(CassandraPersistentProperty property, PotentiallyConvertingIterator iterator) {
private Object[] nextAsArray(Iterator<Object> iterator) {

Object next = iterator.nextConverted(property);
Object next = iterator.next();

if (next instanceof Collection) {
return ((Collection<?>) next).toArray();
Expand Down
37 changes: 4 additions & 33 deletions ...java/org/springframework/data/cassandra/repository/query/ConvertingParameterAccessor.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,10 @@
*/
package org.springframework.data.cassandra.repository.query;

import java.util.Collection;
import java.util.Iterator;

import org.springframework.data.cassandra.core.convert.CassandraConverter;
import org.springframework.data.cassandra.core.cql.QueryOptions;
import org.springframework.data.cassandra.core.mapping.CassandraPersistentProperty;
import org.springframework.data.cassandra.core.mapping.CassandraType;
import org.springframework.data.domain.Pageable;
import org.springframework.data.domain.Range;
Expand Down Expand Up @@ -67,7 +65,7 @@ public Class<?> findDynamicProjection() {

@Override
public Object getBindableValue(int index) {
return potentiallyConvert(index, this.delegate.getBindableValue(index), null);
return potentiallyConvert(index, this.delegate.getBindableValue(index));
}

@Override
Expand Down Expand Up @@ -107,7 +105,7 @@ public Object[] getValues() {

@SuppressWarnings("unchecked")
@Nullable
Object potentiallyConvert(int index, @Nullable Object bindableValue, @Nullable CassandraPersistentProperty property) {
Object potentiallyConvert(int index, @Nullable Object bindableValue) {

if (bindableValue == null) {
return null;
Expand All @@ -123,11 +121,6 @@ Object potentiallyConvert(int index, @Nullable Object bindableValue, @Nullable C
this.converter.convertToColumnType(bindableValue, converter.getColumnTypeResolver().resolve(cassandraType));
}

if (property != null && ((property.isCollectionLike() && bindableValue instanceof Collection)
|| (!property.isCollectionLike() && !(bindableValue instanceof Collection)))) {
return this.converter.convertToColumnType(bindableValue, converter.getColumnTypeResolver().resolve(property));
}

return this.converter.convertToColumnType(bindableValue, converter.getColumnTypeResolver().resolve(bindableValue));
}

Expand All @@ -136,7 +129,7 @@ Object potentiallyConvert(int index, @Nullable Object bindableValue, @Nullable C
*
* @author Mark Paluch
*/
private class ConvertingIterator implements PotentiallyConvertingIterator {
private class ConvertingIterator implements Iterator<Object> {

private final Iterator<Object> delegate;

Expand All @@ -157,34 +150,12 @@ public boolean hasNext() {

@Nullable
public Object next() {
return potentiallyConvert(this.index++, this.delegate.next(), null);
return potentiallyConvert(this.index++, this.delegate.next());
}

public void remove() {
this.delegate.remove();
}

@Nullable
@Override
public Object nextConverted(CassandraPersistentProperty property) {
return potentiallyConvert(this.index++, this.delegate.next(), property);
}
}

/**
* Custom {@link Iterator} that adds a method to access elements in a converted manner.
*
* @author Mark Paluch
*/
interface PotentiallyConvertingIterator extends Iterator<Object> {

/**
* Returns the next element and pass in type information for potential conversion.
*
* @return the converted object, may be {@literal null}.
*/
@Nullable
Object nextConverted(CassandraPersistentProperty property);

}
}
7 changes: 4 additions & 3 deletions ...rg/springframework/data/cassandra/repository/query/ReactiveStringBasedCassandraQuery.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,11 +124,12 @@ protected StringBasedQuery getStringBasedQuery() {
public Mono<SimpleStatement> createQuery(CassandraParameterAccessor parameterAccessor) {

StringBasedQuery query = getStringBasedQuery();

ConvertingParameterAccessor parameterAccessorToUse = new ConvertingParameterAccessor(
getReactiveCassandraOperations().getConverter(), parameterAccessor);
Mono<SpELExpressionEvaluator> spelEvaluator = getSpelEvaluatorFor(query.getExpressionDependencies(),
parameterAccessor);
parameterAccessorToUse);

return spelEvaluator.map(it -> getQueryStatementCreator().select(query, parameterAccessor, it));
return spelEvaluator.map(it -> getQueryStatementCreator().select(query, parameterAccessorToUse, it));
}

@Override
Expand Down
Loading

0 comments on commit 7e831fd

Please sign in to comment.