Lazy Initialisation of ExternalIdentityMonitor#2928
Conversation
OAK-10578: resolve externalIdentityMonitor lazily
Shivam-Agg
changed the title
Shivam-Agg
changed the title
| log.debug("No ExternalIdentityMonitor registered."); | ||
| monitor = ExternalIdentityMonitor.NOOP; | ||
| } | ||
| monitorSupplier = () -> { |
There was a problem hiding this comment.
oak-auth-external enforces 100% branch coverage (mvn verify -pl oak-auth-external -Pcoverage passes on trunk but fails on this PR at 0.9). The supplier's cached monitor != null branch is never exercised because each call site invokes get() only once—please add a test that hits the supplier twice in one login.
| } | ||
|
|
||
| @Test | ||
| public void testMonitorResolvedExactlyOnceOnSuccessfulSync() throws Exception { |
There was a problem hiding this comment.
This largely duplicates testSyncUser (same IDP/sync setup) while only asserting track once. Consider folding verify(wb, times(1)).track(ExternalIdentityMonitor.class) into testSyncUser and removing this test.
| timer.mark("commit"); | ||
| log.debug("validateUser({}) {}", id, timer); | ||
| monitor.doneSyncId(watch.elapsed(NANOSECONDS), syncResult); | ||
| monitorSupplier.get().doneSyncId(watch.elapsed(NANOSECONDS), syncResult); |
There was a problem hiding this comment.
validateUser now resolves the monitor lazily via monitorSupplier.get().doneSyncId(...). Consider asserting never().track right after initialize() and times(1) after login() in testValidateUser so this path documents the same lazy contract as the new sync tests.
| } | ||
|
|
||
| @Test | ||
| public void testNoopMonitorUsedWhenMonitorNotRegistered() throws Exception { |
There was a problem hiding this comment.
Login succeeds and track runs once, but the test does not show ExternalIdentityMonitor.NOOP was selected. Consider stronger assertions (sync side effects) or a failure-path variant without a registered monitor for syncFailed.
2 participants
No description provided.