Skip to content

Commit 6de9f4e

Browse files
marcphilippmarcphilipp
committed
Ensure conversion/aggregation is only performed once per invocation
1 parent d02b53b commit 6de9f4e

9 files changed

+181
-34
lines changed

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ContainerTemplateConstructorParameterResolver.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,8 @@ class ContainerTemplateConstructorParameterResolver extends ParameterizedInvocat
2323
private final Class<?> containerTemplateClass;
2424

2525
ContainerTemplateConstructorParameterResolver(ParameterizedContainerClassContext classContext,
26-
EvaluatedArgumentSet arguments, int invocationIndex) {
27-
super(classContext.getResolverFacade(), arguments, invocationIndex);
26+
EvaluatedArgumentSet arguments, int invocationIndex, ResolutionCache resolutionCache) {
27+
super(classContext.getResolverFacade(), arguments, invocationIndex, resolutionCache);
2828
this.containerTemplateClass = classContext.getAnnotatedElement();
2929
}
3030

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ContainerTemplateInstanceFieldInjectingBeforeEachCallback.java‎

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,22 +15,25 @@
1515

1616
class ContainerTemplateInstanceFieldInjectingBeforeEachCallback implements BeforeEachCallback {
1717

18-
private final ParameterizedContainerClassContext classContext;
18+
private final ResolverFacade resolverFacade;
1919
private final EvaluatedArgumentSet arguments;
2020
private final int invocationIndex;
21+
private final ResolutionCache resolutionCache;
2122

22-
ContainerTemplateInstanceFieldInjectingBeforeEachCallback(ParameterizedContainerClassContext classContext,
23-
EvaluatedArgumentSet arguments, int invocationIndex) {
24-
this.classContext = classContext;
23+
ContainerTemplateInstanceFieldInjectingBeforeEachCallback(ResolverFacade resolverFacade,
24+
EvaluatedArgumentSet arguments, int invocationIndex, ResolutionCache resolutionCache) {
25+
this.resolverFacade = resolverFacade;
2526
this.arguments = arguments;
2627
this.invocationIndex = invocationIndex;
28+
this.resolutionCache = resolutionCache;
2729
}
2830

2931
@Override
3032
public void beforeEach(ExtensionContext extensionContext) throws Exception {
3133
extensionContext.getTestInstance() //
32-
.ifPresent(testInstance -> this.classContext.getResolverFacade() //
33-
.resolveAndInjectFields(testInstance, extensionContext, this.arguments, this.invocationIndex));
34+
.ifPresent(testInstance -> this.resolverFacade //
35+
.resolveAndInjectFields(testInstance, extensionContext, this.arguments, this.invocationIndex,
36+
this.resolutionCache));
3437
}
3538

3639
}

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ContainerTemplateInstanceFieldInjectingPostProcessor.java‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,15 +15,17 @@
1515

1616
class ContainerTemplateInstanceFieldInjectingPostProcessor implements TestInstancePostProcessor {
1717

18-
private final ParameterizedContainerClassContext classContext;
18+
private final ResolverFacade resolverFacade;
1919
private final EvaluatedArgumentSet arguments;
2020
private final int invocationIndex;
21+
private final ResolutionCache resolutionCache;
2122

22-
ContainerTemplateInstanceFieldInjectingPostProcessor(ParameterizedContainerClassContext classContext,
23-
EvaluatedArgumentSet arguments, int invocationIndex) {
24-
this.classContext = classContext;
23+
ContainerTemplateInstanceFieldInjectingPostProcessor(ResolverFacade resolverFacade, EvaluatedArgumentSet arguments,
24+
int invocationIndex, ResolutionCache resolutionCache) {
25+
this.resolverFacade = resolverFacade;
2526
this.arguments = arguments;
2627
this.invocationIndex = invocationIndex;
28+
this.resolutionCache = resolutionCache;
2729
}
2830

2931
@Override
@@ -33,8 +35,7 @@ public ExtensionContextScope getTestInstantiationExtensionContextScope(Extension
3335

3436
@Override
3537
public void postProcessTestInstance(Object testInstance, ExtensionContext extensionContext) {
36-
this.classContext.getResolverFacade() //
37-
.resolveAndInjectFields(testInstance, extensionContext, this.arguments, this.invocationIndex);
38+
this.resolverFacade.resolveAndInjectFields(testInstance, extensionContext, this.arguments, this.invocationIndex,
39+
this.resolutionCache);
3840
}
39-
4041
}

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedContainerInvocationContext.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,8 @@
2727
class ParameterizedContainerInvocationContext extends ParameterizedInvocationContext<ParameterizedContainerClassContext>
2828
implements ContainerTemplateInvocationContext {
2929

30+
private final ResolutionCache resolutionCache = ResolutionCache.enabled();
31+
3032
ParameterizedContainerInvocationContext(ParameterizedContainerClassContext classContext,
3133
ParameterizedInvocationNameFormatter formatter, Arguments arguments, int invocationIndex) {
3234
super(classContext, formatter, arguments, invocationIndex);
@@ -53,18 +55,19 @@ private ContainerTemplateConstructorParameterResolver createExtensionForConstruc
5355
Preconditions.condition(this.declarationContext.getTestInstanceLifecycle() == PER_METHOD,
5456
"Constructor injection is only supported for lifecycle PER_METHOD");
5557
return new ContainerTemplateConstructorParameterResolver(this.declarationContext, this.arguments,
56-
this.invocationIndex);
58+
this.invocationIndex, this.resolutionCache);
5759
}
5860

5961
private Extension createExtensionForFieldInjection() {
62+
ResolverFacade resolverFacade = this.declarationContext.getResolverFacade();
6063
TestInstance.Lifecycle lifecycle = this.declarationContext.getTestInstanceLifecycle();
6164
switch (lifecycle) {
6265
case PER_CLASS:
63-
return new ContainerTemplateInstanceFieldInjectingBeforeEachCallback(this.declarationContext,
64-
this.arguments, this.invocationIndex);
66+
return new ContainerTemplateInstanceFieldInjectingBeforeEachCallback(resolverFacade, this.arguments,
67+
this.invocationIndex, this.resolutionCache);
6568
case PER_METHOD:
66-
return new ContainerTemplateInstanceFieldInjectingPostProcessor(this.declarationContext, this.arguments,
67-
this.invocationIndex);
69+
return new ContainerTemplateInstanceFieldInjectingPostProcessor(resolverFacade, this.arguments,
70+
this.invocationIndex, this.resolutionCache);
6871
}
6972
throw new JUnitException("Unsupported lifecycle: " + lifecycle);
7073
}

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ParameterizedInvocationParameterResolver.java‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,13 +25,15 @@ abstract class ParameterizedInvocationParameterResolver implements ParameterReso
2525
private final ResolverFacade resolverFacade;
2626
private final EvaluatedArgumentSet arguments;
2727
private final int invocationIndex;
28+
private final ResolutionCache resolutionCache;
2829

2930
ParameterizedInvocationParameterResolver(ResolverFacade resolverFacade, EvaluatedArgumentSet arguments,
30-
int invocationIndex) {
31+
int invocationIndex, ResolutionCache resolutionCache) {
3132

3233
this.resolverFacade = resolverFacade;
3334
this.arguments = arguments;
3435
this.invocationIndex = invocationIndex;
36+
this.resolutionCache = resolutionCache;
3537
}
3638

3739
@Override
@@ -51,7 +53,8 @@ public final boolean supportsParameter(ParameterContext parameterContext, Extens
5153
public final Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext)
5254
throws ParameterResolutionException {
5355

54-
return this.resolverFacade.resolve(parameterContext, extensionContext, this.arguments, this.invocationIndex);
56+
return this.resolverFacade.resolve(parameterContext, extensionContext, this.arguments, this.invocationIndex,
57+
this.resolutionCache);
5558
}
5659

5760
protected abstract boolean isSupportedOnConstructorOrMethod(Executable declaringExecutable,

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ResolutionCache.java‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,41 @@
1+
/*
2+
* Copyright 2015-2025 the original author or authors.
3+
*
4+
* All rights reserved. This program and the accompanying materials are
5+
* made available under the terms of the Eclipse Public License v2.0 which
6+
* accompanies this distribution and is available at
7+
*
8+
* https://www.eclipse.org/legal/epl-v20.html
9+
*/
10+
11+
package org.junit.jupiter.params;
12+
13+
import java.util.Map;
14+
import java.util.concurrent.ConcurrentHashMap;
15+
import java.util.function.Supplier;
16+
17+
import org.junit.jupiter.params.support.ParameterDeclaration;
18+
19+
/**
20+
* @since 5.13
21+
*/
22+
interface ResolutionCache {
23+
24+
static ResolutionCache enabled() {
25+
return new Concurrent();
26+
}
27+
28+
ResolutionCache DISABLED = (__, resolver) -> resolver.get();
29+
30+
Object resolve(ParameterDeclaration declaration, Supplier<Object> resolver);
31+
32+
class Concurrent implements ResolutionCache {
33+
34+
private final Map<ParameterDeclaration, Object> cache = new ConcurrentHashMap<>();
35+
36+
@Override
37+
public Object resolve(ParameterDeclaration declaration, Supplier<Object> resolver) {
38+
return cache.computeIfAbsent(declaration, __ -> resolver.get());
39+
}
40+
}
41+
}

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/ResolverFacade.java‎

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -220,25 +220,28 @@ int determineConsumedArgumentCount(EvaluatedArgumentSet arguments) {
220220
* arguments.
221221
*/
222222
Object resolve(ParameterContext parameterContext, ExtensionContext extensionContext, EvaluatedArgumentSet arguments,
223-
int invocationIndex) {
223+
int invocationIndex, ResolutionCache resolutionCache) {
224+
224225
int parameterIndex = toLogicalIndex(parameterContext);
225226
ParameterDeclaration declaration = this.indexedParameterDeclarations.get(parameterIndex) //
226227
.orElseGet(() -> this.aggregatorParameters.stream().filter(
227228
it -> it.getParameterIndex() == parameterIndex).findFirst() //
228229
.orElseThrow(() -> new ParameterResolutionException(
229230
"Parameter index out of bounds: " + parameterIndex)));
230-
return getResolver(extensionContext, declaration, parameterContext.getParameter()) //
231-
.resolve(parameterContext, parameterIndex, arguments, invocationIndex);
231+
return resolutionCache.resolve(declaration,
232+
() -> getResolver(extensionContext, declaration, parameterContext.getParameter()) //
233+
.resolve(parameterContext, parameterIndex, arguments, invocationIndex));
232234
}
233235

234236
void resolveAndInjectFields(Object testInstance, ExtensionContext extensionContext, EvaluatedArgumentSet arguments,
235-
int invocationIndex) {
237+
int invocationIndex, ResolutionCache resolutionCache) {
238+
236239
if (this.indexedParameterDeclarations.sourceElement.equals(testInstance.getClass())) {
237240
getAllParameterDeclarations() //
238241
.filter(FieldParameterDeclaration.class::isInstance) //
239242
.map(FieldParameterDeclaration.class::cast) //
240243
.forEach(declaration -> setField(testInstance, declaration, extensionContext, arguments,
241-
invocationIndex));
244+
invocationIndex, resolutionCache));
242245
}
243246
}
244247

@@ -247,15 +250,16 @@ private Stream<ParameterDeclaration> getAllParameterDeclarations() {
247250
aggregatorParameters.stream());
248251
}
249252

250-
private void setField(Object testInstance, FieldParameterDeclaration parameterDeclaration,
251-
ExtensionContext extensionContext, EvaluatedArgumentSet arguments, int invocationIndex) {
252-
Object argument = resolve(parameterDeclaration, extensionContext, arguments, invocationIndex);
253+
private void setField(Object testInstance, FieldParameterDeclaration declaration, ExtensionContext extensionContext,
254+
EvaluatedArgumentSet arguments, int invocationIndex, ResolutionCache resolutionCache) {
255+
256+
Object argument = resolutionCache.resolve(declaration,
257+
() -> resolve(declaration, extensionContext, arguments, invocationIndex));
253258
try {
254-
parameterDeclaration.getField().set(testInstance, argument);
259+
declaration.getField().set(testInstance, argument);
255260
}
256261
catch (Exception e) {
257-
throw new JUnitException("Failed to inject parameter value into field: " + parameterDeclaration.getField(),
258-
e);
262+
throw new JUnitException("Failed to inject parameter value into field: " + declaration.getField(), e);
259263
}
260264
}
261265

‎junit-jupiter-params/src/main/java/org/junit/jupiter/params/TestTemplateMethodParameterResolver.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ class TestTemplateMethodParameterResolver extends ParameterizedInvocationParamet
2424

2525
TestTemplateMethodParameterResolver(ParameterizedTestMethodContext methodContext, EvaluatedArgumentSet arguments,
2626
int invocationIndex) {
27-
super(methodContext.getResolverFacade(), arguments, invocationIndex);
27+
super(methodContext.getResolverFacade(), arguments, invocationIndex, ResolutionCache.DISABLED);
2828
this.testTemplateMethod = methodContext.getAnnotatedElement();
2929
}
3030

‎jupiter-tests/src/test/java/org/junit/jupiter/params/ParameterizedContainerIntegrationTests.java‎

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import static org.junit.jupiter.api.Assertions.assertEquals;
1616
import static org.junit.jupiter.api.Assertions.assertFalse;
1717
import static org.junit.jupiter.api.Assertions.assertNotNull;
18+
import static org.junit.jupiter.api.Assertions.assertSame;
1819
import static org.junit.jupiter.api.Assertions.assertTrue;
1920
import static org.junit.jupiter.api.Assertions.fail;
2021
import static org.junit.jupiter.api.TestInstance.Lifecycle.PER_CLASS;
@@ -48,6 +49,8 @@
4849
import java.util.stream.Stream;
4950

5051
import org.assertj.core.api.Condition;
52+
import org.junit.jupiter.api.AfterAll;
53+
import org.junit.jupiter.api.BeforeAll;
5154
import org.junit.jupiter.api.DisplayName;
5255
import org.junit.jupiter.api.Named;
5356
import org.junit.jupiter.api.Nested;
@@ -66,6 +69,7 @@
6669
import org.junit.jupiter.params.aggregator.SimpleArgumentsAggregator;
6770
import org.junit.jupiter.params.converter.ArgumentConversionException;
6871
import org.junit.jupiter.params.converter.ConvertWith;
72+
import org.junit.jupiter.params.converter.SimpleArgumentConverter;
6973
import org.junit.jupiter.params.converter.TypedArgumentConverter;
7074
import org.junit.jupiter.params.provider.Arguments;
7175
import org.junit.jupiter.params.provider.ArgumentsProvider;
@@ -449,6 +453,16 @@ void declaredIndexMustBeUnique() {
449453
containerTemplateClass.getName(), containerTemplateClass.getName()))));
450454
}
451455

456+
@ParameterizedTest
457+
@ValueSource(classes = { ArgumentConversionPerInvocationConstructorInjectionTestCase.class,
458+
ArgumentConversionPerInvocationFieldInjectionTestCase.class })
459+
void argumentConverterIsOnlyCalledOncePerInvocation(Class<?> containerTemplateClass) {
460+
461+
var results = executeTestsForClass(containerTemplateClass);
462+
463+
results.allEvents().assertStatistics(stats -> stats.started(5).succeeded(5));
464+
}
465+
452466
// -------------------------------------------------------------------
453467

454468
private static Stream<String> invocationDisplayNames(EngineExecutionResults results) {
@@ -1289,4 +1303,82 @@ void test(TestReporter reporter) {
12891303
));
12901304
}
12911305
}
1306+
1307+
@ParameterizedContainer
1308+
@ValueSource(ints = 1)
1309+
record ArgumentConversionPerInvocationConstructorInjectionTestCase(
1310+
@ConvertWith(Wrapper.Converter.class) Wrapper wrapper) {
1311+
1312+
static Wrapper instance;
1313+
1314+
@BeforeAll
1315+
@AfterAll
1316+
static void clearWrapper() {
1317+
instance = null;
1318+
}
1319+
1320+
@Test
1321+
void test1() {
1322+
setOrCheckWrapper();
1323+
}
1324+
1325+
@Test
1326+
void test2() {
1327+
setOrCheckWrapper();
1328+
}
1329+
1330+
private void setOrCheckWrapper() {
1331+
if (instance == null) {
1332+
instance = wrapper;
1333+
}
1334+
else {
1335+
assertSame(instance, wrapper);
1336+
}
1337+
}
1338+
}
1339+
1340+
@ParameterizedContainer
1341+
@ValueSource(ints = 1)
1342+
static class ArgumentConversionPerInvocationFieldInjectionTestCase {
1343+
1344+
static Wrapper instance;
1345+
1346+
@BeforeAll
1347+
@AfterAll
1348+
static void clearWrapper() {
1349+
instance = null;
1350+
}
1351+
1352+
@Parameter
1353+
@ConvertWith(Wrapper.Converter.class)
1354+
Wrapper wrapper;
1355+
1356+
@Test
1357+
void test1() {
1358+
setOrCheckWrapper();
1359+
}
1360+
1361+
@Test
1362+
void test2() {
1363+
setOrCheckWrapper();
1364+
}
1365+
1366+
private void setOrCheckWrapper() {
1367+
if (instance == null) {
1368+
instance = wrapper;
1369+
}
1370+
else {
1371+
assertSame(instance, wrapper);
1372+
}
1373+
}
1374+
}
1375+
1376+
record Wrapper(int value) {
1377+
static class Converter extends SimpleArgumentConverter {
1378+
@Override
1379+
protected Object convert(Object source, Class<?> targetType) {
1380+
return new Wrapper((Integer) source);
1381+
}
1382+
}
1383+
}
12921384
}

0 commit comments

Comments
 (0)