Skip to content

Commit 0f50884

Browse files
michael-simonsmichael-simons
authored
GH-2428 - Provide mapping functions via supplier.
This avoids potentially stale objects during caches in mapping functions (like in our own) and fixes #2428.
1 parent c259f80 commit 0f50884

15 files changed

+90
-56
lines changed

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import java.util.function.BiFunction;
3434
import java.util.function.Consumer;
3535
import java.util.function.Function;
36+
import java.util.function.Supplier;
3637
import java.util.stream.Collectors;
3738

3839
import org.apache.commons.logging.LogFactory;
@@ -621,7 +622,7 @@ private <T> ExecutableQuery<T> createExecutableQuery(Class<T> domainType, @Nulla
621622
private <T> ExecutableQuery<T> createExecutableQuery(Class<T> domainType, @Nullable Class<?> resultType, @Nullable String cypherStatement,
622623
Map<String, Object> parameters) {
623624

624-
BiFunction<TypeSystem, MapAccessor, ?> mappingFunction = TemplateSupport
625+
Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunction = TemplateSupport
625626
.getAndDecorateMappingFunction(neo4jMappingContext, domainType, resultType);
626627
PreparedQuery<T> preparedQuery = PreparedQuery.queryFor(domainType)
627628
.withCypherQuery(cypherStatement)
@@ -881,7 +882,7 @@ public <T> ExecutableQuery<T> toExecutableQuery(Class<T> domainType,
881882
private <T> ExecutableQuery<T> createExecutableQuery(Class<T> domainType, Class<?> resultType,
882883
QueryFragmentsAndParameters queryFragmentsAndParameters) {
883884

884-
BiFunction<TypeSystem, MapAccessor, ?> mappingFunction = TemplateSupport
885+
Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunction = TemplateSupport
885886
.getAndDecorateMappingFunction(neo4jMappingContext, domainType, resultType);
886887
PreparedQuery<T> preparedQuery = PreparedQuery.queryFor(domainType)
887888
.withQueryFragmentsAndParameters(queryFragmentsAndParameters)

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

Lines changed: 32 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@
4040
import java.util.concurrent.atomic.AtomicBoolean;
4141
import java.util.function.BiFunction;
4242
import java.util.function.Function;
43+
import java.util.function.Supplier;
4344
import java.util.stream.Collectors;
4445

4546
/**
@@ -65,29 +66,35 @@ public static <CT> RequiredBuildStep<CT> queryFor(Class<CT> resultType) {
6566

6667
private final Class<T> resultType;
6768
private final QueryFragmentsAndParameters queryFragmentsAndParameters;
68-
private final @Nullable BiFunction<TypeSystem, Record, T> mappingFunction;
69+
private final @Nullable Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunctionSupplier;
70+
private volatile Optional<BiFunction<TypeSystem, Record, T>> lastMappingFunction = Optional.empty();
6971

7072
private PreparedQuery(OptionalBuildSteps<T> optionalBuildSteps) {
7173
this.resultType = optionalBuildSteps.resultType;
72-
if (optionalBuildSteps.mappingFunction == null) {
73-
this.mappingFunction = null;
74-
} else {
75-
this.mappingFunction = (BiFunction<TypeSystem, Record, T>) new AggregatingMappingFunction(
76-
optionalBuildSteps.mappingFunction);
77-
}
74+
this.mappingFunctionSupplier = optionalBuildSteps.mappingFunctionSupplier;
7875
this.queryFragmentsAndParameters = optionalBuildSteps.queryFragmentsAndParameters;
7976
}
8077

8178
public Class<T> getResultType() {
8279
return this.resultType;
8380
}
8481

85-
public Optional<BiFunction<TypeSystem, Record, T>> getOptionalMappingFunction() {
86-
return Optional.ofNullable(mappingFunction);
82+
@SuppressWarnings("unchecked")
83+
public synchronized Optional<BiFunction<TypeSystem, Record, T>> getOptionalMappingFunction() {
84+
Optional<BiFunction<TypeSystem, Record, T>> currentMappingFunction = Optional.ofNullable(
85+
this.mappingFunctionSupplier)
86+
.map(Supplier::get)
87+
.map(f -> (BiFunction<TypeSystem, Record, T>) new AggregatingMappingFunction(f));
88+
lastMappingFunction = currentMappingFunction;
89+
return currentMappingFunction;
8790
}
8891

89-
boolean resultsHaveBeenAggregated() {
90-
return this.mappingFunction != null && ((AggregatingMappingFunction) this.mappingFunction).hasAggregated();
92+
synchronized boolean resultsHaveBeenAggregated() {
93+
return lastMappingFunction
94+
.filter(AggregatingMappingFunction.class::isInstance)
95+
.map(AggregatingMappingFunction.class::cast)
96+
.map(AggregatingMappingFunction::hasAggregated)
97+
.orElse(false);
9198
}
9299

93100
public QueryFragmentsAndParameters getQueryFragmentsAndParameters() {
@@ -122,7 +129,7 @@ public static class OptionalBuildSteps<CT> {
122129

123130
final Class<CT> resultType;
124131
final QueryFragmentsAndParameters queryFragmentsAndParameters;
125-
@Nullable BiFunction<TypeSystem, MapAccessor, ?> mappingFunction;
132+
@Nullable Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunctionSupplier;
126133

127134
OptionalBuildSteps(Class<CT> resultType, QueryFragmentsAndParameters queryFragmentsAndParameters) {
128135
this.resultType = resultType;
@@ -140,9 +147,21 @@ public OptionalBuildSteps<CT> withParameters(Map<String, Object> newParameters)
140147
return this;
141148
}
142149

150+
/**
151+
* @param newMappingFunction A new mapping function
152+
* @return This builder
153+
* @deprecated since 6.1.7, please use {@link #usingMappingFunction(Supplier)}, otherwise your instance will be cached,
154+
* leading to potentially stale results
155+
*/
156+
@Deprecated
143157
public OptionalBuildSteps<CT> usingMappingFunction(
144158
@Nullable BiFunction<TypeSystem, MapAccessor, ?> newMappingFunction) {
145-
this.mappingFunction = newMappingFunction;
159+
this.mappingFunctionSupplier = newMappingFunction == null ? null : () -> newMappingFunction;
160+
return this;
161+
}
162+
163+
public OptionalBuildSteps<CT> usingMappingFunction(@Nullable Supplier<BiFunction<TypeSystem, MapAccessor, ?>> newMappingFunction) {
164+
this.mappingFunctionSupplier = newMappingFunction;
146165
return this;
147166
}
148167

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import java.util.concurrent.atomic.AtomicReference;
4545
import java.util.function.BiFunction;
4646
import java.util.function.Function;
47+
import java.util.function.Supplier;
4748
import java.util.stream.Collectors;
4849

4950
import org.apache.commons.logging.LogFactory;
@@ -611,7 +612,7 @@ private <T> Mono<ExecutableQuery<T>> createExecutableQuery(Class<T> domainType,
611612
private <T> Mono<ExecutableQuery<T>> createExecutableQuery(Class<T> domainType, @Nullable Class<?> resultType, @Nullable String cypherQuery,
612613
Map<String, Object> parameters) {
613614

614-
BiFunction<TypeSystem, MapAccessor, ?> mappingFunction = TemplateSupport
615+
Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunction = TemplateSupport
615616
.getAndDecorateMappingFunction(neo4jMappingContext, domainType, resultType);
616617
PreparedQuery<T> preparedQuery = PreparedQuery.queryFor(domainType).withCypherQuery(cypherQuery)
617618
.withParameters(parameters)

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

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.function.BiFunction;
2727
import java.util.function.Function;
2828
import java.util.function.Predicate;
29+
import java.util.function.Supplier;
2930
import java.util.stream.Collectors;
3031
import java.util.stream.StreamSupport;
3132

@@ -227,16 +228,18 @@ Statement toStatement() {
227228
* @param <T> The domain type
228229
* @return A mapping function
229230
*/
230-
static <T> BiFunction<TypeSystem, MapAccessor, ?> getAndDecorateMappingFunction(
231+
static <T> Supplier<BiFunction<TypeSystem, MapAccessor, ?>> getAndDecorateMappingFunction(
231232
Neo4jMappingContext mappingContext, Class<T> domainType, @Nullable Class<?> resultType) {
232233

233234
Assert.notNull(mappingContext.getPersistentEntity(domainType), "Cannot get or create persistent entity.");
234-
BiFunction<TypeSystem, MapAccessor, ?> mappingFunction = mappingContext
235-
.getRequiredMappingFunctionFor(domainType);
236-
if (resultType != null && domainType != resultType && !resultType.isInterface()) {
237-
mappingFunction = EntityInstanceWithSource.decorateMappingFunction(mappingFunction);
238-
}
239-
return mappingFunction;
235+
return () -> {
236+
BiFunction<TypeSystem, MapAccessor, ?> mappingFunction = mappingContext.getRequiredMappingFunctionFor(
237+
domainType);
238+
if (resultType != null && domainType != resultType && !resultType.isInterface()) {
239+
mappingFunction = EntityInstanceWithSource.decorateMappingFunction(mappingFunction);
240+
}
241+
return mappingFunction;
242+
};
240243
}
241244

242245
/**

‎src/main/java/org/springframework/data/neo4j/repository/query/AbstractNeo4jQuery.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ private Slice<?> createSlice(boolean incrementLimit, Neo4jParameterAccessor para
150150
protected abstract <T extends Object> PreparedQuery<T> prepareQuery(Class<T> returnedType,
151151
Map<PropertyPath, Boolean> includedProperties, Neo4jParameterAccessor parameterAccessor,
152152
@Nullable Neo4jQueryType queryType,
153-
@Nullable BiFunction<TypeSystem, MapAccessor, ?> mappingFunction,
153+
@Nullable Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunction,
154154
@Nullable UnaryOperator<Integer> limitModifier);
155155

156156
protected Optional<PreparedQuery<Long>> getCountQuery(Neo4jParameterAccessor parameterAccessor) {

‎src/main/java/org/springframework/data/neo4j/repository/query/AbstractReactiveNeo4jQuery.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import java.util.Map;
1919
import java.util.function.BiFunction;
20+
import java.util.function.Supplier;
2021

2122
import org.neo4j.driver.types.MapAccessor;
2223
import org.neo4j.driver.types.TypeSystem;
@@ -91,5 +92,5 @@ public final Object execute(Object[] parameters) {
9192

9293
protected abstract <T extends Object> PreparedQuery<T> prepareQuery(Class<T> returnedType,
9394
Map<PropertyPath, Boolean> includedProperties, Neo4jParameterAccessor parameterAccessor,
94-
@Nullable Neo4jQueryType queryType, @Nullable BiFunction<TypeSystem, MapAccessor, ?> mappingFunction);
95+
@Nullable Neo4jQueryType queryType, @Nullable Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunction);
9596
}

‎src/main/java/org/springframework/data/neo4j/repository/query/CypherdslBasedQuery.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.Map;
1919
import java.util.Optional;
2020
import java.util.function.BiFunction;
21+
import java.util.function.Supplier;
2122
import java.util.function.UnaryOperator;
2223

2324
import org.neo4j.cypherdsl.core.Statement;
@@ -57,7 +58,7 @@ private CypherdslBasedQuery(Neo4jOperations neo4jOperations,
5758
protected <T> PreparedQuery<T> prepareQuery(Class<T> returnedType,
5859
Map<PropertyPath, Boolean> includedProperties,
5960
Neo4jParameterAccessor parameterAccessor, Neo4jQueryType queryType,
60-
BiFunction<TypeSystem, MapAccessor, ?> mappingFunction,
61+
Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunction,
6162
UnaryOperator<Integer> limitModifier) {
6263

6364
Object[] parameters = parameterAccessor.getValues();

‎src/main/java/org/springframework/data/neo4j/repository/query/Neo4jQuerySupport.java

Lines changed: 21 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -102,24 +102,27 @@ static Class<?> getDomainType(QueryMethod queryMethod) {
102102
this.queryType = queryType;
103103
}
104104

105-
protected final BiFunction<TypeSystem, MapAccessor, ?> getMappingFunction(final ResultProcessor resultProcessor) {
106-
107-
final ReturnedType returnedTypeMetadata = resultProcessor.getReturnedType();
108-
final Class<?> returnedType = returnedTypeMetadata.getReturnedType();
109-
final Class<?> domainType = returnedTypeMetadata.getDomainType();
110-
111-
final BiFunction<TypeSystem, MapAccessor, ?> mappingFunction;
112-
113-
if (mappingContext.getConversionService().isSimpleType(returnedType)) {
114-
// Clients automatically selects a single value mapping function.
115-
// It will throw an error if the query contains more than one column.
116-
mappingFunction = null;
117-
} else if (returnedTypeMetadata.isProjecting()) {
118-
mappingFunction = EntityInstanceWithSource.decorateMappingFunction(this.mappingContext.getRequiredMappingFunctionFor(domainType));
119-
} else {
120-
mappingFunction = this.mappingContext.getRequiredMappingFunctionFor(domainType);
121-
}
122-
return mappingFunction;
105+
protected final Supplier<BiFunction<TypeSystem, MapAccessor, ?>> getMappingFunction(final ResultProcessor resultProcessor) {
106+
107+
return () -> {
108+
final ReturnedType returnedTypeMetadata = resultProcessor.getReturnedType();
109+
final Class<?> returnedType = returnedTypeMetadata.getReturnedType();
110+
final Class<?> domainType = returnedTypeMetadata.getDomainType();
111+
112+
final BiFunction<TypeSystem, MapAccessor, ?> mappingFunction;
113+
114+
if (mappingContext.getConversionService().isSimpleType(returnedType)) {
115+
// Clients automatically selects a single value mapping function.
116+
// It will throw an error if the query contains more than one column.
117+
mappingFunction = null;
118+
} else if (returnedTypeMetadata.isProjecting()) {
119+
mappingFunction = EntityInstanceWithSource.decorateMappingFunction(
120+
this.mappingContext.getRequiredMappingFunctionFor(domainType));
121+
} else {
122+
mappingFunction = this.mappingContext.getRequiredMappingFunctionFor(domainType);
123+
}
124+
return mappingFunction;
125+
};
123126
}
124127

125128
private static boolean hasValidReturnTypeForDelete(Neo4jQueryMethod queryMethod) {

‎src/main/java/org/springframework/data/neo4j/repository/query/PartTreeNeo4jQuery.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import java.util.Map;
1919
import java.util.Optional;
2020
import java.util.function.BiFunction;
21+
import java.util.function.Supplier;
2122
import java.util.function.UnaryOperator;
2223

2324
import org.neo4j.driver.types.MapAccessor;
@@ -62,7 +63,7 @@ private PartTreeNeo4jQuery(Neo4jOperations neo4jOperations, Neo4jMappingContext
6263
@Override
6364
protected <T extends Object> PreparedQuery<T> prepareQuery(Class<T> returnedType, Map<PropertyPath, Boolean> includedProperties,
6465
Neo4jParameterAccessor parameterAccessor, @Nullable Neo4jQueryType queryType,
65-
@Nullable BiFunction<TypeSystem, MapAccessor, ?> mappingFunction, UnaryOperator<Integer> limitModifier) {
66+
@Nullable Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunction, UnaryOperator<Integer> limitModifier) {
6667

6768
CypherQueryCreator queryCreator = new CypherQueryCreator(mappingContext, getDomainType(queryMethod),
6869
Optional.ofNullable(queryType).orElseGet(() -> Neo4jQueryType.fromPartTree(tree)), tree, parameterAccessor,

‎src/main/java/org/springframework/data/neo4j/repository/query/ReactiveCypherdslBasedQuery.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717

1818
import java.util.Map;
1919
import java.util.function.BiFunction;
20+
import java.util.function.Supplier;
2021

2122
import org.neo4j.cypherdsl.core.Statement;
2223
import org.neo4j.driver.types.MapAccessor;
@@ -52,7 +53,7 @@ private ReactiveCypherdslBasedQuery(ReactiveNeo4jOperations neo4jOperations,
5253
@Override
5354
protected <T> PreparedQuery<T> prepareQuery(Class<T> returnedType, Map<PropertyPath, Boolean> includedProperties,
5455
Neo4jParameterAccessor parameterAccessor, Neo4jQueryType queryType,
55-
BiFunction<TypeSystem, MapAccessor, ?> mappingFunction) {
56+
Supplier<BiFunction<TypeSystem, MapAccessor, ?>> mappingFunction) {
5657

5758
Object[] parameters = parameterAccessor.getValues();
5859

0 commit comments

Comments
 (0)