Skip to content

Commit d0ff05d

Browse files
michael-simonsmichael-simons
committed
GH-2408 - Evaluate type parameters of converters and apply to complete collections if necessary.
This change applies custom converters to collections if they indicate via their type parameter if they want to work on a whole collection or on single elements. To make this work, a somewhat breaking change is required: The `Neo4jPersistentProperty` cannot return `Function<?, ?>` for their optional reading and writing converters anymore, as we loose the converter information at that place. The chances that someone extends that interface are minimal and can hopefully be mitigated in case anyone does so. The urgency of that feature justifies the change. In addition, the null safe writing and reading must be replaced with a delegating mechanism. This has the advantage that we have simpler stacks, as we don't pass lambdas around anymore. This fixes #2408.
1 parent fca2b6a commit d0ff05d

16 files changed

+474
-72
lines changed

‎src/main/java/org/springframework/data/neo4j/core/Neo4jTemplate.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ private Object convertIdValues(@Nullable Neo4jPersistentProperty idProperty, Obj
312312

313313
return neo4jMappingContext.getConversionService().writeValue(idValues,
314314
ClassTypeInformation.from(idValues.getClass()),
315-
idProperty == null ? null : idProperty.getOptionalWritingConverter());
315+
idProperty == null ? null : idProperty.getOptionalConverter());
316316
}
317317

318318
@Override

‎src/main/java/org/springframework/data/neo4j/core/ReactiveNeo4jTemplate.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ private Object convertIdValues(@Nullable Neo4jPersistentProperty idProperty, Obj
295295
}
296296

297297
return neo4jMappingContext.getConversionService().writeValue(idValues,
298-
ClassTypeInformation.from(idValues.getClass()), idProperty == null ? null : idProperty.getOptionalWritingConverter());
298+
ClassTypeInformation.from(idValues.getClass()), idProperty == null ? null : idProperty.getOptionalConverter());
299299
}
300300

301301
@Override

‎src/main/java/org/springframework/data/neo4j/core/convert/Neo4jConversionService.java‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,6 @@
1515
*/
1616
package org.springframework.data.neo4j.core.convert;
1717

18-
import java.util.function.Function;
19-
2018
import org.apiguardian.api.API;
2119
import org.neo4j.driver.Value;
2220
import org.springframework.dao.TypeMismatchDataAccessException;
@@ -69,7 +67,7 @@ public interface Neo4jConversionService {
6967
*/
7068
@Nullable
7169
Object readValue(
72-
@Nullable Value source, TypeInformation<?> targetType, @Nullable Function<Value, Object> conversionOverride
70+
@Nullable Value source, TypeInformation<?> targetType, @Nullable Neo4jPersistentPropertyConverter<?> conversionOverride
7371
);
7472

7573
/**
@@ -80,7 +78,7 @@ Object readValue(
8078
* @return A driver compatible value object.
8179
*/
8280
Value writeValue(
83-
@Nullable Object value, TypeInformation<?> sourceType, @Nullable Function<Object, Value> conversionOverride
81+
@Nullable Object value, TypeInformation<?> sourceType, @Nullable Neo4jPersistentPropertyConverter<?> conversionOverride
8482
);
8583

8684
/**

‎src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jConversionService.java‎

Lines changed: 32 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.springframework.data.mapping.model.SimpleTypeHolder;
3131
import org.springframework.data.neo4j.core.convert.Neo4jConversionService;
3232
import org.springframework.data.neo4j.core.convert.Neo4jConversions;
33+
import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyConverter;
3334
import org.springframework.data.util.TypeInformation;
3435
import org.springframework.lang.Nullable;
3536

@@ -68,25 +69,31 @@ public boolean hasCustomWriteTarget(Class<?> sourceType) {
6869
@Override
6970
@Nullable
7071
public Object readValue(@Nullable Value source, TypeInformation<?> targetType,
71-
@Nullable Function<Value, Object> conversionOverride) {
72-
73-
BiFunction<Value, Class<?>, Object> conversion = conversionOverride == null ?
74-
(v, t) -> conversionService.convert(v, t) :
75-
(v, t) -> conversionOverride.apply(v);
72+
@Nullable Neo4jPersistentPropertyConverter<?> conversionOverride) {
73+
74+
BiFunction<Value, Class<?>, Object> conversion;
75+
boolean applyConversionToCompleteCollection = false;
76+
if (conversionOverride == null) {
77+
conversion = (v, t) -> conversionService.convert(v, t);
78+
} else {
79+
applyConversionToCompleteCollection = conversionOverride instanceof NullSafeNeo4jPersistentPropertyConverter
80+
&& ((NullSafeNeo4jPersistentPropertyConverter<?>) conversionOverride).isForCollection();
81+
conversion = (v, t) -> conversionOverride.read(v);
82+
}
7683

77-
return readValueImpl(source, targetType, conversion);
84+
return readValueImpl(source, targetType, conversion, applyConversionToCompleteCollection);
7885
}
7986

8087
@Nullable
8188
private Object readValueImpl(@Nullable Value value, TypeInformation<?> type,
82-
BiFunction<Value, Class<?>, Object> conversion) {
89+
BiFunction<Value, Class<?>, Object> conversion, boolean applyConversionToCompleteCollection) {
8390

8491
boolean valueIsLiteralNullOrNullValue = value == null || value == Values.NULL;
8592

8693
try {
8794
Class<?> rawType = type.getType();
8895

89-
if (!valueIsLiteralNullOrNullValue && isCollection(type)) {
96+
if (!valueIsLiteralNullOrNullValue && isCollection(type) && !applyConversionToCompleteCollection) {
9097
Collection<Object> target = CollectionFactory
9198
.createCollection(rawType, type.getComponentType().getType(), value.size());
9299
value.values()
@@ -102,16 +109,25 @@ private Object readValueImpl(@Nullable Value value, TypeInformation<?> type,
102109

103110
@Override
104111
public Value writeValue(@Nullable Object value, TypeInformation<?> sourceType,
105-
@Nullable Function<Object, Value> writingConverter) {
106-
107-
Function<Object, Value> conversion =
108-
writingConverter == null ? v -> conversionService.convert(v, Value.class) : writingConverter;
112+
@Nullable Neo4jPersistentPropertyConverter<?> writingConverter) {
113+
114+
Function<Object, Value> conversion;
115+
boolean applyConversionToCompleteCollection = false;
116+
if (writingConverter == null) {
117+
conversion = v -> conversionService.convert(v, Value.class);
118+
} else {
119+
@SuppressWarnings("unchecked")
120+
Neo4jPersistentPropertyConverter<Object> hlp = (Neo4jPersistentPropertyConverter<Object>) writingConverter;
121+
applyConversionToCompleteCollection = writingConverter instanceof NullSafeNeo4jPersistentPropertyConverter
122+
&& ((NullSafeNeo4jPersistentPropertyConverter<?>) writingConverter).isForCollection();
123+
conversion = hlp::write;
124+
}
109125

110-
return writeValueImpl(value, sourceType, conversion);
126+
return writeValueImpl(value, sourceType, conversion, applyConversionToCompleteCollection);
111127
}
112128

113129
private Value writeValueImpl(@Nullable Object value, TypeInformation<?> type,
114-
Function<Object, Value> conversion) {
130+
Function<Object, Value> conversion, boolean applyConversionToCompleteCollection) {
115131

116132
if (value == null) {
117133
try {
@@ -122,10 +138,9 @@ private Value writeValueImpl(@Nullable Object value, TypeInformation<?> type,
122138
}
123139
}
124140

125-
if (isCollection(type)) {
141+
if (isCollection(type) && !applyConversionToCompleteCollection) {
126142
Collection<?> sourceCollection = (Collection<?>) value;
127-
Object[] targetCollection = (sourceCollection).stream()
128-
.map(conversion::apply).toArray();
143+
Object[] targetCollection = (sourceCollection).stream().map(conversion::apply).toArray();
129144
return Values.value(targetCollection);
130145
}
131146

‎src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jEntityConverter.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ public void write(Object source, Map<String, Object> parameters) {
194194
return;
195195
}
196196

197-
final Value value = conversionService.writeValue(propertyAccessor.getProperty(p), p.getTypeInformation(), p.getOptionalWritingConverter());
197+
final Value value = conversionService.writeValue(propertyAccessor.getProperty(p), p.getTypeInformation(), p.getOptionalConverter());
198198
if (p.isComposite()) {
199199
value.keys().forEach(k -> properties.put(k, value.get(k)));
200200
} else {
@@ -208,7 +208,7 @@ public void write(Object source, Map<String, Object> parameters) {
208208
if (nodeDescription.hasIdProperty()) {
209209
Neo4jPersistentProperty idProperty = nodeDescription.getRequiredIdProperty();
210210
parameters.put(Constants.NAME_OF_ID,
211-
conversionService.writeValue(propertyAccessor.getProperty(idProperty), idProperty.getTypeInformation(), idProperty.getOptionalWritingConverter()));
211+
conversionService.writeValue(propertyAccessor.getProperty(idProperty), idProperty.getTypeInformation(), idProperty.getOptionalConverter()));
212212
}
213213
// in case of relationship properties ignore internal id property
214214
if (nodeDescription.hasVersionProperty()) {
@@ -424,7 +424,7 @@ public Object getParameterValue(PreferredConstructor.Parameter parameter) {
424424
} else if (matchingProperty.isEntityWithRelationshipProperties()) {
425425
return lastMappedEntity;
426426
}
427-
return conversionService.readValue(extractValueOf(matchingProperty, values), parameter.getType(), matchingProperty.getOptionalReadingConverter());
427+
return conversionService.readValue(extractValueOf(matchingProperty, values), parameter.getType(), matchingProperty.getOptionalConverter());
428428
}
429429
};
430430

@@ -449,7 +449,7 @@ private PropertyHandler<Neo4jPersistentProperty> populateFrom(MapAccessor queryR
449449
propertyAccessor.setProperty(property, targetNode);
450450
}
451451
} else {
452-
Object value = conversionService.readValue(extractValueOf(property, queryResult), typeInformation, property.getOptionalReadingConverter());
452+
Object value = conversionService.readValue(extractValueOf(property, queryResult), typeInformation, property.getOptionalConverter());
453453
Class<?> rawType = typeInformation.getType();
454454
propertyAccessor.setProperty(property, getValueOrDefault(ownerIsKotlinType, rawType, value));
455455
}

‎src/main/java/org/springframework/data/neo4j/core/mapping/DefaultNeo4jPersistentProperty.java‎

Lines changed: 2 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,7 @@
1717

1818
import java.lang.reflect.Field;
1919
import java.util.Optional;
20-
import java.util.function.Function;
2120

22-
import org.neo4j.driver.Value;
23-
import org.neo4j.driver.Values;
2421
import org.springframework.data.mapping.Association;
2522
import org.springframework.data.mapping.MappingException;
2623
import org.springframework.data.mapping.PersistentEntity;
@@ -215,29 +212,13 @@ public boolean isEntityWithRelationshipProperties() {
215212
return isEntity() && ((Neo4jPersistentEntity<?>) getOwner()).isRelationshipPropertiesEntity();
216213
}
217214

218-
private static Function<Object, Value> nullSafeWrite(Function<Object, Value> delegate) {
219-
return source -> source == null ? Values.NULL : delegate.apply(source);
220-
}
221-
222215
@Override
223-
public Function<Object, Value> getOptionalWritingConverter() {
216+
public Neo4jPersistentPropertyConverter<?> getOptionalConverter() {
224217
return customConversion.getOptional()
225-
.map(c -> {
226-
Function<Object, Value> originalConversion = c::write;
227-
return this.isComposite() ? originalConversion : nullSafeWrite(originalConversion);
228-
})
218+
.map(Neo4jPersistentPropertyConverter.class::cast)
229219
.orElse(null);
230220
}
231221

232-
private static Function<Value, Object> nullSafeRead(Function<Value, Object> delegate) {
233-
return source -> source == null || source.isNull() ? null : delegate.apply(source);
234-
}
235-
236-
@Override
237-
public Function<Value, Object> getOptionalReadingConverter() {
238-
return customConversion.getOptional().map(c -> nullSafeRead(c::read)).orElse(null);
239-
}
240-
241222
/**
242223
* Computes the target name of this property.
243224
*

‎src/main/java/org/springframework/data/neo4j/core/mapping/DtoInstantiatingConverter.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ Object getPropertyValueFor(Neo4jPersistentProperty targetProperty, PersistentEnt
211211
singleValue = p -> context.getEntityConverter().read(actualType, p);
212212
} else {
213213
ClassTypeInformation<?> actualTargetType = ClassTypeInformation.from(actualType);
214-
singleValue = p -> context.getConversionService().readValue(p, actualTargetType, targetProperty.getOptionalReadingConverter());
214+
singleValue = p -> context.getConversionService().readValue(p, actualTargetType, targetProperty.getOptionalConverter());
215215
}
216216

217217
if (targetProperty.isCollectionLike()) {

‎src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jMappingContext.java‎

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717

1818
import java.lang.reflect.Constructor;
1919
import java.lang.reflect.Modifier;
20+
import java.lang.reflect.ParameterizedType;
21+
import java.lang.reflect.Type;
2022
import java.util.HashMap;
2123
import java.util.List;
2224
import java.util.Locale;
2325
import java.util.Map;
2426
import java.util.Optional;
2527
import java.util.Set;
2628
import java.util.concurrent.ConcurrentHashMap;
29+
import java.util.stream.Collectors;
2730

2831
import org.apiguardian.api.API;
2932
import org.neo4j.cypherdsl.core.Statement;
@@ -34,6 +37,7 @@
3437
import org.springframework.beans.factory.NoSuchBeanDefinitionException;
3538
import org.springframework.beans.factory.config.AutowireCapableBeanFactory;
3639
import org.springframework.context.ApplicationContext;
40+
import org.springframework.core.GenericTypeResolver;
3741
import org.springframework.data.mapping.MappingException;
3842
import org.springframework.data.mapping.PersistentEntity;
3943
import org.springframework.data.mapping.context.AbstractMappingContext;
@@ -346,7 +350,26 @@ Neo4jPersistentPropertyConverter getOptionalCustomConversionsFor(Neo4jPersistent
346350

347351
ConvertWith convertWith = persistentProperty.getRequiredAnnotation(ConvertWith.class);
348352
Neo4jPersistentPropertyConverterFactory persistentPropertyConverterFactory = this.getOrCreateConverterFactoryOfType(convertWith.converterFactory());
349-
return persistentPropertyConverterFactory.getPropertyConverterFor(persistentProperty);
353+
Neo4jPersistentPropertyConverter<?> customConversions = persistentPropertyConverterFactory.getPropertyConverterFor(persistentProperty);
354+
355+
boolean forCollection = false;
356+
if (persistentProperty.isCollectionLike() && convertWith.converter() != ConvertWith.UnsetConverter.class) {
357+
Map<String, Type> typeVariableMap = GenericTypeResolver.getTypeVariableMap(convertWith.converter())
358+
.entrySet()
359+
.stream()
360+
.collect(Collectors.toMap(e -> e.getKey().getName(), Map.Entry::getValue));
361+
Type propertyType = null;
362+
if (typeVariableMap.containsKey("T")) {
363+
propertyType = typeVariableMap.get("T");
364+
} else if (typeVariableMap.containsKey("P")) {
365+
propertyType = typeVariableMap.get("P");
366+
}
367+
forCollection =
368+
propertyType != null && propertyType instanceof ParameterizedType && persistentProperty.getType()
369+
.equals(((ParameterizedType) propertyType).getRawType());
370+
}
371+
372+
return new NullSafeNeo4jPersistentPropertyConverter<>(customConversions, persistentProperty.isComposite(), forCollection);
350373
}
351374

352375
@Override
@@ -378,7 +401,7 @@ public CreateRelationshipStatementHolder createStatement(Neo4jPersistentEntity<?
378401
if (relationshipContext.getRelationship().isDynamic()) {
379402
TypeInformation<?> keyType = relationshipContext.getInverse().getTypeInformation().getRequiredComponentType();
380403
Object key = ((Map.Entry) relatedValue).getKey();
381-
dynamicRelationshipType = conversionService.writeValue(key, keyType, relationshipContext.getInverse().getOptionalWritingConverter()).asString();
404+
dynamicRelationshipType = conversionService.writeValue(key, keyType, relationshipContext.getInverse().getOptionalConverter()).asString();
382405
}
383406
return createStatementForRelationShipWithProperties(
384407
neo4jPersistentEntity, relationshipContext,
@@ -414,7 +437,7 @@ private CreateRelationshipStatementHolder createStatementForRelationshipWithoutP
414437
Neo4jPersistentProperty inverse = relationshipContext.getInverse();
415438
TypeInformation<?> keyType = inverse.getTypeInformation().getRequiredComponentType();
416439
Object key = ((Map.Entry<?, ?>) relatedValue).getKey();
417-
relationshipType = conversionService.writeValue(key, keyType, inverse.getOptionalWritingConverter()).asString();
440+
relationshipType = conversionService.writeValue(key, keyType, inverse.getOptionalConverter()).asString();
418441
}
419442

420443
Statement relationshipCreationQuery = CypherGenerator.INSTANCE.prepareSaveOfRelationship(

‎src/main/java/org/springframework/data/neo4j/core/mapping/Neo4jPersistentProperty.java‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,10 @@
1616
package org.springframework.data.neo4j.core.mapping;
1717

1818
import java.util.Optional;
19-
import java.util.function.Function;
2019

2120
import org.apiguardian.api.API;
22-
import org.neo4j.driver.Value;
2321
import org.springframework.data.mapping.PersistentProperty;
22+
import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyConverter;
2423
import org.springframework.data.neo4j.core.schema.CompositeProperty;
2524
import org.springframework.data.neo4j.core.schema.DynamicLabels;
2625
import org.springframework.lang.Nullable;
@@ -69,10 +68,7 @@ default boolean isDynamicLabels() {
6968
}
7069

7170
@Nullable
72-
Function<Object, Value> getOptionalWritingConverter();
73-
74-
@Nullable
75-
Function<Value, Object> getOptionalReadingConverter();
71+
Neo4jPersistentPropertyConverter<?> getOptionalConverter();
7672

7773
/**
7874
* @return True if this property targets an entity which is a container for relationship properties.

‎src/main/java/org/springframework/data/neo4j/core/mapping/NullSafeNeo4jPersistentPropertyConverter.java‎

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,71 @@
1+
/*
2+
* Copyright 2011-2021 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
package org.springframework.data.neo4j.core.mapping;
17+
18+
import org.neo4j.driver.Value;
19+
import org.neo4j.driver.Values;
20+
import org.springframework.data.neo4j.core.convert.Neo4jPersistentPropertyConverter;
21+
import org.springframework.lang.Nullable;
22+
23+
/**
24+
* All property converters will be wrapped by this class. It adds the information if a converter needs to be applied to
25+
* a complete collection or to individual values.
26+
*
27+
* @author Michael J. Simons
28+
* @param <T> The type of the property this converter converts.
29+
* @soundtrack Roger Taylor - The Outsider
30+
*/
31+
final class NullSafeNeo4jPersistentPropertyConverter<T> implements Neo4jPersistentPropertyConverter<T> {
32+
33+
/**
34+
* The actual delegate doing the conversation.
35+
*/
36+
private final Neo4jPersistentPropertyConverter<T> delegate;
37+
38+
/**
39+
* {@literal false} for all non-composite converters. If true, {@literal null} will be passed to the writing converter
40+
*/
41+
private final boolean passNullOnWrite;
42+
43+
/**
44+
* {@literal true} if the converter needs to be applied to a whole collection.
45+
*/
46+
private final boolean forCollection;
47+
48+
NullSafeNeo4jPersistentPropertyConverter(Neo4jPersistentPropertyConverter<T> delegate, boolean passNullOnWrite,
49+
boolean forCollection) {
50+
this.delegate = delegate;
51+
this.passNullOnWrite = passNullOnWrite;
52+
this.forCollection = forCollection;
53+
}
54+
55+
@Override
56+
public Value write(@Nullable T source) {
57+
if (source == null) {
58+
return passNullOnWrite ? delegate.write(source) : Values.NULL;
59+
}
60+
return delegate.write(source);
61+
}
62+
63+
@Override @Nullable
64+
public T read(@Nullable Value source) {
65+
return source == null || source.isNull() ? null : delegate.read(source);
66+
}
67+
68+
public boolean isForCollection() {
69+
return forCollection;
70+
}
71+
}

0 commit comments

Comments
 (0)