[MS-1087] Add external CoSync biometric models #1487

BurningAXE · 2025-12-02T16:21:16Z

JIRA ticket
Will be released in: version

Root cause analysis (for bugfixes only)

First known affected version: version

Describe how the issue was found and what causes it
Include images to better illustrate the issue

Notable changes

Describe the main changes in the PR with the reasons why they are required
Include images to better illustrate the changes

Testing guidance

Describe how the reviewers can verify that issue is fixed

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)

Copilot

Pull request overview

This PR introduces a versioned external schema (V1) for CoSync biometric models, decoupling the external API contract from internal domain models. This enables stable data exchange with CommCare while allowing internal models to evolve independently.

Key Changes:

Created V1 external schema models with dedicated serialization/deserialization logic for backward and forward compatibility
Implemented conversion functions between internal domain models and V1 external schemas
Added comprehensive test coverage for serialization roundtrips and backward compatibility scenarios

Reviewed changes

Copilot reviewed 22 out of 22 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
`TokenizableStringV1.kt`	V1 external schema for tokenizable strings with custom serialization
`TokenizableStringV1Serialization.kt`	Custom Jackson serializers/deserializers supporting backward compatibility with plain strings
`SampleIdentifierV1.kt`	V1 external schema for fingerprint sample identifiers
`TemplateV1.kt`	V1 schemas for face and fingerprint biometric templates
`BiometricReferenceV1.kt`	V1 polymorphic schema for face and fingerprint references
`ExternalCredentialV1.kt`	V1 schema for external credentials (e.g., ID cards)
`ExternalCredentialTypeV1.kt`	V1 enum for external credential types
`CoSyncEnrolmentRecordEventsV1.kt`	Top-level V1 wrapper with schema version field
`CoSyncEnrolmentRecordEventV1.kt`	V1 polymorphic base for enrolment record events
`CoSyncEnrolmentRecordCreationEventV1.kt`	V1 schema for enrolment record creation events
`CoSyncEnrolmentRecordCreationEventV1Deserializer.kt`	Custom deserializer handling legacy plain string format
`EnrolmentRecordEvents.kt`	New internal domain wrapper for enrolment record events
`SerializationRoundtripTest.kt`	Tests for JSON serialization/deserialization roundtrips and backward compatibility
`CoSyncEnrolmentRecordEventsV1Test.kt`	Tests for V1 wrapper conversion logic
`CoSyncEnrolmentRecordCreationEventV1Test.kt`	Tests for creation event conversion and null handling
`CoSyncEnrolmentRecordCreationEventV1DeserializerTest.kt`	Tests for custom deserializer with various JSON formats
`BiometricReferenceV1Test.kt`	Tests for biometric reference conversion preserving metadata and identifiers
`CommCareEventDataSource.kt`	Updated to use V1 schemas with conversion to domain models
`CommCareIdentityDataSource.kt`	Updated to use V1 schemas with conversion to domain models
`GetEnrolmentCreationEventForSubjectUseCase.kt`	Updated to serialize using V1 external schema
`CoSyncEnrolmentRecordEvents.kt` (deleted)	Removed old unversioned external schema
`CoSyncEnrolmentRecordCreationEventDeserializer.kt` (deleted)	Replaced with V1-specific deserializer

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

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

Copilot · 2025-12-02T16:27:02Z

...src/main/java/com/simprints/infra/events/event/cosync/v1/TokenizableStringV1Serialization.kt

+/**
+ * JSON deserializer for [TokenizableStringV1] that handles backward compatibility.
+ *
+ * Any json object without the explicit className specification will be resolved
+ * as [TokenizableStringV1.Raw] for backward compatibility.
+ *
+ * Examples:
+ * { "className": "TokenizableString.Raw", "value": "person" }
+ *     -> TokenizableStringV1.Raw(value = "person")
+ *
+ * { "className": "TokenizableString.Tokenized", "value": "eq2Efc98d" }
+ *     -> TokenizableStringV1.Tokenized(value = "eq2Efc98d")
+ *
+ * { "className": "Something else", "value": "name" }
+ *     -> TokenizableStringV1.Raw(value = "name")
+ *
+ * { "value": "no class" }
+ *     -> TokenizableStringV1.Raw(value = "no class")
+ */


The documentation states "Any json object without the explicit className specification will be resolved as [TokenizableStringV1.Raw]" but the code on line 75 actually defaults to Raw for any className that is not "TokenizableString.Tokenized". Consider clarifying the documentation to specify that unknown className values (not just missing className) also default to Raw.

...main/java/com/simprints/infra/events/event/cosync/v1/CoSyncEnrolmentRecordCreationEventV1.kt

Copilot · 2025-12-02T16:27:03Z

infra/events/src/main/java/com/simprints/infra/events/event/cosync/v1/ExternalCredentialV1.kt

+import com.simprints.core.domain.externalcredential.ExternalCredential
+
+/**
+ * V1 external schema for external credentials (e.g., MFID).


The comment on line 8 says "V1 external schema for external credentials (e.g., MFID)." However, looking at the ExternalCredentialTypeV1 enum, the actual types are NHISCard, GhanaIdCard, and QRCode. Consider updating this example to use one of these actual types instead of "MFID" for clarity.

Suggested change

* V1 external schema for external credentials (e.g., MFID).

* V1 external schema for external credentials (e.g., GhanaIdCard).

Copilot · 2025-12-02T16:27:03Z

infra/events/src/main/java/com/simprints/infra/events/event/cosync/v1/TokenizableStringV1.kt

+
+        return other is TokenizableStringV1 && other.value == value
+    }
+
+    override fun hashCode(): Int = value.hashCode()


The equals() method (lines 45-49) compares TokenizableStringV1 instances only by their value field, ignoring whether they are Raw or Tokenized. This means TokenizableStringV1.Raw("test") equals TokenizableStringV1.Tokenized("test"), which may not be the intended behavior. Consider whether the type distinction should be part of the equality check to avoid potential bugs when comparing tokenized vs raw values.

Suggested change

return other is TokenizableStringV1 && other.value == value

}

override fun hashCode(): Int = value.hashCode()

if (other == null || this::class != other::class) return false

return (other as TokenizableStringV1).value == value

}

override fun hashCode(): Int = 31 * this::class.hashCode() + value.hashCode()

Copilot · 2025-12-02T16:27:04Z

infra/events/src/main/java/com/simprints/infra/events/event/cosync/v1/ExternalCredentialV1.kt

+ */
+fun ExternalCredentialV1.toDomain() = ExternalCredential(
+    id = id,
+    value = value.toDomain() as com.simprints.core.domain.tokenization.TokenizableString.Tokenized,


The toDomain() function casts value.toDomain() to TokenizableString.Tokenized. However, this cast could fail at runtime if the V1 value is not actually of type TokenizableStringV1.Tokenized. Since the ExternalCredentialV1.value field is already typed as TokenizableStringV1.Tokenized, this cast should be safe, but it's redundant. Consider removing the explicit cast since the type is already guaranteed by the field definition.

Suggested change

value = value.toDomain() as com.simprints.core.domain.tokenization.TokenizableString.Tokenized,

value = value.toDomain(),

Copilot · 2025-12-02T16:27:04Z

...m/simprints/infra/events/event/cosync/v1/CoSyncEnrolmentRecordCreationEventV1Deserializer.kt

+        val moduleId = try {
+            ctxt.readTreeAsValue(payload["moduleId"], TokenizableStringV1::class.java)
+        } catch (_: Exception) {
+            TokenizableStringV1.Raw(payload["moduleId"].asText())
+        }


The deserializer catches all exceptions with catch (_: Exception) when trying to parse TokenizableStringV1. While this provides backward compatibility, it silently swallows all exceptions including unexpected ones (like OutOfMemoryError or other serious issues). Consider catching more specific exceptions (e.g., JsonProcessingException or JsonMappingException) to avoid masking unexpected errors.

Copilot · 2025-12-02T16:27:04Z

...m/simprints/infra/events/event/cosync/v1/CoSyncEnrolmentRecordCreationEventV1Deserializer.kt

+        // Parse attendantId - try as TokenizableStringV1 first, fall back to plain string
+        val attendantId = try {
+            ctxt.readTreeAsValue(payload["attendantId"], TokenizableStringV1::class.java)
+        } catch (_: Exception) {
+            TokenizableStringV1.Raw(payload["attendantId"].asText())
+        }


The deserializer catches all exceptions with catch (_: Exception) when trying to parse TokenizableStringV1. While this provides backward compatibility, it silently swallows all exceptions including unexpected ones. Consider catching more specific exceptions (e.g., JsonProcessingException or JsonMappingException) to avoid masking unexpected errors.

sonarqubecloud · 2025-12-02T16:35:39Z

Quality Gate failed

Failed conditions
B Reliability Rating on New Code (required ≥ A)

See analysis details on SonarQube Cloud

Catch issues before they fail your Quality Gate with our IDE extension SonarQube for IDE

meladRaouf · 2025-12-04T11:58:27Z

.../java/com/simprints/feature/clientapi/usecases/GetEnrolmentCreationEventForSubjectUseCase.kt

        val coSyncSerializationModule = SimpleModule().apply {
-            addSerializer(TokenizableString::class.java, TokenizationClassNameSerializer())
-            addDeserializer(TokenizableString::class.java, TokenizationClassNameDeserializer())
+            addSerializer(TokenizableStringV1::class.java, TokenizableStringV1Serializer())


@BurningAXE I don't yet understand why we are exporting the TokenizableString class info to the calling apps

Because properties in the model are using it? :D If we strip it we'll have to guess about the tokenization state of those fields. Which is already the case with some historical data but does not need to continue.
Do you see any issues with it?

I just see that tokenization handling adds a lot of it complexity

cla-bot bot added the ... label Dec 2, 2025

BurningAXE requested a review from Copilot December 2, 2025 16:21

Copilot started reviewing on behalf of BurningAXE December 2, 2025 16:22 View session

Copilot finished reviewing on behalf of BurningAXE December 2, 2025 16:25

Copilot AI reviewed Dec 2, 2025

View reviewed changes

meladRaouf reviewed Dec 4, 2025

View reviewed changes

[MS-1087] Add external CoSync biometric models

5d17a34

BurningAXE force-pushed the MS-1087-Stable-CoSync-Models branch from 959bf83 to 5d17a34 Compare January 8, 2026 17:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[MS-1087] Add external CoSync biometric models #1487

[MS-1087] Add external CoSync biometric models #1487

Uh oh!

BurningAXE commented Dec 2, 2025

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 2, 2025

Uh oh!

Uh oh!

Copilot AI Dec 2, 2025

Uh oh!

Copilot AI Dec 2, 2025

Uh oh!

Copilot AI Dec 2, 2025

Uh oh!

Copilot AI Dec 2, 2025

Uh oh!

Copilot AI Dec 2, 2025

Uh oh!

sonarqubecloud bot commented Dec 2, 2025

Uh oh!

meladRaouf Dec 4, 2025

Uh oh!

BurningAXE Dec 4, 2025

Uh oh!

meladRaouf Dec 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	* V1 external schema for external credentials (e.g., MFID).
	* V1 external schema for external credentials (e.g., GhanaIdCard).

-        return other is TokenizableStringV1 && other.value == value
-    }
-    override fun hashCode(): Int = value.hashCode()
+        if (other == null || this::class != other::class) return false
+        return (other as TokenizableStringV1).value == value
+    }
+    override fun hashCode(): Int = 31 * this::class.hashCode() + value.hashCode()

	value = value.toDomain() as com.simprints.core.domain.tokenization.TokenizableString.Tokenized,
	value = value.toDomain(),

[MS-1087] Add external CoSync biometric models #1487

Are you sure you want to change the base?

[MS-1087] Add external CoSync biometric models #1487

Uh oh!

Conversation

BurningAXE commented Dec 2, 2025

Root cause analysis (for bugfixes only)

Notable changes

Testing guidance

Additional work checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

sonarqubecloud bot commented Dec 2, 2025

Quality Gate failed

Uh oh!

meladRaouf Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

BurningAXE Dec 4, 2025

Choose a reason for hiding this comment

Uh oh!

meladRaouf Dec 8, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants