Spring/spring-data-neo4j
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Fix conversion of Enums and general collection handling.

Browse Source 
While there have been conversions ins place for enum, they didn’t work for either arrays of enums nor collections thereof. While fixing the later, it became appearent that neither converting collections of anything else than String worked.

This has been fixed by moving the creation of collections into the `Neo4jConverter` itself and not as an afterthought registering in the conversion service. The additional converters are triggered to late in the process and don’t get hold of the original type descriptor anymore it seems. The type descriptor they receive doesn’t know about the element type anymore, as we cannot easily convert Spring Datas type information into Spring Frameworks type descriptor. Therefor, we stay in Spring Data land.

This closes #93.
This commit is contained in:
Michael Simons
2019-11-18 11:33:38 +01:00
committed by GitHub
parent 452d7a24f2
commit 85fee93463
12 changed files with 139 additions and 132 deletions
Download Patch File Download Diff File Expand all files Collapse all files

59
spring-data-neo4j/src/main/java/org/neo4j/springframework/data/core/convert/AdditionalTypes.java
View File

@@ -20,6 +20,7 @@ package org.neo4j.springframework.data.core.convert;
import static org.springframework.data.convert.ConverterBuilder.*;
import java.lang.reflect.Array;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.time.Instant;
@@ -27,18 +28,20 @@ import java.time.ZoneOffset;
import java.time.format.DateTimeFormatter;
import java.time.temporal.TemporalAmount;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.Date;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Set;
import java.util.UUID;
import org.jetbrains.annotations.NotNull;
import org.neo4j.driver.Value;
import org.neo4j.driver.Values;
import org.neo4j.driver.exceptions.value.LossyCoercion;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.ConditionalConverter;
import org.springframework.core.convert.converter.GenericConverter;
import org.springframework.data.convert.ReadingConverter;
import org.springframework.data.convert.WritingConverter;
@@ -182,35 +185,59 @@ final class AdditionalTypes {
@ReadingConverter
@WritingConverter
static class EnumConverter implements GenericConverter {
private final Set<ConvertiblePair> convertiblePairs;
EnumConverter() {
Set<ConvertiblePair> hlp = new HashSet<>();
hlp.add(new ConvertiblePair(Enum.class, Value.class));
hlp.add(new ConvertiblePair(Value.class, Enum.class));
convertiblePairs = Collections.unmodifiableSet(hlp);
}
static class EnumConverter implements GenericConverter, ConditionalConverter {
@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return this.convertiblePairs;
return null;
}
@Override
public boolean matches(TypeDescriptor sourceType, TypeDescriptor targetType) {
if (Value.class.isAssignableFrom(sourceType.getType())) {
return describesSupportedEnumVariant(targetType);
} else if (Value.class.isAssignableFrom(targetType.getType())) {
return describesSupportedEnumVariant(sourceType);
} else {
return false;
}
}
@Override
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
Class<?> concreteTargetType = targetType.getType();
if (source == null) {
return concreteTargetType == Value.class ? Values.NULL : null;
return Value.class.isAssignableFrom(targetType.getType()) ? Values.NULL : null;
}
if (sourceType.getType() == Value.class) {
return Enum.valueOf((Class<Enum>) concreteTargetType, ((Value) source).asString());
if (Value.class.isAssignableFrom(sourceType.getType())) {
return read((Value) source, targetType);
} else {
if (sourceType.isArray()) {
return Values.value(Arrays.stream(((Enum[]) source)).map(Enum::name).toArray());
}
return Values.value(((Enum) source).name());
}
}
@NotNull
private static Object read(Value source, TypeDescriptor targetType) {
if (targetType.isArray()) {
Class<?> componentType = targetType.getElementTypeDescriptor().getType();
Object[] targetArray = (Object[]) Array.newInstance(componentType, source.size());
Arrays.setAll(targetArray, i -> Enum.valueOf((Class<Enum>) componentType, source.get(i).asString()));
return targetArray;
} else {
return Enum.valueOf((Class<Enum>) targetType.getType(), source.asString());
}
}
private static boolean describesSupportedEnumVariant(TypeDescriptor typeDescriptor) {
TypeDescriptor elementType = typeDescriptor.isArray() ?
typeDescriptor.getElementTypeDescriptor() :
typeDescriptor;
return Enum.class.isAssignableFrom(elementType.getType());
}
}
static Float asFloat(Value value) {

89
spring-data-neo4j/src/main/java/org/neo4j/springframework/data/core/convert/Neo4jConversions.java
View File

@@ -18,24 +18,13 @@
*/
package org.neo4j.springframework.data.core.convert;
import static java.util.stream.Collectors.*;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import org.apiguardian.api.API;
import org.neo4j.driver.Value;
import org.neo4j.driver.Values;
import org.springframework.core.CollectionFactory;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.converter.ConverterRegistry;
import org.springframework.core.convert.converter.GenericConverter;
import org.springframework.data.convert.CustomConversions;
import org.springframework.lang.Nullable;
/**
* @author Michael J. Simons
@@ -47,7 +36,6 @@ public final class Neo4jConversions extends CustomConversions {
private static final StoreConversions STORE_CONVERSIONS;
private static final List<Object> STORE_CONVERTERS;
private static final TypeDescriptor TYPE_DESCRIPTOR_OF_VALUE = TypeDescriptor.valueOf(Value.class);
static {
@@ -76,81 +64,4 @@ public final class Neo4jConversions extends CustomConversions {
public Neo4jConversions(Collection<?> converters) {
super(STORE_CONVERSIONS, converters);
}
@Override
public void registerConvertersIn(ConverterRegistry conversionService) {
super.registerConvertersIn(conversionService);
// Those can only be added at this point, as they will delegate to the target conversion service.
conversionService.addConverter(new ValueToCollectionConverter((ConversionService) conversionService));
conversionService.addConverter(new CollectionToValueConverter((ConversionService) conversionService));
}
private static class ValueToCollectionConverter implements GenericConverter {
private static final Set<ConvertiblePair> CONVERTIBLE_TYPES = Collections
.singleton(new ConvertiblePair(Value.class, Collection.class));
private final ConversionService conversionService;
ValueToCollectionConverter(ConversionService conversionService) {
this.conversionService = conversionService;
}
@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return CONVERTIBLE_TYPES;
}
@Override
@Nullable
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
if (source == null) {
return null;
}
Value value = (Value) source;
TypeDescriptor elementDesc = targetType.getElementTypeDescriptor();
Collection<Object> target = CollectionFactory.createCollection(targetType.getType(),
(elementDesc != null ? elementDesc.getType() : null), value.size());
if (elementDesc == null) {
target.addAll(value.asList());
} else {
value.values().forEach(sourceElement -> target.add(this.conversionService.convert(sourceElement,
TYPE_DESCRIPTOR_OF_VALUE, elementDesc)));
}
return target;
}
}
private static class CollectionToValueConverter implements GenericConverter {
private static final Set<ConvertiblePair> CONVERTIBLE_TYPES = Collections
.singleton(new ConvertiblePair(Collection.class, Value.class));
private final ConversionService conversionService;
CollectionToValueConverter(ConversionService conversionService) {
this.conversionService = conversionService;
}
@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return CONVERTIBLE_TYPES;
}
@Override
@Nullable
public Object convert(@Nullable Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
if (source == null) {
return null;
}
Collection<?> sourceCollection = (Collection<?>) source;
return Values.value((sourceCollection).stream().map(v -> conversionService
.convert(v, sourceType.elementTypeDescriptor(v), TYPE_DESCRIPTOR_OF_VALUE))
.collect(toList()));
}
}
}

37
spring-data-neo4j/src/main/java/org/neo4j/springframework/data/core/mapping/DefaultNeo4jConverter.java
View File

@@ -18,11 +18,14 @@
*/
package org.neo4j.springframework.data.core.mapping;
import java.util.Collection;
import org.neo4j.driver.Value;
import org.neo4j.driver.Values;
import org.neo4j.driver.types.TypeSystem;
import org.neo4j.springframework.data.core.convert.Neo4jConversions;
import org.neo4j.springframework.data.core.convert.Neo4jConverter;
import org.springframework.core.CollectionFactory;
import org.springframework.core.convert.ConversionService;
import org.springframework.core.convert.TypeDescriptor;
import org.springframework.core.convert.support.ConfigurableConversionService;
@@ -65,10 +68,19 @@ final class DefaultNeo4jConverter implements Neo4jConverter {
}
try {
return conversionService.convert(value, type.getType());
Class<?> rawType = type.getType();
if (isCollection(type)) {
Collection<Object> target = CollectionFactory.createCollection(rawType,
type.getComponentType().getType(), value.size());
value.values().forEach(
element -> target.add(conversionService.convert(element, type.getComponentType().getType())));
return target;
}
return conversionService.convert(value, rawType);
} catch (Exception e) {
String msg = String.format("Could not convert %s into %s",
(value == null ? "literal null" : value), type.toString());
String msg = String.format("Could not convert %s into %s", value, type.toString());
throw new TypeMismatchDataAccessException(msg, e);
}
}
@@ -80,14 +92,25 @@ final class DefaultNeo4jConverter implements Neo4jConverter {
return Values.NULL;
}
if (isCollection(type)) {
Collection<?> sourceCollection = (Collection<?>) value;
Object[] targetCollection = (sourceCollection).stream().map(element ->
conversionService.convert(element, Value.class)).toArray();
return Values.value(targetCollection);
}
return conversionService.convert(value, Value.class);
}
private static boolean isCollection(TypeInformation<?> type) {
return Collection.class.isAssignableFrom(type.getType());
}
@Override
public <T> PersistentPropertyAccessor<T> decoratePropertyAccessor(TypeSystem typeSystem,
PersistentPropertyAccessor<T> targetPropertyAccessor) {
return new ConvertingPropertyAccessor<>(targetPropertyAccessor, new DelegatingConversionService(conversionService));
return new ConvertingPropertyAccessor<>(targetPropertyAccessor, new DelegatingConversionService());
}
@Override
@@ -107,12 +130,6 @@ final class DefaultNeo4jConverter implements Neo4jConverter {
class DelegatingConversionService implements ConversionService {
private final ConversionService delegate;
DelegatingConversionService(ConversionService delegate) {
this.delegate = delegate;
}
@Override
public boolean canConvert(Class<?> sourceType, Class<?> targetType) {
return sourceType == Value.class;

27
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/integration/Neo4jConversionsIT.java
View File

@@ -23,7 +23,9 @@ import static org.junit.jupiter.api.DynamicTest.*;
import java.time.LocalDate;
import java.util.Arrays;
import java.util.Collection;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.stream.Stream;
@@ -162,17 +164,36 @@ class Neo4jConversionsIT extends Neo4jConversionsITBase {
Value v = session.run("MATCH (n) WHERE labels(n) = [$label] RETURN n[$attribute] as r",
Values.parameters("label", label, "attribute", attribute)).single().get("r");
Object converted = DEFAULT_CONVERSION_SERVICE.convert(v, t.getClass());
assertThat(converted).isEqualTo(t);
TypeDescriptor typeDescriptor = TypeDescriptor.forObject(t);
if (typeDescriptor.isCollection()) {
Collection<?> collection = (Collection<?>) t;
Class<?> targetType = collection.stream().map(Object::getClass).findFirst().get();
List<Object> convertedObjects = v.asList(elem -> DEFAULT_CONVERSION_SERVICE.convert(elem, targetType));
assertThat(convertedObjects).containsAll(collection);
} else {
Object converted = DEFAULT_CONVERSION_SERVICE.convert(v, typeDescriptor.getType());
assertThat(converted).isEqualTo(t);
}
}
}
static void assertWrite(String label, String attribute, Object t) {
Value driverValue;
if (t != null && Collection.class.isAssignableFrom(t.getClass())) {
Collection<?> sourceCollection = (Collection<?>) t;
Object[] targetCollection = (sourceCollection).stream().map(element ->
DEFAULT_CONVERSION_SERVICE.convert(element, Value.class)).toArray();
driverValue = Values.value(targetCollection);
} else {
driverValue = DEFAULT_CONVERSION_SERVICE.convert(t, Value.class);
}
try (Session session = neo4jConnectionSupport.getDriver().session()) {
Map<String, Object> parameters = new HashMap<>();
parameters.put("label", label);
parameters.put("attribute", attribute);
parameters.put("v", DEFAULT_CONVERSION_SERVICE.convert(t, TYPE_DESCRIPTOR_OF_VALUE));
parameters.put("v", driverValue);
long cnt = session
.run("MATCH (n) WHERE labels(n) = [$label] AND n[$attribute] = $v RETURN COUNT(n) AS cnt",

12
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/integration/imperative/TypeConversionIT.java
View File

@@ -34,6 +34,7 @@ import org.junit.jupiter.api.TestFactory;
import org.neo4j.driver.Driver;
import org.neo4j.driver.Session;
import org.neo4j.driver.Value;
import org.neo4j.driver.Values;
import org.neo4j.springframework.data.config.AbstractNeo4jConfig;
import org.neo4j.springframework.data.core.convert.Neo4jConversions;
import org.neo4j.springframework.data.integration.shared.Neo4jConversionsITBase;
@@ -135,7 +136,16 @@ class TypeConversionIT extends Neo4jConversionsITBase {
long id = (long) ReflectionTestUtils.getField(thing, "id");
Object domainValue = ReflectionTestUtils.getField(thing, fieldName);
Value driverValue = conversionService.convert(domainValue, Value.class);
Value driverValue;
if (domainValue != null && Collection.class.isAssignableFrom(domainValue.getClass())) {
Collection<?> sourceCollection = (Collection<?>) domainValue;
Object[] targetCollection = (sourceCollection).stream().map(element ->
conversionService.convert(element, Value.class)).toArray();
driverValue = Values.value(targetCollection);
} else {
driverValue = conversionService.convert(domainValue, Value.class);
}
try (Session session = neo4jConnectionSupport.getDriver().session()) {
Map<String, Object> parameters = new HashMap<>();

4
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/integration/shared/ImmutableAuditableThing.java
View File

@@ -20,7 +20,7 @@ package org.neo4j.springframework.data.integration.shared;
import lombok.AllArgsConstructor;
import lombok.Value;
import lombok.experimental.Wither;
import lombok.With;
import java.time.LocalDateTime;
@@ -37,7 +37,7 @@ import org.springframework.data.annotation.Persistent;
* @author Michael J. Simons
*/
@Value
@Wither
@With
@AllArgsConstructor(onConstructor = @__(@PersistenceConstructor))
@Persistent
public class ImmutableAuditableThing {

11
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/integration/shared/Neo4jConversionsITBase.java
View File

@@ -28,6 +28,7 @@ import java.util.*;
import org.junit.jupiter.api.BeforeAll;
import org.neo4j.driver.Session;
import org.neo4j.driver.Values;
import org.neo4j.springframework.data.integration.shared.ThingWithAllAdditionalTypes.SomeEnum;
import org.neo4j.springframework.data.test.Neo4jExtension;
import org.neo4j.springframework.data.types.CartesianPoint2d;
import org.neo4j.springframework.data.types.CartesianPoint3d;
@@ -88,6 +89,10 @@ public abstract class Neo4jConversionsITBase {
hlp.put("setOfStrings", new HashSet<>(Arrays.asList("Hallo", "wereld")));
hlp.put("anInstant", Instant.from(LocalDateTime.of(2019, 9, 26, 20, 34, 23).atOffset(ZoneOffset.UTC)));
hlp.put("aUUID", UUID.fromString("d4ec9208-4b17-4ec7-a709-19a5e53865a8"));
hlp.put("anEnum", SomeEnum.TheUsualMisfit);
hlp.put("anArrayOfEnums", new SomeEnum[] { SomeEnum.ValueA, SomeEnum.ValueB });
hlp.put("aCollectionOfEnums", Arrays.asList(SomeEnum.ValueC, SomeEnum.TheUsualMisfit));
hlp.put("listOfDoubles", Arrays.asList(1.0));
ADDITIONAL_TYPES = Collections.unmodifiableMap(hlp);
}
@@ -191,7 +196,11 @@ public abstract class Neo4jConversionsITBase {
+ " n.listOfStrings = ['Hello', 'World'],"
+ " n.setOfStrings = ['Hallo', 'wereld'],"
+ " n.anInstant = datetime('2019-09-26T20:34:23Z'),"
+ " n.aUUID = 'd4ec9208-4b17-4ec7-a709-19a5e53865a8'"
+ " n.aUUID = 'd4ec9208-4b17-4ec7-a709-19a5e53865a8',"
+ " n.listOfDoubles = [1.0],"
+ " n.anEnum = 'TheUsualMisfit',"
+ " n.anArrayOfEnums = ['ValueA', 'ValueB'],"
+ " n.aCollectionOfEnums = ['ValueC', 'TheUsualMisfit']"
+ " RETURN id(n) AS id", parameters).single().get("id").asLong();
parameters = new HashMap<>();

4
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/integration/shared/PersonWithAllConstructor.java
View File

@@ -23,7 +23,7 @@ import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.Setter;
import lombok.ToString;
import lombok.experimental.Wither;
import lombok.With;
import java.time.Instant;
import java.time.LocalDate;
@@ -48,7 +48,7 @@ import org.neo4j.springframework.data.core.schema.Property;
public class PersonWithAllConstructor {
@Id @GeneratedValue
@Wither
@With
private final Long id;
private final String name;

16
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/integration/shared/ThingWithAllAdditionalTypes.java
View File

@@ -22,7 +22,7 @@ import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.experimental.Wither;
import lombok.With;
import java.math.BigDecimal;
import java.math.BigInteger;
@@ -50,8 +50,12 @@ import org.neo4j.springframework.data.core.schema.Node;
@Builder
public class ThingWithAllAdditionalTypes {
enum SomeEnum {
ValueA, ValueB, TheUsualMisfit, ValueC
}
@Id @GeneratedValue
@Wither
@With
public final Long id;
private boolean[] booleanArray;
@@ -99,4 +103,12 @@ public class ThingWithAllAdditionalTypes {
private Instant anInstant;
private UUID aUUID;
private SomeEnum anEnum;
private SomeEnum[] anArrayOfEnums;
private List<Double> listOfDoubles;
private List<SomeEnum> aCollectionOfEnums;
}

4
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/integration/shared/ThingWithAllCypherTypes.java
View File

@@ -22,7 +22,7 @@ import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.experimental.Wither;
import lombok.With;
import java.time.LocalDate;
import java.time.LocalDateTime;
@@ -48,7 +48,7 @@ import org.neo4j.springframework.data.core.schema.Node;
public class ThingWithAllCypherTypes {
@Id @GeneratedValue
@Wither
@With
public final Long id;
private boolean aBoolean;

4
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/integration/shared/ThingWithAllSpatialTypes.java
View File

@@ -22,7 +22,7 @@ import lombok.AccessLevel;
import lombok.AllArgsConstructor;
import lombok.Builder;
import lombok.Data;
import lombok.experimental.Wither;
import lombok.With;
import org.neo4j.springframework.data.core.schema.GeneratedValue;
import org.neo4j.springframework.data.core.schema.Id;
@@ -45,7 +45,7 @@ import org.springframework.data.geo.Point;
public class ThingWithAllSpatialTypes {
@Id @GeneratedValue
@Wither
@With
public final Long id;
private Point sdnPoint;

4
spring-data-neo4j/src/test/java/org/neo4j/springframework/data/repository/event/ImmutableSample.java
View File

@@ -21,7 +21,7 @@ package org.neo4j.springframework.data.repository.event;
import lombok.AllArgsConstructor;
import lombok.NoArgsConstructor;
import lombok.Value;
import lombok.experimental.Wither;
import lombok.With;
import java.util.Date;
@@ -33,7 +33,7 @@ import org.springframework.data.annotation.LastModifiedDate;
* @author Michael J. Simons
*/
@Value
@Wither
@With
@AllArgsConstructor
@NoArgsConstructor(force = true)
public class ImmutableSample {