Fix expiration logic for ldap internal credential cache #11395
Conversation
loquisgon
force-pushed
the
fix_ldap_cache_duration
branch
from
0382af2
to
dc66caa
Compare
|
|
||
| public void testIsExpiredWhenMaxDurationIsSmall() throws InterruptedException | ||
| { | ||
| Thread.sleep(1000); |
There was a problem hiding this comment.
Maybe you could avoid the sleeps by changing isExpired to accept "now" as a parameter, and passing in an explicit "created at" time when creating the principal
There was a problem hiding this comment.
What about exposing the private constructor (for testing) and adding the now parameter as you indicate? however passing now to isExpired feels like an error prone decision (callers may make mistakes violating the contract of the expiration logic --- violation of encapsulation)
There was a problem hiding this comment.
Done. I left the original isExpired method and added a new isExpired, visible for testing, with the new parameters.
| Assert.assertTrue(PRINCIPAL.isExpired(100, 1000)); | ||
| } | ||
|
|
||
| public void testIsExpiredWhenMaxDurationIsSmall() throws InterruptedException |
There was a problem hiding this comment.
Exception is never thrown. Same for other tests below. Failing intelliJ inspection because of this.
* Fix expiration logic for ldap internal credential cache * Removed sleeps from tests * Make method package scoped so it can be used in unit tests * Removed unused thrown exceptions
Fixes a bug with the expiration logic for
LdapUserPrincipal. The entry should unconditionally expire whenmaxDurationis up among other conditions. It was not expiring in that case.This PR has: