diff --git a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java index 18745c63019d..d6fbd2ba3d0c 100644 --- a/junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java +++ b/junit-platform-engine/src/main/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStore.java @@ -49,6 +49,7 @@ public final class NamespacedHierarchicalStore implements AutoCloseable { private final ConcurrentMap, StoredValue> storedValues = new ConcurrentHashMap<>(4); private final NamespacedHierarchicalStore parentStore; private final CloseAction closeAction; + private boolean closed; /** * Create a new store with the supplied parent. @@ -57,6 +58,7 @@ public final class NamespacedHierarchicalStore implements AutoCloseable { */ public NamespacedHierarchicalStore(NamespacedHierarchicalStore parentStore) { this(parentStore, null); + this.closed = false; } /** @@ -69,6 +71,7 @@ public NamespacedHierarchicalStore(NamespacedHierarchicalStore parentStore) { public NamespacedHierarchicalStore(NamespacedHierarchicalStore parentStore, CloseAction closeAction) { this.parentStore = parentStore; this.closeAction = closeAction; + this.closed = false; } /** @@ -87,7 +90,7 @@ public NamespacedHierarchicalStore newChild() { */ @Override public void close() { - if (this.closeAction == null) { + if (this.closeAction == null || this.closed) { return; } ThrowableCollector throwableCollector = new ThrowableCollector(__ -> false); @@ -97,6 +100,7 @@ public void close() { .sorted(EvaluatedValue.REVERSE_INSERT_ORDER) // .forEach(it -> throwableCollector.execute(() -> it.close(this.closeAction))); throwableCollector.assertEmpty(); + this.closed = true; } /** @@ -108,6 +112,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 +130,8 @@ public Object get(N namespace, Object key) { * be cast to the required type */ public T get(N namespace, Object key, Class requiredType) throws NamespacedHierarchicalStoreException { + if (this.closed) + illegalQueryAfterClose(); Object value = get(namespace, key); return castToRequiredType(key, value, requiredType); } @@ -139,6 +147,8 @@ public T get(N namespace, Object key, Class requiredType) throws Namespac * @return the stored value; may be {@code null} */ public Object getOrComputeIfAbsent(N namespace, K key, Function defaultCreator) { + if (this.closed) + illegalQueryAfterClose(); Preconditions.notNull(defaultCreator, "defaultCreator must not be null"); CompositeKey compositeKey = new CompositeKey<>(namespace, key); StoredValue storedValue = getStoredValue(compositeKey); @@ -165,6 +175,8 @@ public Object getOrComputeIfAbsent(N namespace, K key, Function def */ public V getOrComputeIfAbsent(N namespace, K key, Function defaultCreator, Class requiredType) throws NamespacedHierarchicalStoreException { + if (this.closed) + illegalQueryAfterClose(); Object value = getOrComputeIfAbsent(namespace, key, defaultCreator); return castToRequiredType(key, value, requiredType); } @@ -184,6 +196,8 @@ public V getOrComputeIfAbsent(N namespace, K key, Function 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 +214,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 +235,8 @@ public Object remove(N namespace, Object key) { * be cast to the required type */ public T remove(N namespace, Object key, Class requiredType) throws NamespacedHierarchicalStoreException { + if (this.closed) + illegalModifyAfterClose(); Object value = remove(namespace, key); return castToRequiredType(key, value, requiredType); } @@ -256,6 +274,14 @@ private T castToRequiredType(Object key, Object value, Class 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 { private final N namespace; diff --git a/platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java b/platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java index c4c87785fe4f..10695f418ac9 100644 --- a/platform-tests/src/test/java/org/junit/platform/engine/support/store/NamespacedHierarchicalStoreTests.java +++ b/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) {