Repository navigation
Add forced IAM token refresh API to the Java toolkit - #17
Conversation
![Alexandre [C] Luc Rehnby-Martin](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
| assertNotEquals(first, replacement); | ||
| assertEquals(replacement, harness.manager.getToken()); | ||
| assertEquals(2, harness.credentialCalls.get()); | ||
| assertEquals(java.util.Arrays.asList(first, replacement), harness.changedTokens); |
| Throwable concurrentFailure = concurrent.join(); | ||
| assertTrue(forcedFailure instanceof ConfigurationException); | ||
| assertSame(forcedFailure, concurrentFailure); | ||
| assertNotEquals(rejected, forcedFailure.getMessage()); |
There was a problem hiding this comment.
maybe something stronger would be an assert that tests it doesn't include it
There was a problem hiding this comment.
Strengthened this to explicitly assert that the rejected token is absent from the failure message.
| assertNotNull(ElastiCacheIamAuthTokenProvider.builder()); | ||
| assertNotNull(ElastiCacheIamAuthTokenManager.builder()); | ||
| Method refreshToken = ElastiCacheIamAuthTokenManager.class.getMethod("refreshToken"); | ||
| assertTrue(refreshToken.getReturnType().equals(String.class)); |
There was a problem hiding this comment.
assertEquals(String.class, refreshToken.getReturnType()) so a failure prints the actual type. Reflection also isn't needed here: a plain call such as String s = manager.refreshToken(); checks the same thing at compile time.
There was a problem hiding this comment.
Removed reflection. The integration test now directly calls String token = manager.refreshToken() and verifies the result.
| } | ||
|
|
||
| /** | ||
| * Forces an immediate token refresh. |
There was a problem hiding this comment.
nit: "immediate" isn't quite true if a refresh is already in retry backoff. refresh() returns the existing inFlight future, so the caller waits out the remaining backoff instead of retrying right away. We should soften the wording here and in the README (e.g. "Invalidates the cached token and refreshes without waiting for the scheduled refresh; joins a refresh already in progress, including one in retry backoff").
There was a problem hiding this comment.
Updated the wording
| * @throws TokenRefreshException if the manager is closed | ||
| * @throws ConfigurationException if usable AWS credentials cannot be resolved | ||
| */ | ||
| public String refreshToken() { |
There was a problem hiding this comment.
In finishFailedRefresh there could be a race: it reads the cached token under the lock but calls refresh.complete(validToken) after releasing it. If refreshToken() runs in between, it clears cached, joins this same in-flight future, and gets back the token it invalidated. Moving the complete call inside the synchronized block, like installAndComplete does, closes the gap.
There was a problem hiding this comment.
Good catch. finishFailedRefresh now completes the shared future while still holding the cache lock, so forced invalidation cannot interleave and receive the rejected fallback token.
3 participants
Summary
ElastiCacheIamAuthTokenManager.refreshToken()with behavior aligned to Node’s existing API.Verification
mvn verify: 50 tests passed.