Make NamespacedHierarchicalStore ops fail if closed

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

Make a close operation on an already closed store a no-op.

Issue: junit-team#3614

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;
 
 	/**
 	 * Create a new store with the supplied parent.
@@ -57,6 +58,7 @@ public final class NamespacedHierarchicalStore<N> implements AutoCloseable {
 	 */
 	public NamespacedHierarchicalStore(NamespacedHierarchicalStore<N> parentStore) {
 		this(parentStore, null);
+		this.closed = false;
 	}
 
 	/**
@@ -69,6 +71,7 @@ public NamespacedHierarchicalStore(NamespacedHierarchicalStore<N> parentStore) {
 	public NamespacedHierarchicalStore(NamespacedHierarchicalStore<N> parentStore, CloseAction<N> closeAction) {
 		this.parentStore = parentStore;
 		this.closeAction = closeAction;
+		this.closed = false;
 	}
 
 	/**
@@ -84,10 +87,13 @@ 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>Closing an already closed store is a no-op.
+	 * @throws IllegalStateException if Modifying or querying from an already
+	 * closed store.
 	 */
 	@Override
 	public void close() {
-		if (this.closeAction == null) {
+		if (this.closeAction == null || this.closed) {
 			return;
 		}
 		ThrowableCollector throwableCollector = new ThrowableCollector(__ -> false);
@@ -97,6 +103,7 @@ public void close() {
 				.sorted(EvaluatedValue.REVERSE_INSERT_ORDER) //
 				.forEach(it -> throwableCollector.execute(() -> it.close(this.closeAction)));
 		throwableCollector.assertEmpty();
+		this.closed = true;
 	}
 
 	/**
@@ -108,6 +115,8 @@ public void close() {
 	 * @return the stored value; may be {@code null}
 	 */
 	public Object get(N namespace, Object key) {
+		if (this.closed)
+			illegalQueryAfterClose();
 		StoredValue storedValue = getStoredValue(new CompositeKey<>(namespace, key));
 		return StoredValue.evaluateIfNotNull(storedValue);
 	}
@@ -124,6 +133,8 @@ public Object get(N namespace, Object key) {
 	 * be cast to the required type
 	 */
 	public <T> T get(N namespace, Object key, Class<T> requiredType) throws NamespacedHierarchicalStoreException {
+		if (this.closed)
+			illegalQueryAfterClose();
 		Object value = get(namespace, key);
 		return castToRequiredType(key, value, requiredType);
 	}
@@ -139,6 +150,8 @@ public <T> T get(N namespace, Object key, Class<T> requiredType) throws Namespac
 	 * @return the stored value; may be {@code null}
 	 */
 	public <K, V> Object getOrComputeIfAbsent(N namespace, K key, Function<K, V> defaultCreator) {
+		if (this.closed)
+			illegalQueryAfterClose();
 		Preconditions.notNull(defaultCreator, "defaultCreator must not be null");
 		CompositeKey<N> compositeKey = new CompositeKey<>(namespace, key);
 		StoredValue storedValue = getStoredValue(compositeKey);
@@ -165,6 +178,8 @@ public <K, V> Object getOrComputeIfAbsent(N namespace, K key, Function<K, V> def
 	 */
 	public <K, V> V getOrComputeIfAbsent(N namespace, K key, Function<K, V> defaultCreator, Class<V> requiredType)
 			throws NamespacedHierarchicalStoreException {
+		if (this.closed)
+			illegalQueryAfterClose();
 		Object value = getOrComputeIfAbsent(namespace, key, defaultCreator);
 		return castToRequiredType(key, value, requiredType);
 	}
@@ -184,6 +199,8 @@ public <K, V> V getOrComputeIfAbsent(N namespace, K key, Function<K, V> defaultC
 	 * be cast to the required type
 	 */
 	public Object put(N namespace, Object key, Object value) throws NamespacedHierarchicalStoreException {
+		if (this.closed)
+			illegalModifyAfterClose();
 		StoredValue oldValue = this.storedValues.put(new CompositeKey<>(namespace, key), storedValue(() -> value));
 		return StoredValue.evaluateIfNotNull(oldValue);
 	}
@@ -200,6 +217,8 @@ public Object put(N namespace, Object key, Object value) throws NamespacedHierar
 	 * @return the previously stored value; may be {@code null}
 	 */
 	public Object remove(N namespace, Object key) {
+		if (this.closed)
+			illegalModifyAfterClose();
 		StoredValue previous = this.storedValues.remove(new CompositeKey<>(namespace, key));
 		return StoredValue.evaluateIfNotNull(previous);
 	}
@@ -219,6 +238,8 @@ public Object remove(N namespace, Object key) {
 	 * be cast to the required type
 	 */
 	public <T> T remove(N namespace, Object key, Class<T> requiredType) throws NamespacedHierarchicalStoreException {
+		if (this.closed)
+			illegalModifyAfterClose();
 		Object value = remove(namespace, key);
 		return castToRequiredType(key, value, requiredType);
 	}
@@ -256,6 +277,14 @@ private <T> T castToRequiredType(Object key, Object value, Class<T> requiredType
 				requiredType.getName(), value.getClass().getName(), value));
 	}
 
+	private void illegalModifyAfterClose() {
+		throw new IllegalStateException("Modifying a closed resource");
+	}
+
+	private void illegalQueryAfterClose() {
+		throw new IllegalStateException("Modifying a closed resource");
+	}
+
 	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 testNoOpAfterClose() 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 throwExceptionForModifyingAfterClose() {
+			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 throwExceptionForQueryingAfterClose() {
+			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 Object createObject(final String display) {