Make operations on NamespacedHierarchicalStore fail if it's closed

Track the active/closed state in NamespacedHierarchicalStore so it can
throw an IllegalStateException when performing a modification or query
on it after it has been already closed.

This commit also makes close() an idempotent operation.

Closes #3614
Closes #3800

documentation/src/docs/asciidoc/release-notes/release-notes-5.11.0-M2.adoc

@@ -36,8 +36,10 @@ repository on GitHub.
 [[release-notes-5.11.0-M2-junit-platform-new-features-and-improvements]]
 ==== New Features and Improvements
 
-* ❓
-
+* `NamespacedHierarchicalStore` will now throw an IllegalStateException for
+  modification or query calls after it has been closed. Closing an already
+  closed store is a no-op.
+  - See link:https://github.com/junit-team/junit5/issues/3614[issue 3614] for details.
 
 [[release-notes-5.11.0-M2-junit-jupiter]]
 === JUnit Jupiter

junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java

@@ -49,6 +49,7 @@ public final class NamespacedHierarchicalStore<N> implements AutoCloseable {
 	private final ConcurrentMap<CompositeKey<N>, StoredValue> storedValues = new ConcurrentHashMap<>(4);
 	private final NamespacedHierarchicalStore<N> parentStore;
 	private final CloseAction<N> closeAction;
+	private boolean closed = false;
 
 	/**
 	 * Create a new store with the supplied parent.
@@ -84,10 +85,11 @@ public NamespacedHierarchicalStore<N> newChild() {
 	 * stored values in reverse insertion order.
 	 *
 	 * <p>Closing a store does not close its parent or any of its children.
+	 * <p>Invocations of this method after the store has already been closed will be ignored.
 	 */
 	@Override
 	public void close() {
-		if (this.closeAction == null) {
+		if (this.closeAction == null || this.closed) {
 			return;
 		}
 		ThrowableCollector throwableCollector = new ThrowableCollector(__ -> false);
@@ -97,6 +99,7 @@ public void close() {
 				.sorted(EvaluatedValue.REVERSE_INSERT_ORDER) //
 				.forEach(it -> throwableCollector.execute(() -> it.close(this.closeAction)));
 		throwableCollector.assertEmpty();
+		this.closed = true;
 	}
 
 	/**
@@ -106,8 +109,12 @@ public void close() {
 	 * @param namespace the namespace; never {@code null}
 	 * @param key the key; never {@code null}
 	 * @return the stored value; may be {@code null}
+	 * @throws IllegalStateException when querying from an already closed store
 	 */
 	public Object get(N namespace, Object key) {
+		if (this.closed) {
+			rejectQueryAfterClose();
+		}
 		StoredValue storedValue = getStoredValue(new CompositeKey<>(namespace, key));
 		return StoredValue.evaluateIfNotNull(storedValue);
 	}
@@ -122,8 +129,12 @@ public Object get(N namespace, Object key) {
 	 * @return the stored value; may be {@code null}
 	 * @throws NamespacedHierarchicalStoreException if the stored value cannot
 	 * be cast to the required type
+	 * @throws IllegalStateException when querying from an already closed store
 	 */
 	public <T> T get(N namespace, Object key, Class<T> requiredType) throws NamespacedHierarchicalStoreException {
+		if (this.closed) {
+			rejectQueryAfterClose();
+		}
 		Object value = get(namespace, key);
 		return castToRequiredType(key, value, requiredType);
 	}
@@ -137,8 +148,12 @@ public <T> T get(N namespace, Object key, Class<T> requiredType) throws Namespac
 	 * @param defaultCreator the function called with the supplied {@code key}
 	 * to create a new value; never {@code null} but may return {@code null}
 	 * @return the stored value; may be {@code null}
+	 * @throws IllegalStateException when querying from an already closed store
 	 */
 	public <K, V> Object getOrComputeIfAbsent(N namespace, K key, Function<K, V> defaultCreator) {
+		if (this.closed) {
+			rejectQueryAfterClose();
+		}
 		Preconditions.notNull(defaultCreator, "defaultCreator must not be null");
 		CompositeKey<N> compositeKey = new CompositeKey<>(namespace, key);
 		StoredValue storedValue = getStoredValue(compositeKey);
@@ -162,9 +177,13 @@ public <K, V> Object getOrComputeIfAbsent(N namespace, K key, Function<K, V> def
 	 * @return the stored value; may be {@code null}
 	 * @throws NamespacedHierarchicalStoreException if the stored value cannot
 	 * be cast to the required type
+	 * @throws IllegalStateException when querying from an already closed store
 	 */
 	public <K, V> V getOrComputeIfAbsent(N namespace, K key, Function<K, V> defaultCreator, Class<V> requiredType)
 			throws NamespacedHierarchicalStoreException {
+		if (this.closed) {
+			rejectQueryAfterClose();
+		}
 		Object value = getOrComputeIfAbsent(namespace, key, defaultCreator);
 		return castToRequiredType(key, value, requiredType);
 	}
@@ -182,8 +201,12 @@ public <K, V> V getOrComputeIfAbsent(N namespace, K key, Function<K, V> defaultC
 	 * @return the previously stored value; may be {@code null}
 	 * @throws NamespacedHierarchicalStoreException if the stored value cannot
 	 * be cast to the required type
+	 * @throws IllegalStateException when modifying an already closed store
 	 */
 	public Object put(N namespace, Object key, Object value) throws NamespacedHierarchicalStoreException {
+		if (this.closed) {
+			rejectModificationAfterClose();
+		}
 		StoredValue oldValue = this.storedValues.put(new CompositeKey<>(namespace, key), storedValue(() -> value));
 		return StoredValue.evaluateIfNotNull(oldValue);
 	}
@@ -198,8 +221,12 @@ public Object put(N namespace, Object key, Object value) throws NamespacedHierar
 	 * @param namespace the namespace; never {@code null}
 	 * @param key the key; never {@code null}
 	 * @return the previously stored value; may be {@code null}
+	 * @throws IllegalStateException when modifying an already closed store
 	 */
 	public Object remove(N namespace, Object key) {
+		if (this.closed) {
+			rejectModificationAfterClose();
+		}
 		StoredValue previous = this.storedValues.remove(new CompositeKey<>(namespace, key));
 		return StoredValue.evaluateIfNotNull(previous);
 	}
@@ -217,8 +244,12 @@ public Object remove(N namespace, Object key) {
 	 * @return the previously stored value; may be {@code null}
 	 * @throws NamespacedHierarchicalStoreException if the stored value cannot
 	 * be cast to the required type
+	 * @throws IllegalStateException when modifying an already closed store
 	 */
 	public <T> T remove(N namespace, Object key, Class<T> requiredType) throws NamespacedHierarchicalStoreException {
+		if (this.closed) {
+			rejectModificationAfterClose();
+		}
 		Object value = remove(namespace, key);
 		return castToRequiredType(key, value, requiredType);
 	}
@@ -256,6 +287,14 @@ private <T> T castToRequiredType(Object key, Object value, Class<T> requiredType
 				requiredType.getName(), value.getClass().getName(), value));
 	}
 
+	private void rejectModificationAfterClose() {
+		throw new IllegalStateException("A NamespacedHierarchicalStore cannot be modified after it has been closed");
+	}
+
+	private void rejectQueryAfterClose() {
+		throw new IllegalStateException("A NamespacedHierarchicalStore cannot be queried after it has been closed");
+	}
+
 	private static class CompositeKey<N> {
 
 		private final N namespace;

platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java

@@ -19,6 +19,7 @@
 import static org.junit.platform.commons.test.ConcurrencyTestingUtils.executeConcurrently;
 import static org.mockito.Mockito.inOrder;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -435,6 +436,42 @@ void doesNotIgnoreStoredValuesThatThrewUnrecoverableFailuresDuringCleanup() {
 
 			verifyNoInteractions(closeAction);
 		}
+
+		@Test
+		void closeIsIdempotent() throws Throwable {
+			store.put(namespace, "key1", "value1");
+
+			verifyNoInteractions(closeAction);
+
+			store.close();
+
+			verify(closeAction, times(1)).close(namespace, "key1", "value1");
+
+			store.close();
+
+			verifyNoMoreInteractions(closeAction);
+		}
+
+		@Test
+		void rejectsModificationAfterClose() {
+			store.close();
+
+			assertThrows(IllegalStateException.class, () -> store.put(namespace, "key1", "value1"));
+			assertThrows(IllegalStateException.class, () -> store.remove(namespace, "key1"));
+			assertThrows(IllegalStateException.class, () -> store.remove(namespace, "key1", Number.class));
+		}
+
+		@Test
+		void rejectsQueryAfterClose() {
+			store.close();
+
+			assertThrows(IllegalStateException.class, () -> store.get(namespace, "key1"));
+			assertThrows(IllegalStateException.class, () -> store.get(namespace, "key1", Integer.class));
+			assertThrows(IllegalStateException.class,
+				() -> store.getOrComputeIfAbsent(namespace, "key1", k -> "tzadina 9ba7"));
+			assertThrows(IllegalStateException.class,
+				() -> store.getOrComputeIfAbsent(namespace, "key1", k -> 1337, Integer.class));
+		}
 	}
 
 	private static Object createObject(String display) {