Skip to content

MS-438 Prevent invalid person creation event #708

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
luhmirin-s merged 2 commits into from
May 27, 2024
Merged

MS-438 Prevent invalid person creation event #708

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f500b3
MS-438 Add capture event ID to the step result data
luhmirin-s May 7, 2024
20285c3
MS-438 Compose CreatePerson event only based on information from step…
luhmirin-s May 7, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions face/capture/src/main/java/com/simprints/face/capture/FaceCaptureResult.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ data class FaceCaptureResult(

@Keep
data class Item(
val captureEventId: String?,
val index: Int,
val sample: Sample?,
) : Serializable
Expand Down
15 changes: 7 additions & 8 deletions face/capture/src/main/java/com/simprints/face/capture/screens/FaceCaptureViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ internal class FaceCaptureViewModel @Inject constructor(
get() = _finishFlowEvent
private val _finishFlowEvent = MutableLiveData<LiveDataEventWithContent<FaceCaptureResult>>()


val invalidLicense: LiveData<LiveDataEvent>
get() = _invalidLicense
private val _invalidLicense = MutableLiveData<LiveDataEvent>()
Expand Down Expand Up @@ -106,12 +105,13 @@ internal class FaceCaptureViewModel @Inject constructor(

val items = faceDetections.mapIndexed { index, detection ->
FaceCaptureResult.Item(
index,
FaceCaptureResult.Sample(
detection.id,
detection.face?.template ?: ByteArray(0),
detection.securedImageRef,
detection.face?.format ?: "",
captureEventId = detection.id,
index = index,
sample = FaceCaptureResult.Sample(
faceId = detection.id,
template = detection.face?.template ?: ByteArray(0),
imageRef = detection.securedImageRef,
format = detection.face?.format ?: "",
)
)
}
Expand Down Expand Up @@ -159,5 +159,4 @@ internal class FaceCaptureViewModel @Inject constructor(
eventReporter.addCaptureConfirmationEvent(startTime, isContinue)
}


}
81 changes: 30 additions & 51 deletions ...tor/src/main/java/com/simprints/feature/orchestrator/usecases/CreatePersonEventUseCase.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,20 +5,16 @@ import com.simprints.core.domain.face.uniqueId
import com.simprints.core.domain.fingerprint.FingerprintSample
import com.simprints.core.domain.fingerprint.uniqueId
import com.simprints.core.tools.time.TimeHelper
import com.simprints.core.tools.utils.EncodingUtils
import com.simprints.face.capture.FaceCaptureResult
import com.simprints.fingerprint.capture.FingerprintCaptureResult
import com.simprints.infra.events.SessionEventRepository
import com.simprints.infra.events.event.domain.models.PersonCreationEvent
import com.simprints.infra.events.event.domain.models.face.FaceCaptureBiometricsEvent
import com.simprints.infra.events.event.domain.models.fingerprint.FingerprintCaptureBiometricsEvent
import java.io.Serializable
import javax.inject.Inject

internal class CreatePersonEventUseCase @Inject constructor(
private val eventRepository: SessionEventRepository,
private val timeHelper: TimeHelper,
private val encodingUtils: EncodingUtils,
) {

suspend operator fun invoke(results: List<Serializable>) {
Expand All @@ -27,70 +23,53 @@ internal class CreatePersonEventUseCase @Inject constructor(
// If a personCreationEvent is already in the current session,
// we don' want to add it again (the capture steps would still be the same)
if (sessionEvents.none { it is PersonCreationEvent }) {
val faceCaptureBiometricsEvents = sessionEvents.filterIsInstance<FaceCaptureBiometricsEvent>()
val fingerprintCaptureBiometricsEvents = sessionEvents.filterIsInstance<FingerprintCaptureBiometricsEvent>()
val faceCaptures = extractFaceCaptures(results)
val fingerprintCaptures = extractFingerprintCaptures(results)

val faceSamples = extractFaceSamples(results)
val fingerprintSamples = extractFingerprintSamples(results)
val personCreationEvent = build(faceCaptures, fingerprintCaptures)

val personCreationEvent = build(
faceCaptureBiometricsEvents,
fingerprintCaptureBiometricsEvents,
faceSamples,
fingerprintSamples
)
if (personCreationEvent.hasBiometricData()) {
Copy link
Copy Markdown
Contributor

@BurningAXE BurningAXE May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we log an error in the opposite case?

Copy link
Copy Markdown
Contributor Author

@luhmirin-s luhmirin-s May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could, but I am pretty sceptical that anyone will ever look into it.

eventRepository.addOrUpdateEvent(personCreationEvent)
}
}
}

private fun extractFaceSamples(responses: List<Serializable>) = responses
private fun extractFaceCaptures(responses: List<Serializable>) = responses
.filterIsInstance<FaceCaptureResult>()
.flatMap { it.results }
.mapNotNull { it.sample }
.map { FaceSample(it.template, it.format) }

private fun extractFingerprintSamples(responses: List<Serializable>) = responses
private fun extractFingerprintCaptures(responses: List<Serializable>) = responses
.filterIsInstance<FingerprintCaptureResult>()
.flatMap { it.results }
.mapNotNull { it.sample }
.map { FingerprintSample(it.fingerIdentifier, it.template, it.templateQualityScore, it.format) }

private fun build(
faceCaptureBiometricsEvents: List<FaceCaptureBiometricsEvent>,
fingerprintCaptureBiometricsEvents: List<FingerprintCaptureBiometricsEvent>,
faceSamplesForPersonCreation: List<FaceSample>?,
fingerprintSamplesForPersonCreation: List<FingerprintSample>?
) = PersonCreationEvent(
startTime = timeHelper.now(),
fingerprintCaptureIds = extractFingerprintCaptureEventIdsBasedOnPersonTemplate(
fingerprintCaptureBiometricsEvents,
fingerprintSamplesForPersonCreation?.map { encodingUtils.byteArrayToBase64(it.template) }
),
fingerprintReferenceId = fingerprintSamplesForPersonCreation?.uniqueId(),
faceCaptureIds = extractFaceCaptureEventIdsBasedOnPersonTemplate(
faceCaptureBiometricsEvents,
faceSamplesForPersonCreation?.map { encodingUtils.byteArrayToBase64(it.template) }
),
faceReferenceId = faceSamplesForPersonCreation?.uniqueId()
)
faceSamplesForPersonCreation: List<FaceCaptureResult.Item>,
fingerprintSamplesForPersonCreation: List<FingerprintCaptureResult.Item>,
): PersonCreationEvent {
val fingerprintCaptureIds = fingerprintSamplesForPersonCreation
.mapNotNull { it.captureEventId }
.ifEmpty { null }
val fingerprintReferenceId = fingerprintSamplesForPersonCreation
.mapNotNull { it.sample }
.map { FingerprintSample(it.fingerIdentifier, it.template, it.templateQualityScore, it.format) }
.uniqueId()

private fun extractFingerprintCaptureEventIdsBasedOnPersonTemplate(
captureEvents: List<FingerprintCaptureBiometricsEvent>,
personTemplates: List<String>?
): List<String>? = captureEvents
.filter { personTemplates?.contains(it.payload.fingerprint.template) == true }
.map { it.payload.id }
.ifEmpty { null }
val faceCaptureIds = faceSamplesForPersonCreation
.mapNotNull { it.captureEventId }
.ifEmpty { null }
val faceReferenceId = faceSamplesForPersonCreation
.mapNotNull { it.sample }
.map { FaceSample(it.template, it.format) }
.uniqueId()

private fun extractFaceCaptureEventIdsBasedOnPersonTemplate(
captureEvents: List<FaceCaptureBiometricsEvent>,
personTemplates: List<String>?
): List<String>? = captureEvents
.filter { personTemplates?.contains(it.payload.face.template) == true }
.map { it.payload.id }
.ifEmpty { null }
return PersonCreationEvent(
startTime = timeHelper.now(),
fingerprintCaptureIds = fingerprintCaptureIds,
fingerprintReferenceId = fingerprintReferenceId,
faceCaptureIds = faceCaptureIds,
faceReferenceId = faceReferenceId,
)
}

private fun PersonCreationEvent.hasBiometricData() =
payload.fingerprintCaptureIds?.isNotEmpty() == true || payload.faceCaptureIds?.isNotEmpty() == true
Expand Down
2 changes: 2 additions & 0 deletions ...rc/test/java/com/simprints/feature/orchestrator/cache/OrchestratorCacheIntegrationTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import com.simprints.feature.orchestrator.steps.Step
import com.simprints.feature.orchestrator.steps.StepId
import com.simprints.feature.orchestrator.steps.StepStatus
import com.simprints.fingerprint.capture.FingerprintCaptureResult
import com.simprints.infra.events.sampledata.SampleDefaults.GUID1
import com.simprints.infra.security.SecurityManager
import io.mockk.MockKAnnotations
import io.mockk.every
Expand Down Expand Up @@ -62,6 +63,7 @@ class OrchestratorCacheIntegrationTest {
result = FingerprintCaptureResult(
results = listOf(
FingerprintCaptureResult.Item(
captureEventId = GUID1,
identifier = IFingerIdentifier.LEFT_THUMB,
sample = null
)
Expand Down
58 changes: 27 additions & 31 deletions ...src/test/java/com/simprints/feature/orchestrator/usecases/CreatePersonEventUseCaseTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,14 @@
package com.simprints.feature.orchestrator.usecases

import com.google.common.truth.Truth.assertThat
import com.simprints.core.domain.fingerprint.IFingerIdentifier
import com.simprints.core.tools.time.TimeHelper
import com.simprints.core.tools.utils.EncodingUtils
import com.simprints.core.tools.time.Timestamp
import com.simprints.face.capture.FaceCaptureResult
import com.simprints.fingerprint.capture.FingerprintCaptureResult
import com.simprints.infra.events.SessionEventRepository
import com.simprints.infra.events.event.domain.models.PersonCreationEvent
import com.simprints.infra.events.event.domain.models.face.FaceCaptureBiometricsEvent
import com.simprints.infra.events.event.domain.models.face.FaceCaptureEvent
import com.simprints.infra.events.event.domain.models.fingerprint.FingerprintCaptureBiometricsEvent
import com.simprints.infra.events.event.domain.models.fingerprint.FingerprintCaptureEvent
import com.simprints.core.domain.fingerprint.IFingerIdentifier
import com.simprints.core.tools.time.Timestamp
import com.simprints.infra.events.SessionEventRepository
import com.simprints.testtools.common.coroutines.TestCoroutineRule
import io.mockk.MockKAnnotations
import io.mockk.coEvery
Expand All @@ -36,30 +32,24 @@ internal class CreatePersonEventUseCaseTest {
@MockK
lateinit var timeHelper: TimeHelper

@MockK
lateinit var encodingUtils: EncodingUtils

private lateinit var useCase: CreatePersonEventUseCase

@Before
fun setUp() {
MockKAnnotations.init(this, relaxed = true)

every { timeHelper.now() } returns Timestamp(0L)
every { encodingUtils.byteArrayToBase64(any()) } returns TEMPLATE

coEvery { eventRepository.getCurrentSessionScope() } returns mockk {
every { id } returns "sessionId"
}

useCase = CreatePersonEventUseCase(eventRepository, timeHelper, encodingUtils)
useCase = CreatePersonEventUseCase(eventRepository, timeHelper)
}

@Test
fun `Does not create event if has person creation in session`() = runTest {
coEvery { eventRepository.getEventsInCurrentSession() } returns listOf(
mockk<FingerprintCaptureBiometricsEvent>(),
mockk<FaceCaptureBiometricsEvent>(),
mockk<PersonCreationEvent>(),
)

Expand All @@ -70,10 +60,7 @@ internal class CreatePersonEventUseCaseTest {

@Test
fun `Does not create event if no biometric data`() = runTest {
Copy link
Copy Markdown
Contributor

@BurningAXE BurningAXE May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need more tests to cover the case that is prevented with this PR?

Copy link
Copy Markdown
Contributor Author

@luhmirin-s luhmirin-s May 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The issue is prevented by removing the repo and only using the step result data. I think the current tests cover that; IIRC, there are some extra checks in places where the data is added to step results.

coEvery { eventRepository.getEventsInCurrentSession() } returns listOf(
mockk<FingerprintCaptureEvent>(),
mockk<FaceCaptureEvent>(),
)
coEvery { eventRepository.getEventsInCurrentSession() } returns listOf()

useCase(listOf())

Expand All @@ -82,18 +69,13 @@ internal class CreatePersonEventUseCaseTest {

@Test
fun `Create event if there is face biometric data`() = runTest {
coEvery { eventRepository.getEventsInCurrentSession() } returns listOf(
mockk<FaceCaptureBiometricsEvent> {
every { payload.id } returns "eventFaceId1"
every { payload.face.template } returns TEMPLATE
},
)
coEvery { eventRepository.getEventsInCurrentSession() } returns listOf()

useCase(listOf(FaceCaptureResult(listOf(createFaceCaptureResultItem()))))

coVerify {
eventRepository.addOrUpdateEvent(withArg<PersonCreationEvent> {
assertThat(it.payload.faceCaptureIds).isEqualTo(listOf("eventFaceId1"))
assertThat(it.payload.faceCaptureIds).isEqualTo(listOf(FACE_ID))
})
}
}
Expand All @@ -102,7 +84,7 @@ internal class CreatePersonEventUseCaseTest {
fun `Create event if there is fingerprint biometric data`() = runTest {
coEvery { eventRepository.getEventsInCurrentSession() } returns listOf(
mockk<FingerprintCaptureBiometricsEvent> {
every { payload.id } returns "eventFinger1"
every { payload.id } returns FINGER_ID
every { payload.fingerprint.template } returns TEMPLATE
},
)
Expand All @@ -111,22 +93,36 @@ internal class CreatePersonEventUseCaseTest {

coVerify {
eventRepository.addOrUpdateEvent(withArg<PersonCreationEvent> {
assertThat(it.payload.fingerprintCaptureIds).isEqualTo(listOf("eventFinger1"))
assertThat(it.payload.fingerprintCaptureIds).isEqualTo(listOf(FINGER_ID))
})
}
}

private fun createFingerprintCaptureResultItem() = FingerprintCaptureResult.Item(
IFingerIdentifier.RIGHT_THUMB,
FingerprintCaptureResult.Sample(IFingerIdentifier.RIGHT_THUMB, byteArrayOf(), 0, null, "format")
captureEventId = FINGER_ID,
identifier = IFingerIdentifier.RIGHT_THUMB,
sample = FingerprintCaptureResult.Sample(
IFingerIdentifier.RIGHT_THUMB,
TEMPLATE.toByteArray(),
0,
null,
"format"
)
)


private fun createFaceCaptureResultItem() =
FaceCaptureResult.Item(0, FaceCaptureResult.Sample("faceId", byteArrayOf(), null, "format"))
FaceCaptureResult.Item(
captureEventId = FACE_ID,
index = 0,
sample = FaceCaptureResult.Sample(FACE_ID, TEMPLATE.toByteArray(), null, "format")
)


companion object {

const val TEMPLATE = "template"
private const val TEMPLATE = "template"
private const val FINGER_ID = "eventFinger1"
private const val FACE_ID = "eventFinger1"
}
}
38 changes: 23 additions & 15 deletions ...a/com/simprints/feature/orchestrator/usecases/MapStepsForLastBiometricEnrolUseCaseTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import com.simprints.infra.config.store.models.Finger
import com.simprints.matcher.FaceMatchResult
import com.simprints.matcher.FingerprintMatchResult
import com.simprints.core.domain.fingerprint.IFingerIdentifier
import com.simprints.infra.events.sampledata.SampleDefaults.GUID1
import org.junit.Before
import org.junit.Test
import java.io.Serializable
Expand Down Expand Up @@ -48,13 +49,17 @@ internal class MapStepsForLastBiometricEnrolUseCaseTest {
val result = useCase(listOf(
FaceCaptureResult(
results = listOf(
FaceCaptureResult.Item(0, null),
FaceCaptureResult.Item(0, FaceCaptureResult.Sample(
faceId = "faceId",
template = byteArrayOf(),
imageRef = null,
format = "format"
))
FaceCaptureResult.Item(captureEventId = null, index = 0, sample = null),
FaceCaptureResult.Item(
captureEventId = GUID1,
index = 0,
sample = FaceCaptureResult.Sample(
faceId = "faceId",
template = byteArrayOf(),
imageRef = null,
format = "format"
)
)
),
)
))
Expand All @@ -81,15 +86,18 @@ internal class MapStepsForLastBiometricEnrolUseCaseTest {
val result = useCase(listOf(
FingerprintCaptureResult(
results = listOf(
FingerprintCaptureResult.Item(IFingerIdentifier.LEFT_THUMB, null),
FingerprintCaptureResult.Item(null, IFingerIdentifier.LEFT_THUMB, null),
FingerprintCaptureResult.Item(
IFingerIdentifier.RIGHT_THUMB, FingerprintCaptureResult.Sample(
fingerIdentifier = IFingerIdentifier.RIGHT_THUMB,
template = byteArrayOf(),
templateQualityScore = 0,
imageRef = null,
format = "format"
))
identifier = IFingerIdentifier.RIGHT_THUMB,
captureEventId = GUID1,
sample = FingerprintCaptureResult.Sample(
fingerIdentifier = IFingerIdentifier.RIGHT_THUMB,
template = byteArrayOf(),
templateQualityScore = 0,
imageRef = null,
format = "format"
)
)
),
)
))
Expand Down
1 change: 1 addition & 0 deletions ...print/capture/src/main/java/com/simprints/fingerprint/capture/FingerprintCaptureResult.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ data class FingerprintCaptureResult(

@Keep
data class Item(
val captureEventId: String?,
val identifier: IFingerIdentifier,
val sample: Sample?,
) : Serializable
Expand Down
Loading