feat(jans-fido2): add JUnit 5 coverage for Fido2UserMetrics entity be…#14137
feat(jans-fido2): add JUnit 5 coverage for Fido2UserMetrics entity be…#14137imran-ishaq wants to merge 2 commits into
Conversation
…havior Signed-off-by: imran <imranishaq7071@gmail.com>
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
Prepare
Description
Adds JUnit 5 test coverage for
Fido2UserMetrics, the per-user ORM entity backing every FIDO2 user-adoption metric stored underou=fido2-user-metrics,o=jans. This is the first PR in the FIDO2 metrics test rollout that exercises real computed behavior — counter mutations with timestamp side effects, null-safe ratio computations, threshold-based bucketing for engagement (HIGH/MEDIUM/LOWat 50 / 10) and adoption (NEW/LEARNING/ADOPTED/EXPERT), 30-day window logic for new/active user classification, and the customequals/hashCodeidentity-tuple contract used by collections/dedup inFido2UserMetricsService. 19 grouped@Testmethods, runs in ~0.151s. Test-only change; no production code orpom.xmlchanges. Step 5 of the FIDO2 metrics test rollout.Target issue
Fido2UserMetrics(jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2UserMetrics.java) is the per-user ORM entity backing every FIDO2 user-adoption metric stored underou=fido2-user-metrics,o=jans. Unlike the earlier DTOs in this rollout, this class carries real computed logic that downstream services and the REST analytics endpoints depend on:incrementRegistrations(boolean),incrementAuthentications(boolean),incrementFallbackEvents()— counter mutations with side effects onlastActivityDate/lastUpdated.getRegistrationSuccessRate(),getAuthenticationSuccessRate(),getOverallSuccessRate(),getFallbackRate()— null-safe ratio computations. Without guards,Double.NaN/Double.POSITIVE_INFINITYwould leak into stored aggregates and through the JSON analytics responses.updateEngagementLevel()— threshold-based bucketing intoHIGH(≥50 ops),MEDIUM(≥10),LOW(otherwise). An off-by-one mistake at the boundaries silently re-buckets every user.updateAdoptionStage()— threshold-based bucketing intoNEW(0 regs),LEARNING(1),ADOPTED(2–5),EXPERT(>5).isNewUser(),isActiveUser()— 30-day window logic anchored toSystem.currentTimeMillis(), with null-date guards.equals(Object)/hashCode()based on the(userId, username, firstRegistrationDate)triple — used by collections/dedup inFido2UserMetricsService.isActive=true, and stampslastUpdated; the 2-arg constructor additionally assigns a randomUUIDtoid.None of this is tested today. This is the first step in the rollout that exercises real computation rather than getter/setter wiring, and the per-method consequences of a silent regression are large: corrupted stored aggregates, NaN leakage through the JSON API, mis-bucketed engagement reports, and broken collection/dedup semantics in
Fido2UserMetricsService.This PR is step 5 of the FIDO2 metrics test rollout.
closes #14133
Implementation Details
Adds a single new JUnit 5 test class —
jans-fido2/model/src/test/java/io/jans/fido2/model/metric/Fido2UserMetricsTest.java— with 19 grouped@Testmethods. Coverage map:isActive=true,lastUpdated~now)testDefaultConstructorInitialStatetestTwoArgConstructorincrementRegistrations(true)testIncrementRegistrationsSuccessincrementRegistrations(false)testIncrementRegistrationsFailureincrementAuthentications(true/false)testIncrementAuthenticationsSuccessAndFailureincrementFallbackEvents()testIncrementFallbackEventsgetRegistrationSuccessRatehappy pathtestGetRegistrationSuccessRateHappyPathgetRegistrationSuccessRatenull + zero guardstestGetRegistrationSuccessRateZeroAndNullGuardsgetAuthenticationSuccessRatehappy path + guardstestGetAuthenticationSuccessRateHappyPathAndGuardsgetOverallSuccessRatecross-counter math (4 regs/6 auths → 0.5)testGetOverallSuccessRateHappyPathgetOverallSuccessRatenull/zero/one-side-null guardstestGetOverallSuccessRateNullAndZeroGuardsgetFallbackRatehappy path + null totals + null fallbacktestGetFallbackRateHappyPathAndGuardsupdateEngagementLevelboundary at 50 (49→MEDIUM, 50→HIGH)testUpdateEngagementLevelBoundary50updateEngagementLevelboundary at 10 (9→LOW, 10→MEDIUM)testUpdateEngagementLevelBoundary10updateEngagementLevelnull totals → LOWtestUpdateEngagementLevelNullTotalsupdateAdoptionStagefull ladder (0→NEW, 1→LEARNING, 5→ADOPTED, 6→EXPERT, null→NEW)testUpdateAdoptionStageBoundariesisNewUser29d → true, 31d → false, null → falsetestIsNewUserisActiveUser29d → true, 31d → false, null → falsetestIsActiveUserequals/hashCodeidentity-tuple contracttestEqualsContractForIdentityTupleDesign choices worth flagging:
>vs>=regressions on 50 and 10 for engagement; 0/1/5/6 transitions for adoption) andNaNleakage from unguarded division. Happy paths catch neither — so each threshold ladder is tested at the value above and below the boundary, plus null inputs.Thread.sleepand no clock injection. Timestamp side-effect assertions use a small wall-clock window pattern: capturebefore = System.currentTimeMillis()before the call,after = System.currentTimeMillis()after, assertlastUpdated.getTime()falls in[before, after].isNewUser/isActiveUseruse fixedDateoffsets relative toSystem.currentTimeMillis()(29-day / 31-day) rather than wallclock comparisons inside the assertion. Robust without flakiness.equals/hashCodecontract coverage spans: reflexive (x.equals(x)), null (x.equals(null) == false), wrong-type (x.equals("string") == false), all three identity-tuple-diff cases (differentuserId, differentusername, differentfirstRegistrationDate), AND the inverse — unrelated fields like counters andengagementLeveldiffering must preserve equality. This is exactly what theFido2UserMetricsServicededup invariant depends on.getOverallSuccessRate/getFallbackRateincludes the one-side-null case, where the null side must be treated as 0 and the other side drives the rate — a subtle path that a happy-path-only test would miss entirely.Scope:
src/test.src/main.pom.xmlchanges (jans-orm-model, which provides theEntrysuperclass, is already on the model module's compile classpath).Local verification:
mvn -pl model -am testfromjans-fido2/— 19 tests, 0 failures, 0 errors, runtime ~0.151s.Test and Document the changes
Please check the below before submitting your PR. The PR will not be merged if there are no commits that start with
docs:to indicate documentation changes or if the below checklist is not selected.