Skip to content

Commit c6daa2f

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 76720fc commit c6daa2f

15 files changed

+451
-71
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
@@ -224,7 +224,7 @@ private Object convertIdValues(@Nullable Neo4jPersistentProperty idProperty, Obj
224224

225225
return neo4jMappingContext.getConversionService().writeValue(idValues,
226226
ClassTypeInformation.from(idValues.getClass()),
227-
idProperty == null ? null : idProperty.getOptionalWritingConverter());
227+
idProperty == null ? null : idProperty.getOptionalConverter());
228228
}
229229

230230
@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
@@ -226,7 +226,7 @@ private Object convertIdValues(@Nullable Neo4jPersistentProperty idProperty, Obj
226226
}
227227

228228
return neo4jMappingContext.getConversionService().writeValue(idValues,
229-
ClassTypeInformation.from(idValues.getClass()), idProperty == null ? null : idProperty.getOptionalWritingConverter());
229+
ClassTypeInformation.from(idValues.getClass()), idProperty == null ? null : idProperty.getOptionalConverter());
230230
}
231231

232232
@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
@@ -186,7 +186,7 @@ public void write(Object source, Map<String, Object> parameters) {
186186
return;
187187
}
188188

189-
final Value value = conversionService.writeValue(propertyAccessor.getProperty(p), p.getTypeInformation(), p.getOptionalWritingConverter());
189+
final Value value = conversionService.writeValue(propertyAccessor.getProperty(p), p.getTypeInformation(), p.getOptionalConverter());
190190
if (p.isComposite()) {
191191
value.keys().forEach(k -> properties.put(k, value.get(k)));
192192
} else {
@@ -200,7 +200,7 @@ public void write(Object source, Map<String, Object> parameters) {
200200
if (nodeDescription.hasIdProperty()) {
201201
Neo4jPersistentProperty idProperty = nodeDescription.getRequiredIdProperty();
202202
parameters.put(Constants.NAME_OF_ID,
203-
conversionService.writeValue(propertyAccessor.getProperty(idProperty), idProperty.getTypeInformation(), idProperty.getOptionalWritingConverter()));
203+
conversionService.writeValue(propertyAccessor.getProperty(idProperty), idProperty.getTypeInformation(), idProperty.getOptionalConverter()));
204204
}
205205
// in case of relationship properties ignore internal id property
206206
if (nodeDescription.hasVersionProperty()) {
@@ -387,7 +387,7 @@ public Object getParameterValue(PreferredConstructor.Parameter parameter) {
387387
} else if (matchingProperty.isEntityWithRelationshipProperties()) {
388388
return lastMappedEntity;
389389
}
390-
return conversionService.readValue(extractValueOf(matchingProperty, values), parameter.getType(), matchingProperty.getOptionalReadingConverter());
390+
return conversionService.readValue(extractValueOf(matchingProperty, values), parameter.getType(), matchingProperty.getOptionalConverter());
391391
}
392392
};
393393

@@ -411,7 +411,7 @@ private PropertyHandler<Neo4jPersistentProperty> populateFrom(MapAccessor queryR
411411
propertyAccessor.setProperty(property, targetNode);
412412
}
413413
} else {
414-
Object value = conversionService.readValue(extractValueOf(property, queryResult), typeInformation, property.getOptionalReadingConverter());
414+
Object value = conversionService.readValue(extractValueOf(property, queryResult), typeInformation, property.getOptionalConverter());
415415
Class<?> rawType = typeInformation.getType();
416416
propertyAccessor.setProperty(property, getValueOrDefault(ownerIsKotlinType, rawType, value));
417417
}

‎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
@@ -16,10 +16,7 @@
1616
package org.springframework.data.neo4j.core.mapping;
1717

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

21-
import org.neo4j.driver.Value;
22-
import org.neo4j.driver.Values;
2320
import org.springframework.data.mapping.Association;
2421
import org.springframework.data.mapping.MappingException;
2522
import org.springframework.data.mapping.PersistentEntity;
@@ -190,29 +187,13 @@ public boolean isEntity() {
190187
return super.isEntity() && isAssociation() || (isEntityWithRelationshipProperties() && !isComposite());
191188
}
192189

193-
private static Function<Object, Value> nullSafeWrite(Function<Object, Value> delegate) {
194-
return source -> source == null ? Values.NULL : delegate.apply(source);
195-
}
196-
197190
@Override
198-
public Function<Object, Value> getOptionalWritingConverter() {
191+
public Neo4jPersistentPropertyConverter<?> getOptionalConverter() {
199192
return customConversion.getOptional()
200-
.map(c -> {
201-
Function<Object, Value> originalConversion = c::write;
202-
return this.isComposite() ? originalConversion : nullSafeWrite(originalConversion);
203-
})
193+
.map(Neo4jPersistentPropertyConverter.class::cast)
204194
.orElse(null);
205195
}
206196

207-
private static Function<Value, Object> nullSafeRead(Function<Value, Object> delegate) {
208-
return source -> source == null || source.isNull() ? null : delegate.apply(source);
209-
}
210-
211-
@Override
212-
public Function<Value, Object> getOptionalReadingConverter() {
213-
return customConversion.getOptional().map(c -> nullSafeRead(c::read)).orElse(null);
214-
}
215-
216197
@Override
217198
public boolean isEntityWithRelationshipProperties() {
218199
return super.isEntity() && ((Neo4jPersistentEntity<?>) getOwner()).isRelationshipPropertiesEntity();

‎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;
@@ -344,7 +348,26 @@ Neo4jPersistentPropertyConverter getOptionalCustomConversionsFor(Neo4jPersistent
344348

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

350373
@Override
@@ -376,7 +399,7 @@ public CreateRelationshipStatementHolder createStatement(Neo4jPersistentEntity<?
376399
if (relationshipContext.getRelationship().isDynamic()) {
377400
TypeInformation<?> keyType = relationshipContext.getInverse().getTypeInformation().getRequiredComponentType();
378401
Object key = ((Map.Entry<String, ?>) relatedValue).getKey();
379-
dynamicRelationshipType = conversionService.writeValue(key, keyType, relationshipContext.getInverse().getOptionalWritingConverter()).asString();
402+
dynamicRelationshipType = conversionService.writeValue(key, keyType, relationshipContext.getInverse().getOptionalConverter()).asString();
380403
}
381404
return createStatementForRelationShipWithProperties(
382405
neo4jPersistentEntity, relationshipContext,
@@ -412,7 +435,7 @@ private CreateRelationshipStatementHolder createStatementForRelationshipWithoutP
412435
Neo4jPersistentProperty inverse = relationshipContext.getInverse();
413436
TypeInformation<?> keyType = inverse.getTypeInformation().getRequiredComponentType();
414437
Object key = ((Map.Entry<?, ?>) relatedValue).getKey();
415-
relationshipType = conversionService.writeValue(key, keyType, inverse.getOptionalWritingConverter()).asString();
438+
relationshipType = conversionService.writeValue(key, keyType, inverse.getOptionalConverter()).asString();
416439
}
417440

418441
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.data.neo4j.core.schema.RelationshipProperties;
@@ -78,10 +77,7 @@ default boolean isRelationshipWithProperties() {
7877
}
7978

8079
@Nullable
81-
Function<Object, Value> getOptionalWritingConverter();
82-
83-
@Nullable
84-
Function<Value, Object> getOptionalReadingConverter();
80+
Neo4jPersistentPropertyConverter<?> getOptionalConverter();
8581

8682
/**
8783
* @deprecated since 6.0.4, see #isEntityWithRelationshipProperties()

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