Skip to content

Commit 68bb2e3

Browse files
marcphilippmarcphilipp
authored
Close resource in inverse order of addition (#2387)
Resolves #2211.
1 parent 10aef49 commit 68bb2e3

File tree

9 files changed

+143
-44
lines changed

9 files changed

+143
-44
lines changed

‎documentation/src/docs/asciidoc/release-notes/release-notes-5.7.0.adoc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,8 @@ on GitHub.
3131

3232
==== Bug Fixes
3333

34-
* ❓
34+
* `CloseableResource` instances stored in `ExtensionContext.Store` are now closed in the
35+
reverse order they were added in. Previously, the order was undefined and unstable.
3536

3637
==== Deprecations and Breaking Changes
3738

‎documentation/src/docs/asciidoc/user-guide/extensions.adoc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -569,8 +569,8 @@ methods available for storing and retrieving values via the `{ExtensionContext_S
569569
.`ExtensionContext.Store.CloseableResource`
570570
NOTE: An extension context store is bound to its extension context lifecycle. When an
571571
extension context lifecycle ends it closes its associated store. All stored values
572-
that are instances of `CloseableResource` are notified by
573-
an invocation of their `close()` method.
572+
that are instances of `CloseableResource` are notified by an invocation of their `close()`
573+
method in the inverse order they were added in.
574574

575575
[[extensions-supported-utilities]]
576576
=== Supported Utilities in Extensions

‎junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ExtensionContext.java

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -396,6 +396,9 @@ interface Store {
396396
* <p>Note that the {@code CloseableResource} API is only honored for
397397
* objects stored within an extension context {@link Store Store}.
398398
*
399+
* <p>The resources stored in a {@link Store Store} are closed in the
400+
* inverse order they were added in.
401+
*
399402
* @since 5.1
400403
*/
401404
@API(status = STABLE, since = "5.1")

‎junit-jupiter-api/src/testFixtures/java/org/junit/jupiter/api/extension/ExtensionContextParameterResolver.java

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/*
2+
* Copyright 2015-2020 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.api.extension;
12+
13+
public class ExtensionContextParameterResolver implements ParameterResolver {
14+
15+
@Override
16+
public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext)
17+
throws ParameterResolutionException {
18+
return ExtensionContext.class.equals(parameterContext.getParameter().getType());
19+
}
20+
21+
@Override
22+
public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext)
23+
throws ParameterResolutionException {
24+
return extensionContext;
25+
}
26+
}

‎junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/ExtensionValuesStore.java

Lines changed: 46 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,12 @@
1515
import static org.junit.platform.commons.util.ReflectionUtils.getWrapperType;
1616
import static org.junit.platform.commons.util.ReflectionUtils.isAssignableTo;
1717

18+
import java.util.Comparator;
1819
import java.util.Objects;
1920
import java.util.Optional;
2021
import java.util.concurrent.ConcurrentHashMap;
2122
import java.util.concurrent.ConcurrentMap;
23+
import java.util.concurrent.atomic.AtomicInteger;
2224
import java.util.concurrent.locks.Lock;
2325
import java.util.concurrent.locks.ReentrantLock;
2426
import java.util.function.Function;
@@ -27,6 +29,7 @@
2729
import org.apiguardian.api.API;
2830
import org.junit.jupiter.api.extension.ExtensionContext;
2931
import org.junit.jupiter.api.extension.ExtensionContext.Namespace;
32+
import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource;
3033
import org.junit.jupiter.api.extension.ExtensionContextException;
3134
import org.junit.platform.engine.support.hierarchical.ThrowableCollector;
3235

@@ -39,34 +42,36 @@
3942
@API(status = INTERNAL, since = "5.0")
4043
public class ExtensionValuesStore {
4144

45+
private static final Comparator<StoredValue> REVERSE_INSERT_ORDER = Comparator.<StoredValue, Integer> comparing(
46+
it -> it.order).reversed();
47+
48+
private final AtomicInteger insertOrderSequence = new AtomicInteger();
49+
private final ConcurrentMap<CompositeKey, StoredValue> storedValues = new ConcurrentHashMap<>(4);
4250
private final ExtensionValuesStore parentStore;
43-
private final ConcurrentMap<CompositeKey, Supplier<Object>> storedValues = new ConcurrentHashMap<>(4);
4451

4552
public ExtensionValuesStore(ExtensionValuesStore parentStore) {
4653
this.parentStore = parentStore;
4754
}
4855

4956
/**
50-
* Close all values that implement {@link ExtensionContext.Store.CloseableResource}.
57+
* Close all values that implement {@link CloseableResource}.
5158
*
5259
* @implNote Only close values stored in this instance. This implementation
5360
* does not close values in parent stores.
5461
*/
5562
public void closeAllStoredCloseableValues() {
5663
ThrowableCollector throwableCollector = createThrowableCollector();
57-
for (Supplier<Object> supplier : storedValues.values()) {
58-
Object value = supplier.get();
59-
if (value instanceof ExtensionContext.Store.CloseableResource) {
60-
ExtensionContext.Store.CloseableResource resource = (ExtensionContext.Store.CloseableResource) value;
61-
throwableCollector.execute(resource::close);
62-
}
63-
}
64+
storedValues.values().stream() //
65+
.filter(storedValue -> storedValue.evaluate() instanceof CloseableResource) //
66+
.sorted(REVERSE_INSERT_ORDER) //
67+
.map(storedValue -> (CloseableResource) storedValue.evaluate()) //
68+
.forEach(resource -> throwableCollector.execute(resource::close));
6469
throwableCollector.assertEmpty();
6570
}
6671

6772
Object get(Namespace namespace, Object key) {
68-
Supplier<Object> storedValue = getStoredValue(new CompositeKey(namespace, key));
69-
return (storedValue != null ? storedValue.get() : null);
73+
StoredValue storedValue = getStoredValue(new CompositeKey(namespace, key));
74+
return (storedValue != null ? storedValue.evaluate() : null);
7075
}
7176

7277
<T> T get(Namespace namespace, Object key, Class<T> requiredType) {
@@ -76,12 +81,12 @@ <T> T get(Namespace namespace, Object key, Class<T> requiredType) {
7681

7782
<K, V> Object getOrComputeIfAbsent(Namespace namespace, K key, Function<K, V> defaultCreator) {
7883
CompositeKey compositeKey = new CompositeKey(namespace, key);
79-
Supplier<Object> storedValue = getStoredValue(compositeKey);
84+
StoredValue storedValue = getStoredValue(compositeKey);
8085
if (storedValue == null) {
81-
Supplier<Object> newValue = new MemoizingSupplier(() -> defaultCreator.apply(key));
86+
StoredValue newValue = storedValue(new MemoizingSupplier(() -> defaultCreator.apply(key)));
8287
storedValue = Optional.ofNullable(storedValues.putIfAbsent(compositeKey, newValue)).orElse(newValue);
8388
}
84-
return storedValue.get();
89+
return storedValue.evaluate();
8590
}
8691

8792
<K, V> V getOrComputeIfAbsent(Namespace namespace, K key, Function<K, V> defaultCreator, Class<V> requiredType) {
@@ -90,30 +95,32 @@ <K, V> V getOrComputeIfAbsent(Namespace namespace, K key, Function<K, V> default
9095
}
9196

9297
void put(Namespace namespace, Object key, Object value) {
93-
storedValues.put(new CompositeKey(namespace, key), () -> value);
98+
storedValues.put(new CompositeKey(namespace, key), storedValue(() -> value));
99+
}
100+
101+
private StoredValue storedValue(Supplier<Object> value) {
102+
return new StoredValue(insertOrderSequence.getAndIncrement(), value);
94103
}
95104

96105
Object remove(Namespace namespace, Object key) {
97-
Supplier<Object> previous = storedValues.remove(new CompositeKey(namespace, key));
98-
return (previous != null ? previous.get() : null);
106+
StoredValue previous = storedValues.remove(new CompositeKey(namespace, key));
107+
return (previous != null ? previous.evaluate() : null);
99108
}
100109

101110
<T> T remove(Namespace namespace, Object key, Class<T> requiredType) {
102111
Object value = remove(namespace, key);
103112
return castToRequiredType(key, value, requiredType);
104113
}
105114

106-
private Supplier<Object> getStoredValue(CompositeKey compositeKey) {
107-
Supplier<Object> storedValue = storedValues.get(compositeKey);
115+
private StoredValue getStoredValue(CompositeKey compositeKey) {
116+
StoredValue storedValue = storedValues.get(compositeKey);
108117
if (storedValue != null) {
109118
return storedValue;
110119
}
111-
else if (parentStore != null) {
120+
if (parentStore != null) {
112121
return parentStore.getStoredValue(compositeKey);
113122
}
114-
else {
115-
return null;
116-
}
123+
return null;
117124
}
118125

119126
@SuppressWarnings("unchecked")
@@ -161,6 +168,22 @@ public int hashCode() {
161168

162169
}
163170

171+
private static class StoredValue {
172+
173+
private final int order;
174+
private final Supplier<Object> supplier;
175+
176+
public StoredValue(int order, Supplier<Object> supplier) {
177+
this.order = order;
178+
this.supplier = supplier;
179+
}
180+
181+
private Object evaluate() {
182+
return supplier.get();
183+
}
184+
185+
}
186+
164187
private static class MemoizingSupplier implements Supplier<Object> {
165188

166189
private static final Object NO_VALUE_SET = new Object();

‎junit-jupiter-engine/src/test/java/org/junit/jupiter/api/extension/CloseableResourceIntegrationTests.java

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
/*
2+
* Copyright 2015-2020 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.api.extension;
12+
13+
import static org.junit.platform.testkit.engine.EventConditions.reportEntry;
14+
15+
import java.util.Map;
16+
17+
import org.jetbrains.annotations.NotNull;
18+
import org.junit.jupiter.api.Test;
19+
import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
20+
21+
public class CloseableResourceIntegrationTests extends AbstractJupiterTestEngineTests {
22+
23+
@Test
24+
void closesCloseableResourcesInReverseInsertOrder() {
25+
executeTestsForClass(TestCase.class).allEvents().reportingEntryPublished() //
26+
.assertEventsMatchExactly( //
27+
reportEntry(Map.of("3", "closed")), //
28+
reportEntry(Map.of("2", "closed")), //
29+
reportEntry(Map.of("1", "closed")));
30+
}
31+
32+
@ExtendWith(ExtensionContextParameterResolver.class)
33+
static class TestCase {
34+
@Test
35+
void closesCloseableResourcesInExtensionContext(ExtensionContext extensionContext) {
36+
ExtensionContext.Store store = extensionContext.getStore(ExtensionContext.Namespace.GLOBAL);
37+
store.put("foo", reportEntryOnClose(extensionContext, "1"));
38+
store.put("bar", reportEntryOnClose(extensionContext, "2"));
39+
store.put("baz", reportEntryOnClose(extensionContext, "3"));
40+
}
41+
42+
@NotNull
43+
private ExtensionContext.Store.CloseableResource reportEntryOnClose(ExtensionContext extensionContext,
44+
String key) {
45+
return () -> extensionContext.publishReportEntry(Map.of(key, "closed"));
46+
}
47+
}
48+
}

‎junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/execution/ExtensionValuesStoreTests.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -378,7 +378,7 @@ void orderOfNamespacePartsDoesMatter() {
378378
}
379379

380380
@Test
381-
void additionNamespacePartMakesADifferenc() {
381+
void additionNamespacePartMakesADifference() {
382382

383383
Namespace ns1 = Namespace.create("part1", "part2");
384384
Namespace ns2 = Namespace.create("part1");

‎junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/ExtensionContextExecutionTests.java

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,7 @@
2121
import org.junit.jupiter.api.extension.BeforeAllCallback;
2222
import org.junit.jupiter.api.extension.ExtendWith;
2323
import org.junit.jupiter.api.extension.ExtensionContext;
24-
import org.junit.jupiter.api.extension.ParameterContext;
25-
import org.junit.jupiter.api.extension.ParameterResolutionException;
26-
import org.junit.jupiter.api.extension.ParameterResolver;
24+
import org.junit.jupiter.api.extension.ExtensionContextParameterResolver;
2725
import org.junit.jupiter.engine.AbstractJupiterTestEngineTests;
2826

2927
class ExtensionContextExecutionTests extends AbstractJupiterTestEngineTests {
@@ -45,20 +43,6 @@ void extensionContextHierarchy(ExtensionContext methodExtensionContext) {
4543
assertThat(engineExtensionContext.orElse(null).getParent()).isEmpty();
4644
}
4745

48-
static class ExtensionContextParameterResolver implements ParameterResolver {
49-
@Override
50-
public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext)
51-
throws ParameterResolutionException {
52-
return ExtensionContext.class.equals(parameterContext.getParameter().getType());
53-
}
54-
55-
@Override
56-
public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext)
57-
throws ParameterResolutionException {
58-
return extensionContext;
59-
}
60-
}
61-
6246
@Test
6347
void twoTestClassesCanShareStateViaEngineExtensionContext() {
6448
Parent.counter.set(0);

‎junit-platform-testkit/src/main/java/org/junit/platform/testkit/engine/EventConditions.java

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212

1313
import static java.util.function.Predicate.isEqual;
1414
import static java.util.stream.Collectors.toList;
15+
import static org.apiguardian.api.API.Status.EXPERIMENTAL;
1516
import static org.apiguardian.api.API.Status.MAINTAINED;
1617
import static org.assertj.core.api.Assertions.allOf;
1718
import static org.junit.platform.commons.util.FunctionUtils.where;
@@ -29,6 +30,7 @@
2930
import java.util.ArrayList;
3031
import java.util.Arrays;
3132
import java.util.List;
33+
import java.util.Map;
3234
import java.util.function.Predicate;
3335

3436
import org.apiguardian.api.API;
@@ -40,6 +42,7 @@
4042
import org.junit.platform.engine.TestExecutionResult;
4143
import org.junit.platform.engine.TestExecutionResult.Status;
4244
import org.junit.platform.engine.UniqueId;
45+
import org.junit.platform.engine.reporting.ReportEntry;
4346
import org.junit.platform.engine.support.descriptor.EngineDescriptor;
4447

4548
/**
@@ -411,4 +414,15 @@ public static Condition<Event> reason(Predicate<String> predicate) {
411414
return new Condition<>(byPayload(String.class, predicate), "event with custom reason predicate");
412415
}
413416

417+
/**
418+
* Create a new {@link Condition} that matches if and only if an
419+
* {@link Event}'s {@linkplain Event#getPayload() payload} is an instance of
420+
* {@link ReportEntry} that contains the supplied key-value pairs.
421+
*/
422+
@API(status = EXPERIMENTAL, since = "1.7")
423+
public static Condition<Event> reportEntry(Map<String, String> keyValuePairs) {
424+
return new Condition<>(byPayload(ReportEntry.class, it -> it.getKeyValuePairs().equals(keyValuePairs)),
425+
"event for report entry with key-value pairs %s", keyValuePairs);
426+
}
427+
414428
}

0 commit comments

Comments
 (0)