From c4fbd5b3af7279e60efe4cba9ccef0d8683b4271 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 7 Jul 2025 15:17:22 +0300 Subject: [PATCH 1/9] MS-1058 Rename existing image remote data source to Firestore uploader --- .../infra/images/ImageRepositoryImpl.kt | 6 ++-- .../simprints/infra/images/ImagesModule.kt | 6 ++-- ...eRemoteDataSource.kt => SampleUploader.kt} | 4 +-- .../FirestoreSampleUploader.kt} | 10 ++++--- .../infra/images/ImageRepositoryImplTest.kt | 18 +++++------ .../FirestoreSampleUploaderTest.kt} | 30 +++++++++++-------- 6 files changed, 38 insertions(+), 36 deletions(-) rename infra/images/src/main/java/com/simprints/infra/images/remote/{ImageRemoteDataSource.kt => SampleUploader.kt} (89%) rename infra/images/src/main/java/com/simprints/infra/images/remote/{ImageRemoteDataSourceImpl.kt => firestore/FirestoreSampleUploader.kt} (88%) rename infra/images/src/test/java/com/simprints/infra/images/remote/{ImageRemoteDataSourceImplTest.kt => firestore/FirestoreSampleUploaderTest.kt} (78%) diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt index 98d39ba1d9..e1c66e7117 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt @@ -4,14 +4,14 @@ import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef -import com.simprints.infra.images.remote.ImageRemoteDataSource +import com.simprints.infra.images.remote.SampleUploader import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber import javax.inject.Inject internal class ImageRepositoryImpl @Inject internal constructor( private val localDataSource: ImageLocalDataSource, - private val remoteDataSource: ImageRemoteDataSource, + private val sampleUploader: SampleUploader, private val metadataStore: ImageMetadataStore, ) : ImageRepository { override suspend fun storeImageSecurely( @@ -34,7 +34,7 @@ internal class ImageRepositoryImpl @Inject internal constructor( try { localDataSource.decryptImage(imageRef)?.let { stream -> val metadata = metadataStore.getMetadata(imageRef.relativePath) - val uploadResult = remoteDataSource.uploadImage(stream, imageRef, metadata) + val uploadResult = sampleUploader.uploadSample(stream, imageRef, metadata) if (uploadResult.isUploadSuccessful()) { localDataSource.deleteImage(imageRef) metadataStore.deleteMetadata(imageRef.relativePath) diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImagesModule.kt b/infra/images/src/main/java/com/simprints/infra/images/ImagesModule.kt index 493ec9a417..00e3a8181c 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImagesModule.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImagesModule.kt @@ -5,8 +5,8 @@ import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.local.ImageLocalDataSourceImpl import com.simprints.infra.images.metadata.database.ImageMetadataDao import com.simprints.infra.images.metadata.database.ImageMetadataDatabase -import com.simprints.infra.images.remote.ImageRemoteDataSource -import com.simprints.infra.images.remote.ImageRemoteDataSourceImpl +import com.simprints.infra.images.remote.SampleUploader +import com.simprints.infra.images.remote.firestore.FirestoreSampleUploader import dagger.Binds import dagger.Module import dagger.Provides @@ -25,7 +25,7 @@ abstract class ImagesModule { internal abstract fun bindImageLocalDataSource(impl: ImageLocalDataSourceImpl): ImageLocalDataSource @Binds - internal abstract fun bindImageRemoteDataSource(impl: ImageRemoteDataSourceImpl): ImageRemoteDataSource + internal abstract fun bindImageRemoteDataSource(impl: FirestoreSampleUploader): SampleUploader } @Module diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/ImageRemoteDataSource.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/SampleUploader.kt similarity index 89% rename from infra/images/src/main/java/com/simprints/infra/images/remote/ImageRemoteDataSource.kt rename to infra/images/src/main/java/com/simprints/infra/images/remote/SampleUploader.kt index 4eb9a9e3e5..567374649d 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/ImageRemoteDataSource.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/SampleUploader.kt @@ -6,7 +6,7 @@ import java.io.FileInputStream /** * Interface for remote image file operations */ -internal interface ImageRemoteDataSource { +internal interface SampleUploader { /** * Uploads an image * @@ -17,7 +17,7 @@ internal interface ImageRemoteDataSource { * @return the result of the operation. * @see [UploadResult] */ - suspend fun uploadImage( + suspend fun uploadSample( imageStream: FileInputStream, imageRef: SecuredImageRef, metadata: Map, diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/ImageRemoteDataSourceImpl.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt similarity index 88% rename from infra/images/src/main/java/com/simprints/infra/images/remote/ImageRemoteDataSourceImpl.kt rename to infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt index bc0a61a85c..9de66ee9dc 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/ImageRemoteDataSourceImpl.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt @@ -1,20 +1,22 @@ -package com.simprints.infra.images.remote +package com.simprints.infra.images.remote.firestore import com.google.firebase.storage.FirebaseStorage import com.google.firebase.storage.StorageMetadata import com.simprints.infra.authstore.AuthStore import com.simprints.infra.config.sync.ConfigManager import com.simprints.infra.images.model.SecuredImageRef +import com.simprints.infra.images.remote.SampleUploader +import com.simprints.infra.images.remote.UploadResult import com.simprints.infra.logging.Simber import kotlinx.coroutines.tasks.await import java.io.FileInputStream import javax.inject.Inject -internal class ImageRemoteDataSourceImpl @Inject constructor( +internal class FirestoreSampleUploader @Inject constructor( private val configManager: ConfigManager, private val authStore: AuthStore, -) : ImageRemoteDataSource { - override suspend fun uploadImage( +) : SampleUploader { + override suspend fun uploadSample( imageStream: FileInputStream, imageRef: SecuredImageRef, metadata: Map, diff --git a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt index bc228ffbe3..dbd1e9a5da 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt @@ -1,19 +1,15 @@ package com.simprints.infra.images -import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth.* import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef -import com.simprints.infra.images.remote.ImageRemoteDataSource +import com.simprints.infra.images.remote.SampleUploader import com.simprints.infra.images.remote.UploadResult import com.simprints.testtools.common.coroutines.TestCoroutineRule -import io.mockk.MockKAnnotations -import io.mockk.coEvery -import io.mockk.coJustRun -import io.mockk.coVerify +import io.mockk.* import io.mockk.impl.annotations.MockK -import io.mockk.mockk import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Rule @@ -28,7 +24,7 @@ internal class ImageRepositoryImplTest { lateinit var localDataSource: ImageLocalDataSource @MockK - lateinit var remoteDataSource: ImageRemoteDataSource + lateinit var sampleUploader: SampleUploader @MockK lateinit var metadataStore: ImageMetadataStore @@ -38,7 +34,7 @@ internal class ImageRepositoryImplTest { @Before fun setUp() { MockKAnnotations.init(this) - repository = ImageRepositoryImpl(localDataSource, remoteDataSource, metadataStore) + repository = ImageRepositoryImpl(localDataSource, sampleUploader, metadataStore) initValidImageMocks() } @@ -144,7 +140,7 @@ internal class ImageRepositoryImplTest { coJustRun { metadataStore.deleteAllMetadata() } coEvery { - remoteDataSource.uploadImage(mockStream, validImage, emptyMap()) + sampleUploader.uploadSample(mockStream, validImage, emptyMap()) } returns UploadResult( validImage, UploadResult.Status.SUCCESSFUL, @@ -160,7 +156,7 @@ internal class ImageRepositoryImplTest { } returns mockStream coEvery { - remoteDataSource.uploadImage(mockStream, invalidImage, emptyMap()) + sampleUploader.uploadSample(mockStream, invalidImage, emptyMap()) } returns UploadResult( invalidImage, UploadResult.Status.FAILED, diff --git a/infra/images/src/test/java/com/simprints/infra/images/remote/ImageRemoteDataSourceImplTest.kt b/infra/images/src/test/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploaderTest.kt similarity index 78% rename from infra/images/src/test/java/com/simprints/infra/images/remote/ImageRemoteDataSourceImplTest.kt rename to infra/images/src/test/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploaderTest.kt index 7434920494..d1b3206ea3 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/remote/ImageRemoteDataSourceImplTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploaderTest.kt @@ -1,7 +1,7 @@ -package com.simprints.infra.images.remote +package com.simprints.infra.images.remote.firestore import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.google.common.truth.Truth.assertThat +import com.google.common.truth.Truth import com.google.firebase.storage.FirebaseStorage import com.simprints.infra.authstore.AuthStore import com.simprints.infra.config.sync.ConfigManager @@ -23,7 +23,7 @@ import java.io.FileInputStream @Suppress("DEPRECATION") @RunWith(AndroidJUnit4::class) -class ImageRemoteDataSourceImplTest { +class FirestoreSampleUploaderTest { @MockK private lateinit var configManager: ConfigManager @@ -36,7 +36,7 @@ class ImageRemoteDataSourceImplTest { @MockK private lateinit var mockSecuredImageRef: SecuredImageRef - private lateinit var remoteDataSource: ImageRemoteDataSourceImpl + private lateinit var remoteDataSource: FirestoreSampleUploader @Before fun setup() { @@ -44,7 +44,7 @@ class ImageRemoteDataSourceImplTest { every { mockSecuredImageRef.relativePath.parts } returns arrayOf("Test1") - remoteDataSource = ImageRemoteDataSourceImpl(configManager, authStore) + remoteDataSource = FirestoreSampleUploader(configManager, authStore) // We need to mock statics and global extensions mockkStatic(FirebaseStorage::class) @@ -68,9 +68,9 @@ class ImageRemoteDataSourceImplTest { every { FirebaseStorage.getInstance(any(), any()) } returns storageMock - val result = remoteDataSource.uploadImage(mockImageStream, mockSecuredImageRef, emptyMap()) + val result = remoteDataSource.uploadSample(mockImageStream, mockSecuredImageRef, emptyMap()) - assertThat(result.isUploadSuccessful()).isTrue() + Truth.assertThat(result.isUploadSuccessful()).isTrue() } @Test @@ -83,18 +83,22 @@ class ImageRemoteDataSourceImplTest { every { FirebaseStorage.getInstance(any(), any()) } returns storageMock - val result = remoteDataSource.uploadImage(mockImageStream, mockSecuredImageRef, mapOf("key" to "value")) + val result = remoteDataSource.uploadSample( + mockImageStream, + mockSecuredImageRef, + mapOf("key" to "value"), + ) - assertThat(result.isUploadSuccessful()).isTrue() + Truth.assertThat(result.isUploadSuccessful()).isTrue() } @Test fun `null project returns failed upload`() = runTest { every { authStore.getLegacyAppFallback().options.projectId } returns null - val result = remoteDataSource.uploadImage(mockImageStream, mockSecuredImageRef, emptyMap()) + val result = remoteDataSource.uploadSample(mockImageStream, mockSecuredImageRef, emptyMap()) - assertThat(result.isUploadSuccessful()).isFalse() + Truth.assertThat(result.isUploadSuccessful()).isFalse() } @Test @@ -102,9 +106,9 @@ class ImageRemoteDataSourceImplTest { coEvery { configManager.getProject(any()).imageBucket } returns "" every { authStore.getLegacyAppFallback().options.projectId } returns "projectId" - val result = remoteDataSource.uploadImage(mockImageStream, mockSecuredImageRef, emptyMap()) + val result = remoteDataSource.uploadSample(mockImageStream, mockSecuredImageRef, emptyMap()) - assertThat(result.isUploadSuccessful()).isFalse() + Truth.assertThat(result.isUploadSuccessful()).isFalse() } private fun setupStorageMock() = mockk(relaxed = true) { From b6fe4f14edc8deba2e71251de44242533a33bbee Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Wed, 9 Jul 2025 15:18:46 +0300 Subject: [PATCH 2/9] MS-1058 Move file path creation to image repo to ensure consistency per modality --- .../capture/screens/FaceCaptureViewModel.kt | 4 +- .../capture/usecases/SaveFaceImageUseCase.kt | 53 ------------------ .../capture/usecases/SaveFaceSampleUseCase.kt | 32 +++++++++++ .../screens/FaceCaptureViewModelTest.kt | 4 +- ...seTest.kt => SaveFaceSampleUseCaseTest.kt} | 25 +++------ .../screen/FingerprintCaptureViewModel.kt | 4 +- ...ase.kt => SaveFingerprintSampleUseCase.kt} | 55 ++++++------------- .../screen/FingerprintCaptureViewModelTest.kt | 20 +++---- ...kt => SaveFingerprintSampleUseCaseTest.kt} | 24 +++----- .../simprints/infra/images/ImageRepository.kt | 19 ++++--- .../infra/images/ImageRepositoryImpl.kt | 42 +++++++++++--- .../images/usecase/CreateSamplePathUseCase.kt | 27 +++++++++ .../infra/images/ImageRepositoryImplTest.kt | 6 +- .../usecase/CreateSamplePathUseCaseTest.kt | 42 ++++++++++++++ 14 files changed, 199 insertions(+), 158 deletions(-) delete mode 100644 face/capture/src/main/java/com/simprints/face/capture/usecases/SaveFaceImageUseCase.kt create mode 100644 face/capture/src/main/java/com/simprints/face/capture/usecases/SaveFaceSampleUseCase.kt rename face/capture/src/test/java/com/simprints/face/capture/usecases/{SaveFaceImageUseCaseTest.kt => SaveFaceSampleUseCaseTest.kt} (76%) rename fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/{SaveImageUseCase.kt => SaveFingerprintSampleUseCase.kt} (64%) rename fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/usecase/{SaveImageUseCaseTest.kt => SaveFingerprintSampleUseCaseTest.kt} (86%) create mode 100644 infra/images/src/main/java/com/simprints/infra/images/usecase/CreateSamplePathUseCase.kt create mode 100644 infra/images/src/test/java/com/simprints/infra/images/usecase/CreateSamplePathUseCaseTest.kt diff --git a/face/capture/src/main/java/com/simprints/face/capture/screens/FaceCaptureViewModel.kt b/face/capture/src/main/java/com/simprints/face/capture/screens/FaceCaptureViewModel.kt index e8c309b1e3..4797939f2b 100644 --- a/face/capture/src/main/java/com/simprints/face/capture/screens/FaceCaptureViewModel.kt +++ b/face/capture/src/main/java/com/simprints/face/capture/screens/FaceCaptureViewModel.kt @@ -14,7 +14,7 @@ import com.simprints.face.capture.FaceCaptureResult import com.simprints.face.capture.models.FaceDetection import com.simprints.face.capture.usecases.BitmapToByteArrayUseCase import com.simprints.face.capture.usecases.IsUsingAutoCaptureUseCase -import com.simprints.face.capture.usecases.SaveFaceImageUseCase +import com.simprints.face.capture.usecases.SaveFaceSampleUseCase import com.simprints.face.capture.usecases.ShouldShowInstructionsScreenUseCase import com.simprints.face.capture.usecases.SimpleCaptureEventReporter import com.simprints.face.infra.biosdkresolver.ResolveFaceBioSdkUseCase @@ -47,7 +47,7 @@ import javax.inject.Inject internal class FaceCaptureViewModel @Inject constructor( private val authStore: AuthStore, private val configManager: ConfigManager, - private val saveFaceImage: SaveFaceImageUseCase, + private val saveFaceImage: SaveFaceSampleUseCase, private val eventReporter: SimpleCaptureEventReporter, private val bitmapToByteArray: BitmapToByteArrayUseCase, private val licenseRepository: LicenseRepository, diff --git a/face/capture/src/main/java/com/simprints/face/capture/usecases/SaveFaceImageUseCase.kt b/face/capture/src/main/java/com/simprints/face/capture/usecases/SaveFaceImageUseCase.kt deleted file mode 100644 index d5a34b7a38..0000000000 --- a/face/capture/src/main/java/com/simprints/face/capture/usecases/SaveFaceImageUseCase.kt +++ /dev/null @@ -1,53 +0,0 @@ -package com.simprints.face.capture.usecases - -import com.simprints.infra.events.session.SessionEventRepository -import com.simprints.infra.images.ImageRepository -import com.simprints.infra.images.model.Path -import com.simprints.infra.images.model.SecuredImageRef -import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FACE_CAPTURE -import com.simprints.infra.logging.Simber -import javax.inject.Inject - -internal class SaveFaceImageUseCase @Inject constructor( - private val coreImageRepository: ImageRepository, - private val sessionEventRepository: SessionEventRepository, -) { - suspend operator fun invoke( - imageBytes: ByteArray, - captureEventId: String, - ): SecuredImageRef? = determinePath(captureEventId)?.let { path -> - Simber.d("Saving face image ${path.compose()}", tag = FACE_CAPTURE) - val sessionScope = sessionEventRepository.getCurrentSessionScope() - val projectId = sessionScope.projectId - val securedImageRef = - coreImageRepository.storeImageSecurely(imageBytes, projectId, path) - - if (securedImageRef != null) { - SecuredImageRef(securedImageRef.relativePath) - } else { - Simber.i("Saving image failed for captureId $captureEventId", tag = FACE_CAPTURE) - null - } - } - - private suspend fun determinePath(captureEventId: String): Path? = try { - val sessionScope = sessionEventRepository.getCurrentSessionScope() - val sessionId = sessionScope.id - Path( - arrayOf( - SESSIONS_PATH, - sessionId, - FACES_PATH, - "$captureEventId.jpg", - ), - ) - } catch (t: Throwable) { - Simber.e("Error determining path for captureId=$captureEventId", t, tag = FACE_CAPTURE) - null - } - - companion object { - const val SESSIONS_PATH = "sessions" - const val FACES_PATH = "faces" - } -} diff --git a/face/capture/src/main/java/com/simprints/face/capture/usecases/SaveFaceSampleUseCase.kt b/face/capture/src/main/java/com/simprints/face/capture/usecases/SaveFaceSampleUseCase.kt new file mode 100644 index 0000000000..f6ea99b859 --- /dev/null +++ b/face/capture/src/main/java/com/simprints/face/capture/usecases/SaveFaceSampleUseCase.kt @@ -0,0 +1,32 @@ +package com.simprints.face.capture.usecases + +import com.simprints.infra.config.store.models.GeneralConfiguration +import com.simprints.infra.events.session.SessionEventRepository +import com.simprints.infra.images.ImageRepository +import com.simprints.infra.images.model.SecuredImageRef +import javax.inject.Inject + +internal class SaveFaceSampleUseCase @Inject constructor( + private val coreImageRepository: ImageRepository, + private val sessionEventRepository: SessionEventRepository, +) { + suspend operator fun invoke( + imageBytes: ByteArray, + captureEventId: String, + ): SecuredImageRef? { + val sessionScope = try { + sessionEventRepository.getCurrentSessionScope() + } catch (_: Throwable) { + return null + } + + return coreImageRepository.storeSample( + projectId = sessionScope.projectId, + sessionId = sessionScope.id, + modality = GeneralConfiguration.Modality.FACE, + sampleId = captureEventId, + fileExtension = "jpg", + sampleBytes = imageBytes, + ) + } +} diff --git a/face/capture/src/test/java/com/simprints/face/capture/screens/FaceCaptureViewModelTest.kt b/face/capture/src/test/java/com/simprints/face/capture/screens/FaceCaptureViewModelTest.kt index 54032ebb32..3f143e143b 100644 --- a/face/capture/src/test/java/com/simprints/face/capture/screens/FaceCaptureViewModelTest.kt +++ b/face/capture/src/test/java/com/simprints/face/capture/screens/FaceCaptureViewModelTest.kt @@ -6,7 +6,7 @@ import com.simprints.core.tools.time.Timestamp import com.simprints.face.capture.models.FaceDetection import com.simprints.face.capture.usecases.BitmapToByteArrayUseCase import com.simprints.face.capture.usecases.IsUsingAutoCaptureUseCase -import com.simprints.face.capture.usecases.SaveFaceImageUseCase +import com.simprints.face.capture.usecases.SaveFaceSampleUseCase import com.simprints.face.capture.usecases.ShouldShowInstructionsScreenUseCase import com.simprints.face.capture.usecases.SimpleCaptureEventReporter import com.simprints.face.infra.basebiosdk.initialization.FaceBioSdkInitializer @@ -54,7 +54,7 @@ class FaceCaptureViewModelTest { private lateinit var configManager: ConfigManager @MockK - private lateinit var faceImageUseCase: SaveFaceImageUseCase + private lateinit var faceImageUseCase: SaveFaceSampleUseCase @MockK private lateinit var eventReporter: SimpleCaptureEventReporter diff --git a/face/capture/src/test/java/com/simprints/face/capture/usecases/SaveFaceImageUseCaseTest.kt b/face/capture/src/test/java/com/simprints/face/capture/usecases/SaveFaceSampleUseCaseTest.kt similarity index 76% rename from face/capture/src/test/java/com/simprints/face/capture/usecases/SaveFaceImageUseCaseTest.kt rename to face/capture/src/test/java/com/simprints/face/capture/usecases/SaveFaceSampleUseCaseTest.kt index 62d0697ef1..8d5cba5785 100644 --- a/face/capture/src/test/java/com/simprints/face/capture/usecases/SaveFaceImageUseCaseTest.kt +++ b/face/capture/src/test/java/com/simprints/face/capture/usecases/SaveFaceSampleUseCaseTest.kt @@ -15,20 +15,20 @@ import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test -class SaveFaceImageUseCaseTest { +class SaveFaceSampleUseCaseTest { @MockK lateinit var imageRepo: ImageRepository @MockK lateinit var eventRepo: SessionEventRepository - private lateinit var useCase: SaveFaceImageUseCase + private lateinit var useCase: SaveFaceSampleUseCase @Before fun setUp() { MockKAnnotations.init(this, relaxed = true) - useCase = SaveFaceImageUseCase(imageRepo, eventRepo) + useCase = SaveFaceSampleUseCase(imageRepo, eventRepo) } @Test @@ -40,6 +40,7 @@ class SaveFaceImageUseCaseTest { val expectedPath = Path( arrayOf( + "sessions", "sessions", "sessionId", "faces", @@ -47,7 +48,7 @@ class SaveFaceImageUseCaseTest { ), ) coEvery { - imageRepo.storeImageSecurely(any(), "projectId", any()) + imageRepo.storeSample("projectId", "sessionId", any(), any(), any(), any()) } returns SecuredImageRef(expectedPath) val imageBytes = byteArrayOf() @@ -55,17 +56,7 @@ class SaveFaceImageUseCaseTest { assertThat(useCase.invoke(imageBytes, captureEventId)).isNotNull() - coVerify { - imageRepo.storeImageSecurely( - withArg { - assert(it.isEmpty()) - }, - "projectId", - withArg { - assert(expectedPath.compose().contains(it.compose())) - }, - ) - } + coVerify { imageRepo.storeSample(any(), any(), any(), any(), any(), any()) } } @Test @@ -85,13 +76,13 @@ class SaveFaceImageUseCaseTest { every { id } returns "sessionId" } coEvery { - imageRepo.storeImageSecurely(any(), "projectId", any()) + imageRepo.storeSample("projectId", "sessionId", any(), "captureEventId", "jpg", any()) } returns null val imageBytes = byteArrayOf() val captureEventId = "captureEventId" assertThat(useCase.invoke(imageBytes, captureEventId)).isNull() - coVerify { imageRepo.storeImageSecurely(any(), "projectId", any()) } + coVerify { imageRepo.storeSample(any(), any(), any(), any(), any(), any()) } } } diff --git a/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/screen/FingerprintCaptureViewModel.kt b/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/screen/FingerprintCaptureViewModel.kt index daa3ed497e..069dd719ae 100644 --- a/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/screen/FingerprintCaptureViewModel.kt +++ b/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/screen/FingerprintCaptureViewModel.kt @@ -29,7 +29,7 @@ import com.simprints.fingerprint.capture.usecase.AddCaptureEventsUseCase import com.simprints.fingerprint.capture.usecase.GetNextFingerToAddUseCase import com.simprints.fingerprint.capture.usecase.GetStartStateUseCase import com.simprints.fingerprint.capture.usecase.IsNoFingerDetectedLimitReachedUseCase -import com.simprints.fingerprint.capture.usecase.SaveImageUseCase +import com.simprints.fingerprint.capture.usecase.SaveFingerprintSampleUseCase import com.simprints.fingerprint.infra.basebiosdk.exceptions.BioSdkException import com.simprints.fingerprint.infra.biosdk.BioSdkWrapper import com.simprints.fingerprint.infra.biosdk.ResolveBioSdkWrapperUseCase @@ -72,7 +72,7 @@ internal class FingerprintCaptureViewModel @Inject constructor( private val configManager: ConfigManager, private val timeHelper: TimeHelper, private val resolveBioSdkWrapperUseCase: ResolveBioSdkWrapperUseCase, - private val saveImage: SaveImageUseCase, + private val saveImage: SaveFingerprintSampleUseCase, private val getNextFingerToAdd: GetNextFingerToAddUseCase, private val getStartState: GetStartStateUseCase, private val addCaptureEvents: AddCaptureEventsUseCase, diff --git a/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveImageUseCase.kt b/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCase.kt similarity index 64% rename from fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveImageUseCase.kt rename to fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCase.kt index 1a47e712ef..513972eb81 100644 --- a/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveImageUseCase.kt +++ b/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCase.kt @@ -5,16 +5,16 @@ import com.simprints.fingerprint.capture.extensions.deduceFileExtension import com.simprints.fingerprint.capture.extensions.toInt import com.simprints.fingerprint.capture.state.CaptureState import com.simprints.fingerprint.infra.scanner.v2.scanner.ScannerInfo +import com.simprints.infra.config.store.models.GeneralConfiguration import com.simprints.infra.config.store.models.Vero2Configuration import com.simprints.infra.events.session.SessionEventRepository import com.simprints.infra.images.ImageRepository -import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FACE_CAPTURE import com.simprints.infra.logging.Simber import javax.inject.Inject -internal class SaveImageUseCase @Inject constructor( +internal class SaveFingerprintSampleUseCase @Inject constructor( private val coreImageRepository: ImageRepository, private val coreEventRepository: SessionEventRepository, private val scannerInfo: ScannerInfo, @@ -49,15 +49,20 @@ internal class SaveImageUseCase @Inject constructor( dpi: Int, scannerId: String?, un20SerialNumber: String?, - ): SecuredImageRef? = determinePath(captureEventId, fileExtension)?.let { path -> - Simber.d("Saving fingerprint image $path", tag = FACE_CAPTURE) - val currentSession = coreEventRepository.getCurrentSessionScope() - val projectId = currentSession.projectId + ): SecuredImageRef? { + val currentSession = try { + coreEventRepository.getCurrentSessionScope() + } catch (_: Throwable) { + return null + } - val securedImageRef = coreImageRepository.storeImageSecurely( - imageBytes = imageBytes, - projectId = projectId, - relativePath = Path(path.parts), + return coreImageRepository.storeSample( + projectId = currentSession.projectId, + sessionId = currentSession.id, + modality = GeneralConfiguration.Modality.FINGERPRINT, + sampleId = captureEventId, + fileExtension = fileExtension, + sampleBytes = imageBytes, metadata = mutableMapOf( META_KEY_FINGER_ID to finger.name, META_KEY_DPI to dpi.toString(), @@ -66,37 +71,9 @@ internal class SaveImageUseCase @Inject constructor( un20SerialNumber?.let { this[META_KEY_UN20_SERIAL_NUMBER] = it } }, ) - - if (securedImageRef != null) { - SecuredImageRef(Path(securedImageRef.relativePath.parts)) - } else { - Simber.i("Saving image failed for captureId $captureEventId", tag = FACE_CAPTURE) - null - } - } - - private suspend fun determinePath( - captureEventId: String, - fileExtension: String, - ): Path? = try { - val sessionId = coreEventRepository.getCurrentSessionScope().id - Path( - arrayOf( - SESSIONS_PATH, - sessionId, - FINGERPRINTS_PATH, - "$captureEventId.$fileExtension", - ), - ) - } catch (t: Throwable) { - Simber.e("Failed to determine path for captureId $captureEventId", t, tag = FACE_CAPTURE) - null } - companion object { - const val SESSIONS_PATH = "sessions" - const val FINGERPRINTS_PATH = "fingerprints" - + companion object Companion { private const val META_KEY_DPI = "dpi" private const val META_KEY_FINGER_ID = "finger" private const val META_KEY_SCANNER_ID = "scannerID" diff --git a/fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/screen/FingerprintCaptureViewModelTest.kt b/fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/screen/FingerprintCaptureViewModelTest.kt index 75dd864814..7cc121f2e6 100644 --- a/fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/screen/FingerprintCaptureViewModelTest.kt +++ b/fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/screen/FingerprintCaptureViewModelTest.kt @@ -21,7 +21,7 @@ import com.simprints.fingerprint.capture.usecase.AddCaptureEventsUseCase import com.simprints.fingerprint.capture.usecase.GetNextFingerToAddUseCase import com.simprints.fingerprint.capture.usecase.GetStartStateUseCase import com.simprints.fingerprint.capture.usecase.IsNoFingerDetectedLimitReachedUseCase -import com.simprints.fingerprint.capture.usecase.SaveImageUseCase +import com.simprints.fingerprint.capture.usecase.SaveFingerprintSampleUseCase import com.simprints.fingerprint.infra.basebiosdk.exceptions.BioSdkException import com.simprints.fingerprint.infra.biosdk.BioSdkWrapper import com.simprints.fingerprint.infra.biosdk.ResolveBioSdkWrapperUseCase @@ -99,7 +99,7 @@ class FingerprintCaptureViewModelTest { private lateinit var timeHelper: TimeHelper @MockK - private lateinit var saveImageUseCase: SaveImageUseCase + private lateinit var saveFingerprintSampleUseCase: SaveFingerprintSampleUseCase @MockK private lateinit var addCaptureEventsUseCase: AddCaptureEventsUseCase @@ -153,7 +153,7 @@ class FingerprintCaptureViewModelTest { configManager = configManager, timeHelper = timeHelper, resolveBioSdkWrapperUseCase = resolveBioSdkWrapperUseCase, - saveImage = saveImageUseCase, + saveImage = saveFingerprintSampleUseCase, getNextFingerToAdd = getNextFingerToAddUseCase, getStartState = getStartStateUseCase, addCaptureEvents = addCaptureEventsUseCase, @@ -518,7 +518,7 @@ class FingerprintCaptureViewModelTest { ) coVerify(exactly = 12) { addCaptureEventsUseCase.invoke(any(), any(), any(), any()) } vm.handleConfirmFingerprintsAndContinue() - coVerify(exactly = 4) { saveImageUseCase.invoke(any(), any(), any(), any()) } + coVerify(exactly = 4) { saveFingerprintSampleUseCase.invoke(any(), any(), any(), any()) } coVerify { addBiometricReferenceCreatedEvents.invoke(any(), any()) } vm.finishWithFingerprints.assertEventReceivedWithContentAssertions { actualFingerprints -> @@ -577,7 +577,7 @@ class FingerprintCaptureViewModelTest { coVerify(exactly = 2) { addCaptureEventsUseCase.invoke(any(), any(), any(), any()) } vm.handleConfirmFingerprintsAndContinue() - coVerify(exactly = 2) { saveImageUseCase.invoke(any(), any(), any(), any()) } + coVerify(exactly = 2) { saveFingerprintSampleUseCase.invoke(any(), any(), any(), any()) } coVerify { addBiometricReferenceCreatedEvents.invoke(any(), any()) } vm.finishWithFingerprints.assertEventReceivedWithContentAssertions { actualFingerprints -> @@ -634,7 +634,7 @@ class FingerprintCaptureViewModelTest { ) coVerify(exactly = 2) { addCaptureEventsUseCase.invoke(any(), any(), any(), any()) } vm.handleConfirmFingerprintsAndContinue() - coVerify(exactly = 0) { saveImageUseCase.invoke(any(), any(), any(), any()) } + coVerify(exactly = 0) { saveFingerprintSampleUseCase.invoke(any(), any(), any(), any()) } coVerify { addBiometricReferenceCreatedEvents.invoke(any(), any()) } vm.finishWithFingerprints.assertEventReceivedWithContentAssertions { actualFingerprints -> @@ -849,7 +849,7 @@ class FingerprintCaptureViewModelTest { coVerify(exactly = 14) { addCaptureEventsUseCase.invoke(any(), any(), any(), any()) } vm.handleConfirmFingerprintsAndContinue() - coVerify(exactly = 3) { saveImageUseCase.invoke(any(), any(), any(), any()) } + coVerify(exactly = 3) { saveFingerprintSampleUseCase.invoke(any(), any(), any(), any()) } vm.finishWithFingerprints.assertEventReceivedWithContentAssertions { actualFingerprints -> assertThat(actualFingerprints?.results).hasSize(3) @@ -960,7 +960,7 @@ class FingerprintCaptureViewModelTest { coVerify(exactly = 14) { addCaptureEventsUseCase.invoke(any(), any(), any(), any()) } // If eager, expect that images were saved before confirm was pressed, including bad scans - coVerify(exactly = 8) { saveImageUseCase.invoke(any(), any(), any(), any()) } + coVerify(exactly = 8) { saveFingerprintSampleUseCase.invoke(any(), any(), any(), any()) } vm.handleConfirmFingerprintsAndContinue() @@ -1057,7 +1057,7 @@ class FingerprintCaptureViewModelTest { vm.handleConfirmFingerprintsAndContinue() // Save image is called even if scanResult.image == null - coVerify(exactly = 3) { saveImageUseCase.invoke(any(), any(), any(), any()) } + coVerify(exactly = 3) { saveFingerprintSampleUseCase.invoke(any(), any(), any(), any()) } vm.finishWithFingerprints.assertEventReceivedWithContentAssertions { actualFingerprints -> assertThat(actualFingerprints?.results).hasSize(3) @@ -1468,7 +1468,7 @@ class FingerprintCaptureViewModelTest { private fun withImageTransfer(strategy: ImageSavingStrategy = ImageSavingStrategy.ONLY_USED_IN_REFERENCE) { every { vero2Configuration.imageSavingStrategy } returns strategy - coEvery { saveImageUseCase.invoke(any(), any(), any(), any()) } returns mockk { + coEvery { saveFingerprintSampleUseCase.invoke(any(), any(), any(), any()) } returns mockk { every { relativePath } returns Path(emptyArray()) } } diff --git a/fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/usecase/SaveImageUseCaseTest.kt b/fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCaseTest.kt similarity index 86% rename from fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/usecase/SaveImageUseCaseTest.kt rename to fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCaseTest.kt index f62a76de1a..711955473b 100644 --- a/fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/usecase/SaveImageUseCaseTest.kt +++ b/fingerprint/capture/src/test/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCaseTest.kt @@ -5,6 +5,7 @@ import com.simprints.core.domain.fingerprint.IFingerIdentifier import com.simprints.fingerprint.capture.state.CaptureState import com.simprints.fingerprint.capture.state.ScanResult import com.simprints.fingerprint.infra.scanner.v2.scanner.ScannerInfo +import com.simprints.infra.config.store.models.GeneralConfiguration import com.simprints.infra.config.store.models.Vero2Configuration import com.simprints.infra.events.session.SessionEventRepository import com.simprints.infra.images.ImageRepository @@ -20,7 +21,7 @@ import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Test -class SaveImageUseCaseTest { +class SaveFingerprintSampleUseCaseTest { @MockK lateinit var imageRepo: ImageRepository @@ -30,7 +31,7 @@ class SaveImageUseCaseTest { @MockK lateinit var vero2Configuration: Vero2Configuration - private lateinit var useCase: SaveImageUseCase + private lateinit var useCase: SaveFingerprintSampleUseCase private lateinit var scannerInfo: ScannerInfo @Before @@ -40,7 +41,7 @@ class SaveImageUseCaseTest { every { vero2Configuration.imageSavingStrategy } returns Vero2Configuration.ImageSavingStrategy.EAGER every { vero2Configuration.captureStrategy } returns Vero2Configuration.CaptureStrategy.SECUGEN_ISO_1300_DPI - useCase = SaveImageUseCase(imageRepo, eventRepo, scannerInfo) + useCase = SaveFingerprintSampleUseCase(imageRepo, eventRepo, scannerInfo) } @Test @@ -91,7 +92,7 @@ class SaveImageUseCaseTest { ), ) coEvery { - imageRepo.storeImageSecurely(any(), "projectId", any(), any()) + imageRepo.storeSample("projectId", "sessionId", any(), any(), any(), any(), any()) } returns SecuredImageRef(expectedPath) assertThat( @@ -103,16 +104,7 @@ class SaveImageUseCaseTest { ), ).isNotNull() - coVerify { - imageRepo.storeImageSecurely( - withArg { assert(it.isEmpty()) }, - "projectId", - withArg { - assert(expectedPath.compose().contains(it.compose())) - }, - withArg { assert(it == expectedMetadata) }, - ) - } + coVerify { imageRepo.storeSample(any(), any(), any(), any(), any(), any(), eq(expectedMetadata)) } } @Test @@ -136,7 +128,7 @@ class SaveImageUseCaseTest { every { id } returns "sessionId" } coEvery { - imageRepo.storeImageSecurely(any(), "projectId", any(), any()) + imageRepo.storeSample(any(), any(), GeneralConfiguration.Modality.FINGERPRINT, any(), any(), any(), any()) } returns null assertThat( @@ -148,7 +140,7 @@ class SaveImageUseCaseTest { ), ).isNull() - coVerify { imageRepo.storeImageSecurely(any(), "projectId", any(), any()) } + coVerify { imageRepo.storeSample(any(), any(), any(), any(), any(), any(), any()) } } private fun createCollectedStub(image: ByteArray?) = CaptureState.ScanProcess.Collected( diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImageRepository.kt b/infra/images/src/main/java/com/simprints/infra/images/ImageRepository.kt index 37c306ee7c..1f03152f4a 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImageRepository.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImageRepository.kt @@ -1,6 +1,6 @@ package com.simprints.infra.images -import com.simprints.infra.images.model.Path +import com.simprints.infra.config.store.models.GeneralConfiguration import com.simprints.infra.images.model.SecuredImageRef /** @@ -8,20 +8,23 @@ import com.simprints.infra.images.model.SecuredImageRef */ interface ImageRepository { /** - * Encrypts and stores an image file locally + * Encrypts and stores an sample file locally. + * Path of the sample file within the images root folder will + * be created based on the session ID, modality and file extension parameters. * - * @param imageBytes the image, in bytes + * @param sampleBytes the sample file to be stored in bytes * @param projectId the id of the project - * @param relativePath the path of the image within the images root folder, including file name * @param metadata arbitrary key-value pairs to be associated with the image * * @return a reference to the newly stored image, if successful, otherwise null - * @see [com.simprints.infra.images.local.ImageLocalDataSource.encryptAndStoreImage] */ - suspend fun storeImageSecurely( - imageBytes: ByteArray, + suspend fun storeSample( projectId: String, - relativePath: Path, + sessionId: String, + modality: GeneralConfiguration.Modality, + sampleId: String, + fileExtension: String, + sampleBytes: ByteArray, metadata: Map = emptyMap(), ): SecuredImageRef? diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt index e1c66e7117..80102f25eb 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt @@ -1,10 +1,13 @@ package com.simprints.infra.images +import com.simprints.infra.config.store.models.GeneralConfiguration import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore -import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.remote.SampleUploader +import com.simprints.infra.images.usecase.CreateSamplePathUseCase +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FACE_CAPTURE +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FINGER_CAPTURE import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber import javax.inject.Inject @@ -13,16 +16,39 @@ internal class ImageRepositoryImpl @Inject internal constructor( private val localDataSource: ImageLocalDataSource, private val sampleUploader: SampleUploader, private val metadataStore: ImageMetadataStore, + private val createSamplePathUseCase: CreateSamplePathUseCase, ) : ImageRepository { - override suspend fun storeImageSecurely( - imageBytes: ByteArray, + override suspend fun storeSample( projectId: String, - relativePath: Path, + sessionId: String, + modality: GeneralConfiguration.Modality, + sampleId: String, + fileExtension: String, + sampleBytes: ByteArray, metadata: Map, - ): SecuredImageRef? = localDataSource - .encryptAndStoreImage(imageBytes, projectId, relativePath) - // Only store metadata if the image was stored successfully - ?.also { metadataStore.storeMetadata(relativePath, metadata) } + ): SecuredImageRef? { + val logTag = if (modality == GeneralConfiguration.Modality.FACE) FACE_CAPTURE else FINGER_CAPTURE + + val relativePath = try { + createSamplePathUseCase(sessionId, modality, sampleId, fileExtension) + } catch (t: Throwable) { + Simber.e("Error determining path for $sessionId", t, tag = logTag) + return null + } + + Simber.d("Saving $modality sample to ${relativePath.compose()}", tag = logTag) + + return localDataSource + .encryptAndStoreImage(sampleBytes, projectId, relativePath) + .also { + if (it == null) { + Simber.i("Saving image failed for captureId $sessionId", tag = logTag) + } else { + // Only store metadata if the image was stored successfully + metadataStore.storeMetadata(relativePath, metadata) + } + } + } override suspend fun getNumberOfImagesToUpload(projectId: String): Int = localDataSource.listImages(projectId).count() diff --git a/infra/images/src/main/java/com/simprints/infra/images/usecase/CreateSamplePathUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/usecase/CreateSamplePathUseCase.kt new file mode 100644 index 0000000000..6bb1335246 --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/usecase/CreateSamplePathUseCase.kt @@ -0,0 +1,27 @@ +package com.simprints.infra.images.usecase + +import com.simprints.infra.config.store.models.GeneralConfiguration +import com.simprints.infra.images.model.Path +import javax.inject.Inject + +internal class CreateSamplePathUseCase @Inject constructor() { + operator fun invoke( + sessionId: String, + modality: GeneralConfiguration.Modality, + sampleId: String, + fileExtension: String, + ) = Path( + arrayOf( + SESSIONS_PATH, + sessionId, + if (modality == GeneralConfiguration.Modality.FACE) FACES_PATH else FINGERPRINTS_PATH, + "$sampleId.$fileExtension", + ), + ) + + companion object { + const val SESSIONS_PATH = "sessions" + const val FACES_PATH = "faces" + const val FINGERPRINTS_PATH = "fingerprints" + } +} diff --git a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt index dbd1e9a5da..32f6acfc0e 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt @@ -7,6 +7,7 @@ import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.remote.SampleUploader import com.simprints.infra.images.remote.UploadResult +import com.simprints.infra.images.usecase.CreateSamplePathUseCase import com.simprints.testtools.common.coroutines.TestCoroutineRule import io.mockk.* import io.mockk.impl.annotations.MockK @@ -29,12 +30,15 @@ internal class ImageRepositoryImplTest { @MockK lateinit var metadataStore: ImageMetadataStore + @MockK + lateinit var createSamplePathUseCase: CreateSamplePathUseCase + private lateinit var repository: ImageRepository @Before fun setUp() { MockKAnnotations.init(this) - repository = ImageRepositoryImpl(localDataSource, sampleUploader, metadataStore) + repository = ImageRepositoryImpl(localDataSource, sampleUploader, metadataStore, createSamplePathUseCase) initValidImageMocks() } diff --git a/infra/images/src/test/java/com/simprints/infra/images/usecase/CreateSamplePathUseCaseTest.kt b/infra/images/src/test/java/com/simprints/infra/images/usecase/CreateSamplePathUseCaseTest.kt new file mode 100644 index 0000000000..de78c1ea07 --- /dev/null +++ b/infra/images/src/test/java/com/simprints/infra/images/usecase/CreateSamplePathUseCaseTest.kt @@ -0,0 +1,42 @@ +package com.simprints.infra.images.usecase + +import com.google.common.truth.Truth.* +import com.simprints.infra.config.store.models.GeneralConfiguration +import org.junit.Before +import org.junit.Test + +class CreateSamplePathUseCaseTest { + private lateinit var useCase: CreateSamplePathUseCase + + @Before + fun setUp() { + useCase = CreateSamplePathUseCase() + } + + @Test + fun `invoke should create a valid path for face sample`() { + val expectedPath = "sessions/sessionId/faces/captureEventId.jpg" + val result = useCase( + sessionId = "sessionId", + modality = GeneralConfiguration.Modality.FACE, + sampleId = "captureEventId", + fileExtension = "jpg", + ) + + assertThat(result.compose()).isEqualTo(expectedPath) + } + + @Test + fun `invoke should create a valid path for fingerprint sample`() { + val expectedPath = "sessions/sessionId/fingerprints/captureEventId.swq" + + val result = useCase( + sessionId = "sessionId", + modality = GeneralConfiguration.Modality.FINGERPRINT, + sampleId = "captureEventId", + fileExtension = "swq", + ) + + assertThat(result.compose()).isEqualTo(expectedPath) + } +} From dbd4c33f3baae5779cf1aff307a51924f35498c2 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Wed, 9 Jul 2025 18:16:40 +0300 Subject: [PATCH 3/9] MS-1058 Move all of the image upload logic into firestore uploader --- .../infra/images/ImageRepositoryImpl.kt | 39 +--- .../simprints/infra/images/ImagesModule.kt | 5 - .../infra/images/remote/SampleUploader.kt | 19 +- .../firestore/FirestoreSampleUploader.kt | 93 +++++---- .../images/usecase/GetUploaderUseCase.kt | 24 +++ .../infra/images/ImageRepositoryImplTest.kt | 177 +++++++----------- .../firestore/FirestoreSampleUploaderTest.kt | 168 +++++++++++------ 7 files changed, 266 insertions(+), 259 deletions(-) create mode 100644 infra/images/src/main/java/com/simprints/infra/images/usecase/GetUploaderUseCase.kt diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt index 80102f25eb..7e6b4200af 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt @@ -4,19 +4,18 @@ import com.simprints.infra.config.store.models.GeneralConfiguration import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.SecuredImageRef -import com.simprints.infra.images.remote.SampleUploader import com.simprints.infra.images.usecase.CreateSamplePathUseCase +import com.simprints.infra.images.usecase.GetUploaderUseCase import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FACE_CAPTURE import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FINGER_CAPTURE -import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber import javax.inject.Inject internal class ImageRepositoryImpl @Inject internal constructor( private val localDataSource: ImageLocalDataSource, - private val sampleUploader: SampleUploader, private val metadataStore: ImageMetadataStore, private val createSamplePathUseCase: CreateSamplePathUseCase, + private val getSampleUploader: GetUploaderUseCase, ) : ImageRepository { override suspend fun storeSample( projectId: String, @@ -28,13 +27,7 @@ internal class ImageRepositoryImpl @Inject internal constructor( metadata: Map, ): SecuredImageRef? { val logTag = if (modality == GeneralConfiguration.Modality.FACE) FACE_CAPTURE else FINGER_CAPTURE - - val relativePath = try { - createSamplePathUseCase(sessionId, modality, sampleId, fileExtension) - } catch (t: Throwable) { - Simber.e("Error determining path for $sessionId", t, tag = logTag) - return null - } + val relativePath = createSamplePathUseCase(sessionId, modality, sampleId, fileExtension) Simber.d("Saving $modality sample to ${relativePath.compose()}", tag = logTag) @@ -52,31 +45,7 @@ internal class ImageRepositoryImpl @Inject internal constructor( override suspend fun getNumberOfImagesToUpload(projectId: String): Int = localDataSource.listImages(projectId).count() - override suspend fun uploadStoredImagesAndDelete(projectId: String): Boolean { - var allImagesUploaded = true - - val images = localDataSource.listImages(projectId) - images.forEach { imageRef -> - try { - localDataSource.decryptImage(imageRef)?.let { stream -> - val metadata = metadataStore.getMetadata(imageRef.relativePath) - val uploadResult = sampleUploader.uploadSample(stream, imageRef, metadata) - if (uploadResult.isUploadSuccessful()) { - localDataSource.deleteImage(imageRef) - metadataStore.deleteMetadata(imageRef.relativePath) - } else { - allImagesUploaded = false - Simber.i("Failed to upload image without exception", tag = SYNC) - } - } - } catch (t: Throwable) { - allImagesUploaded = false - Simber.e("Failed to upload images", t, tag = SYNC) - } - } - - return allImagesUploaded - } + override suspend fun uploadStoredImagesAndDelete(projectId: String): Boolean = getSampleUploader().uploadAllSamples(projectId) override suspend fun deleteStoredImages() { metadataStore.deleteAllMetadata() diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImagesModule.kt b/infra/images/src/main/java/com/simprints/infra/images/ImagesModule.kt index 00e3a8181c..8677870c11 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImagesModule.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImagesModule.kt @@ -5,8 +5,6 @@ import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.local.ImageLocalDataSourceImpl import com.simprints.infra.images.metadata.database.ImageMetadataDao import com.simprints.infra.images.metadata.database.ImageMetadataDatabase -import com.simprints.infra.images.remote.SampleUploader -import com.simprints.infra.images.remote.firestore.FirestoreSampleUploader import dagger.Binds import dagger.Module import dagger.Provides @@ -23,9 +21,6 @@ abstract class ImagesModule { @Binds internal abstract fun bindImageLocalDataSource(impl: ImageLocalDataSourceImpl): ImageLocalDataSource - - @Binds - internal abstract fun bindImageRemoteDataSource(impl: FirestoreSampleUploader): SampleUploader } @Module diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/SampleUploader.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/SampleUploader.kt index 567374649d..0e8b138d04 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/SampleUploader.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/SampleUploader.kt @@ -1,25 +1,12 @@ package com.simprints.infra.images.remote -import com.simprints.infra.images.model.SecuredImageRef -import java.io.FileInputStream - /** * Interface for remote image file operations */ internal interface SampleUploader { /** - * Uploads an image - * - * @param imageStream the image file as a stream - * @param imageRef a reference to the image to be uploaded - * @param metadata arbitrary key-value pairs to be associated with the image - * - * @return the result of the operation. - * @see [UploadResult] + * Uploads all locally stored samples. + * On successful upload, the file and the associated metadata are deleted. */ - suspend fun uploadSample( - imageStream: FileInputStream, - imageRef: SecuredImageRef, - metadata: Map, - ): UploadResult + suspend fun uploadAllSamples(projectId: String): Boolean } diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt index 9de66ee9dc..ca99c75013 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt @@ -2,11 +2,14 @@ package com.simprints.infra.images.remote.firestore import com.google.firebase.storage.FirebaseStorage import com.google.firebase.storage.StorageMetadata +import com.google.firebase.storage.StorageReference import com.simprints.infra.authstore.AuthStore import com.simprints.infra.config.sync.ConfigManager +import com.simprints.infra.images.local.ImageLocalDataSource +import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.remote.SampleUploader -import com.simprints.infra.images.remote.UploadResult +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber import kotlinx.coroutines.tasks.await import java.io.FileInputStream @@ -15,57 +18,67 @@ import javax.inject.Inject internal class FirestoreSampleUploader @Inject constructor( private val configManager: ConfigManager, private val authStore: AuthStore, + private val localDataSource: ImageLocalDataSource, + private val metadataStore: ImageMetadataStore, ) : SampleUploader { - override suspend fun uploadSample( - imageStream: FileInputStream, - imageRef: SecuredImageRef, - metadata: Map, - ): UploadResult { - val firebaseProjectName = authStore.getLegacyAppFallback().options.projectId - - return if (firebaseProjectName != null) { - val projectId = authStore.signedInProjectId + override suspend fun uploadAllSamples(projectId: String): Boolean { + val firebaseApp = authStore.getLegacyAppFallback() + val firebaseProjectName = firebaseApp.options.projectId - if (projectId.isEmpty()) { - Simber.i("AuthStore projectId is empty") - return UploadResult(imageRef, UploadResult.Status.FAILED) - } + if (!firebaseProjectName.isNullOrBlank()) { + var allImagesUploaded = true val bucketUrl = configManager.getProject(projectId).imageBucket - val rootRef = FirebaseStorage - .getInstance( - authStore.getLegacyAppFallback(), - bucketUrl, - ).reference + .getInstance(firebaseApp, bucketUrl) + .reference - var fileRef = rootRef - imageRef.relativePath.parts.forEach { pathPart -> - fileRef = fileRef.child(pathPart) + localDataSource.listImages(projectId).forEach { imageRef -> + try { + localDataSource.decryptImage(imageRef)?.let { stream -> + val metadata = metadataStore.getMetadata(imageRef.relativePath) + val uploadSuccessful = uploadSample(rootRef, stream, imageRef, metadata) + if (uploadSuccessful) { + localDataSource.deleteImage(imageRef) + metadataStore.deleteMetadata(imageRef.relativePath) + } else { + allImagesUploaded = false + Simber.i("Failed to upload image without exception", tag = SYNC) + } + } + } catch (t: Throwable) { + allImagesUploaded = false + Simber.e("Failed to upload images", t, tag = SYNC) + } } - Simber.d("Uploading ${fileRef.path}") + return allImagesUploaded + } else { + Simber.i("Firebase projectId is null") + return false + } + } - val uploadTask = if (metadata.isEmpty()) { - fileRef.putStream(imageStream).await() - } else { - val storeMetadata = StorageMetadata - .Builder() - .also { metadata.forEach { (key, value) -> it.setCustomMetadata(key, value) } } - .build() - fileRef.putStream(imageStream, storeMetadata).await() - } + private suspend fun uploadSample( + rootRef: StorageReference, + imageStream: FileInputStream, + imageRef: SecuredImageRef, + metadata: Map, + ): Boolean { + val fileRef = imageRef.relativePath.parts + .fold(rootRef) { ref, pathPart -> ref.child(pathPart) } - val status = if (uploadTask.task.isSuccessful) { - UploadResult.Status.SUCCESSFUL - } else { - UploadResult.Status.FAILED - } + Simber.d("Uploading ${fileRef.path}") - UploadResult(imageRef, status) + val uploadTask = if (metadata.isEmpty()) { + fileRef.putStream(imageStream).await() } else { - Simber.i("Firebase projectId is null") - UploadResult(imageRef, UploadResult.Status.FAILED) + val storeMetadata = StorageMetadata + .Builder() + .also { metadata.forEach { (key, value) -> it.setCustomMetadata(key, value) } } + .build() + fileRef.putStream(imageStream, storeMetadata).await() } + return uploadTask.task.isSuccessful } } diff --git a/infra/images/src/main/java/com/simprints/infra/images/usecase/GetUploaderUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/usecase/GetUploaderUseCase.kt new file mode 100644 index 0000000000..cf4e86fd91 --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/usecase/GetUploaderUseCase.kt @@ -0,0 +1,24 @@ +package com.simprints.infra.images.usecase + +import com.simprints.infra.config.store.ConfigRepository +import com.simprints.infra.config.store.models.experimental +import com.simprints.infra.images.remote.SampleUploader +import com.simprints.infra.images.remote.firestore.FirestoreSampleUploader +import javax.inject.Inject + +internal class GetUploaderUseCase @Inject constructor( + private val configRepository: ConfigRepository, + private val firestoreUploader: FirestoreSampleUploader, +) { + suspend operator fun invoke(): SampleUploader = configRepository + .getProjectConfiguration() + .experimental() + .sampleUploadWithSignedUrlEnabled + .let { + if (it) { + TODO() + } else { + firestoreUploader + } + } +} diff --git a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt index 32f6acfc0e..3e2966f15c 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt @@ -1,13 +1,14 @@ package com.simprints.infra.images import com.google.common.truth.Truth.* +import com.simprints.infra.config.store.models.GeneralConfiguration import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.remote.SampleUploader -import com.simprints.infra.images.remote.UploadResult import com.simprints.infra.images.usecase.CreateSamplePathUseCase +import com.simprints.infra.images.usecase.GetUploaderUseCase import com.simprints.testtools.common.coroutines.TestCoroutineRule import io.mockk.* import io.mockk.impl.annotations.MockK @@ -15,7 +16,6 @@ import kotlinx.coroutines.test.runTest import org.junit.Before import org.junit.Rule import org.junit.Test -import java.io.FileInputStream internal class ImageRepositoryImplTest { @get:Rule @@ -33,82 +33,96 @@ internal class ImageRepositoryImplTest { @MockK lateinit var createSamplePathUseCase: CreateSamplePathUseCase + @MockK + lateinit var getUploaderUseCase: GetUploaderUseCase + private lateinit var repository: ImageRepository @Before fun setUp() { - MockKAnnotations.init(this) - repository = ImageRepositoryImpl(localDataSource, sampleUploader, metadataStore, createSamplePathUseCase) - initValidImageMocks() - } + MockKAnnotations.init(this, relaxed = true) - @Test - fun withEmptyList_shouldConsiderUploadOperationSuccessful() = runTest { - coEvery { localDataSource.listImages(PROJECT_ID) } returns emptyList() - - val successful = repository.uploadStoredImagesAndDelete(PROJECT_ID) + every { createSamplePathUseCase.invoke(any(), any(), any(), any()) } returns Path(VALID_PATH) + coEvery { sampleUploader.uploadAllSamples(any()) } returns true + coEvery { getUploaderUseCase.invoke() } returns sampleUploader - assertThat(successful).isTrue() + repository = ImageRepositoryImpl( + localDataSource = localDataSource, + metadataStore = metadataStore, + createSamplePathUseCase = createSamplePathUseCase, + getSampleUploader = getUploaderUseCase, + ) } @Test - fun withAllFilesValid_shouldUploadAndDeleteSuccessfully() = runTest { - configureLocalImageFiles(includeInvalidFile = false) - - val successful = repository.uploadStoredImagesAndDelete(PROJECT_ID) + fun `save sample to local storage`() = runTest { + coEvery { localDataSource.encryptAndStoreImage(any(), any(), any()) } returns mockValidImage() + val imageRef = repository.storeSample( + projectId = PROJECT_ID, + sessionId = "sessionId", + modality = GeneralConfiguration.Modality.FACE, + sampleId = "sampleId", + fileExtension = "jpg", + sampleBytes = ByteArray(10), + ) - assertThat(successful).isTrue() + assertThat(imageRef).isNotNull() + coVerify { + metadataStore.storeMetadata(any(), any()) + localDataSource.encryptAndStoreImage(any(), any(), any()) + } } @Test - fun withException_shouldReturnNotSuccessful() = runTest { - coEvery { - localDataSource.decryptImage(mockThrowingImage()) - } throws Exception("Cannot decrypt") - - coEvery { localDataSource.listImages(any()) } returns listOf( - mockValidImage(), - mockThrowingImage(), + fun `save sample with metadata to local storage`() = runTest { + coEvery { localDataSource.encryptAndStoreImage(any(), any(), any()) } returns mockValidImage() + val imageRef = repository.storeSample( + projectId = PROJECT_ID, + sessionId = "sessionId", + modality = GeneralConfiguration.Modality.FACE, + sampleId = "sampleId", + fileExtension = "jpg", + sampleBytes = ByteArray(10), + metadata = mapOf("k" to "v"), ) - val successful = repository.uploadStoredImagesAndDelete(PROJECT_ID) - assertThat(successful).isFalse() + assertThat(imageRef).isNotNull() + coVerify { + metadataStore.storeMetadata(any(), any()) + localDataSource.encryptAndStoreImage(any(), any(), any()) + } } @Test - fun withDecryptedAndNotUploadedImage_shouldReturnNotSuccessful() = runTest { - initValidImageFailedUploadMocks() - configureLocalImageFiles(includeInvalidFile = true) - - val successful = repository.uploadStoredImagesAndDelete(PROJECT_ID) + fun `returns null if was not able to save image`() = runTest { + coEvery { localDataSource.encryptAndStoreImage(any(), any(), any()) } returns null + val imageRef = repository.storeSample( + projectId = PROJECT_ID, + sessionId = "sessionId", + modality = GeneralConfiguration.Modality.FACE, + sampleId = "sampleId", + fileExtension = "jpg", + sampleBytes = ByteArray(10), + metadata = mapOf("k" to "v"), + ) - assertThat(successful).isFalse() + assertThat(imageRef).isNull() + coVerify(exactly = 0) { + metadataStore.storeMetadata(any(), any()) + } } @Test - fun shouldDeleteAnImageAfterTheUpload() = runTest { - configureLocalImageFiles(includeInvalidFile = false) - + fun `delegates sample upload to uploader`() = runTest { val successful = repository.uploadStoredImagesAndDelete(PROJECT_ID) - coVerify(exactly = 3) { localDataSource.decryptImage(any()) } - coVerify(exactly = 3) { localDataSource.deleteImage(any()) } - coVerify(exactly = 3) { metadataStore.deleteMetadata(any()) } assertThat(successful).isTrue() + coVerify { sampleUploader.uploadAllSamples(any()) } } @Test - fun shouldDecryptImageBeforeUploading() = runTest { - configureLocalImageFiles(numberOfValidFiles = 5, includeInvalidFile = false) - - repository.uploadStoredImagesAndDelete(PROJECT_ID) - - coVerify(exactly = 5) { localDataSource.decryptImage(any()) } - } - - @Test - fun shouldDeleteStoredImages() = runTest { - configureLocalImageFiles(numberOfValidFiles = 5, includeInvalidFile = false) + fun `deletes stored images and metadata`() = runTest { + configureLocalImageFiles(numberOfValidFiles = 5) repository.deleteStoredImages() @@ -117,68 +131,19 @@ internal class ImageRepositoryImplTest { } @Test - fun shouldGetImagesCount() = runTest { - val nImagesInLocal = 5 - configureLocalImageFiles(numberOfValidFiles = nImagesInLocal, includeInvalidFile = false) + fun `returns number of images to upload`() = runTest { + configureLocalImageFiles(numberOfValidFiles = 5) val imageCount = repository.getNumberOfImagesToUpload(PROJECT_ID) - assertThat(imageCount).isEqualTo(nImagesInLocal) - } - - private fun initValidImageMocks() { - val validImage = mockValidImage() - val mockStream = mockk() - - coEvery { - localDataSource.deleteImage(validImage) - } returns true - - coEvery { - localDataSource.decryptImage(validImage) - } returns mockStream - - coJustRun { metadataStore.storeMetadata(any(), any()) } - coEvery { metadataStore.getMetadata(any()) } returns emptyMap() - coJustRun { metadataStore.deleteMetadata(any()) } - coJustRun { metadataStore.deleteAllMetadata() } - - coEvery { - sampleUploader.uploadSample(mockStream, validImage, emptyMap()) - } returns UploadResult( - validImage, - UploadResult.Status.SUCCESSFUL, - ) - } - - private fun initValidImageFailedUploadMocks() { - val invalidImage = mockInvalidImage() - val mockStream = mockk() - - coEvery { - localDataSource.decryptImage(invalidImage) - } returns mockStream - - coEvery { - sampleUploader.uploadSample(mockStream, invalidImage, emptyMap()) - } returns UploadResult( - invalidImage, - UploadResult.Status.FAILED, - ) + assertThat(imageCount).isEqualTo(5) } - private fun configureLocalImageFiles( - numberOfValidFiles: Int = 3, - includeInvalidFile: Boolean, - ) { + private fun configureLocalImageFiles(numberOfValidFiles: Int) { val files = mutableListOf().apply { repeat(numberOfValidFiles) { add(mockValidImage()) } - - if (includeInvalidFile) { - add(mockInvalidImage()) - } } coEvery { localDataSource.listImages(PROJECT_ID) } returns files @@ -187,14 +152,8 @@ internal class ImageRepositoryImplTest { private fun mockValidImage() = SecuredImageRef(Path(VALID_PATH)) - private fun mockInvalidImage() = SecuredImageRef(Path(INVALID_PATH)) - - private fun mockThrowingImage() = SecuredImageRef(Path(THROWING_PATH)) - companion object { private const val VALID_PATH = "valid.txt" - private const val INVALID_PATH = "invalid" - private const val THROWING_PATH = "throw.exe" private const val PROJECT_ID = "projectId" } } diff --git a/infra/images/src/test/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploaderTest.kt b/infra/images/src/test/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploaderTest.kt index d1b3206ea3..1bb2ab638e 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploaderTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploaderTest.kt @@ -1,18 +1,16 @@ package com.simprints.infra.images.remote.firestore -import androidx.test.ext.junit.runners.AndroidJUnit4 -import com.google.common.truth.Truth +import androidx.test.ext.junit.runners.* +import com.google.common.truth.Truth.* import com.google.firebase.storage.FirebaseStorage import com.simprints.infra.authstore.AuthStore import com.simprints.infra.config.sync.ConfigManager +import com.simprints.infra.images.local.ImageLocalDataSource +import com.simprints.infra.images.metadata.ImageMetadataStore +import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef -import io.mockk.MockKAnnotations -import io.mockk.coEvery -import io.mockk.every +import io.mockk.* import io.mockk.impl.annotations.MockK -import io.mockk.mockk -import io.mockk.mockkStatic -import io.mockk.unmockkStatic import kotlinx.coroutines.tasks.await import kotlinx.coroutines.test.runTest import org.junit.After @@ -31,10 +29,13 @@ class FirestoreSampleUploaderTest { private lateinit var authStore: AuthStore @MockK - private lateinit var mockImageStream: FileInputStream + private lateinit var mockSecuredImageRef: SecuredImageRef @MockK - private lateinit var mockSecuredImageRef: SecuredImageRef + private lateinit var metadataStore: ImageMetadataStore + + @MockK + private lateinit var localDataSource: ImageLocalDataSource private lateinit var remoteDataSource: FirestoreSampleUploader @@ -44,7 +45,12 @@ class FirestoreSampleUploaderTest { every { mockSecuredImageRef.relativePath.parts } returns arrayOf("Test1") - remoteDataSource = FirestoreSampleUploader(configManager, authStore) + remoteDataSource = FirestoreSampleUploader( + configManager = configManager, + authStore = authStore, + localDataSource = localDataSource, + metadataStore = metadataStore, + ) // We need to mock statics and global extensions mockkStatic(FirebaseStorage::class) @@ -59,75 +65,129 @@ class FirestoreSampleUploaderTest { } @Test - fun `test image upload flow`() = runTest { - coEvery { configManager.getProject(any()).imageBucket } returns "gs://`simprints-dev.appspot.com" - every { authStore.getLegacyAppFallback().options.projectId } returns "projectId" - every { authStore.signedInProjectId } returns "projectId" + fun `null project returns failed upload`() = runTest { + every { authStore.getLegacyAppFallback().options.projectId } returns null - val storageMock = setupStorageMock() + val result = remoteDataSource.uploadAllSamples(PROJECT_ID) - every { FirebaseStorage.getInstance(any(), any()) } returns storageMock + assertThat(result).isFalse() + } - val result = remoteDataSource.uploadSample(mockImageStream, mockSecuredImageRef, emptyMap()) + @Test + fun `empty project returns failed upload`() = runTest { + every { authStore.getLegacyAppFallback().options.projectId } returns "" + + val result = remoteDataSource.uploadAllSamples(PROJECT_ID) - Truth.assertThat(result.isUploadSuccessful()).isTrue() + assertThat(result).isFalse() } @Test - fun `test image with metadata upload flow`() = runTest { - coEvery { configManager.getProject(any()).imageBucket } returns "gs://`simprints-dev.appspot.com" - every { authStore.getLegacyAppFallback().options.projectId } returns "projectId" - every { authStore.signedInProjectId } returns "projectId" + fun `when no samples to upload returns success`() = runTest { + setupProjectConfig() + setupStorageMock() + configureLocalImageFiles(numberOfValidFiles = 0) - val storageMock = setupStorageMock() + coEvery { localDataSource.listImages(PROJECT_ID) } returns emptyList() - every { FirebaseStorage.getInstance(any(), any()) } returns storageMock + assertThat(remoteDataSource.uploadAllSamples(PROJECT_ID)).isTrue() + } - val result = remoteDataSource.uploadSample( - mockImageStream, - mockSecuredImageRef, - mapOf("key" to "value"), - ) + @Test + fun `test upload and delete all samples successfully`() = runTest { + setupProjectConfig() + setupStorageMock() + configureLocalImageFiles(numberOfValidFiles = 3) + + assertThat(remoteDataSource.uploadAllSamples(PROJECT_ID)).isTrue() + coVerify(exactly = 3) { localDataSource.decryptImage(any()) } + coVerify(exactly = 3) { localDataSource.deleteImage(any()) } + coVerify(exactly = 3) { metadataStore.deleteMetadata(any()) } + } + + @Test + fun `test upload and delete all samples with metadata successfully`() = runTest { + setupProjectConfig() + setupStorageMock() + + configureLocalImageFiles(numberOfValidFiles = 3, metadata = mapOf("k" to "v")) - Truth.assertThat(result.isUploadSuccessful()).isTrue() + assertThat(remoteDataSource.uploadAllSamples(PROJECT_ID)).isTrue() + coVerify(exactly = 3) { localDataSource.decryptImage(any()) } + coVerify(exactly = 3) { localDataSource.deleteImage(any()) } + coVerify(exactly = 3) { metadataStore.deleteMetadata(any()) } } @Test - fun `null project returns failed upload`() = runTest { - every { authStore.getLegacyAppFallback().options.projectId } returns null + fun `test failed decryption should not return success`() = runTest { + setupProjectConfig() + setupStorageMock() - val result = remoteDataSource.uploadSample(mockImageStream, mockSecuredImageRef, emptyMap()) + coEvery { localDataSource.listImages(any()) } returns listOf(mockImage()) + coEvery { localDataSource.decryptImage(any()) } throws Exception("Cannot decrypt") - Truth.assertThat(result.isUploadSuccessful()).isFalse() + assertThat(remoteDataSource.uploadAllSamples(PROJECT_ID)).isFalse() } @Test - fun `null storage bucket returns failed upload`() = runTest { - coEvery { configManager.getProject(any()).imageBucket } returns "" - every { authStore.getLegacyAppFallback().options.projectId } returns "projectId" + fun `test failed upload should not return success`() = runTest { + setupProjectConfig() + setupStorageMock(success = false) + configureLocalImageFiles(numberOfValidFiles = 1) - val result = remoteDataSource.uploadSample(mockImageStream, mockSecuredImageRef, emptyMap()) + assertThat(remoteDataSource.uploadAllSamples(PROJECT_ID)).isFalse() + } - Truth.assertThat(result.isUploadSuccessful()).isFalse() + private fun setupProjectConfig() { + coEvery { configManager.getProject(any()).imageBucket } returns "gs://`simprints-dev.appspot.com" + every { authStore.getLegacyAppFallback().options.projectId } returns "projectId" + every { authStore.signedInProjectId } returns "projectId" } - private fun setupStorageMock() = mockk(relaxed = true) { - every { reference.child(any()) } returns mockk { - every { path } returns "testPath" - every { putStream(any()) } returns mockk { - coEvery { await() } returns mockk { - every { task } returns mockk { - every { isSuccessful } returns true - } + private fun setupStorageMock(success: Boolean = true) { + every { FirebaseStorage.getInstance(any(), any()) } returns mockk(relaxed = true) { + every { reference.child(any()) } returns mockk { + every { path } returns "testPath" + every { putStream(any()) } returns mockk { + coEvery { await().task.isSuccessful } returns success } - } - every { putStream(any(), any()) } returns mockk { - coEvery { await() } returns mockk { - every { task } returns mockk { - every { isSuccessful } returns true - } + every { putStream(any(), any()) } returns mockk { + coEvery { await().task.isSuccessful } returns success } } } } + + private fun configureLocalImageFiles( + numberOfValidFiles: Int, + metadata: Map = emptyMap(), + ) { + val files = mutableListOf().apply { + repeat(numberOfValidFiles) { + add(mockImage()) + } + } + coEvery { localDataSource.listImages(PROJECT_ID) } returns files + + val validImage = mockImage() + val mockStream = mockk() + + coEvery { + localDataSource.deleteImage(validImage) + } returns true + + coEvery { + localDataSource.decryptImage(validImage) + } returns mockStream + + coEvery { metadataStore.getMetadata(any()) } returns metadata + coJustRun { metadataStore.deleteMetadata(any()) } + } + + private fun mockImage() = SecuredImageRef(Path(VALID_PATH)) + + companion object { + private const val VALID_PATH = "valid.txt" + private const val PROJECT_ID = "projectId" + } } From 3441d91c378a385540ee3ab312a20d912f6bf0e7 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Thu, 10 Jul 2025 11:20:20 +0300 Subject: [PATCH 4/9] MS-1058 Add file extension meta data to every sample --- .../usecase/SaveFingerprintSampleUseCase.kt | 2 +- .../com/simprints/infra/images/ImageRepository.kt | 2 +- .../simprints/infra/images/ImageRepositoryImpl.kt | 15 +++++++++++++-- .../infra/images/ImageRepositoryImplTest.kt | 4 ++-- 4 files changed, 17 insertions(+), 6 deletions(-) diff --git a/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCase.kt b/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCase.kt index 513972eb81..293aa215b5 100644 --- a/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCase.kt +++ b/fingerprint/capture/src/main/java/com/simprints/fingerprint/capture/usecase/SaveFingerprintSampleUseCase.kt @@ -63,7 +63,7 @@ internal class SaveFingerprintSampleUseCase @Inject constructor( sampleId = captureEventId, fileExtension = fileExtension, sampleBytes = imageBytes, - metadata = mutableMapOf( + optionalMetadata = mutableMapOf( META_KEY_FINGER_ID to finger.name, META_KEY_DPI to dpi.toString(), ).apply { diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImageRepository.kt b/infra/images/src/main/java/com/simprints/infra/images/ImageRepository.kt index 1f03152f4a..3f422b03c4 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImageRepository.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImageRepository.kt @@ -25,7 +25,7 @@ interface ImageRepository { sampleId: String, fileExtension: String, sampleBytes: ByteArray, - metadata: Map = emptyMap(), + optionalMetadata: Map = emptyMap(), ): SecuredImageRef? /** diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt index 7e6b4200af..ff71b41d58 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt @@ -24,7 +24,7 @@ internal class ImageRepositoryImpl @Inject internal constructor( sampleId: String, fileExtension: String, sampleBytes: ByteArray, - metadata: Map, + optionalMetadata: Map, ): SecuredImageRef? { val logTag = if (modality == GeneralConfiguration.Modality.FACE) FACE_CAPTURE else FINGER_CAPTURE val relativePath = createSamplePathUseCase(sessionId, modality, sampleId, fileExtension) @@ -38,7 +38,14 @@ internal class ImageRepositoryImpl @Inject internal constructor( Simber.i("Saving image failed for captureId $sessionId", tag = logTag) } else { // Only store metadata if the image was stored successfully - metadataStore.storeMetadata(relativePath, metadata) + metadataStore.storeMetadata(relativePath, optionalMetadata) + // Also append mandatory metadata keys + metadataStore.storeMetadata( + relativePath, + mapOf( + META_KEY_FORMAT to fileExtension, + ), + ) } } } @@ -53,4 +60,8 @@ internal class ImageRepositoryImpl @Inject internal constructor( localDataSource.deleteImage(image) } } + + companion object { + private const val META_KEY_FORMAT = "format" + } } diff --git a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt index 3e2966f15c..a1a6401587 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt @@ -83,7 +83,7 @@ internal class ImageRepositoryImplTest { sampleId = "sampleId", fileExtension = "jpg", sampleBytes = ByteArray(10), - metadata = mapOf("k" to "v"), + optionalMetadata = mapOf("k" to "v"), ) assertThat(imageRef).isNotNull() @@ -103,7 +103,7 @@ internal class ImageRepositoryImplTest { sampleId = "sampleId", fileExtension = "jpg", sampleBytes = ByteArray(10), - metadata = mapOf("k" to "v"), + optionalMetadata = mapOf("k" to "v"), ) assertThat(imageRef).isNull() From 88e25295ad74d82eb6fc6add9fdb1472eee096c2 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Thu, 10 Jul 2025 15:45:05 +0300 Subject: [PATCH 5/9] MS-1058 Add utility to extract data from samples path --- .../infra/images/ImageRepositoryImpl.kt | 6 +- .../com/simprints/infra/images/model/Path.kt | 2 +- .../images/usecase/CreateSamplePathUseCase.kt | 27 ------ .../images/usecase/SamplePathConvertor.kt | 49 +++++++++++ .../infra/images/ImageRepositoryImplTest.kt | 8 +- .../usecase/CreateSamplePathUseCaseTest.kt | 42 --------- .../images/usecase/SamplePathConvertorTest.kt | 87 +++++++++++++++++++ 7 files changed, 144 insertions(+), 77 deletions(-) delete mode 100644 infra/images/src/main/java/com/simprints/infra/images/usecase/CreateSamplePathUseCase.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConvertor.kt delete mode 100644 infra/images/src/test/java/com/simprints/infra/images/usecase/CreateSamplePathUseCaseTest.kt create mode 100644 infra/images/src/test/java/com/simprints/infra/images/usecase/SamplePathConvertorTest.kt diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt index ff71b41d58..df866a23e5 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt @@ -4,8 +4,8 @@ import com.simprints.infra.config.store.models.GeneralConfiguration import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.SecuredImageRef -import com.simprints.infra.images.usecase.CreateSamplePathUseCase import com.simprints.infra.images.usecase.GetUploaderUseCase +import com.simprints.infra.images.usecase.SamplePathConvertor import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FACE_CAPTURE import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FINGER_CAPTURE import com.simprints.infra.logging.Simber @@ -14,7 +14,7 @@ import javax.inject.Inject internal class ImageRepositoryImpl @Inject internal constructor( private val localDataSource: ImageLocalDataSource, private val metadataStore: ImageMetadataStore, - private val createSamplePathUseCase: CreateSamplePathUseCase, + private val samplePathConvertor: SamplePathConvertor, private val getSampleUploader: GetUploaderUseCase, ) : ImageRepository { override suspend fun storeSample( @@ -27,7 +27,7 @@ internal class ImageRepositoryImpl @Inject internal constructor( optionalMetadata: Map, ): SecuredImageRef? { val logTag = if (modality == GeneralConfiguration.Modality.FACE) FACE_CAPTURE else FINGER_CAPTURE - val relativePath = createSamplePathUseCase(sessionId, modality, sampleId, fileExtension) + val relativePath = samplePathConvertor.create(sessionId, modality, sampleId, fileExtension) Simber.d("Saving $modality sample to ${relativePath.compose()}", tag = logTag) diff --git a/infra/images/src/main/java/com/simprints/infra/images/model/Path.kt b/infra/images/src/main/java/com/simprints/infra/images/model/Path.kt index 245d9937f3..1b230d7de4 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/model/Path.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/model/Path.kt @@ -76,7 +76,7 @@ data class Path( override fun hashCode(): Int = parts.contentHashCode() * 2 + 27 - override fun toString(): String = super.toString() + override fun toString(): String = compose() companion object { /** diff --git a/infra/images/src/main/java/com/simprints/infra/images/usecase/CreateSamplePathUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/usecase/CreateSamplePathUseCase.kt deleted file mode 100644 index 6bb1335246..0000000000 --- a/infra/images/src/main/java/com/simprints/infra/images/usecase/CreateSamplePathUseCase.kt +++ /dev/null @@ -1,27 +0,0 @@ -package com.simprints.infra.images.usecase - -import com.simprints.infra.config.store.models.GeneralConfiguration -import com.simprints.infra.images.model.Path -import javax.inject.Inject - -internal class CreateSamplePathUseCase @Inject constructor() { - operator fun invoke( - sessionId: String, - modality: GeneralConfiguration.Modality, - sampleId: String, - fileExtension: String, - ) = Path( - arrayOf( - SESSIONS_PATH, - sessionId, - if (modality == GeneralConfiguration.Modality.FACE) FACES_PATH else FINGERPRINTS_PATH, - "$sampleId.$fileExtension", - ), - ) - - companion object { - const val SESSIONS_PATH = "sessions" - const val FACES_PATH = "faces" - const val FINGERPRINTS_PATH = "fingerprints" - } -} diff --git a/infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConvertor.kt b/infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConvertor.kt new file mode 100644 index 0000000000..6491d622ec --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConvertor.kt @@ -0,0 +1,49 @@ +package com.simprints.infra.images.usecase + +import com.simprints.infra.config.store.models.GeneralConfiguration +import com.simprints.infra.images.model.Path +import javax.inject.Inject + +internal class SamplePathConvertor @Inject constructor() { + fun create( + sessionId: String, + modality: GeneralConfiguration.Modality, + sampleId: String, + fileExtension: String, + ) = Path( + arrayOf( + SESSIONS_PATH, + sessionId, + if (modality == GeneralConfiguration.Modality.FACE) FACES_PATH else FINGERPRINTS_PATH, + "$sampleId.$fileExtension", + ), + ) + + fun extract(path: Path): PathData? { + // path structure is: projects/{projectId}/sessions/{sessionId}/{faces|fingerprints}/{sampleId}.{extension} + val parts = path.parts.takeIf { it.size > 3 } ?: return null + val sessionsPathIndex = parts.indexOf(SESSIONS_PATH).takeIf { it >= 0 } ?: return null + val sessionId = parts[sessionsPathIndex + 1] + val modality = parts[sessionsPathIndex + 2].let { + when (it) { + FACES_PATH -> GeneralConfiguration.Modality.FACE + else -> GeneralConfiguration.Modality.FINGERPRINT + } + } + val sampleId = parts[sessionsPathIndex + 3].substringBefore(".") + + return PathData(sessionId, modality, sampleId) + } + + data class PathData( + val sessionId: String, + val modality: GeneralConfiguration.Modality, + val sampleId: String, + ) + + companion object Companion { + const val SESSIONS_PATH = "sessions" + const val FACES_PATH = "faces" + const val FINGERPRINTS_PATH = "fingerprints" + } +} diff --git a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt index a1a6401587..373f18ec77 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt @@ -7,8 +7,8 @@ import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.remote.SampleUploader -import com.simprints.infra.images.usecase.CreateSamplePathUseCase import com.simprints.infra.images.usecase.GetUploaderUseCase +import com.simprints.infra.images.usecase.SamplePathConvertor import com.simprints.testtools.common.coroutines.TestCoroutineRule import io.mockk.* import io.mockk.impl.annotations.MockK @@ -31,7 +31,7 @@ internal class ImageRepositoryImplTest { lateinit var metadataStore: ImageMetadataStore @MockK - lateinit var createSamplePathUseCase: CreateSamplePathUseCase + lateinit var samplePathConvertor: SamplePathConvertor @MockK lateinit var getUploaderUseCase: GetUploaderUseCase @@ -42,14 +42,14 @@ internal class ImageRepositoryImplTest { fun setUp() { MockKAnnotations.init(this, relaxed = true) - every { createSamplePathUseCase.invoke(any(), any(), any(), any()) } returns Path(VALID_PATH) + every { samplePathConvertor.create(any(), any(), any(), any()) } returns Path(VALID_PATH) coEvery { sampleUploader.uploadAllSamples(any()) } returns true coEvery { getUploaderUseCase.invoke() } returns sampleUploader repository = ImageRepositoryImpl( localDataSource = localDataSource, metadataStore = metadataStore, - createSamplePathUseCase = createSamplePathUseCase, + samplePathConvertor = samplePathConvertor, getSampleUploader = getUploaderUseCase, ) } diff --git a/infra/images/src/test/java/com/simprints/infra/images/usecase/CreateSamplePathUseCaseTest.kt b/infra/images/src/test/java/com/simprints/infra/images/usecase/CreateSamplePathUseCaseTest.kt deleted file mode 100644 index de78c1ea07..0000000000 --- a/infra/images/src/test/java/com/simprints/infra/images/usecase/CreateSamplePathUseCaseTest.kt +++ /dev/null @@ -1,42 +0,0 @@ -package com.simprints.infra.images.usecase - -import com.google.common.truth.Truth.* -import com.simprints.infra.config.store.models.GeneralConfiguration -import org.junit.Before -import org.junit.Test - -class CreateSamplePathUseCaseTest { - private lateinit var useCase: CreateSamplePathUseCase - - @Before - fun setUp() { - useCase = CreateSamplePathUseCase() - } - - @Test - fun `invoke should create a valid path for face sample`() { - val expectedPath = "sessions/sessionId/faces/captureEventId.jpg" - val result = useCase( - sessionId = "sessionId", - modality = GeneralConfiguration.Modality.FACE, - sampleId = "captureEventId", - fileExtension = "jpg", - ) - - assertThat(result.compose()).isEqualTo(expectedPath) - } - - @Test - fun `invoke should create a valid path for fingerprint sample`() { - val expectedPath = "sessions/sessionId/fingerprints/captureEventId.swq" - - val result = useCase( - sessionId = "sessionId", - modality = GeneralConfiguration.Modality.FINGERPRINT, - sampleId = "captureEventId", - fileExtension = "swq", - ) - - assertThat(result.compose()).isEqualTo(expectedPath) - } -} diff --git a/infra/images/src/test/java/com/simprints/infra/images/usecase/SamplePathConvertorTest.kt b/infra/images/src/test/java/com/simprints/infra/images/usecase/SamplePathConvertorTest.kt new file mode 100644 index 0000000000..8151f4d926 --- /dev/null +++ b/infra/images/src/test/java/com/simprints/infra/images/usecase/SamplePathConvertorTest.kt @@ -0,0 +1,87 @@ +package com.simprints.infra.images.usecase + +import com.google.common.truth.Truth.* +import com.simprints.infra.config.store.models.GeneralConfiguration +import com.simprints.infra.images.model.Path +import org.junit.Before +import org.junit.Test + +class SamplePathConvertorTest { + private lateinit var pathUtil: SamplePathConvertor + + @Before + fun setUp() { + pathUtil = SamplePathConvertor() + } + + @Test + fun `should create a valid path for face sample`() { + val expectedPath = "sessions/sessionId/faces/captureEventId.jpg" + val result = pathUtil.create( + sessionId = "sessionId", + modality = GeneralConfiguration.Modality.FACE, + sampleId = "captureEventId", + fileExtension = "jpg", + ) + + assertThat(result.compose()).isEqualTo(expectedPath) + } + + @Test + fun `should create a valid path for fingerprint sample`() { + val expectedPath = "sessions/sessionId/fingerprints/captureEventId.swq" + + val result = pathUtil.create( + sessionId = "sessionId", + modality = GeneralConfiguration.Modality.FINGERPRINT, + sampleId = "captureEventId", + fileExtension = "swq", + ) + + assertThat(result.compose()).isEqualTo(expectedPath) + } + + @Test + fun `extracts fingerprint sample data from path`() { + val result = pathUtil.extract(Path("sessions/sessionId/fingerprints/captureEventId.swq")) + + assertThat(result).isNotNull() + assertThat(result?.sessionId).isEqualTo("sessionId") + assertThat(result?.sampleId).isEqualTo("captureEventId") + assertThat(result?.modality).isEqualTo(GeneralConfiguration.Modality.FINGERPRINT) + } + + @Test + fun `extracts face sample data from path`() { + val result = pathUtil.extract(Path("sessions/sessionId/faces/captureEventId.swq")) + + assertThat(result).isNotNull() + assertThat(result?.sessionId).isEqualTo("sessionId") + assertThat(result?.sampleId).isEqualTo("captureEventId") + assertThat(result?.modality).isEqualTo(GeneralConfiguration.Modality.FACE) + } + + @Test + fun `extracts sample data from path with project segments`() { + val result = pathUtil.extract(Path("projects/projectId/sessions/sessionId/fingerprints/captureEventId.swq")) + + assertThat(result).isNotNull() + assertThat(result?.sessionId).isEqualTo("sessionId") + assertThat(result?.sampleId).isEqualTo("captureEventId") + assertThat(result?.modality).isEqualTo(GeneralConfiguration.Modality.FINGERPRINT) + } + + @Test + fun `returns null if path is too short`() { + val result = pathUtil.extract(Path("captureEventId.swq")) + + assertThat(result).isNull() + } + + @Test + fun `returns null if path does not contain sessions segment`() { + val result = pathUtil.extract(Path("sessionId/fingerprints/captureEventId.swq")) + + assertThat(result).isNull() + } +} From ccae3f323ea9dc17292880a1ac77a0d1c8e8c4f5 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Thu, 10 Jul 2025 16:46:48 +0300 Subject: [PATCH 6/9] MS-1058 Add signed url sample uploader --- .../ApiEventSampleUpSyncRequestPayload.kt | 2 +- .../samples/SampleUpSyncRequestEvent.kt | 4 +- infra/images/build.gradle.kts | 3 +- .../signedurl/SignedUrlSampleUploader.kt | 213 ++++++++++++++++++ .../api/ApiSampleUploadUrlRequest.kt | 12 + .../api/ApiSampleUploadUrlResponse.kt | 9 + .../signedurl/api/SampleUploadApiInterface.kt | 37 +++ .../signedurl/api/SampleUploadRequestBody.kt | 25 ++ .../usecase/CalculateFileMd5AndSizeUseCase.kt | 53 +++++ .../images/usecase/GetUploaderUseCase.kt | 10 +- .../signedurl/SignedUrlSampleUploaderTest.kt | 5 + .../CalculateFileMd5AndSizeUseCaseTest.kt | 49 ++++ .../httpclient/BuildOkHttpClientUseCase.kt | 9 +- 13 files changed, 418 insertions(+), 13 deletions(-) create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/ApiSampleUploadUrlRequest.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/ApiSampleUploadUrlResponse.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/SampleUploadApiInterface.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/SampleUploadRequestBody.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCase.kt create mode 100644 infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt create mode 100644 infra/images/src/test/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCaseTest.kt diff --git a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/event/remote/models/samples/ApiEventSampleUpSyncRequestPayload.kt b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/event/remote/models/samples/ApiEventSampleUpSyncRequestPayload.kt index 46474be1eb..2feaf75161 100644 --- a/infra/event-sync/src/main/java/com/simprints/infra/eventsync/event/remote/models/samples/ApiEventSampleUpSyncRequestPayload.kt +++ b/infra/event-sync/src/main/java/com/simprints/infra/eventsync/event/remote/models/samples/ApiEventSampleUpSyncRequestPayload.kt @@ -13,7 +13,7 @@ internal data class ApiEventSampleUpSyncRequestPayload( val endTime: ApiTimestamp?, val requestId: String, val sampleId: String, - val size: Int, + val size: Long, val errorType: String?, ) : ApiEventPayload(startTime) { constructor(domainPayload: SampleUpSyncRequestEvent.SampleUpSyncRequestPayload) : this( diff --git a/infra/events/src/main/java/com/simprints/infra/events/event/domain/models/samples/SampleUpSyncRequestEvent.kt b/infra/events/src/main/java/com/simprints/infra/events/event/domain/models/samples/SampleUpSyncRequestEvent.kt index bbf8b43bd0..89389d57de 100644 --- a/infra/events/src/main/java/com/simprints/infra/events/event/domain/models/samples/SampleUpSyncRequestEvent.kt +++ b/infra/events/src/main/java/com/simprints/infra/events/event/domain/models/samples/SampleUpSyncRequestEvent.kt @@ -22,7 +22,7 @@ class SampleUpSyncRequestEvent( endedAt: Timestamp, requestId: String, sampleId: String, - size: Int, + size: Long, errorType: String? = null, ) : this( UUID.randomUUID().toString(), @@ -44,7 +44,7 @@ class SampleUpSyncRequestEvent( override val endedAt: Timestamp?, val requestId: String, val sampleId: String, - val size: Int, + val size: Long, val errorType: String?, override val eventVersion: Int, override val type: EventType = EventType.SAMPLE_UP_SYNC_REQUEST, diff --git a/infra/images/build.gradle.kts b/infra/images/build.gradle.kts index 5ed3944f35..136dd7b163 100644 --- a/infra/images/build.gradle.kts +++ b/infra/images/build.gradle.kts @@ -7,13 +7,13 @@ plugins { android { namespace = "com.simprints.infra.images" - } dependencies { implementation(project(":infra:auth-store")) implementation(project(":infra:config-store")) implementation(project(":infra:config-sync")) + implementation(project(":infra:events")) // Firebase implementation(libs.firebase.storage) @@ -22,4 +22,5 @@ dependencies { implementation(libs.androidX.security) implementation(libs.kotlin.coroutinesPlayServices) + implementation(libs.retrofit.core) } diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt new file mode 100644 index 0000000000..4be6e2812f --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt @@ -0,0 +1,213 @@ +package com.simprints.infra.images.remote.signedurl + +import com.simprints.core.tools.time.TimeHelper +import com.simprints.infra.authstore.AuthStore +import com.simprints.infra.config.store.ConfigRepository +import com.simprints.infra.events.EventRepository +import com.simprints.infra.events.event.domain.models.samples.SampleUpSyncRequestEvent +import com.simprints.infra.events.event.domain.models.scope.EventScope +import com.simprints.infra.events.event.domain.models.scope.EventScopeEndCause +import com.simprints.infra.events.event.domain.models.scope.EventScopeType +import com.simprints.infra.images.local.ImageLocalDataSource +import com.simprints.infra.images.metadata.ImageMetadataStore +import com.simprints.infra.images.model.SecuredImageRef +import com.simprints.infra.images.remote.SampleUploader +import com.simprints.infra.images.remote.signedurl.api.ApiSampleUploadUrlRequest +import com.simprints.infra.images.remote.signedurl.api.SampleUploadApiInterface +import com.simprints.infra.images.remote.signedurl.api.SampleUploadRequestBody +import com.simprints.infra.images.usecase.CalculateFileMd5AndSizeUseCase +import com.simprints.infra.images.usecase.SamplePathConvertor +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC +import com.simprints.infra.logging.Simber +import com.simprints.infra.network.SimNetwork +import kotlinx.coroutines.isActive +import java.util.UUID +import javax.inject.Inject +import kotlin.coroutines.coroutineContext + +internal class SignedUrlSampleUploader @Inject constructor( + private val timeHelper: TimeHelper, + private val authStore: AuthStore, + private val eventRepository: EventRepository, + private val configRepository: ConfigRepository, + private val localDataSource: ImageLocalDataSource, + private val metadataStore: ImageMetadataStore, + private val samplePathUtil: SamplePathConvertor, + private val calculateFileMd5AndSize: CalculateFileMd5AndSizeUseCase, +) : SampleUploader { + override suspend fun uploadAllSamples(projectId: String): Boolean { + val client = getClient() + val batchSize = getBatchSize() + + val urlRequestScope = eventRepository.createEventScope(type = EventScopeType.SAMPLE_UP_SYNC) + var allImagesUploaded = true + + val sampleBatches = localDataSource.listImages(projectId).chunked(batchSize) + for (batch in sampleBatches) { + if (!coroutineContext.isActive) { + // Do not process next batch if coroutine is being cancelled + allImagesUploaded = false + break + } + + // Read batch of images to calculate hashes, size and other meta data + val batchUploadData = batch.mapNotNull { imageRef -> + try { + prepareImageUploadData(imageRef).also { + if (it == null) { + allImagesUploaded = false + Simber.i("Failed to read image file without exception", tag = SYNC) + } + } + } catch (t: Throwable) { + allImagesUploaded = false + Simber.e("Failed to read image file", t, tag = SYNC) + null + } + } + + // Fetch upload urls for each image + val sampleUrlMap = fetchUploadUrlsPerSample(batchUploadData, batchSize, client, projectId) + + for (sample in batchUploadData) { + if (!coroutineContext.isActive) { + // Do not upload next image if coroutine is being cancelled + allImagesUploaded = false + break + } + + val url = sampleUrlMap[sample.sampleId] + if (url == null) { + allImagesUploaded = false + Simber.i("Failed to fetch sample url", tag = SYNC) + continue + } + + // Upload the sample to the fetched URL and + val success = uploadSampleWithEventTracking(client, sample, url, urlRequestScope) + if (success) { + localDataSource.deleteImage(sample.imageRef) + metadataStore.deleteMetadata(sample.imageRef.relativePath) + } else { + allImagesUploaded = false + } + } + } + eventRepository.closeEventScope(urlRequestScope, EventScopeEndCause.WORKFLOW_ENDED) + + return allImagesUploaded + } + + private suspend fun getClient(): SimNetwork.SimApiClient = + authStore.buildClient(SampleUploadApiInterface::class) + + private suspend fun prepareImageUploadData(imageRef: SecuredImageRef): SampleUploadData? = localDataSource + .decryptImage(imageRef) + ?.use { stream -> calculateFileMd5AndSize(stream) } + ?.let { (md5, size) -> + val (sessionId, modality, sampleId) = samplePathUtil.extract(imageRef.relativePath) ?: return null + val metadata = metadataStore.getMetadata(imageRef.relativePath) + + SampleUploadData( + imageRef = imageRef, + sampleId = sampleId, + sessionId = sessionId, + md5 = md5, + size = size, + modality = modality.name, + metadata = metadata, + ) + } + + private suspend fun fetchUploadUrlsPerSample( + batchUploadData: List, + batchSize: Int, + client: SimNetwork.SimApiClient, + projectId: String, + ): Map = batchUploadData + .map { + ApiSampleUploadUrlRequest( + sampleId = it.sampleId, + sessionId = it.sessionId, + modality = it.modality, + md5 = it.md5, + metadata = it.metadata, + ) + }.chunked(batchSize) + .map { batch -> + + try { + client.executeCall { api -> api.getSampleUploadUrl(projectId, batch) } + } catch (_: Exception) { + emptyList() + } + }.flatten() + .associate { it.sampleId to it.url } + + private suspend fun getBatchSize(): Int = configRepository + .getProjectConfiguration() + .synchronization + .samples.signedUrlBatchSize + + private suspend fun uploadSampleWithEventTracking( + client: SimNetwork.SimApiClient, + sample: SampleUploadData, + url: String, + urlRequestScope: EventScope, + ): Boolean { + val requestId = UUID.randomUUID().toString() + val requestStartTime = timeHelper.now() + + val errorType = uploadSample(client, sample, url, requestId) + eventRepository.addOrUpdateEvent( + scope = urlRequestScope, + event = SampleUpSyncRequestEvent( + createdAt = requestStartTime, + endedAt = timeHelper.now(), + requestId = requestId, + sampleId = sample.sampleId, + size = sample.size, + errorType = errorType, + ), + ) + return errorType.isNullOrBlank() + } + + private suspend fun uploadSample( + client: SimNetwork.SimApiClient, + sampleData: SampleUploadData, + url: String, + requestId: String, + ): String? = localDataSource.decryptImage(sampleData.imageRef)?.use { stream -> + try { + val response = client.executeCall { api -> + api.uploadFile( + uploadUrl = url, + requestId = requestId, + md5 = sampleData.md5, + requestBody = SampleUploadRequestBody(stream, sampleData.size), + ) + } + if (response.isSuccessful) { + null + } else { + response.errorBody()?.string().also { + Simber.i("Failed to upload image: $it", tag = SYNC) + } + } + } catch (e: Exception) { + Simber.e("Failed to upload image", e, tag = SYNC) + e.javaClass.simpleName + } + } + + private data class SampleUploadData( + val imageRef: SecuredImageRef, + val sampleId: String, + val sessionId: String, + val modality: String, + val md5: String, + val size: Long, + val metadata: Map, + ) +} diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/ApiSampleUploadUrlRequest.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/ApiSampleUploadUrlRequest.kt new file mode 100644 index 0000000000..d763cc11ed --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/ApiSampleUploadUrlRequest.kt @@ -0,0 +1,12 @@ +package com.simprints.infra.images.remote.signedurl.api + +import androidx.annotation.Keep + +@Keep +internal data class ApiSampleUploadUrlRequest( + val sampleId: String, + val sessionId: String, + val md5: String, + val modality: String, + val metadata: Map, +) diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/ApiSampleUploadUrlResponse.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/ApiSampleUploadUrlResponse.kt new file mode 100644 index 0000000000..daf6e85ff8 --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/ApiSampleUploadUrlResponse.kt @@ -0,0 +1,9 @@ +package com.simprints.infra.images.remote.signedurl.api + +import androidx.annotation.Keep + +@Keep +internal data class ApiSampleUploadUrlResponse( + val url: String, + val sampleId: String, +) diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/SampleUploadApiInterface.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/SampleUploadApiInterface.kt new file mode 100644 index 0000000000..47ffbb5f31 --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/SampleUploadApiInterface.kt @@ -0,0 +1,37 @@ +package com.simprints.infra.images.remote.signedurl.api + +import androidx.annotation.Keep +import com.simprints.infra.network.SimRemoteInterface +import okhttp3.ResponseBody +import retrofit2.Response +import retrofit2.http.Body +import retrofit2.http.Header +import retrofit2.http.POST +import retrofit2.http.PUT +import retrofit2.http.Path +import retrofit2.http.Url + +@Keep +internal interface SampleUploadApiInterface : SimRemoteInterface { + @POST("projects/{projectId}/samples") + suspend fun getSampleUploadUrl( + @Path("projectId") projectId: String, + @Body samples: List, + ): List + + /** + * Uploads the provided sample to the provided url. + * + * Since sample files are stored in an encrypted files and are accessed via non-resettable + * input stream, body logging will consume the stream and the actual upload will fail. + * + * **Therefore for this upload to work the highest log level is HEADER.** + */ + @PUT + suspend fun uploadFile( + @Url uploadUrl: String, + @Header("X-Request-ID") requestId: String, + @Header("Content-MD5") md5: String, + @Body requestBody: SampleUploadRequestBody, + ): Response +} diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/SampleUploadRequestBody.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/SampleUploadRequestBody.kt new file mode 100644 index 0000000000..9863a87007 --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/api/SampleUploadRequestBody.kt @@ -0,0 +1,25 @@ +package com.simprints.infra.images.remote.signedurl.api + +import com.simprints.core.ExcludedFromGeneratedTestCoverageReports +import okhttp3.MediaType +import okhttp3.MediaType.Companion.toMediaTypeOrNull +import okhttp3.RequestBody +import okio.BufferedSink +import okio.source +import java.io.FileInputStream +import java.io.IOException + +@ExcludedFromGeneratedTestCoverageReports("OkHttp specific code that is not possible to test in isolation") +internal class SampleUploadRequestBody( + private val inputStream: FileInputStream, + private val size: Long, +) : RequestBody() { + override fun contentType(): MediaType? = "application/octet-stream".toMediaTypeOrNull() + + override fun contentLength(): Long = size + + @Throws(IOException::class) + override fun writeTo(sink: BufferedSink) { + inputStream.source().use { source -> sink.writeAll(source) } + } +} diff --git a/infra/images/src/main/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCase.kt new file mode 100644 index 0000000000..2e956570bd --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCase.kt @@ -0,0 +1,53 @@ +package com.simprints.infra.images.usecase + +import com.simprints.core.tools.utils.EncodingUtils +import java.io.IOException +import java.io.InputStream +import java.security.DigestInputStream +import java.security.MessageDigest +import javax.inject.Inject + +/** + * This operation consumes the input stream and since the encrypted file stream does + * not support mark/reset, the file needs to be re-opened again on the upload. + * + * For this reason un-encrypted file size is also calculated during md5 calculation + * to be used between stream reads. + */ +internal class CalculateFileMd5AndSizeUseCase @Inject constructor( + private val encodingUtils: EncodingUtils, +) { + operator fun invoke(inputStream: InputStream): CalculationResult = try { + val md5 = MessageDigest.getInstance("MD5") + var size = 0L + + DigestInputStream(inputStream, md5).use { dis -> + val buffer = ByteArray(BUFFER_SIZE) + var bytesRead = 0 + while (dis.read(buffer).also { bytesRead = it } != -1) { + // Appending directly in the check causes off-by-one error due to "-1" marker + size += bytesRead + // No explicit update to 'md' needed here, DigestInputStream handles it. + } + } + + val digest = md5.digest() + + CalculationResult( + md5 = encodingUtils.byteArrayToBase64(digest), + size = size, + ) + } catch (e: IOException) { + e.printStackTrace() + CalculationResult("", 0) + } + + companion object Companion { + private const val BUFFER_SIZE = 8 * 1024 // 8K as suggested by AI + } + + internal data class CalculationResult( + val md5: String, + val size: Long, + ) +} diff --git a/infra/images/src/main/java/com/simprints/infra/images/usecase/GetUploaderUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/usecase/GetUploaderUseCase.kt index cf4e86fd91..61986f9637 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/usecase/GetUploaderUseCase.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/usecase/GetUploaderUseCase.kt @@ -4,21 +4,17 @@ import com.simprints.infra.config.store.ConfigRepository import com.simprints.infra.config.store.models.experimental import com.simprints.infra.images.remote.SampleUploader import com.simprints.infra.images.remote.firestore.FirestoreSampleUploader +import com.simprints.infra.images.remote.signedurl.SignedUrlSampleUploader import javax.inject.Inject internal class GetUploaderUseCase @Inject constructor( private val configRepository: ConfigRepository, private val firestoreUploader: FirestoreSampleUploader, + private val signedUrlUploader: SignedUrlSampleUploader, ) { suspend operator fun invoke(): SampleUploader = configRepository .getProjectConfiguration() .experimental() .sampleUploadWithSignedUrlEnabled - .let { - if (it) { - TODO() - } else { - firestoreUploader - } - } + .let { if (it) signedUrlUploader else firestoreUploader } } diff --git a/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt new file mode 100644 index 0000000000..2b2ef847b5 --- /dev/null +++ b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt @@ -0,0 +1,5 @@ +package com.simprints.infra.images.remote.signedurl + +class SignedUrlSampleUploaderTest { + // TODO +} diff --git a/infra/images/src/test/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCaseTest.kt b/infra/images/src/test/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCaseTest.kt new file mode 100644 index 0000000000..6a04323e73 --- /dev/null +++ b/infra/images/src/test/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCaseTest.kt @@ -0,0 +1,49 @@ +package com.simprints.infra.images.usecase + +import com.google.common.truth.Truth.* +import com.simprints.core.tools.utils.EncodingUtils +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import java.io.ByteArrayInputStream +import java.util.Base64 + +internal class CalculateFileMd5AndSizeUseCaseTest { + // Core version relies on Android implementation, that does not work in tests + private val encodingUtil = object : EncodingUtils { + override fun byteArrayToBase64(bytes: ByteArray): String = Base64.getEncoder().encodeToString(bytes) + + override fun base64ToBytes(base64: String): ByteArray = TODO("No-op") + } + + private val testByteArray: ByteArray + get() = "testString".toByteArray(Charsets.UTF_8) + + private lateinit var useCase: CalculateFileMd5AndSizeUseCase + + @Before + fun setUp() { + useCase = CalculateFileMd5AndSizeUseCase(encodingUtil) + } + + @Test + fun `correctly computes MD5 checksum of provided input stream`() = runTest { + val stream = ByteArrayInputStream(testByteArray) + + val result = useCase.invoke(stream) + + // Calculated using online calculator + assertThat(result.md5).isEqualTo("U2eI9Nvf/uz7uPNQqUHuow==") + assertThat(result.size).isEqualTo(testByteArray.size) + } + + @Test + fun `correctly computes MD5 checksum of provided input stream with multiple buffer reads`() = runTest { + val largeBufferSize = 30 * 1024 + val stream = ByteArrayInputStream(ByteArray(largeBufferSize, { 1 })) + + val result = useCase.invoke(stream) + + assertThat(result.size).isEqualTo(largeBufferSize) + } +} diff --git a/infra/network/src/main/java/com/simprints/infra/network/httpclient/BuildOkHttpClientUseCase.kt b/infra/network/src/main/java/com/simprints/infra/network/httpclient/BuildOkHttpClientUseCase.kt index ceb935d6a5..1c04bb1be9 100644 --- a/infra/network/src/main/java/com/simprints/infra/network/httpclient/BuildOkHttpClientUseCase.kt +++ b/infra/network/src/main/java/com/simprints/infra/network/httpclient/BuildOkHttpClientUseCase.kt @@ -88,8 +88,13 @@ internal class BuildOkHttpClientUseCase @Inject constructor( if (!currentAuthToken.isNullOrBlank()) { addInterceptor(buildAuthenticationInterceptor(currentAuthToken!!)) } - }.addNetworkInterceptor(ChuckerInterceptor.Builder(ctx).build()) - .addInterceptor(buildDeviceIdInterceptor(deviceId)) + }.addNetworkInterceptor( + ChuckerInterceptor + .Builder(ctx) + // Skip sample storage domain to avoid interfering with encrypted file streaming + .skipDomains("storage.googleapis.com") + .build(), + ).addInterceptor(buildDeviceIdInterceptor(deviceId)) .addInterceptor(buildVersionInterceptor(versionName)) .addInterceptor(buildGZipInterceptor()) .apply { From 37b43236b8570c2e2c3ff1c89c8ac4b6d2a5107a Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 14 Jul 2025 10:19:43 +0300 Subject: [PATCH 7/9] MS-1058 Fix path converter naming --- .../com/simprints/infra/images/ImageRepositoryImpl.kt | 6 +++--- .../images/remote/signedurl/SignedUrlSampleUploader.kt | 4 ++-- .../{SamplePathConvertor.kt => SamplePathConverter.kt} | 2 +- .../com/simprints/infra/images/ImageRepositoryImplTest.kt | 8 ++++---- .../infra/images/usecase/SamplePathConvertorTest.kt | 4 ++-- 5 files changed, 12 insertions(+), 12 deletions(-) rename infra/images/src/main/java/com/simprints/infra/images/usecase/{SamplePathConvertor.kt => SamplePathConverter.kt} (96%) diff --git a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt index df866a23e5..73f87648d2 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/ImageRepositoryImpl.kt @@ -5,7 +5,7 @@ import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.usecase.GetUploaderUseCase -import com.simprints.infra.images.usecase.SamplePathConvertor +import com.simprints.infra.images.usecase.SamplePathConverter import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FACE_CAPTURE import com.simprints.infra.logging.LoggingConstants.CrashReportTag.FINGER_CAPTURE import com.simprints.infra.logging.Simber @@ -14,7 +14,7 @@ import javax.inject.Inject internal class ImageRepositoryImpl @Inject internal constructor( private val localDataSource: ImageLocalDataSource, private val metadataStore: ImageMetadataStore, - private val samplePathConvertor: SamplePathConvertor, + private val samplePathConverter: SamplePathConverter, private val getSampleUploader: GetUploaderUseCase, ) : ImageRepository { override suspend fun storeSample( @@ -27,7 +27,7 @@ internal class ImageRepositoryImpl @Inject internal constructor( optionalMetadata: Map, ): SecuredImageRef? { val logTag = if (modality == GeneralConfiguration.Modality.FACE) FACE_CAPTURE else FINGER_CAPTURE - val relativePath = samplePathConvertor.create(sessionId, modality, sampleId, fileExtension) + val relativePath = samplePathConverter.create(sessionId, modality, sampleId, fileExtension) Simber.d("Saving $modality sample to ${relativePath.compose()}", tag = logTag) diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt index 4be6e2812f..8b7488d82b 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt @@ -16,7 +16,7 @@ import com.simprints.infra.images.remote.signedurl.api.ApiSampleUploadUrlRequest import com.simprints.infra.images.remote.signedurl.api.SampleUploadApiInterface import com.simprints.infra.images.remote.signedurl.api.SampleUploadRequestBody import com.simprints.infra.images.usecase.CalculateFileMd5AndSizeUseCase -import com.simprints.infra.images.usecase.SamplePathConvertor +import com.simprints.infra.images.usecase.SamplePathConverter import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber import com.simprints.infra.network.SimNetwork @@ -32,7 +32,7 @@ internal class SignedUrlSampleUploader @Inject constructor( private val configRepository: ConfigRepository, private val localDataSource: ImageLocalDataSource, private val metadataStore: ImageMetadataStore, - private val samplePathUtil: SamplePathConvertor, + private val samplePathUtil: SamplePathConverter, private val calculateFileMd5AndSize: CalculateFileMd5AndSizeUseCase, ) : SampleUploader { override suspend fun uploadAllSamples(projectId: String): Boolean { diff --git a/infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConvertor.kt b/infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConverter.kt similarity index 96% rename from infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConvertor.kt rename to infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConverter.kt index 6491d622ec..824a6091cc 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConvertor.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/usecase/SamplePathConverter.kt @@ -4,7 +4,7 @@ import com.simprints.infra.config.store.models.GeneralConfiguration import com.simprints.infra.images.model.Path import javax.inject.Inject -internal class SamplePathConvertor @Inject constructor() { +internal class SamplePathConverter @Inject constructor() { fun create( sessionId: String, modality: GeneralConfiguration.Modality, diff --git a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt index 373f18ec77..e63c8f76f6 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/ImageRepositoryImplTest.kt @@ -8,7 +8,7 @@ import com.simprints.infra.images.model.Path import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.remote.SampleUploader import com.simprints.infra.images.usecase.GetUploaderUseCase -import com.simprints.infra.images.usecase.SamplePathConvertor +import com.simprints.infra.images.usecase.SamplePathConverter import com.simprints.testtools.common.coroutines.TestCoroutineRule import io.mockk.* import io.mockk.impl.annotations.MockK @@ -31,7 +31,7 @@ internal class ImageRepositoryImplTest { lateinit var metadataStore: ImageMetadataStore @MockK - lateinit var samplePathConvertor: SamplePathConvertor + lateinit var samplePathConverter: SamplePathConverter @MockK lateinit var getUploaderUseCase: GetUploaderUseCase @@ -42,14 +42,14 @@ internal class ImageRepositoryImplTest { fun setUp() { MockKAnnotations.init(this, relaxed = true) - every { samplePathConvertor.create(any(), any(), any(), any()) } returns Path(VALID_PATH) + every { samplePathConverter.create(any(), any(), any(), any()) } returns Path(VALID_PATH) coEvery { sampleUploader.uploadAllSamples(any()) } returns true coEvery { getUploaderUseCase.invoke() } returns sampleUploader repository = ImageRepositoryImpl( localDataSource = localDataSource, metadataStore = metadataStore, - samplePathConvertor = samplePathConvertor, + samplePathConverter = samplePathConverter, getSampleUploader = getUploaderUseCase, ) } diff --git a/infra/images/src/test/java/com/simprints/infra/images/usecase/SamplePathConvertorTest.kt b/infra/images/src/test/java/com/simprints/infra/images/usecase/SamplePathConvertorTest.kt index 8151f4d926..aa988374ec 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/usecase/SamplePathConvertorTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/usecase/SamplePathConvertorTest.kt @@ -7,11 +7,11 @@ import org.junit.Before import org.junit.Test class SamplePathConvertorTest { - private lateinit var pathUtil: SamplePathConvertor + private lateinit var pathUtil: SamplePathConverter @Before fun setUp() { - pathUtil = SamplePathConvertor() + pathUtil = SamplePathConverter() } @Test From 9fa628fc709d058174f2d12002d9b843fe099c53 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 14 Jul 2025 15:35:55 +0300 Subject: [PATCH 8/9] MS-1058 Refactor signed url uploader for readability and testability --- .../remote/signedurl/SampleUploadData.kt | 13 + .../signedurl/SignedUrlSampleUploader.kt | 154 ++--------- .../FetchUploadUrlsPerSampleUseCase.kt | 33 +++ .../usecase/PrepareImageUploadDataUseCase.kt | 35 +++ .../UploadSampleWithTrackingUseCase.kt | 80 ++++++ .../signedurl/SignedUrlSampleUploaderTest.kt | 253 +++++++++++++++++- .../FetchUploadUrlsPerSampleUseCaseTest.kt | 92 +++++++ .../PrepareImageUploadDataUseCaseTest.kt | 91 +++++++ .../UploadSampleWithTrackingUseCaseTest.kt | 130 +++++++++ 9 files changed, 745 insertions(+), 136 deletions(-) create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SampleUploadData.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/FetchUploadUrlsPerSampleUseCase.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/PrepareImageUploadDataUseCase.kt create mode 100644 infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCase.kt create mode 100644 infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/FetchUploadUrlsPerSampleUseCaseTest.kt create mode 100644 infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/PrepareImageUploadDataUseCaseTest.kt create mode 100644 infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCaseTest.kt diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SampleUploadData.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SampleUploadData.kt new file mode 100644 index 0000000000..f1883ce459 --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SampleUploadData.kt @@ -0,0 +1,13 @@ +package com.simprints.infra.images.remote.signedurl + +import com.simprints.infra.images.model.SecuredImageRef + +internal data class SampleUploadData( + val imageRef: SecuredImageRef, + val sampleId: String, + val sessionId: String, + val modality: String, + val md5: String, + val size: Long, + val metadata: Map, +) diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt index 8b7488d82b..eb7c4596ec 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt @@ -1,49 +1,43 @@ package com.simprints.infra.images.remote.signedurl -import com.simprints.core.tools.time.TimeHelper -import com.simprints.infra.authstore.AuthStore import com.simprints.infra.config.store.ConfigRepository import com.simprints.infra.events.EventRepository -import com.simprints.infra.events.event.domain.models.samples.SampleUpSyncRequestEvent -import com.simprints.infra.events.event.domain.models.scope.EventScope import com.simprints.infra.events.event.domain.models.scope.EventScopeEndCause import com.simprints.infra.events.event.domain.models.scope.EventScopeType import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore -import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.remote.SampleUploader -import com.simprints.infra.images.remote.signedurl.api.ApiSampleUploadUrlRequest -import com.simprints.infra.images.remote.signedurl.api.SampleUploadApiInterface -import com.simprints.infra.images.remote.signedurl.api.SampleUploadRequestBody -import com.simprints.infra.images.usecase.CalculateFileMd5AndSizeUseCase -import com.simprints.infra.images.usecase.SamplePathConverter +import com.simprints.infra.images.remote.signedurl.usecase.FetchUploadUrlsPerSampleUseCase +import com.simprints.infra.images.remote.signedurl.usecase.PrepareImageUploadDataUseCase +import com.simprints.infra.images.remote.signedurl.usecase.UploadSampleWithTrackingUseCase import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber -import com.simprints.infra.network.SimNetwork import kotlinx.coroutines.isActive -import java.util.UUID import javax.inject.Inject import kotlin.coroutines.coroutineContext internal class SignedUrlSampleUploader @Inject constructor( - private val timeHelper: TimeHelper, - private val authStore: AuthStore, - private val eventRepository: EventRepository, private val configRepository: ConfigRepository, private val localDataSource: ImageLocalDataSource, + private val eventRepository: EventRepository, private val metadataStore: ImageMetadataStore, - private val samplePathUtil: SamplePathConverter, - private val calculateFileMd5AndSize: CalculateFileMd5AndSizeUseCase, + private val prepareImageUploadData: PrepareImageUploadDataUseCase, + private val uploadSampleWithTracking: UploadSampleWithTrackingUseCase, + private val fetchUploadUrlsPerSample: FetchUploadUrlsPerSampleUseCase, ) : SampleUploader { override suspend fun uploadAllSamples(projectId: String): Boolean { - val client = getClient() + var allImagesUploaded = true val batchSize = getBatchSize() - val urlRequestScope = eventRepository.createEventScope(type = EventScopeType.SAMPLE_UP_SYNC) - var allImagesUploaded = true - val sampleBatches = localDataSource.listImages(projectId).chunked(batchSize) - for (batch in sampleBatches) { + val sampleReferenceBatches = localDataSource + .listImages(projectId) + // Preparing the file for upload requires reading each of them to calculate md5 and size, + // therefore splitting the list into batches before preparing allows to avoid some work in + // cases where there are large amounts of files and the coroutine is being interrupted, + // even if the result is that some requested batches are not at max size. + .chunked(batchSize) + for (batch in sampleReferenceBatches) { if (!coroutineContext.isActive) { // Do not process next batch if coroutine is being cancelled allImagesUploaded = false @@ -67,7 +61,7 @@ internal class SignedUrlSampleUploader @Inject constructor( } // Fetch upload urls for each image - val sampleUrlMap = fetchUploadUrlsPerSample(batchUploadData, batchSize, client, projectId) + val sampleIdToUrlMap = fetchUploadUrlsPerSample(projectId, batchUploadData) for (sample in batchUploadData) { if (!coroutineContext.isActive) { @@ -76,15 +70,15 @@ internal class SignedUrlSampleUploader @Inject constructor( break } - val url = sampleUrlMap[sample.sampleId] + val url = sampleIdToUrlMap[sample.sampleId] if (url == null) { allImagesUploaded = false Simber.i("Failed to fetch sample url", tag = SYNC) continue } - // Upload the sample to the fetched URL and - val success = uploadSampleWithEventTracking(client, sample, url, urlRequestScope) + // Upload the sample to the fetched URL and clean up the local storage if successful + val success = uploadSampleWithTracking(urlRequestScope, url, sample) if (success) { localDataSource.deleteImage(sample.imageRef) metadataStore.deleteMetadata(sample.imageRef.relativePath) @@ -98,116 +92,8 @@ internal class SignedUrlSampleUploader @Inject constructor( return allImagesUploaded } - private suspend fun getClient(): SimNetwork.SimApiClient = - authStore.buildClient(SampleUploadApiInterface::class) - - private suspend fun prepareImageUploadData(imageRef: SecuredImageRef): SampleUploadData? = localDataSource - .decryptImage(imageRef) - ?.use { stream -> calculateFileMd5AndSize(stream) } - ?.let { (md5, size) -> - val (sessionId, modality, sampleId) = samplePathUtil.extract(imageRef.relativePath) ?: return null - val metadata = metadataStore.getMetadata(imageRef.relativePath) - - SampleUploadData( - imageRef = imageRef, - sampleId = sampleId, - sessionId = sessionId, - md5 = md5, - size = size, - modality = modality.name, - metadata = metadata, - ) - } - - private suspend fun fetchUploadUrlsPerSample( - batchUploadData: List, - batchSize: Int, - client: SimNetwork.SimApiClient, - projectId: String, - ): Map = batchUploadData - .map { - ApiSampleUploadUrlRequest( - sampleId = it.sampleId, - sessionId = it.sessionId, - modality = it.modality, - md5 = it.md5, - metadata = it.metadata, - ) - }.chunked(batchSize) - .map { batch -> - - try { - client.executeCall { api -> api.getSampleUploadUrl(projectId, batch) } - } catch (_: Exception) { - emptyList() - } - }.flatten() - .associate { it.sampleId to it.url } - private suspend fun getBatchSize(): Int = configRepository .getProjectConfiguration() .synchronization .samples.signedUrlBatchSize - - private suspend fun uploadSampleWithEventTracking( - client: SimNetwork.SimApiClient, - sample: SampleUploadData, - url: String, - urlRequestScope: EventScope, - ): Boolean { - val requestId = UUID.randomUUID().toString() - val requestStartTime = timeHelper.now() - - val errorType = uploadSample(client, sample, url, requestId) - eventRepository.addOrUpdateEvent( - scope = urlRequestScope, - event = SampleUpSyncRequestEvent( - createdAt = requestStartTime, - endedAt = timeHelper.now(), - requestId = requestId, - sampleId = sample.sampleId, - size = sample.size, - errorType = errorType, - ), - ) - return errorType.isNullOrBlank() - } - - private suspend fun uploadSample( - client: SimNetwork.SimApiClient, - sampleData: SampleUploadData, - url: String, - requestId: String, - ): String? = localDataSource.decryptImage(sampleData.imageRef)?.use { stream -> - try { - val response = client.executeCall { api -> - api.uploadFile( - uploadUrl = url, - requestId = requestId, - md5 = sampleData.md5, - requestBody = SampleUploadRequestBody(stream, sampleData.size), - ) - } - if (response.isSuccessful) { - null - } else { - response.errorBody()?.string().also { - Simber.i("Failed to upload image: $it", tag = SYNC) - } - } - } catch (e: Exception) { - Simber.e("Failed to upload image", e, tag = SYNC) - e.javaClass.simpleName - } - } - - private data class SampleUploadData( - val imageRef: SecuredImageRef, - val sampleId: String, - val sessionId: String, - val modality: String, - val md5: String, - val size: Long, - val metadata: Map, - ) } diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/FetchUploadUrlsPerSampleUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/FetchUploadUrlsPerSampleUseCase.kt new file mode 100644 index 0000000000..7627086c86 --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/FetchUploadUrlsPerSampleUseCase.kt @@ -0,0 +1,33 @@ +package com.simprints.infra.images.remote.signedurl.usecase + +import com.simprints.infra.authstore.AuthStore +import com.simprints.infra.images.remote.signedurl.SampleUploadData +import com.simprints.infra.images.remote.signedurl.api.ApiSampleUploadUrlRequest +import com.simprints.infra.images.remote.signedurl.api.SampleUploadApiInterface +import javax.inject.Inject + +internal class FetchUploadUrlsPerSampleUseCase @Inject constructor( + private val authStore: AuthStore, +) { + suspend operator fun invoke( + projectId: String, + batchUploadData: List, + ): Map = batchUploadData + .map { + ApiSampleUploadUrlRequest( + sampleId = it.sampleId, + sessionId = it.sessionId, + modality = it.modality, + md5 = it.md5, + metadata = it.metadata, + ) + }.let { batch -> + try { + authStore + .buildClient(SampleUploadApiInterface::class) + .executeCall { api -> api.getSampleUploadUrl(projectId, batch) } + } catch (_: Exception) { + emptyList() + } + }.associate { it.sampleId to it.url } +} diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/PrepareImageUploadDataUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/PrepareImageUploadDataUseCase.kt new file mode 100644 index 0000000000..534919217f --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/PrepareImageUploadDataUseCase.kt @@ -0,0 +1,35 @@ +package com.simprints.infra.images.remote.signedurl.usecase + +import com.simprints.infra.images.local.ImageLocalDataSource +import com.simprints.infra.images.metadata.ImageMetadataStore +import com.simprints.infra.images.model.SecuredImageRef +import com.simprints.infra.images.remote.signedurl.SampleUploadData +import com.simprints.infra.images.usecase.CalculateFileMd5AndSizeUseCase +import com.simprints.infra.images.usecase.SamplePathConverter +import javax.inject.Inject + +internal class PrepareImageUploadDataUseCase @Inject constructor( + private val localDataSource: ImageLocalDataSource, + private val calculateFileMd5AndSize: CalculateFileMd5AndSizeUseCase, + private val samplePathUtil: SamplePathConverter, + private val metadataStore: ImageMetadataStore, +) { + suspend operator fun invoke(imageRef: SecuredImageRef): SampleUploadData? = localDataSource + .decryptImage(imageRef) + ?.use { stream -> calculateFileMd5AndSize(stream) } + ?.let { (md5, size) -> + val (sessionId, modality, sampleId) = samplePathUtil.extract(imageRef.relativePath) + ?: return null + val metadata = metadataStore.getMetadata(imageRef.relativePath) + + SampleUploadData( + imageRef = imageRef, + sampleId = sampleId, + sessionId = sessionId, + md5 = md5, + size = size, + modality = modality.name, + metadata = metadata, + ) + } +} diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCase.kt new file mode 100644 index 0000000000..bf1d49e85f --- /dev/null +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCase.kt @@ -0,0 +1,80 @@ +package com.simprints.infra.images.remote.signedurl.usecase + +import com.simprints.core.tools.time.TimeHelper +import com.simprints.infra.authstore.AuthStore +import com.simprints.infra.events.EventRepository +import com.simprints.infra.events.event.domain.models.samples.SampleUpSyncRequestEvent +import com.simprints.infra.events.event.domain.models.scope.EventScope +import com.simprints.infra.images.local.ImageLocalDataSource +import com.simprints.infra.images.remote.signedurl.SampleUploadData +import com.simprints.infra.images.remote.signedurl.api.SampleUploadApiInterface +import com.simprints.infra.images.remote.signedurl.api.SampleUploadRequestBody +import com.simprints.infra.logging.LoggingConstants +import com.simprints.infra.logging.Simber +import java.util.UUID +import javax.inject.Inject + +internal class UploadSampleWithTrackingUseCase @Inject constructor( + private val timeHelper: TimeHelper, + private val authStore: AuthStore, + private val localDataSource: ImageLocalDataSource, + private val eventRepository: EventRepository, +) { + suspend operator fun invoke( + urlRequestScope: EventScope, + url: String, + sample: SampleUploadData, + ): Boolean { + val requestId = UUID.randomUUID().toString() + val requestStartTime = timeHelper.now() + + val errorType = uploadSample(requestId, url, sample) + eventRepository.addOrUpdateEvent( + scope = urlRequestScope, + event = SampleUpSyncRequestEvent( + createdAt = requestStartTime, + endedAt = timeHelper.now(), + requestId = requestId, + sampleId = sample.sampleId, + size = sample.size, + errorType = errorType, + ), + ) + return errorType.isNullOrBlank() + } + + private suspend fun uploadSample( + requestId: String, + url: String, + sampleData: SampleUploadData, + ): String? { + val fileStream = localDataSource.decryptImage(sampleData.imageRef) + if (fileStream == null) { + return "Image decryption failed" + } + + return fileStream.use { stream -> + try { + val client = authStore.buildClient(SampleUploadApiInterface::class) + val response = client.executeCall { api -> + api.uploadFile( + uploadUrl = url, + requestId = requestId, + md5 = sampleData.md5, + requestBody = SampleUploadRequestBody(stream, sampleData.size), + ) + } + if (response.isSuccessful) { + null + } else { + response.errorBody()?.string().also { + Simber.i("Failed to upload image: $it", tag = LoggingConstants.CrashReportTag.SYNC) + } + } + } catch (e: Exception) { + Simber.e("Failed to upload image", e, tag = LoggingConstants.CrashReportTag.SYNC) + e.javaClass.simpleName + } + } + } +} diff --git a/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt index 2b2ef847b5..7f426e57ba 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt @@ -1,5 +1,254 @@ package com.simprints.infra.images.remote.signedurl -class SignedUrlSampleUploaderTest { - // TODO +import com.google.common.truth.Truth.* +import com.simprints.infra.config.store.ConfigRepository +import com.simprints.infra.events.EventRepository +import com.simprints.infra.events.event.domain.models.scope.EventScope +import com.simprints.infra.events.event.domain.models.scope.EventScopeEndCause +import com.simprints.infra.images.local.ImageLocalDataSource +import com.simprints.infra.images.metadata.ImageMetadataStore +import com.simprints.infra.images.model.Path +import com.simprints.infra.images.model.SecuredImageRef +import com.simprints.infra.images.remote.signedurl.usecase.FetchUploadUrlsPerSampleUseCase +import com.simprints.infra.images.remote.signedurl.usecase.PrepareImageUploadDataUseCase +import com.simprints.infra.images.remote.signedurl.usecase.UploadSampleWithTrackingUseCase +import io.mockk.* +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.test.runTest +import okio.IOException +import org.junit.Before +import kotlin.test.Test + +internal class SignedUrlSampleUploaderTest { + @MockK + lateinit var configRepository: ConfigRepository + + @MockK + lateinit var localDataSource: ImageLocalDataSource + + @MockK + lateinit var eventRepository: EventRepository + + @MockK + lateinit var metadataStore: ImageMetadataStore + + @MockK + lateinit var fetchUploadUrlsPerSampleUseCase: FetchUploadUrlsPerSampleUseCase + + @MockK + lateinit var uploadSampleWithTrackingUseCase: UploadSampleWithTrackingUseCase + + @MockK + lateinit var prepareImageUploadDataUseCase: PrepareImageUploadDataUseCase + + private lateinit var signedUrlSampleUploader: SignedUrlSampleUploader + + @Before + fun setUp() { + MockKAnnotations.init(this, relaxed = true) + + coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() + coJustRun { eventRepository.closeEventScope(any(), any()) } + + signedUrlSampleUploader = SignedUrlSampleUploader( + configRepository = configRepository, + localDataSource = localDataSource, + eventRepository = eventRepository, + metadataStore = metadataStore, + prepareImageUploadData = prepareImageUploadDataUseCase, + uploadSampleWithTracking = uploadSampleWithTrackingUseCase, + fetchUploadUrlsPerSample = fetchUploadUrlsPerSampleUseCase, + ) + } + + @Test + fun `Successfully uploads a single sample to signed url`() = runTest { + mockBatchSize(1) + coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() + coEvery { localDataSource.listImages(any()) } returns listOf(mockImageRef(SAMPLE_ID)) + coEvery { prepareImageUploadDataUseCase(any()) } returns mockSampleUploadData(SAMPLE_ID) + coEvery { fetchUploadUrlsPerSampleUseCase(any(), any()) } returns mapOf(SAMPLE_ID to SIGNED_URL) + coEvery { uploadSampleWithTrackingUseCase(any(), any(), any()) } returns true + + assertThat(signedUrlSampleUploader.uploadAllSamples(PROJECT_ID)).isTrue() + + coVerify(exactly = 1) { + // Scope was created and closed + eventRepository.createEventScope(any(), any()) + eventRepository.closeEventScope(any(), EventScopeEndCause.WORKFLOW_ENDED) + + // sample uploaded to provided url + prepareImageUploadDataUseCase(any()) + uploadSampleWithTrackingUseCase.invoke(any(), SIGNED_URL, any()) + + // Local data was cleared + localDataSource.deleteImage(any()) + metadataStore.deleteMetadata(any()) + } + } + + @Test + fun `Successfully uploads a multiple samples in batches`() = runTest { + mockBatchSize(2) + coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() + coEvery { localDataSource.listImages(any()) } returns List(5) { mockImageRef(SAMPLE_ID) } + coEvery { prepareImageUploadDataUseCase(any()) } answers { + val sampleId = (it.invocation.args[0] as SecuredImageRef).relativePath.toString() + mockSampleUploadData(sampleId) + } + coEvery { fetchUploadUrlsPerSampleUseCase(any(), any()) } returns mapOf(SAMPLE_ID to SIGNED_URL) + coEvery { uploadSampleWithTrackingUseCase(any(), any(), any()) } returns true + + assertThat(signedUrlSampleUploader.uploadAllSamples(PROJECT_ID)).isTrue() + + // Single event scoper per call + coVerify(exactly = 1) { + eventRepository.createEventScope(any(), any()) + eventRepository.closeEventScope(any(), EventScopeEndCause.WORKFLOW_ENDED) + } + + // Once per batch + coVerify(exactly = 3) { + fetchUploadUrlsPerSampleUseCase(any(), any()) + } + + // Once per sample + coVerify(exactly = 5) { + prepareImageUploadDataUseCase(any()) + uploadSampleWithTrackingUseCase.invoke(any(), SIGNED_URL, any()) + localDataSource.deleteImage(any()) + metadataStore.deleteMetadata(any()) + } + } + + @Test + fun `Gracefully skips failed upload data collection`() = runTest { + mockBatchSize(3) + coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() + coEvery { localDataSource.listImages(any()) } returns List(3) { mockImageRef("${SAMPLE_ID}_$it") } + coEvery { prepareImageUploadDataUseCase(any()) } answers { + val sampleId = (it.invocation.args[0] as SecuredImageRef).relativePath.toString() + if (sampleId == "${SAMPLE_ID}_1") null else mockSampleUploadData(sampleId) + } + coEvery { fetchUploadUrlsPerSampleUseCase(any(), any()) } returns mapOf( + "${SAMPLE_ID}_0" to SIGNED_URL, + "${SAMPLE_ID}_2" to SIGNED_URL, + ) + coEvery { uploadSampleWithTrackingUseCase(any(), any(), any()) } returns true + + assertThat(signedUrlSampleUploader.uploadAllSamples(PROJECT_ID)).isFalse() + + coVerify(exactly = 1) { + fetchUploadUrlsPerSampleUseCase(any(), match { it.size == 2 }) + } + // Once per sample + coVerify(exactly = 2) { + uploadSampleWithTrackingUseCase.invoke(any(), SIGNED_URL, any()) + localDataSource.deleteImage(any()) + metadataStore.deleteMetadata(any()) + } + } + + @Test + fun `Gracefully skips throwing upload data collection`() = runTest { + mockBatchSize(3) + coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() + coEvery { localDataSource.listImages(any()) } returns List(3) { mockImageRef("${SAMPLE_ID}_$it") } + coEvery { prepareImageUploadDataUseCase(any()) } answers { + val sampleId = (it.invocation.args[0] as SecuredImageRef).relativePath.toString() + if (sampleId == "${SAMPLE_ID}_1") throw IOException("Failed") else mockSampleUploadData(sampleId) + } + coEvery { fetchUploadUrlsPerSampleUseCase(any(), any()) } returns mapOf( + "${SAMPLE_ID}_0" to SIGNED_URL, + "${SAMPLE_ID}_2" to SIGNED_URL, + ) + coEvery { uploadSampleWithTrackingUseCase(any(), any(), any()) } returns true + + assertThat(signedUrlSampleUploader.uploadAllSamples(PROJECT_ID)).isFalse() + + coVerify(exactly = 1) { + fetchUploadUrlsPerSampleUseCase(any(), match { it.size == 2 }) + } + // Once per sample + coVerify(exactly = 2) { + uploadSampleWithTrackingUseCase.invoke(any(), SIGNED_URL, any()) + localDataSource.deleteImage(any()) + metadataStore.deleteMetadata(any()) + } + } + + @Test + fun `Gracefully skips missing url`() = runTest { + mockBatchSize(3) + coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() + coEvery { localDataSource.listImages(any()) } returns List(3) { mockImageRef("${SAMPLE_ID}_$it") } + coEvery { prepareImageUploadDataUseCase(any()) } answers { + val sampleId = (it.invocation.args[0] as SecuredImageRef).relativePath.toString() + mockSampleUploadData(sampleId) + } + + coEvery { fetchUploadUrlsPerSampleUseCase(any(), any()) } returns mapOf( + "${SAMPLE_ID}_0" to SIGNED_URL, + // No sampleId_1 + "${SAMPLE_ID}_2" to SIGNED_URL, + ) + + coEvery { uploadSampleWithTrackingUseCase(any(), any(), any()) } returns true + + val result = signedUrlSampleUploader.uploadAllSamples(PROJECT_ID) + + assertThat(result).isFalse() + + // Once per sample (skipping sample_1) + coVerify(exactly = 2) { + uploadSampleWithTrackingUseCase.invoke(any(), SIGNED_URL, any()) + localDataSource.deleteImage(any()) + metadataStore.deleteMetadata(any()) + } + } + + @Test + fun `Do not delete local data if upload fails`() = runTest { + mockBatchSize(1) + coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() + coEvery { localDataSource.listImages(any()) } returns listOf(mockImageRef(SAMPLE_ID)) + coEvery { prepareImageUploadDataUseCase(any()) } returns mockSampleUploadData(SAMPLE_ID) + coEvery { fetchUploadUrlsPerSampleUseCase(any(), any()) } returns mapOf(SAMPLE_ID to SIGNED_URL) + coEvery { uploadSampleWithTrackingUseCase(any(), any(), any()) } returns false + + assertThat(signedUrlSampleUploader.uploadAllSamples(PROJECT_ID)).isFalse() + + coVerify(exactly = 0) { + localDataSource.deleteImage(any()) + metadataStore.deleteMetadata(any()) + } + } + + private fun mockBatchSize(batchSize: Int) { + coEvery { + configRepository + .getProjectConfiguration() + .synchronization.samples.signedUrlBatchSize + } returns batchSize + } + + private fun mockImageRef(path: String) = SecuredImageRef( + relativePath = Path(path), + ) + + private fun mockSampleUploadData(sampleId: String) = SampleUploadData( + imageRef = mockImageRef(sampleId), + sampleId = sampleId, + sessionId = "sessionId", + modality = "modality", + md5 = "md5", + size = 10L, + metadata = emptyMap(), + ) + + companion object { + private const val SAMPLE_ID = "sampleId" + private const val PROJECT_ID = "projectId" + private const val SIGNED_URL = "signed.url" + } } diff --git a/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/FetchUploadUrlsPerSampleUseCaseTest.kt b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/FetchUploadUrlsPerSampleUseCaseTest.kt new file mode 100644 index 0000000000..5ac3aa625a --- /dev/null +++ b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/FetchUploadUrlsPerSampleUseCaseTest.kt @@ -0,0 +1,92 @@ +package com.simprints.infra.images.remote.signedurl.usecase + +import com.google.common.truth.Truth.* +import com.simprints.infra.authstore.AuthStore +import com.simprints.infra.images.model.Path +import com.simprints.infra.images.model.SecuredImageRef +import com.simprints.infra.images.remote.signedurl.SampleUploadData +import com.simprints.infra.images.remote.signedurl.api.ApiSampleUploadUrlRequest +import com.simprints.infra.images.remote.signedurl.api.ApiSampleUploadUrlResponse +import com.simprints.infra.images.remote.signedurl.api.SampleUploadApiInterface +import com.simprints.infra.network.SimNetwork +import com.simprints.testtools.common.alias.InterfaceInvocation +import io.mockk.* +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import kotlin.reflect.KClass + +internal class FetchUploadUrlsPerSampleUseCaseTest { + @MockK + lateinit var authStore: AuthStore + + @MockK + lateinit var apiClient: SimNetwork.SimApiClient + + @MockK + lateinit var apiInterface: SampleUploadApiInterface + + private lateinit var useCase: FetchUploadUrlsPerSampleUseCase + + @Before + fun setUp() { + MockKAnnotations.init(this, relaxed = true) + + coEvery { authStore.buildClient(any>()) } returns apiClient + coEvery { apiClient.executeCall(any()) } coAnswers { + val args = this.args + @Suppress("UNCHECKED_CAST") + (args[0] as InterfaceInvocation).invoke(apiInterface) + } + + useCase = FetchUploadUrlsPerSampleUseCase(authStore) + } + + @Test + fun `Successfully fetches upload urls for provided sample data`() = runTest { + coEvery { apiInterface.getSampleUploadUrl(any(), any>()) } returns listOf( + ApiSampleUploadUrlResponse(sampleId = "sampleId", url = "url"), + ) + + val result = useCase(PROJECT_ID, listOf(mockSampleUploadData())) + + assertThat(result["sampleId"]).isEqualTo("url") + coVerify(exactly = 1) { apiInterface.getSampleUploadUrl(PROJECT_ID, any()) } + } + + @Test + fun `Gracefully handles upload url request error`() = runTest { + coEvery { apiInterface.getSampleUploadUrl(any(), any()) } throws Exception("Failed") + + val result = useCase(PROJECT_ID, listOf(mockSampleUploadData())) + + assertThat(result).isEmpty() + } + + private fun mockSampleUploadData() = SampleUploadData( + imageRef = SecuredImageRef(Path("sampleId")), + sampleId = "sampleId", + sessionId = "sessionId", + modality = "modality", + md5 = "md5", + size = 10L, + metadata = emptyMap(), + ) + + companion object { + private const val PROJECT_ID = "projectId" + } +} + +// coEvery { authStore.buildClient(any>()) } returns apiClient +// coEvery { apiClient.executeCall(any()) } coAnswers { +// val args = this.args +// @Suppress("UNCHECKED_CAST") +// (args[0] as InterfaceInvocation).invoke(apiInterface) +// } +// coEvery { apiClient.executeCall>(any()) } coAnswers { +// val args = this.args +// @Suppress("UNCHECKED_CAST") +// (args[0] as InterfaceInvocation>).invoke(apiInterface) +// } diff --git a/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/PrepareImageUploadDataUseCaseTest.kt b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/PrepareImageUploadDataUseCaseTest.kt new file mode 100644 index 0000000000..01642834ee --- /dev/null +++ b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/PrepareImageUploadDataUseCaseTest.kt @@ -0,0 +1,91 @@ +package com.simprints.infra.images.remote.signedurl.usecase + +import com.google.common.truth.Truth.* +import com.simprints.infra.config.store.models.GeneralConfiguration +import com.simprints.infra.images.local.ImageLocalDataSource +import com.simprints.infra.images.metadata.ImageMetadataStore +import com.simprints.infra.images.model.SecuredImageRef +import com.simprints.infra.images.usecase.CalculateFileMd5AndSizeUseCase +import com.simprints.infra.images.usecase.SamplePathConverter +import io.mockk.* +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import java.io.FileInputStream + +internal class PrepareImageUploadDataUseCaseTest { + @MockK + lateinit var localDataSource: ImageLocalDataSource + + @MockK + lateinit var calculateFileMd5AndSize: CalculateFileMd5AndSizeUseCase + + @MockK + lateinit var samplePathUtil: SamplePathConverter + + @MockK + lateinit var metadataStore: ImageMetadataStore + + @MockK + lateinit var fileStream: FileInputStream + + @MockK + lateinit var imageRef: SecuredImageRef + + private lateinit var useCase: PrepareImageUploadDataUseCase + + @Before + fun setUp() { + MockKAnnotations.init(this, relaxed = true) + + useCase = PrepareImageUploadDataUseCase( + localDataSource = localDataSource, + calculateFileMd5AndSize = calculateFileMd5AndSize, + samplePathUtil = samplePathUtil, + metadataStore = metadataStore, + ) + } + + @Test + fun `Successfully prepares data for sample upload`() = runTest { + coEvery { localDataSource.decryptImage(any()) } returns fileStream + coEvery { calculateFileMd5AndSize(any()) } returns + CalculateFileMd5AndSizeUseCase.CalculationResult("base64-md5", 10L) + every { samplePathUtil.extract(any()) } returns + SamplePathConverter.PathData("sessionId", GeneralConfiguration.Modality.FINGERPRINT, "sampleId") + coEvery { metadataStore.getMetadata(any()) } returns mapOf("k" to "v") + + val result = useCase(imageRef) + + verify(exactly = 1) { fileStream.close() } + assertThat(result?.imageRef).isEqualTo(imageRef) + assertThat(result?.sampleId).isEqualTo("sampleId") + assertThat(result?.sessionId).isEqualTo("sessionId") + assertThat(result?.md5).isEqualTo("base64-md5") + assertThat(result?.size).isEqualTo(10L) + assertThat(result?.modality).isEqualTo("FINGERPRINT") + assertThat(result?.metadata).isEqualTo(mapOf("k" to "v")) + } + + @Test + fun `Gracefully handles missing file`() = runTest { + coEvery { localDataSource.decryptImage(any()) } returns null + + val result = useCase(imageRef) + + assertThat(result).isNull() + } + + @Test + fun `Gracefully handles path extraction failure`() = runTest { + coEvery { localDataSource.decryptImage(any()) } returns fileStream + coEvery { calculateFileMd5AndSize(any()) } returns + CalculateFileMd5AndSizeUseCase.CalculationResult("base64-md5", 10L) + every { samplePathUtil.extract(any()) } returns null + + val result = useCase(imageRef) + + assertThat(result).isNull() + } +} diff --git a/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCaseTest.kt b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCaseTest.kt new file mode 100644 index 0000000000..887b4f2a61 --- /dev/null +++ b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCaseTest.kt @@ -0,0 +1,130 @@ +package com.simprints.infra.images.remote.signedurl.usecase + +import com.google.common.truth.Truth.* +import com.simprints.core.tools.time.TimeHelper +import com.simprints.core.tools.time.Timestamp +import com.simprints.infra.authstore.AuthStore +import com.simprints.infra.events.EventRepository +import com.simprints.infra.events.event.domain.models.samples.SampleUpSyncRequestEvent +import com.simprints.infra.events.event.domain.models.scope.EventScope +import com.simprints.infra.images.local.ImageLocalDataSource +import com.simprints.infra.images.model.Path +import com.simprints.infra.images.model.SecuredImageRef +import com.simprints.infra.images.remote.signedurl.SampleUploadData +import com.simprints.infra.images.remote.signedurl.api.ApiSampleUploadUrlResponse +import com.simprints.infra.images.remote.signedurl.api.SampleUploadApiInterface +import com.simprints.infra.network.SimNetwork +import com.simprints.testtools.common.alias.InterfaceInvocation +import io.mockk.* +import io.mockk.impl.annotations.MockK +import kotlinx.coroutines.test.runTest +import org.junit.Before +import org.junit.Test +import java.io.FileInputStream +import java.io.IOException +import kotlin.reflect.KClass + +internal class UploadSampleWithTrackingUseCaseTest { + @MockK + lateinit var timeHelper: TimeHelper + + @MockK + lateinit var authStore: AuthStore + + @MockK + lateinit var apiClient: SimNetwork.SimApiClient + + @MockK + lateinit var apiInterface: SampleUploadApiInterface + + @MockK + lateinit var localDataSource: ImageLocalDataSource + + @MockK + lateinit var fileStream: FileInputStream + + @MockK + lateinit var eventRepository: EventRepository + + @MockK + lateinit var eventScope: EventScope + private lateinit var useCase: UploadSampleWithTrackingUseCase + + @Before + fun setUp() { + MockKAnnotations.init(this, relaxed = true) + + every { timeHelper.now() } returns Timestamp(1L) + coEvery { authStore.buildClient(any>()) } returns apiClient + coEvery { apiClient.executeCall(any()) } coAnswers { + val args = this.args + @Suppress("UNCHECKED_CAST") + (args[0] as InterfaceInvocation).invoke(apiInterface) + } + + useCase = UploadSampleWithTrackingUseCase( + timeHelper = timeHelper, + authStore = authStore, + localDataSource = localDataSource, + eventRepository = eventRepository, + ) + } + + @Test + fun `Successfully upload sample and reports event`() = runTest { + coEvery { localDataSource.decryptImage(any()) } returns fileStream + coEvery { apiInterface.uploadFile(any(), any(), any(), any()) } returns mockk { + every { isSuccessful } returns true + } + + val result = useCase(eventScope, "url", mockUploadData()) + + coVerify(exactly = 1) { + eventRepository.addOrUpdateEvent(eventScope, match { it is SampleUpSyncRequestEvent }) + } + + assertThat(result).isTrue() + } + + @Test + fun `Handles image decryption failure`() = runTest { + coEvery { localDataSource.decryptImage(any()) } returns null + + val result = useCase(eventScope, "url", mockUploadData()) + + assertThat(result).isFalse() + } + + @Test + fun `Handles image upload failure`() = runTest { + coEvery { localDataSource.decryptImage(any()) } returns fileStream + coEvery { apiInterface.uploadFile(any(), any(), any(), any()) } returns mockk { + every { isSuccessful } returns false + every { errorBody()?.string() } returns "Failure" + } + + val result = useCase(eventScope, "url", mockUploadData()) + + assertThat(result).isFalse() + } + + @Test + fun `Handles image upload exception`() = runTest { + coEvery { localDataSource.decryptImage(any()) } returns fileStream + coEvery { apiInterface.uploadFile(any(), any(), any(), any()) } throws IOException("Failure") + + val result = useCase(eventScope, "url", mockUploadData()) + + assertThat(result).isFalse() + } + + private fun mockUploadData(): SampleUploadData = SampleUploadData( + imageRef = SecuredImageRef(Path("sampleId")), + sampleId = "sampleId", + sessionId = "sessionId", + modality = "modality", + md5 = "md5", + size = 10L, + metadata = emptyMap(), + ) +} From 323299604d6774976f852a1f98f9cb68790ff7f5 Mon Sep 17 00:00:00 2001 From: Sergejs Luhmirins Date: Mon, 14 Jul 2025 18:25:52 +0300 Subject: [PATCH 9/9] MS-1058 Add more informative logging --- .../firestore/FirestoreSampleUploader.kt | 58 +++++++++---------- .../signedurl/SignedUrlSampleUploader.kt | 16 +++-- .../UploadSampleWithTrackingUseCase.kt | 6 +- .../usecase/CalculateFileMd5AndSizeUseCase.kt | 4 +- .../signedurl/SignedUrlSampleUploaderTest.kt | 10 +--- .../infra/logging/LoggingConstants.kt | 1 + .../httpclient/BuildOkHttpClientUseCase.kt | 5 +- 7 files changed, 55 insertions(+), 45 deletions(-) diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt index ca99c75013..7de80828c0 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/firestore/FirestoreSampleUploader.kt @@ -9,6 +9,7 @@ import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.metadata.ImageMetadataStore import com.simprints.infra.images.model.SecuredImageRef import com.simprints.infra.images.remote.SampleUploader +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SAMPLE_UPLOAD import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC import com.simprints.infra.logging.Simber import kotlinx.coroutines.tasks.await @@ -23,40 +24,39 @@ internal class FirestoreSampleUploader @Inject constructor( ) : SampleUploader { override suspend fun uploadAllSamples(projectId: String): Boolean { val firebaseApp = authStore.getLegacyAppFallback() - val firebaseProjectName = firebaseApp.options.projectId - - if (!firebaseProjectName.isNullOrBlank()) { - var allImagesUploaded = true + if (firebaseApp.options.projectId.isNullOrBlank()) { + Simber.i("Firebase projectId is null", tag = SAMPLE_UPLOAD) + return false + } + var allImagesUploaded = true - val bucketUrl = configManager.getProject(projectId).imageBucket - val rootRef = FirebaseStorage - .getInstance(firebaseApp, bucketUrl) - .reference + Simber.i("Starting sample upload to Firestore") + val bucketUrl = configManager.getProject(projectId).imageBucket + val rootRef = FirebaseStorage + .getInstance(firebaseApp, bucketUrl) + .reference - localDataSource.listImages(projectId).forEach { imageRef -> - try { - localDataSource.decryptImage(imageRef)?.let { stream -> - val metadata = metadataStore.getMetadata(imageRef.relativePath) - val uploadSuccessful = uploadSample(rootRef, stream, imageRef, metadata) - if (uploadSuccessful) { - localDataSource.deleteImage(imageRef) - metadataStore.deleteMetadata(imageRef.relativePath) - } else { - allImagesUploaded = false - Simber.i("Failed to upload image without exception", tag = SYNC) - } + localDataSource.listImages(projectId).forEach { imageRef -> + Simber.i("Reading sample file: ${imageRef.relativePath.parts.last()}", tag = SAMPLE_UPLOAD) + try { + localDataSource.decryptImage(imageRef)?.let { stream -> + val metadata = metadataStore.getMetadata(imageRef.relativePath) + val uploadSuccessful = uploadSample(rootRef, stream, imageRef, metadata) + if (uploadSuccessful) { + localDataSource.deleteImage(imageRef) + metadataStore.deleteMetadata(imageRef.relativePath) + } else { + allImagesUploaded = false + Simber.i("Failed to upload image without exception", tag = SAMPLE_UPLOAD) } - } catch (t: Throwable) { - allImagesUploaded = false - Simber.e("Failed to upload images", t, tag = SYNC) } + } catch (t: Throwable) { + allImagesUploaded = false + Simber.e("Failed to upload images", t, tag = SYNC) } - - return allImagesUploaded - } else { - Simber.i("Firebase projectId is null") - return false } + + return allImagesUploaded } private suspend fun uploadSample( @@ -68,7 +68,7 @@ internal class FirestoreSampleUploader @Inject constructor( val fileRef = imageRef.relativePath.parts .fold(rootRef) { ref, pathPart -> ref.child(pathPart) } - Simber.d("Uploading ${fileRef.path}") + Simber.i("Uploading ${fileRef.path.last()}", tag = SAMPLE_UPLOAD) val uploadTask = if (metadata.isEmpty()) { fileRef.putStream(imageStream).await() diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt index eb7c4596ec..8ea3e5eed9 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploader.kt @@ -10,7 +10,7 @@ import com.simprints.infra.images.remote.SampleUploader import com.simprints.infra.images.remote.signedurl.usecase.FetchUploadUrlsPerSampleUseCase import com.simprints.infra.images.remote.signedurl.usecase.PrepareImageUploadDataUseCase import com.simprints.infra.images.remote.signedurl.usecase.UploadSampleWithTrackingUseCase -import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SAMPLE_UPLOAD import com.simprints.infra.logging.Simber import kotlinx.coroutines.isActive import javax.inject.Inject @@ -30,6 +30,7 @@ internal class SignedUrlSampleUploader @Inject constructor( val batchSize = getBatchSize() val urlRequestScope = eventRepository.createEventScope(type = EventScopeType.SAMPLE_UP_SYNC) + Simber.i("Starting image upload in batches of $batchSize (Scope ID: ${urlRequestScope.id}") val sampleReferenceBatches = localDataSource .listImages(projectId) // Preparing the file for upload requires reading each of them to calculate md5 and size, @@ -50,12 +51,15 @@ internal class SignedUrlSampleUploader @Inject constructor( prepareImageUploadData(imageRef).also { if (it == null) { allImagesUploaded = false - Simber.i("Failed to read image file without exception", tag = SYNC) + Simber.i( + "Failed to read image file without exception", + tag = SAMPLE_UPLOAD, + ) } } } catch (t: Throwable) { allImagesUploaded = false - Simber.e("Failed to read image file", t, tag = SYNC) + Simber.e("Failed to read image file", t, tag = SAMPLE_UPLOAD) null } } @@ -63,17 +67,20 @@ internal class SignedUrlSampleUploader @Inject constructor( // Fetch upload urls for each image val sampleIdToUrlMap = fetchUploadUrlsPerSample(projectId, batchUploadData) + Simber.i("${sampleIdToUrlMap.size} signed URLs fetched") + for (sample in batchUploadData) { if (!coroutineContext.isActive) { // Do not upload next image if coroutine is being cancelled allImagesUploaded = false break } + Simber.i("Uploading ${sample.sampleId}") val url = sampleIdToUrlMap[sample.sampleId] if (url == null) { allImagesUploaded = false - Simber.i("Failed to fetch sample url", tag = SYNC) + Simber.i("Failed to fetch sample url", tag = SAMPLE_UPLOAD) continue } @@ -82,6 +89,7 @@ internal class SignedUrlSampleUploader @Inject constructor( if (success) { localDataSource.deleteImage(sample.imageRef) metadataStore.deleteMetadata(sample.imageRef.relativePath) + Simber.i("Uploaded ${sample.sampleId} successfully") } else { allImagesUploaded = false } diff --git a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCase.kt index bf1d49e85f..36bc1d3f2d 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCase.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/remote/signedurl/usecase/UploadSampleWithTrackingUseCase.kt @@ -9,7 +9,7 @@ import com.simprints.infra.images.local.ImageLocalDataSource import com.simprints.infra.images.remote.signedurl.SampleUploadData import com.simprints.infra.images.remote.signedurl.api.SampleUploadApiInterface import com.simprints.infra.images.remote.signedurl.api.SampleUploadRequestBody -import com.simprints.infra.logging.LoggingConstants +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SAMPLE_UPLOAD import com.simprints.infra.logging.Simber import java.util.UUID import javax.inject.Inject @@ -68,11 +68,11 @@ internal class UploadSampleWithTrackingUseCase @Inject constructor( null } else { response.errorBody()?.string().also { - Simber.i("Failed to upload image: $it", tag = LoggingConstants.CrashReportTag.SYNC) + Simber.i("Failed to upload image: $it", tag = SAMPLE_UPLOAD) } } } catch (e: Exception) { - Simber.e("Failed to upload image", e, tag = LoggingConstants.CrashReportTag.SYNC) + Simber.e("Failed to upload image", e, tag = SAMPLE_UPLOAD) e.javaClass.simpleName } } diff --git a/infra/images/src/main/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCase.kt b/infra/images/src/main/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCase.kt index 2e956570bd..3c844eb546 100644 --- a/infra/images/src/main/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCase.kt +++ b/infra/images/src/main/java/com/simprints/infra/images/usecase/CalculateFileMd5AndSizeUseCase.kt @@ -1,6 +1,8 @@ package com.simprints.infra.images.usecase import com.simprints.core.tools.utils.EncodingUtils +import com.simprints.infra.logging.LoggingConstants.CrashReportTag.SYNC +import com.simprints.infra.logging.Simber import java.io.IOException import java.io.InputStream import java.security.DigestInputStream @@ -38,7 +40,7 @@ internal class CalculateFileMd5AndSizeUseCase @Inject constructor( size = size, ) } catch (e: IOException) { - e.printStackTrace() + Simber.e("Failed to calculate md5 for a sample file", e, tag = SYNC) CalculationResult("", 0) } diff --git a/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt index 7f426e57ba..5842a952e5 100644 --- a/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt +++ b/infra/images/src/test/java/com/simprints/infra/images/remote/signedurl/SignedUrlSampleUploaderTest.kt @@ -47,7 +47,9 @@ internal class SignedUrlSampleUploaderTest { fun setUp() { MockKAnnotations.init(this, relaxed = true) - coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() + coEvery { eventRepository.createEventScope(any(), any()) } returns mockk { + every { id } returns "scopeId" + } coJustRun { eventRepository.closeEventScope(any(), any()) } signedUrlSampleUploader = SignedUrlSampleUploader( @@ -64,7 +66,6 @@ internal class SignedUrlSampleUploaderTest { @Test fun `Successfully uploads a single sample to signed url`() = runTest { mockBatchSize(1) - coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() coEvery { localDataSource.listImages(any()) } returns listOf(mockImageRef(SAMPLE_ID)) coEvery { prepareImageUploadDataUseCase(any()) } returns mockSampleUploadData(SAMPLE_ID) coEvery { fetchUploadUrlsPerSampleUseCase(any(), any()) } returns mapOf(SAMPLE_ID to SIGNED_URL) @@ -90,7 +91,6 @@ internal class SignedUrlSampleUploaderTest { @Test fun `Successfully uploads a multiple samples in batches`() = runTest { mockBatchSize(2) - coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() coEvery { localDataSource.listImages(any()) } returns List(5) { mockImageRef(SAMPLE_ID) } coEvery { prepareImageUploadDataUseCase(any()) } answers { val sampleId = (it.invocation.args[0] as SecuredImageRef).relativePath.toString() @@ -124,7 +124,6 @@ internal class SignedUrlSampleUploaderTest { @Test fun `Gracefully skips failed upload data collection`() = runTest { mockBatchSize(3) - coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() coEvery { localDataSource.listImages(any()) } returns List(3) { mockImageRef("${SAMPLE_ID}_$it") } coEvery { prepareImageUploadDataUseCase(any()) } answers { val sampleId = (it.invocation.args[0] as SecuredImageRef).relativePath.toString() @@ -152,7 +151,6 @@ internal class SignedUrlSampleUploaderTest { @Test fun `Gracefully skips throwing upload data collection`() = runTest { mockBatchSize(3) - coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() coEvery { localDataSource.listImages(any()) } returns List(3) { mockImageRef("${SAMPLE_ID}_$it") } coEvery { prepareImageUploadDataUseCase(any()) } answers { val sampleId = (it.invocation.args[0] as SecuredImageRef).relativePath.toString() @@ -180,7 +178,6 @@ internal class SignedUrlSampleUploaderTest { @Test fun `Gracefully skips missing url`() = runTest { mockBatchSize(3) - coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() coEvery { localDataSource.listImages(any()) } returns List(3) { mockImageRef("${SAMPLE_ID}_$it") } coEvery { prepareImageUploadDataUseCase(any()) } answers { val sampleId = (it.invocation.args[0] as SecuredImageRef).relativePath.toString() @@ -210,7 +207,6 @@ internal class SignedUrlSampleUploaderTest { @Test fun `Do not delete local data if upload fails`() = runTest { mockBatchSize(1) - coEvery { eventRepository.createEventScope(any(), any()) } returns mockk() coEvery { localDataSource.listImages(any()) } returns listOf(mockImageRef(SAMPLE_ID)) coEvery { prepareImageUploadDataUseCase(any()) } returns mockSampleUploadData(SAMPLE_ID) coEvery { fetchUploadUrlsPerSampleUseCase(any(), any()) } returns mapOf(SAMPLE_ID to SIGNED_URL) diff --git a/infra/logging/src/main/java/com/simprints/infra/logging/LoggingConstants.kt b/infra/logging/src/main/java/com/simprints/infra/logging/LoggingConstants.kt index 9d745f99b8..e76356f4ce 100644 --- a/infra/logging/src/main/java/com/simprints/infra/logging/LoggingConstants.kt +++ b/infra/logging/src/main/java/com/simprints/infra/logging/LoggingConstants.kt @@ -46,6 +46,7 @@ object LoggingConstants { ORCHESTRATION, MIGRATION, REALM_DB_MIGRATION, + SAMPLE_UPLOAD, } // Tags eligible for Firebase Analytics logging diff --git a/infra/network/src/main/java/com/simprints/infra/network/httpclient/BuildOkHttpClientUseCase.kt b/infra/network/src/main/java/com/simprints/infra/network/httpclient/BuildOkHttpClientUseCase.kt index 1c04bb1be9..a1c92d31f3 100644 --- a/infra/network/src/main/java/com/simprints/infra/network/httpclient/BuildOkHttpClientUseCase.kt +++ b/infra/network/src/main/java/com/simprints/infra/network/httpclient/BuildOkHttpClientUseCase.kt @@ -91,7 +91,10 @@ internal class BuildOkHttpClientUseCase @Inject constructor( }.addNetworkInterceptor( ChuckerInterceptor .Builder(ctx) - // Skip sample storage domain to avoid interfering with encrypted file streaming + // Chucker's logging of binary request bodies (e.g., sample uploads) consumes the entire + // request input stream. For encrypted sample files, this stream cannot be reset, + // leading to an exhausted or closed stream by the time OkHttp processes it. + // To prevent interference with sample uploads, we skip the file storage domain entirely. .skipDomains("storage.googleapis.com") .build(), ).addInterceptor(buildDeviceIdInterceptor(deviceId))