Skip to content
This repository has been archived by the owner on May 3, 2021. It is now read-only.

Commit

Permalink
GH-93 - Fix conversion of Enums and general collection handling.
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@michael-simons
michael-simons committed Nov 18, 2019
1 parent d247c39 commit 2cb245d
Show file tree
Hide file tree
Showing 13 changed files with 147 additions and 132 deletions.
10 changes: 9 additions & 1 deletion docs/conversions.adoc
View file
Expand Up @@ -160,4 +160,12 @@ Primitive types of wrapper types are equally supported.
|Point with CRS 4326 and x/y corresponding to lat/long
|

|===
|Instances of `Enum`
|String (The name value of the enum)
|

|Instances of `Enum[]`
|List of String (The name value of the enum)
|

|===
57 changes: 42 additions & 15 deletions ...a-neo4j-rx/src/main/java/org/neo4j/springframework/data/core/convert/AdditionalTypes.java
View file
Expand Up @@ -20,25 +20,28 @@

import static org.springframework.data.convert.ConverterBuilder.*;

import java.lang.reflect.Array;
import java.math.BigDecimal;
import java.math.BigInteger;
import java.time.Instant;
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;
Expand Down Expand Up @@ -182,35 +185,59 @@ static Value value(Date date) {

@ReadingConverter
@WritingConverter
static class EnumConverter implements GenericConverter {
static class EnumConverter implements GenericConverter, ConditionalConverter {

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);
@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return null;
}

@Override
public Set<ConvertiblePair> getConvertibleTypes() {
return this.convertiblePairs;
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) {
Expand Down
89 changes: 0 additions & 89 deletions ...-neo4j-rx/src/main/java/org/neo4j/springframework/data/core/convert/Neo4jConversions.java
View file
Expand Up @@ -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
Expand All @@ -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 {

Expand Down Expand Up @@ -76,81 +64,4 @@ public Neo4jConversions() {
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 changes: 27 additions & 10 deletions ...j-rx/src/main/java/org/neo4j/springframework/data/core/mapping/DefaultNeo4jConverter.java
View file
Expand Up @@ -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;
Expand Down Expand Up @@ -65,10 +68,19 @@ public Object readValue(@Nullable Value value, TypeInformation<?> type) {
}

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);
}
}
Expand All @@ -80,14 +92,25 @@ public Value writeValue(@Nullable Object value, TypeInformation<?> type) {
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
Expand All @@ -107,12 +130,6 @@ public Object getParameterValue(PreferredConstructor.Parameter parameter) {

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;
Expand Down
27 changes: 24 additions & 3 deletions ...neo4j-rx/src/test/java/org/neo4j/springframework/data/integration/Neo4jConversionsIT.java
View file
Expand Up @@ -23,7 +23,9 @@

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;

Expand Down Expand Up @@ -162,17 +164,36 @@ static void assertRead(String label, String attribute, Object t) {
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",
Expand Down
12 changes: 11 additions & 1 deletion ...src/test/java/org/neo4j/springframework/data/integration/imperative/TypeConversionIT.java
View file
Expand Up @@ -34,6 +34,7 @@
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;
Expand Down Expand Up @@ -135,7 +136,16 @@ void assertWrite(Object thing, String fieldName, ConversionService conversionSer

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<>();
Expand Down

0 comments on commit 2cb245d

Please sign in to comment.