gnodet-bot commented on code in PR #26747:
URL: https://github.com/apache/camel/pull/26747#discussion_r4082315183
##########
components/camel-keycloak/src/test/java/org/apache/camel/component/keycloak/security/cache/ConcurrentMapTokenCacheTest.java:
##########
@@ -172,4 +174,54 @@ void testConcurrentAccess() throws InterruptedException {
assertEquals(threadCount, cache.size());
}
+
+ @Test
+ void testExpiredResultNotServed() {
+ // A result whose token has already expired must not be served, even
while the configured TTL has not elapsed.
+ Map<String, Object> claims = new HashMap<>();
Review Comment:
🟡 **Misleading test comment — the mechanism is still `put()`'s early-return,
not TTL bounding.**
This test is unchanged from the version flagged in the previous review.
`put()` still has `if (result.isExpired()) { return; }` at the top, so an
already-expired token is never inserted; `get()` returns `null` because the key
is absent — not because `effectiveTtlMillis()` computed a short TTL and the
entry expired.
The `testResultExpiringBeforeTtlNotServedAfterExp` test now correctly covers
`effectiveTtlMillis()`, so the coverage gap is filled. But this test's comment
`"even while the configured TTL has not elapsed"` implies it is testing TTL
bounding, which it is not. The test is actually verifying the early-return
guard in `put()`. Rename or update the comment to match:
```suggestion
// A result whose token has already expired must not be served:
put() must reject it immediately
// (isExpired() early-return) rather than inserting it with a
negative TTL.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
