feat: §17 memo, §18 close(), §19 telemetry, §16.6 switch (D5) - #39
Merged
Conversation
Re-vendors CONTRACT.md at 1.8.2. §16's arithmetic is UNCHANGED here, which
makes this SDK the exception among the five that had invented a policy.
§16. internal/Retry was already conformant — 3 attempts, 200 ms base, 5 s cap,
full jitter over [0, backoff], Retry-After honored via Math.max — and it is the
implementation whose parameters contract 1.8 adopted. Of the five SDKs that had
each invented a policy, this was the only one that got both full jitter and
Retry-After-as-a-floor right. Two things were missing rather than wrong:
- the §16.6 disable switch, now Builder.retryDisabled(). There is deliberately
no builder method for the attempt cap, base or delay cap: §16.1 permits
lowering the cap or disabling retry, never raising either.
- the §16.5 retry event. withRetry gains an attempt-aware overload that
passes the 1-based attempt into the operation and emits Retry before each
wait. The attempt number is not cosmetic: §19.2 rule 5 wants one request
pair per attempt so a caller can count real wire calls, and emitting every
pair as attempt 1 would make a retried call look like a single slow one.
I verified before touching anything that checkAccess actually routes through
withRetry — that is the TypeScript failure mode, where an exported, unit-tested,
green helper turned out to be called by nothing. Here all three authz paths were
genuinely wired, so the new conformance tests are a regression lock rather than
a fix.
§17. DecisionMemo is generic in the decision type rather than importing
AxiamClient.AccessResult, so the internal package keeps its existing shape —
DiscoveryCache<T> already establishes that convention, and an internal helper
reaching back into the public client class would invert the dependency. Off by
default, clamped to 5 s, synchronized because a Java client is routinely shared
across a request-handling thread pool.
§18. close() is idempotent via compareAndSet, so a concurrent double-close does
the work once rather than racing the executor shutdown. It clears the memo and
does not log out: the server-side session outlives the client object, and a
close() that logged out would end every user's session on each deploy.
§19. TelemetryEvent is a SEALED hierarchy of records. That is the Java-specific
version of the "no field can carry a secret" guarantee — a sealed interface
means no code outside this library can add a variant, so the closed field set is
enforced by the compiler rather than by review.
One test needed rethinking: logout() rejects with AuthError when there is no
session, so the memo-clear test could not simply call it. The clear runs BEFORE
that check, deliberately — the trigger is the caller's intent to change
credentials, not the server's answer — and the test now asserts exactly that,
which is a sharper assertion than the original would have been.
Gates: 471 tests pass (20 new), JaCoCo 0.93 line-coverage gate satisfied,
TLS-bypass gate clean.
…ilder methods Three CI jobs failed on one mistake. I inserted retryDisabled(), decisionMemoTtl() and telemetryHook() between oidcClockSkew's javadoc comment and the method it documents, so the comment attached to my first new method instead and oidcClockSkew was left bare. pom.xml runs maven-javadoc-plugin with doclint=all and failOnWarnings=true — javadoc.io serves the attached -javadoc.jar verbatim and it is this SDK's only published API reference, so a missing entry is a real user-facing gap rather than a style nit. "warning: no comment" at AxiamClient.java:472 therefore failed the Javadoc gate, and the same broken build took down the JaCoCo and Spring Boot example jobs with it. The comment is now back above oidcClockSkew, and the duplicate my first fix left stacked above retryDisabled is gone — javadoc silently ignores all but the last of stacked comments, so that would have built green while leaving dead text in the source. Verified with the exact CI commands: mvn compile javadoc:javadoc is clean, mvn test passes 471 with the JaCoCo gate met, and the Spring Boot example verifies with its 8 integration tests.
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.
Re-vendors
CONTRACT.mdat 1.8.2. §16's arithmetic is unchanged here — this SDK is the exception among the five that had invented a policy.Fifth of eleven, after rust#45, typescript#47, python#34 and go#31.
§16 was already right
internal/Retrydoes 3 attempts, 200 ms base, 5 s cap, full jitter over[0, backoff], and honorsRetry-AfterviaMath.max. It is the implementation whose parameters contract 1.8 adopted — and of the five SDKs that had each invented one, the only one that got both full jitter andRetry-After-as-a-floor right.I checked the TypeScript failure mode first. Before touching anything I verified that
checkAccessactually routes throughwithRetry— that is where an exported, unit-tested, green helper turned out to be called by nothing. Here all three authz paths were genuinely wired, so the new §16 conformance tests are a regression lock, not a fix.Two things were missing rather than wrong:
Builder.retryDisabled()(§16.6). No builder method for the attempt cap, base or delay cap: §16.1 permits lowering the cap or disabling retry, never raising either.withRetrygains an attempt-aware overload that passes the 1-based attempt into the operation and emitsRetrybefore each wait.§17 — generic, so
internalstaysinternalDecisionMemo<T>is generic in the decision type rather than importingAxiamClient.AccessResult.DiscoveryCache<T>already establishes that convention in this package, and an internal helper reaching back into the public client class would invert the dependency.Off by default, clamped to
MAX_TTL(5 s),synchronized— a Java client is routinely shared across a request-handling thread pool.§18 — idempotent via
compareAndSetNot merely "calling it twice doesn't throw":
compareAndSetmeans a concurrent double-close does the work once rather than two threads racing the executor shutdown and connection-pool eviction.close()does not log out and never reaches the network — asserted against the wire, because alogoutwired intoclosesucceeds silently.§19 — sealed, which is the Java-specific version of the guarantee
TelemetryEventis a sealed hierarchy of records. No code outside this library can add a variant, so "no event payload can carry a secret" is enforced by the compiler rather than by review — the strongest form of that guarantee across the SDKs so far.One
RequestStart/RequestEndpair per attempt.A test that needed rethinking
logout()rejects withAuthErrorwhen there is no session, so the memo-clear test could not simply call it on a fresh client.The clear runs before that check, deliberately: the trigger is the caller's intent to change credentials, not the server's answer. A logout that failed still means this caller is done with the principal whose decisions are cached, and entries are keyed by subject rather than session — so keeping them would let a re-authentication as a different principal inherit them. The test now asserts exactly that, which is sharper than what I first wrote.
Verification
mvn testcheckscripts/tls-bypass-gate.shsrc/andexamples/mvn compileThe pre-existing
RetryTestpasses unchanged — the arithmetic it covers was not touched.Notes
examples/telemetry-hook/TelemetryHookExample.java, with the Micrometer mapping written out. It pulls no metrics dependency, which is the point of §19.Generated by Claude Code