Skip to content

feat(jans-fido2): add JUnit 5 coverage for UserMetricsUpdateRequest DTO#14138

Draft
imran-ishaq wants to merge 1 commit into
mainfrom
feat/jans-fido2-model-user-metrics-update-request-tests
Draft

feat(jans-fido2): add JUnit 5 coverage for UserMetricsUpdateRequest DTO#14138
imran-ishaq wants to merge 1 commit into
mainfrom
feat/jans-fido2-model-user-metrics-update-request-tests

Conversation

@imran-ishaq
Copy link
Copy Markdown
Contributor

Prepare


Description

Adds JUnit 5 test coverage for UserMetricsUpdateRequest, the parameter-object DTO passed into Fido2UserMetricsService for every user registration, authentication, or fallback event that updates per-user adoption/engagement statistics. The new test class locks the 3-argument constructor's argument order (userId, username, success), the isSuccess() boolean-getter naming convention (reflectively, including a negative check that getSuccess() does NOT exist), and the Serializable + serialVersionUID = 1L invariant. Test-only change; no production code or pom.xml changes. Step 4 of the FIDO2 metrics test rollout.

Target issue

UserMetricsUpdateRequest (jans-fido2/model/src/main/java/io/jans/fido2/model/metric/UserMetricsUpdateRequest.java) is the parameter-object DTO passed from the recording code path into Fido2UserMetricsService whenever a user registration, authentication, or fallback event needs to update per-user adoption/engagement statistics. It carries 12 fields covering user identity, success flag, authenticator/device/browser/OS metadata, duration, network info, and fallback details. It exposes a default constructor and a 3-argument convenience constructor (userId, username, success), and uses the isSuccess() boolean-getter naming convention.

None of this is tested today. Two specific regressions would be silent failures:

  1. The 3-arg constructor's two String parameters (userId, username) are the most likely to get accidentally swapped during a refactor — without a test, every user-metrics record would be written with the values transposed and no compile-time signal would fire.
  2. A "make-it-consistent" rename from isSuccess()getSuccess() would break Jackson default introspection, JavaBeans Introspector, and MapStruct, but compile fine — silently corrupting serialized payloads and downstream service contracts.

This PR is step 4 of the FIDO2 metrics test rollout.

closes #14120

Implementation Details

Adds a single new JUnit 5 test class — jans-fido2/model/src/test/java/io/jans/fido2/model/metric/UserMetricsUpdateRequestTest.java — with 5 grouped @Test methods, one per spec'd behavior:

Behavior Test method
Default constructor leaves fields unset testDefaultConstructorLeavesFieldsUnset
3-arg constructor wires the right fields to the right slots testThreeArgConstructorWiresUserIdAndUsernameAndSuccess
Getter/setter roundtrip (sample per type group) testGetterSetterRoundTripCoversEachValueType
isSuccess() boolean-getter naming locked reflectively testIsSuccessUsesBooleanGetterNamingConvention
Serializable contract + serialVersionUID = 1L locked reflectively testSerializableContractIsIntact

Design choices worth flagging:

  • Sampled rather than exhaustive getter/setter coverage — one field per type group (authenticatorType for String, success for boolean, durationMs for Long). Catches the same wiring class of bugs as testing all 12 individually without ~24 near-identical methods.
  • Argument-order lock for the 3-arg ctor. The test passes ("u-123", "alice", true) and asserts each field landed in the right slot, then asserts the remaining 9 fields stayed null. This is the load-bearing test against the most likely refactor regression in this DTO.
  • Reflective naming lock for isSuccess(). The test reflectively confirms isSuccess() exists and returns primitive boolean, and that getSuccess() does NOT exist. A direct call wouldn't catch the rename; the negative reflective check is what locks the convention.
  • Reflective Serializable invariant. The test asserts the class still implements Serializable and that serialVersionUID is private static final long = 1L. Bumping the UID silently breaks deserialization of in-flight queued payloads — exactly the kind of regression that surfaces only at runtime, in production, far from the change. A full Java native serialization round-trip was considered and rejected as out of scope; the reflective invariant covers the actual production failure mode.

Scope:

  • Test-only addition under src/test.
  • No production-code changes under src/main.
  • No pom.xml changes (the JUnit 5 test-scope dependencies were already declared from step 3).

Local verification: mvn -pl model -am test from jans-fido2/ — 5 tests, 0 failures, 0 errors, runtime ~0.075s.


Test and Document the changes

  • Static code analysis has been run locally and issues have been fixed
  • Relevant unit and integration tests have been added/updated
  • Relevant documentation has been updated if any (i.e. user guides, installation and configuration guides, technical design docs etc)

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.

  • I confirm that there is no impact on the docs due to the code changes in this PR.

Signed-off-by: imran <imranishaq7071@gmail.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 22, 2026

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: c57eede6-012f-468d-bd03-6e5d7c204536

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/jans-fido2-model-user-metrics-update-request-tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@mo-auto
Copy link
Copy Markdown
Member

mo-auto commented May 22, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mo-auto mo-auto added comp-jans-fido2 Component affected by issue or PR kind-feature Issue or PR is a new feature request labels May 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp-jans-fido2 Component affected by issue or PR kind-feature Issue or PR is a new feature request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(jans-fido2): add unit tests for UserMetricsUpdateRequest DTO

2 participants