#131 Refactor JobStore decorators onto a shared forwarding base - #134
Merged
Conversation
Move the full-SPI forwarding decorator into threadmill-core as com.hemju.threadmill.core.store.ForwardingJobStore and make TracingJobStore and MeteredJobStore extend it, overriding only the operations they instrument. The interface's default methods never produce a compiler failure when a decorator forgets to forward them; TracingJobStore missed supportsExternalTransactions() and createRemoteWakeChannel(String), so a traced PostgreSQL store lost join_transaction support and its LISTEN/NOTIFY wake channel behind the decorator. The test-support ForwardingJobStore becomes a deprecated alias. New JobStoreDecoratorContract.assertForwardsEveryOperation enumerates JobStore's methods by reflection against a recording proxy so a future SPI addition that falls through to a default fails by name; the shared store contract now also runs through the plain base and the tracing decorator, with named external-transaction and remote-wake regressions on all three. Closes #131
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #131.
Problem
Threadmill had three hand-written full-
JobStoreforwarding decorators: the test-supportForwardingJobStore,TracingJobStore, andMeteredJobStore. The SPI'sdefaultmethods never produce a compiler failure when a decorator forgets to forward them, andTracingJobStoreimplementedJobStoredirectly and missedsupportsExternalTransactions()andcreateRemoteWakeChannel(String). A traced PostgreSQL store therefore reported no external-transaction support (join_transactionunavailable) and noLISTEN/NOTIFYwake channel.Change
com.hemju.threadmill.core.store.ForwardingJobStore(new,threadmill-core): forwards every SPI operation, the interface defaults included.delegate()isfinaland always returns the immediate delegate, so the Spring auto-config's unwrap loop still walks a chain one layer at a time.TracingJobStoreandMeteredJobStorenow extend the base and override only what they instrument. The metrics decorator drops 24 pure forwards; tracing keeps a span per I/O operation and inherits the capability reads without a span.com.hemju.threadmill.test.ForwardingJobStorebecomes a deprecated alias for the core class (test-support is a published artifact); every in-repo usage moved to the core class.JobStoreDecoratorContract.assertForwardsEveryOperation(new,threadmill-test-support): wraps a recordingProxy, enumeratesJobStore.class.getMethods()by reflection, and requires each method to reach the delegate exactly once with the caller's arguments and to return the delegate's result. Because the method list comes from reflection, an SPI method added later is covered automatically; a new parameter/return type fails loudly until a sample is added.AbstractJobStoreContractTestnow also runs through the plain base (ForwardingJobStoreContractTest) and the tracing decorator (TracingJobStoreContractTest), alongside the existingMeteredJobStoreContractTest.threadmill-tracinggains atestImplementationdependency onthreadmill-test-support; itsgradle.lockfilepicks up awaitility/hamcrest on the test classpaths (already inverification-metadata.xml).CHANGELOG.md(Unreleased), test-support and tracing READMEs.Verification
./gradlew checkgreen (all modules, Spotless included).