Skip to content

Commit e599f93

Browse files
michael-simonsmichael-simons
authored
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 84857ea commit e599f93

16 files changed

+473
-73
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
@@ -319,7 +319,7 @@ private Object convertIdValues(@Nullable Neo4jPersistentProperty idProperty, Obj
319319

320320
return neo4jMappingContext.getConversionService().writeValue(idValues,
321321
ClassTypeInformation.from(idValues.getClass()),
322-
idProperty == null ? null : idProperty.getOptionalWritingConverter());
322+
idProperty == null ? null : idProperty.getOptionalConverter());
323323
}
324324

325325
@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
@@ -301,7 +301,7 @@ private Object convertIdValues(@Nullable Neo4jPersistentProperty idProperty, Obj
301301
}
302302

303303
return neo4jMappingContext.getConversionService().writeValue(idValues,
304-
ClassTypeInformation.from(idValues.getClass()), idProperty == null ? null : idProperty.getOptionalWritingConverter());
304+
ClassTypeInformation.from(idValues.getClass()), idProperty == null ? null : idProperty.getOptionalConverter());
305305
}
306306

307307
@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
@@ -193,7 +193,7 @@ public void write(Object source, Map<String, Object> parameters) {
193193
return;
194194
}
195195

196-
final Value value = conversionService.writeValue(propertyAccessor.getProperty(p), p.getTypeInformation(), p.getOptionalWritingConverter());
196+
final Value value = conversionService.writeValue(propertyAccessor.getProperty(p), p.getTypeInformation(), p.getOptionalConverter());
197197
if (p.isComposite()) {
198198
value.keys().forEach(k -> properties.put(k, value.get(k)));
199199
} else {
@@ -207,7 +207,7 @@ public void write(Object source, Map<String, Object> parameters) {
207207
if (nodeDescription.hasIdProperty()) {
208208
Neo4jPersistentProperty idProperty = nodeDescription.getRequiredIdProperty();
209209
parameters.put(Constants.NAME_OF_ID,
210-
conversionService.writeValue(propertyAccessor.getProperty(idProperty), idProperty.getTypeInformation(), idProperty.getOptionalWritingConverter()));
210+
conversionService.writeValue(propertyAccessor.getProperty(idProperty), idProperty.getTypeInformation(), idProperty.getOptionalConverter()));
211211
}
212212
// in case of relationship properties ignore internal id property
213213
if (nodeDescription.hasVersionProperty()) {
@@ -427,7 +427,7 @@ public <T> T getParameterValue(PreferredConstructor.Parameter<T, Neo4jPersistent
427427
} else if (matchingProperty.isEntityWithRelationshipProperties()) {
428428
result = lastMappedEntity;
429429
} else {
430-
result = conversionService.readValue(extractValueOf(matchingProperty, values), parameter.getType(), matchingProperty.getOptionalReadingConverter());
430+
result = conversionService.readValue(extractValueOf(matchingProperty, values), parameter.getType(), matchingProperty.getOptionalConverter());
431431
}
432432
return (T) result;
433433
}
@@ -454,7 +454,7 @@ private PropertyHandler<Neo4jPersistentProperty> populateFrom(MapAccessor queryR
454454
propertyAccessor.setProperty(property, targetNode);
455455
}
456456
} else {
457-
Object value = conversionService.readValue(extractValueOf(property, queryResult), typeInformation, property.getOptionalReadingConverter());
457+
Object value = conversionService.readValue(extractValueOf(property, queryResult), typeInformation, property.getOptionalConverter());
458458
Class<?> rawType = typeInformation.getType();
459459
propertyAccessor.setProperty(property, getValueOrDefault(ownerIsKotlinType, rawType, value));
460460
}

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

Lines changed: 1 addition & 22 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.annotation.ReadOnlyProperty;
2522
import org.springframework.data.mapping.Association;
2623
import org.springframework.data.mapping.MappingException;
@@ -217,31 +214,13 @@ public boolean isEntityWithRelationshipProperties() {
217214
return isEntity() && ((Neo4jPersistentEntity<?>) getOwner()).isRelationshipPropertiesEntity();
218215
}
219216

220-
private static Function<Object, Value> nullSafeWrite(Function<Object, Value> delegate) {
221-
return source -> source == null ? Values.NULL : delegate.apply(source);
222-
}
223-
224217
@Override
225-
public Function<Object, Value> getOptionalWritingConverter() {
218+
public Neo4jPersistentPropertyConverter<?> getOptionalConverter() {
226219
return customConversion.getOptional()
227220
.map(Neo4jPersistentPropertyConverter.class::cast)
228-
.map(c -> {
229-
@SuppressWarnings("unchecked")
230-
Function<Object, Value> originalConversion = c::write;
231-
return this.isComposite() ? originalConversion : nullSafeWrite(originalConversion);
232-
})
233221
.orElse(null);
234222
}
235223

236-
private static Function<Value, Object> nullSafeRead(Function<Value, Object> delegate) {
237-
return source -> source == null || source.isNull() ? null : delegate.apply(source);
238-
}
239-
240-
@Override
241-
public Function<Value, Object> getOptionalReadingConverter() {
242-
return customConversion.getOptional().map(c -> nullSafeRead(c::read)).orElse(null);
243-
}
244-
245224
/**
246225
* Computes the target name of this property.
247226
*

‎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
@@ -225,7 +225,7 @@ Object getPropertyValueFor(Neo4jPersistentProperty targetProperty, PersistentEnt
225225
singleValue = p -> context.getEntityConverter().read(actualType, p);
226226
} else {
227227
ClassTypeInformation<?> actualTargetType = ClassTypeInformation.from(actualType);
228-
singleValue = p -> context.getConversionService().readValue(p, actualTargetType, targetProperty.getOptionalReadingConverter());
228+
singleValue = p -> context.getConversionService().readValue(p, actualTargetType, targetProperty.getOptionalConverter());
229229
}
230230

231231
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;
@@ -349,7 +353,26 @@ Neo4jPersistentPropertyConverter<?> getOptionalCustomConversionsFor(Neo4jPersist
349353

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

355378
@Override
@@ -381,7 +404,7 @@ public CreateRelationshipStatementHolder createStatement(Neo4jPersistentEntity<?
381404
if (relationshipContext.getRelationship().isDynamic()) {
382405
TypeInformation<?> keyType = relationshipContext.getInverse().getTypeInformation().getRequiredComponentType();
383406
Object key = ((Map.Entry) relatedValue).getKey();
384-
dynamicRelationshipType = conversionService.writeValue(key, keyType, relationshipContext.getInverse().getOptionalWritingConverter()).asString();
407+
dynamicRelationshipType = conversionService.writeValue(key, keyType, relationshipContext.getInverse().getOptionalConverter()).asString();
385408
}
386409
return createStatementForRelationShipWithProperties(
387410
neo4jPersistentEntity, relationshipContext,
@@ -417,7 +440,7 @@ private CreateRelationshipStatementHolder createStatementForRelationshipWithoutP
417440
Neo4jPersistentProperty inverse = relationshipContext.getInverse();
418441
TypeInformation<?> keyType = inverse.getTypeInformation().getRequiredComponentType();
419442
Object key = ((Map.Entry<?, ?>) relatedValue).getKey();
420-
relationshipType = conversionService.writeValue(key, keyType, inverse.getOptionalWritingConverter()).asString();
443+
relationshipType = conversionService.writeValue(key, keyType, inverse.getOptionalConverter()).asString();
421444
}
422445

423446
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)