MS-1135 Simplifying module APIs with common domain models by luhmirin-s · Pull Request #1464 · Simprints/Android-Simprints-ID

luhmirin-s · 2025-11-13T09:58:20Z

JIRA ticket
Will be released in: 2026.1.0

Notable changes

This change introduces a set of modality-independent common data classes that should be reused across modules:
- Modality enum
- ModalitySdkType marker for the per-modality bio SDK enums
- SampleIdentifier enum (formerly IFingerprint and copies)
- Sample and Identity - template and metadata tied to a subject
- CaptureSample and CaptureIdentity - template and metadata not yet tied to a subject
- MatchConfidence - generic matching result for a specific subject

Testing guidance

Run all kinds of basic flows. Everything should work exactly as before the changes.

Additional work checklist

Effect on other features and security has been considered
Design document marked as "In development" (if applicable)
External (Gitbook) and internal (Confluence) Documentation is up to date (or ticket created)
Test cases in Testiny are up to date (or ticket created)
Other teams notified about the changes (if applicable)

BurningAXE · 2025-11-18T14:47:23Z

I've been advocating for some time now to get rid of the wrong (and therefore confusing) usage of "sample" in SID's code. It looks like now is the perfect time to tackle this!
According to biometric terminology:

A biometric sample (3.3.21) is an analog or digital representation of biometric characteristics prior to biometric feature extraction. E.g.: an image of a fingerprint.

We already use the correct terminology for uploading images in the new API. Which brings even more confusion when we use the same term for what is essentially a template with metadata:

A biometric template is set of stored biometric features comparable directly to probe biometric features. E.g.: an ISO fingerprint template.

So my suggestion is to replace the term "sample" with "template":
Sample -> Template
CaptureSample -> CaptureTemplate
SampleIdentifier -> TemplateIdentifier

Since we already have a template property inside the current Sample, to avoid having Template.template we can rename it to templateData.

What do you think? IMO it's straight-forward enough but open to transferring the discussion to a DD if deemed a serious and controversial change.

Copilot

Pull Request Overview

This PR refactors the codebase to introduce modality-independent common data classes that are reused across modules, simplifying APIs and improving maintainability. The changes consolidate previously scattered modality-specific types into unified domain models.

Key Changes:

Introduced unified Sample, Identity, CaptureSample, CaptureIdentity, and MatchConfidence classes that work across modalities
Replaced IFingerIdentifier with SampleIdentifier enum (adding NONE value for non-fingerprint samples)
Consolidated Modality enum from GeneralConfiguration.Modality and Modes into com.simprints.core.domain.common.Modality
Removed modality-specific identity classes (FingerprintIdentity, FaceIdentity) in favor of generic Identity
Removed modality-specific sample classes (FingerprintSample, FaceSample) in favor of generic Sample

Reviewed Changes

Copilot reviewed 229 out of 229 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
`infra/core/src/main/java/com/simprints/core/domain/sample/`	New common domain models: `Sample`, `Identity`, `CaptureSample`, `CaptureIdentity`, `MatchConfidence`, `SampleIdentifier`
`infra/core/src/main/java/com/simprints/core/domain/common/`	New `Modality` enum and `ModalitySdkType` marker interface
`infra/matching/src/main/java/com/simprints/infra/matching/`	Updated matching infrastructure to use common `MatchConfidence` and unified sample types
`infra/enrolment-records/repository/`	Replaced modality-specific identity/sample types with generic `Identity`/`Sample` throughout repository layer
`infra/config-store/src/main/java/com/simprints/infra/config/store/models/`	Updated configuration models to use `Modality` and `SampleIdentifier`; removed old `Finger` enum
`infra/event-sync/`, `infra/events/`	Updated event models and sync logic to use new common types
`fingerprint/infra/*/matching/`	Updated fingerprint matching implementations to work with new generic sample types
`testing/data-generator/`, `infra/images/`	Updated test utilities and image handling to use new common types

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

BurningAXE

No objections on the change per-se but I'd really like to see us move away from the Sample terminology for templates.

BurningAXE · 2025-11-18T15:29:40Z

            )
            actualFingerprints?.results?.forEach {
-                assertThat(it.sample?.template).isEqualTo(TEMPLATE)
-                assertThat(it.sample?.imageRef).isNotNull()


imageRef no more present?

It was never used downstream.

BurningAXE · 2025-11-18T16:10:43Z

-    ) : MatchResultItem
-}
+data class MatchResult(
+    val results: List<MatchConfidence>,


OK, I see it now - doesn't make sense to have MatchResult inside MatchResult. I guess MatchResultItem was not that bad a name. Or maybe MatchComparison/MatchResultComparison?

Yeah, this was the initial reason for the change. Also, there is "MatchResult" in the StdLib, which sneaks into the imports all the time.

Will change it to MatchComparisonResult to be the most explicit.

luhmirin-s · 2025-11-19T08:27:41Z

What do you think? IMO it's straight-forward enough but open to transferring the discussion to a DD if deemed a serious and controversial change.

@BurningAXE While I agree in principle and it seems straightforward, I would prefer discussing with the rest of the team in a DD first. This change is already substantial, and I have tried to maintain the same/similar terminology as much as possible.

Update: create a ticket to work on the DD - https://simprints.atlassian.net/browse/MS-1257

…le identifier

…n classes

sonarqubecloud · 2025-11-19T09:30:08Z

Quality Gate passed

Issues
12 New issues
0 Accepted issues

Measures
0 Security Hotspots
82.0% Coverage on New Code
0.2% Duplication on New Code

See analysis details on SonarQube Cloud

alexandr-simprints · 2025-11-19T09:30:22Z

-        .filterIsInstance<EnrolLastBiometricStepResult.FingerprintMatchResult>()
-        .lastOrNull()
+        .filterIsInstance<EnrolLastBiometricStepResult.MatchResult>()
+        .lastOrNull { it.sdk is FingerprintConfiguration.BioSdk }


Getting specific modality match results based on SDK seems a little bit out of place. Is there a more obvious way to do that? For instance, keeping the 'result' type (Face, Finger) in enum

There can be a helper function that returns the modality of a SDK so you can then filter by modality

alexandr-simprints · 2025-11-19T09:38:49Z

-    } ?: emptyList()
+    ) = results
+        .filterIsInstance<MatchResult>()
+        .lastOrNull { it.sdk is FingerprintConfiguration.BioSdk }


Same comment as above - shall we at least create an extension function for such filtering? Getting modality results based on SDK types classes if non really obvious

Both of those cases get refactored away in the next PR, since this one is mostly about consolidating data models.

There is no need for those helper functions since, in most cases, business logic should rely on generic configuration for the specific SDK instead of modality.

cla-bot Bot added the ... label Nov 13, 2025

luhmirin-s marked this pull request as ready for review November 13, 2025 15:14

luhmirin-s requested review from a team, BurningAXE, TristramN, alex-vt, alexandr-simprints, meladRaouf and ybourgery and removed request for a team November 13, 2025 15:14

BurningAXE requested a review from Copilot November 18, 2025 15:06

Copilot started reviewing on behalf of BurningAXE November 18, 2025 15:06 View session

Copilot finished reviewing on behalf of BurningAXE November 18, 2025 15:07

Copilot AI reviewed Nov 18, 2025

View reviewed changes

BurningAXE approved these changes Nov 18, 2025

View reviewed changes

luhmirin-s added 9 commits November 19, 2025 10:32

MS-1135 Move mobility enum to core.domain from configuration

a92ce79

MS-1135 Replace duplicated finger identifier enums with a common samp…

158e582

…le identifier

MS-1135 Replace FaceSample and FingerprintSample with single data class

ddd9afb

MS-1135 Simplify capture result to use common data model

5ee24ff

MS-1135 Simplify match module parameters with CaptureSample

7806807

MS-1135 Simplify enrol-last capture data models to reuse common domai…

9ae848a

…n classes

MS-1135 Simplify matching module result with abstract bio sdk type

7553533

MS-1135 Use common domain models in matching interfaces

db8d1a1

MS-1135 Simplify capture step responses with a common data model

01387de

luhmirin-s force-pushed the feature/MS-1135-simplifying-module-api branch from 59b38bf to 1650d12 Compare November 19, 2025 08:44

MS-1135 Rename MatchConfidence to MatchComparisonResult

b303c4d

luhmirin-s force-pushed the feature/MS-1135-simplifying-module-api branch from 1650d12 to b303c4d Compare November 19, 2025 09:14

alexandr-simprints approved these changes Nov 19, 2025

View reviewed changes

meladRaouf approved these changes Nov 19, 2025

View reviewed changes

luhmirin-s merged commit d6b4c8b into main Nov 20, 2025
13 checks passed

luhmirin-s deleted the feature/MS-1135-simplifying-module-api branch November 20, 2025 08:12

Conversation

luhmirin-s commented Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Notable changes

Testing guidance

Additional work checklist

Uh oh!

BurningAXE commented Nov 18, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

BurningAXE left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

luhmirin-s commented Nov 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud Bot commented Nov 19, 2025

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

luhmirin-s commented Nov 13, 2025 •

edited

Loading

luhmirin-s commented Nov 19, 2025 •

edited

Loading