Skip to content

feat(jans-fido2): add unit tests for Fido2MetricsAggregation computed surface #2864

Description

@imran-ishaq

Is your feature request related to a problem? Please describe.
Fido2MetricsAggregation (jans-fido2/model/src/main/java/io/jans/fido2/model/metric/Fido2MetricsAggregation.java) is the ORM entity that stores hourly/daily/weekly/monthly summary rows for the FIDO2 metrics feature. Most of its surface is plain getters/setters, but it has several small bits of computed behavior that matter and are completely untested today:

  • A 4-arg constructor that builds the composite id as aggregationType + "_" + period and stamps lastUpdated in UTC.
  • getPeriod() — derives the period string back out of id by splitting on the first _.
  • getLongMetric(key) / getDoubleMetric(key) — null-safe Number coercion that returns null (not NaN, not NPE) when the underlying map entry is absent or non-numeric.
  • getDeviceTypes() / getErrorCounts() — Map accessors that coerce Integer values to Long for JSON-serialization compatibility (exactly the shape Jackson hands back for JSON numbers).
  • incrementMetric(key, increment) — null-safe accumulator (treats both nulls as 0).
  • A custom equals/hashCode based on (id, aggregationType, startTime, endTime) — ignoring uniqueUsers and metricsData.

Each method has explicit null-safety and type-coercion handling — silent failure modes would surface as wrong aggregation totals, dropped Integer→Long conversions, or NPE against partially-loaded ORM entries, all without compile-time signal. The class is small and self-contained — ideal for a quick test pass that locks the behaviors downstream aggregation/analytics code silently depends on.

This is step 7 of the FIDO2 metrics test rollout (steps 1–6 covered Fido2MetricType, Fido2MetricsConstants, Fido2MetricsData, UserMetricsUpdateRequest, Fido2UserMetrics rate calculations, and Fido2UserMetrics state-mutation methods).

Describe the solution you'd like
Add a new JUnit 5 test class:

  • File: jans-fido2/model/src/test/java/io/jans/fido2/model/metric/Fido2MetricsAggregationTest.java

The test should cover, per behavior group (one @Test per group):

  • Default constructor: metricsData is a non-null empty map after new Fido2MetricsAggregation(); all other fields are null.
  • 4-arg constructor composes the id and stamps lastUpdated: new Fido2MetricsAggregation("DAILY", "2026-05-22", start, end) produces id == "DAILY_2026-05-22", sets aggregationType, startTime, endTime to the passed values, and stamps lastUpdated to a non-null value within ±1s of now. The default-ctor invariant (metricsData non-null and empty) must still hold via this() delegation.
  • getPeriod(): with id == "DAILY_2026-05-22", returns "2026-05-22"; with id == "HOURLY_2026-05-22_14" (multi-_), returns "2026-05-22_14" to lock split-on-FIRST-_; with id == "NOPERIOD" (no underscore), returns "NOPERIOD"; with id == null, returns null.
  • getLongMetric / getDoubleMetric null-safety: return null when metricsData itself is null (use setMetricsData(null)), and when the key is absent.
  • getLongMetric widens Integer/truncates Double: when the underlying entry is a Long, returns the value; when it's an Integer, asserts it's widened to Long; when it's a Double, asserts longValue() truncates toward zero.
  • getDoubleMetric accepts Integer: when the underlying entry is an Integer, returns the value as a Double (proves the Number path doesn't reject non-Double numerics).
  • getLongMetric rejects non-numbers: stash "not-a-number" (a String) under a key, assert both getLongMetric(key) and getDoubleMetric(key) return null (not a ClassCastException).
  • Convenience setters/getters wire to the right constant keys: call setRegistrationAttempts(42L) and assert (a) getRegistrationAttempts() == 42L, and (b) getMetricsData().get(Fido2MetricsConstants.REGISTRATION_ATTEMPTS).equals(42L). Pick one more pair (setAuthenticationSuccessRate(0.85) / getAuthenticationSuccessRate()) to prove the Double path stores under AUTHENTICATION_SUCCESS_RATE. No need to test all 14 pairs.
  • getDeviceTypes() coerces Integer→Long: populate the inner map with Integer values (the exact shape Jackson hands back for JSON numbers) and assert getDeviceTypes() returns a Map<String, Long> with widened values.
  • getDeviceTypes() / getErrorCounts() return empty (not null) when absent: assert empty map, not null, when the key is missing from metricsData.
  • incrementMetric null-safety on both sides: starting from an empty map, incrementMetric("foo", 5L) results in getLongMetric("foo") == 5L; calling it again with 3L gives 8L; calling it with null increment keeps it at 8L; calling it on a fresh instance with null increment results in 0L (current treated as 0, increment as 0).
  • equals / hashCode contract: two instances with identical (id, aggregationType, startTime, endTime) are equal and share a hash; differing in any of the four breaks equality; differing in unrelated fields (uniqueUsers, metricsData) preserves equality. Include reflexive x.equals(x) and x.equals(null) / x.equals("string") cases.

Conventions to follow (same as Fido2MetricTypeTest through Fido2UserMetricsRateCalculationsTest):

  • JUnit 5 (org.junit.jupiter.api).
  • No Mockito, no async, no CDI — pure entity behavior.
  • No additional dependencies — junit-jupiter-api and junit-jupiter-engine test-scope deps are already declared in jans-fido2/model/pom.xml.
  • Floating-point assertions use assertEquals(expected, actual, 1e-9) to avoid spurious comparison failures.
  • One @Test per behavior group with a descriptive name (e.g., testFourArgConstructorComposesIdAndStampsLastUpdated, testGetLongMetricReturnsNullForNonNumberValue, testIncrementMetricIsNullSafeOnBothSides).

Acceptance criteria:

  • New test file exists at the path above.
  • All behaviors listed are covered.
  • mvn -pl model -am test from jans-fido2/ passes locally.
  • Test class runs in well under 1 second (no I/O, no Thread.sleep).
  • No changes to production code under src/main.
  • No pom changes required.

Describe alternatives you've considered

  • Exhaustively testing every one of the ~14 convenience getter/setter pairs: rejected — they're mechanical wrappers around getLongMetric / setMetric against a constant key. Testing one Long pair + one Double pair proves the wiring; the rest follows.
  • Splitting equals/hashCode into its own issue: rejected — it's a few assertions; bundling keeps the PR coherent.
  • Adding Java native serialization roundtrip tests: rejected as overkill — no custom readObject/writeObject, plain JavaBeans.
  • Testing getPeriod() only with a single happy-path id: rejected — the split-on-FIRST-_ behavior is the load-bearing detail; a multi-_ id (e.g., HOURLY_2026-05-22_14) is needed to catch a refactor that switches to split("_") or lastIndexOf("_").
  • Asserting lastUpdated is exactly new Date() from the ctor: rejected — clock skew between before/after reads makes that brittle. A ±1s window is tight enough to catch a missing stamp and loose enough to absorb scheduling jitter.

Additional context
Roadmap position: step 7 of the FIDO2 metrics test rollout, intentionally chosen as a small breather between the heavier server-side work. Next planned step is to start covering Fido2MetricsService itself — that work will be split across several focused issues by responsibility (storage/ingest, query, aggregation, analytics).

Same Maven module (jans-fido2/model) as issues #1#6; no pom changes required.

Suggested label: kind-feature.

Metadata

Metadata

Assignees

Labels

enhancementNew feature or requestkind-featureIssue or PR is a new feature request

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions