diff --git a/documentation/src/docs/asciidoc/release-notes/release-notes-5.7.0.adoc b/documentation/src/docs/asciidoc/release-notes/release-notes-5.7.0.adoc index c56d47d3b0cf..845904fa6713 100644 --- a/documentation/src/docs/asciidoc/release-notes/release-notes-5.7.0.adoc +++ b/documentation/src/docs/asciidoc/release-notes/release-notes-5.7.0.adoc @@ -31,7 +31,8 @@ on GitHub. ==== Bug Fixes -* ❓ +* `CloseableResource` instances stored in `ExtensionContext.Store` are now closed in the + reverse order they were added in. Previously, the order was undefined and unstable. ==== Deprecations and Breaking Changes diff --git a/documentation/src/docs/asciidoc/user-guide/extensions.adoc b/documentation/src/docs/asciidoc/user-guide/extensions.adoc index c02b91281daa..3ad1310f8a26 100644 --- a/documentation/src/docs/asciidoc/user-guide/extensions.adoc +++ b/documentation/src/docs/asciidoc/user-guide/extensions.adoc @@ -569,8 +569,8 @@ methods available for storing and retrieving values via the `{ExtensionContext_S .`ExtensionContext.Store.CloseableResource` NOTE: An extension context store is bound to its extension context lifecycle. When an extension context lifecycle ends it closes its associated store. All stored values -that are instances of `CloseableResource` are notified by -an invocation of their `close()` method. +that are instances of `CloseableResource` are notified by an invocation of their `close()` +method in the inverse order they were added in. [[extensions-supported-utilities]] === Supported Utilities in Extensions diff --git a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ExtensionContext.java b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ExtensionContext.java index dc8295f7ffb1..6a244f950e54 100644 --- a/junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ExtensionContext.java +++ b/junit-jupiter-api/src/main/java/org/junit/jupiter/api/extension/ExtensionContext.java @@ -396,6 +396,9 @@ interface Store { *

Note that the {@code CloseableResource} API is only honored for * objects stored within an extension context {@link Store Store}. * + *

The resources stored in a {@link Store Store} are closed in the + * inverse order they were added in. + * * @since 5.1 */ @API(status = STABLE, since = "5.1") diff --git a/junit-jupiter-api/src/testFixtures/java/org/junit/jupiter/api/extension/ExtensionContextParameterResolver.java b/junit-jupiter-api/src/testFixtures/java/org/junit/jupiter/api/extension/ExtensionContextParameterResolver.java new file mode 100644 index 000000000000..4c6df517335c --- /dev/null +++ b/junit-jupiter-api/src/testFixtures/java/org/junit/jupiter/api/extension/ExtensionContextParameterResolver.java @@ -0,0 +1,26 @@ +/* + * Copyright 2015-2020 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.jupiter.api.extension; + +public class ExtensionContextParameterResolver implements ParameterResolver { + + @Override + public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) + throws ParameterResolutionException { + return ExtensionContext.class.equals(parameterContext.getParameter().getType()); + } + + @Override + public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) + throws ParameterResolutionException { + return extensionContext; + } +} diff --git a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/ExtensionValuesStore.java b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/ExtensionValuesStore.java index 66a12d5e4f4e..b7df116a62f4 100644 --- a/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/ExtensionValuesStore.java +++ b/junit-jupiter-engine/src/main/java/org/junit/jupiter/engine/execution/ExtensionValuesStore.java @@ -15,10 +15,12 @@ import static org.junit.platform.commons.util.ReflectionUtils.getWrapperType; import static org.junit.platform.commons.util.ReflectionUtils.isAssignableTo; +import java.util.Comparator; import java.util.Objects; import java.util.Optional; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; +import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReentrantLock; import java.util.function.Function; @@ -27,6 +29,7 @@ import org.apiguardian.api.API; import org.junit.jupiter.api.extension.ExtensionContext; import org.junit.jupiter.api.extension.ExtensionContext.Namespace; +import org.junit.jupiter.api.extension.ExtensionContext.Store.CloseableResource; import org.junit.jupiter.api.extension.ExtensionContextException; import org.junit.platform.engine.support.hierarchical.ThrowableCollector; @@ -39,34 +42,36 @@ @API(status = INTERNAL, since = "5.0") public class ExtensionValuesStore { + private static final Comparator REVERSE_INSERT_ORDER = Comparator. comparing( + it -> it.order).reversed(); + + private final AtomicInteger insertOrderSequence = new AtomicInteger(); + private final ConcurrentMap storedValues = new ConcurrentHashMap<>(4); private final ExtensionValuesStore parentStore; - private final ConcurrentMap> storedValues = new ConcurrentHashMap<>(4); public ExtensionValuesStore(ExtensionValuesStore parentStore) { this.parentStore = parentStore; } /** - * Close all values that implement {@link ExtensionContext.Store.CloseableResource}. + * Close all values that implement {@link CloseableResource}. * * @implNote Only close values stored in this instance. This implementation * does not close values in parent stores. */ public void closeAllStoredCloseableValues() { ThrowableCollector throwableCollector = createThrowableCollector(); - for (Supplier supplier : storedValues.values()) { - Object value = supplier.get(); - if (value instanceof ExtensionContext.Store.CloseableResource) { - ExtensionContext.Store.CloseableResource resource = (ExtensionContext.Store.CloseableResource) value; - throwableCollector.execute(resource::close); - } - } + storedValues.values().stream() // + .filter(storedValue -> storedValue.evaluate() instanceof CloseableResource) // + .sorted(REVERSE_INSERT_ORDER) // + .map(storedValue -> (CloseableResource) storedValue.evaluate()) // + .forEach(resource -> throwableCollector.execute(resource::close)); throwableCollector.assertEmpty(); } Object get(Namespace namespace, Object key) { - Supplier storedValue = getStoredValue(new CompositeKey(namespace, key)); - return (storedValue != null ? storedValue.get() : null); + StoredValue storedValue = getStoredValue(new CompositeKey(namespace, key)); + return (storedValue != null ? storedValue.evaluate() : null); } T get(Namespace namespace, Object key, Class requiredType) { @@ -76,12 +81,12 @@ T get(Namespace namespace, Object key, Class requiredType) { Object getOrComputeIfAbsent(Namespace namespace, K key, Function defaultCreator) { CompositeKey compositeKey = new CompositeKey(namespace, key); - Supplier storedValue = getStoredValue(compositeKey); + StoredValue storedValue = getStoredValue(compositeKey); if (storedValue == null) { - Supplier newValue = new MemoizingSupplier(() -> defaultCreator.apply(key)); + StoredValue newValue = storedValue(new MemoizingSupplier(() -> defaultCreator.apply(key))); storedValue = Optional.ofNullable(storedValues.putIfAbsent(compositeKey, newValue)).orElse(newValue); } - return storedValue.get(); + return storedValue.evaluate(); } V getOrComputeIfAbsent(Namespace namespace, K key, Function defaultCreator, Class requiredType) { @@ -90,12 +95,16 @@ V getOrComputeIfAbsent(Namespace namespace, K key, Function default } void put(Namespace namespace, Object key, Object value) { - storedValues.put(new CompositeKey(namespace, key), () -> value); + storedValues.put(new CompositeKey(namespace, key), storedValue(() -> value)); + } + + private StoredValue storedValue(Supplier value) { + return new StoredValue(insertOrderSequence.getAndIncrement(), value); } Object remove(Namespace namespace, Object key) { - Supplier previous = storedValues.remove(new CompositeKey(namespace, key)); - return (previous != null ? previous.get() : null); + StoredValue previous = storedValues.remove(new CompositeKey(namespace, key)); + return (previous != null ? previous.evaluate() : null); } T remove(Namespace namespace, Object key, Class requiredType) { @@ -103,17 +112,15 @@ T remove(Namespace namespace, Object key, Class requiredType) { return castToRequiredType(key, value, requiredType); } - private Supplier getStoredValue(CompositeKey compositeKey) { - Supplier storedValue = storedValues.get(compositeKey); + private StoredValue getStoredValue(CompositeKey compositeKey) { + StoredValue storedValue = storedValues.get(compositeKey); if (storedValue != null) { return storedValue; } - else if (parentStore != null) { + if (parentStore != null) { return parentStore.getStoredValue(compositeKey); } - else { - return null; - } + return null; } @SuppressWarnings("unchecked") @@ -161,6 +168,22 @@ public int hashCode() { } + private static class StoredValue { + + private final int order; + private final Supplier supplier; + + public StoredValue(int order, Supplier supplier) { + this.order = order; + this.supplier = supplier; + } + + private Object evaluate() { + return supplier.get(); + } + + } + private static class MemoizingSupplier implements Supplier { private static final Object NO_VALUE_SET = new Object(); diff --git a/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/extension/CloseableResourceIntegrationTests.java b/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/extension/CloseableResourceIntegrationTests.java new file mode 100644 index 000000000000..604dedd43c71 --- /dev/null +++ b/junit-jupiter-engine/src/test/java/org/junit/jupiter/api/extension/CloseableResourceIntegrationTests.java @@ -0,0 +1,48 @@ +/* + * Copyright 2015-2020 the original author or authors. + * + * All rights reserved. This program and the accompanying materials are + * made available under the terms of the Eclipse Public License v2.0 which + * accompanies this distribution and is available at + * + * https://www.eclipse.org/legal/epl-v20.html + */ + +package org.junit.jupiter.api.extension; + +import static org.junit.platform.testkit.engine.EventConditions.reportEntry; + +import java.util.Map; + +import org.jetbrains.annotations.NotNull; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.engine.AbstractJupiterTestEngineTests; + +public class CloseableResourceIntegrationTests extends AbstractJupiterTestEngineTests { + + @Test + void closesCloseableResourcesInReverseInsertOrder() { + executeTestsForClass(TestCase.class).allEvents().reportingEntryPublished() // + .assertEventsMatchExactly( // + reportEntry(Map.of("3", "closed")), // + reportEntry(Map.of("2", "closed")), // + reportEntry(Map.of("1", "closed"))); + } + + @ExtendWith(ExtensionContextParameterResolver.class) + static class TestCase { + @Test + void closesCloseableResourcesInExtensionContext(ExtensionContext extensionContext) { + ExtensionContext.Store store = extensionContext.getStore(ExtensionContext.Namespace.GLOBAL); + store.put("foo", reportEntryOnClose(extensionContext, "1")); + store.put("bar", reportEntryOnClose(extensionContext, "2")); + store.put("baz", reportEntryOnClose(extensionContext, "3")); + } + + @NotNull + private ExtensionContext.Store.CloseableResource reportEntryOnClose(ExtensionContext extensionContext, + String key) { + return () -> extensionContext.publishReportEntry(Map.of(key, "closed")); + } + } +} diff --git a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/execution/ExtensionValuesStoreTests.java b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/execution/ExtensionValuesStoreTests.java index eb927bc907f1..5036d8bf5008 100644 --- a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/execution/ExtensionValuesStoreTests.java +++ b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/execution/ExtensionValuesStoreTests.java @@ -378,7 +378,7 @@ void orderOfNamespacePartsDoesMatter() { } @Test - void additionNamespacePartMakesADifferenc() { + void additionNamespacePartMakesADifference() { Namespace ns1 = Namespace.create("part1", "part2"); Namespace ns2 = Namespace.create("part1"); diff --git a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/ExtensionContextExecutionTests.java b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/ExtensionContextExecutionTests.java index 638724492e97..86469c8f8a92 100644 --- a/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/ExtensionContextExecutionTests.java +++ b/junit-jupiter-engine/src/test/java/org/junit/jupiter/engine/extension/ExtensionContextExecutionTests.java @@ -21,9 +21,7 @@ import org.junit.jupiter.api.extension.BeforeAllCallback; import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtensionContext; -import org.junit.jupiter.api.extension.ParameterContext; -import org.junit.jupiter.api.extension.ParameterResolutionException; -import org.junit.jupiter.api.extension.ParameterResolver; +import org.junit.jupiter.api.extension.ExtensionContextParameterResolver; import org.junit.jupiter.engine.AbstractJupiterTestEngineTests; class ExtensionContextExecutionTests extends AbstractJupiterTestEngineTests { @@ -45,20 +43,6 @@ void extensionContextHierarchy(ExtensionContext methodExtensionContext) { assertThat(engineExtensionContext.orElse(null).getParent()).isEmpty(); } - static class ExtensionContextParameterResolver implements ParameterResolver { - @Override - public boolean supportsParameter(ParameterContext parameterContext, ExtensionContext extensionContext) - throws ParameterResolutionException { - return ExtensionContext.class.equals(parameterContext.getParameter().getType()); - } - - @Override - public Object resolveParameter(ParameterContext parameterContext, ExtensionContext extensionContext) - throws ParameterResolutionException { - return extensionContext; - } - } - @Test void twoTestClassesCanShareStateViaEngineExtensionContext() { Parent.counter.set(0); diff --git a/junit-platform-testkit/src/main/java/org/junit/platform/testkit/engine/EventConditions.java b/junit-platform-testkit/src/main/java/org/junit/platform/testkit/engine/EventConditions.java index 236b7daddfe8..55fafae055ad 100644 --- a/junit-platform-testkit/src/main/java/org/junit/platform/testkit/engine/EventConditions.java +++ b/junit-platform-testkit/src/main/java/org/junit/platform/testkit/engine/EventConditions.java @@ -12,6 +12,7 @@ import static java.util.function.Predicate.isEqual; import static java.util.stream.Collectors.toList; +import static org.apiguardian.api.API.Status.EXPERIMENTAL; import static org.apiguardian.api.API.Status.MAINTAINED; import static org.assertj.core.api.Assertions.allOf; import static org.junit.platform.commons.util.FunctionUtils.where; @@ -29,6 +30,7 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.List; +import java.util.Map; import java.util.function.Predicate; import org.apiguardian.api.API; @@ -40,6 +42,7 @@ import org.junit.platform.engine.TestExecutionResult; import org.junit.platform.engine.TestExecutionResult.Status; import org.junit.platform.engine.UniqueId; +import org.junit.platform.engine.reporting.ReportEntry; import org.junit.platform.engine.support.descriptor.EngineDescriptor; /** @@ -411,4 +414,15 @@ public static Condition reason(Predicate predicate) { return new Condition<>(byPayload(String.class, predicate), "event with custom reason predicate"); } + /** + * Create a new {@link Condition} that matches if and only if an + * {@link Event}'s {@linkplain Event#getPayload() payload} is an instance of + * {@link ReportEntry} that contains the supplied key-value pairs. + */ + @API(status = EXPERIMENTAL, since = "1.7") + public static Condition reportEntry(Map keyValuePairs) { + return new Condition<>(byPayload(ReportEntry.class, it -> it.getKeyValuePairs().equals(keyValuePairs)), + "event for report entry with key-value pairs %s", keyValuePairs); + } + }