NamespacedHierarchicalStore should fail on modifications after close

[leonard84]

Status Quo

The NamespacedHierarchicalStore doesn't track its state. Currently it is possible to call put() or any other modifying action after the store has already been closed via close(), allowing for potential misuse and bugs to creep in unknowingly.

Proposal

Any modification or query after close() should result in an IllegalStateException or similar. The method close() should be idempotent: any subsequent call to close() should effectively be a no-op.

Deliverables

• NamespacedHierarchicalStore tracks its active/closed state
• NamespacedHierarchicalStore throws an exception for modification or query calls after it has been closed
• NamespacedHierarchicalStore.close() is idempotent

[sbrannen]

Thanks for raising the issue, @leonard84.

I've slated this for 5.11 M1.

Would you be interested in submitting a PR?

[mobounya]

Oops did not notice that this does not have an up-for-grabs label, can i still submit a PR for this ?

[marcphilipp]

@mobounya Yes, feel free to do so! I've added the label now.

[sbrannen]

The current implementation for tracking closed state does not mark the store as closed if an exception is thrown while invoking close actions or if there is no configured CloseAction.

See:

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

Lines 448 to 449 in 0ce27d2

	// TODO Reinstate assertion once we ensure the store is closed even if an exception is thrown.
	// assertClosed();

I am therefore reopening this issue to address those two issues.