From 7601e0ee939940f28795703e4a2bddb8aa38581c Mon Sep 17 00:00:00 2001 From: Chen Cen Date: Sun, 3 Jul 2022 13:55:30 -0700 Subject: [PATCH] [Identity] Send image_upload analytics --- .../IdentityAnalyticsRequestFactory.kt | 31 +++++-- .../navigation/IdentityUploadFragment.kt | 18 ++-- .../networking/DefaultIdentityRepository.kt | 64 +++++++++------ .../identity/networking/IdentityRepository.kt | 3 +- .../identity/viewmodel/IdentityViewModel.kt | 82 +++++++++++++++---- .../navigation/IdentityUploadFragmentTest.kt | 6 +- .../navigation/PassportUploadFragmentTest.kt | 4 +- .../viewmodel/IdentityViewModelTest.kt | 51 ++++++++++-- 8 files changed, 195 insertions(+), 64 deletions(-) diff --git a/identity/src/main/java/com/stripe/android/identity/analytics/IdentityAnalyticsRequestFactory.kt b/identity/src/main/java/com/stripe/android/identity/analytics/IdentityAnalyticsRequestFactory.kt index 70052a40b55..80eea5bb05a 100644 --- a/identity/src/main/java/com/stripe/android/identity/analytics/IdentityAnalyticsRequestFactory.kt +++ b/identity/src/main/java/com/stripe/android/identity/analytics/IdentityAnalyticsRequestFactory.kt @@ -198,6 +198,25 @@ internal class IdentityAnalyticsRequestFactory @Inject constructor( ) ) + fun imageUpload( + value: Long, + compressionQuality: Float, + scanType: IdentityScanState.ScanType, + id: String?, + fileName: String?, + fileSize: Long + ) = requestFactory.createRequest( + eventName = EVENT_IMAGE_UPLOAD, + additionalParams = additionalParamWithEventMetadata( + PARAM_VALUE to value, + PARAM_COMPRESSION_QUALITY to compressionQuality, + PARAM_SCAN_TYPE to scanType.toParam(), + PARAM_ID to id, + PARAM_FILE_NAME to fileName, + PARAM_FILE_SIZE to fileSize + ) + ) + private fun IdentityScanState.ScanType.toParam(): String = when (this) { IdentityScanState.ScanType.ID_FRONT -> ID @@ -205,9 +224,7 @@ internal class IdentityAnalyticsRequestFactory @Inject constructor( IdentityScanState.ScanType.PASSPORT -> PASSPORT IdentityScanState.ScanType.DL_FRONT -> DRIVER_LICENSE IdentityScanState.ScanType.DL_BACK -> DRIVER_LICENSE - else -> { - throw IllegalArgumentException("Unknown type: $this") - } + IdentityScanState.ScanType.SELFIE -> SELFIE } private fun IdentityScanState.ScanType.toSide(): String = @@ -228,6 +245,7 @@ internal class IdentityAnalyticsRequestFactory @Inject constructor( const val ID = "id" const val PASSPORT = "passport" const val DRIVER_LICENSE = "driver_license" + const val SELFIE = "selfie" const val FRONT = "front" const val BACK = "back" @@ -245,6 +263,7 @@ internal class IdentityAnalyticsRequestFactory @Inject constructor( const val EVENT_AVERAGE_FPS = "average_fps" const val EVENT_MODEL_PERFORMANCE = "model_performance" const val EVENT_TIME_TO_SCREEN = "time_to_screen" + const val EVENT_IMAGE_UPLOAD = "image_upload" const val PARAM_EVENT_META_DATA = "event_metadata" const val PARAM_FROM_FALLBACK_URL = "from_fallback_url" @@ -275,14 +294,16 @@ internal class IdentityAnalyticsRequestFactory @Inject constructor( const val PARAM_NETWORK_TIME = "network_time" const val PARAM_FROM_SCREEN_NAME = "from_screen_name" const val PARAM_TO_SCREEN_NAME = "to_screen_name" + const val PARAM_COMPRESSION_QUALITY = "compression_quality" + const val PARAM_ID = "id" + const val PARAM_FILE_NAME = "file_name" + const val PARAM_FILE_SIZE = "file_size" const val SCREEN_NAME_CONSENT = "consent" const val SCREEN_NAME_DOC_SELECT = "document_select" - const val SCREEN_NAME_LIVE_CAPTURE = "live_capture" const val SCREEN_NAME_LIVE_CAPTURE_PASSPORT = "live_capture_passport" const val SCREEN_NAME_LIVE_CAPTURE_ID = "live_capture_id" const val SCREEN_NAME_LIVE_CAPTURE_DRIVER_LICENSE = "live_capture_driver_license" - const val SCREEN_NAME_FILE_UPLOAD = "file_upload" const val SCREEN_NAME_FILE_UPLOAD_PASSPORT = "file_upload_passport" const val SCREEN_NAME_FILE_UPLOAD_ID = "file_upload_id" const val SCREEN_NAME_FILE_UPLOAD_DRIVER_LICENSE = "file_upload_driver_license" diff --git a/identity/src/main/java/com/stripe/android/identity/navigation/IdentityUploadFragment.kt b/identity/src/main/java/com/stripe/android/identity/navigation/IdentityUploadFragment.kt index 595178e30af..f32aa10d45f 100644 --- a/identity/src/main/java/com/stripe/android/identity/navigation/IdentityUploadFragment.kt +++ b/identity/src/main/java/com/stripe/android/identity/navigation/IdentityUploadFragment.kt @@ -258,7 +258,8 @@ internal abstract class IdentityUploadFragment( uploadResult( uri = it, uploadMethod = DocumentUploadParam.UploadMethod.MANUALCAPTURE, - isFront = true + isFront = true, + scanType ) } } else if (scanType == backScanType) { @@ -266,7 +267,8 @@ internal abstract class IdentityUploadFragment( uploadResult( uri = it, uploadMethod = DocumentUploadParam.UploadMethod.MANUALCAPTURE, - isFront = false + isFront = false, + scanType ) } } @@ -283,7 +285,8 @@ internal abstract class IdentityUploadFragment( uploadResult( uri = it, uploadMethod = DocumentUploadParam.UploadMethod.FILEUPLOAD, - isFront = true + isFront = true, + scanType ) } } else if (scanType == backScanType) { @@ -291,7 +294,8 @@ internal abstract class IdentityUploadFragment( uploadResult( uri = it, uploadMethod = DocumentUploadParam.UploadMethod.FILEUPLOAD, - isFront = false + isFront = false, + scanType ) } } @@ -319,7 +323,8 @@ internal abstract class IdentityUploadFragment( private fun uploadResult( uri: Uri, uploadMethod: DocumentUploadParam.UploadMethod, - isFront: Boolean + isFront: Boolean, + scanType: IdentityScanState.ScanType ) { if (isFront) { showFrontUploading() @@ -331,7 +336,8 @@ internal abstract class IdentityUploadFragment( uri = uri, isFront = isFront, docCapturePage = docCapturePage, - uploadMethod = uploadMethod + uploadMethod = uploadMethod, + scanType = scanType ) } } diff --git a/identity/src/main/java/com/stripe/android/identity/networking/DefaultIdentityRepository.kt b/identity/src/main/java/com/stripe/android/identity/networking/DefaultIdentityRepository.kt index 99ad39e3c1f..2220e26d89f 100644 --- a/identity/src/main/java/com/stripe/android/identity/networking/DefaultIdentityRepository.kt +++ b/identity/src/main/java/com/stripe/android/identity/networking/DefaultIdentityRepository.kt @@ -2,6 +2,7 @@ package com.stripe.android.identity.networking import android.util.Log import androidx.annotation.VisibleForTesting +import com.stripe.android.camera.framework.time.Clock import com.stripe.android.core.exception.APIConnectionException import com.stripe.android.core.exception.APIException import com.stripe.android.core.model.StripeFile @@ -96,7 +97,8 @@ internal class DefaultIdentityRepository @Inject constructor( verificationId: String, ephemeralKey: String, imageFile: File, - filePurpose: StripeFilePurpose + filePurpose: StripeFilePurpose, + onSuccessExecutionTimeBlock: (Long) -> Unit ): StripeFile = executeRequestWithModelJsonParser( request = IdentityFileUploadRequest( fileParams = StripeFileParams( @@ -108,7 +110,8 @@ internal class DefaultIdentityRepository @Inject constructor( ), verificationId = verificationId ), - responseJsonParser = stripeFileJsonParser + responseJsonParser = stripeFileJsonParser, + onSuccessExecutionTimeBlock = onSuccessExecutionTimeBlock ) override suspend fun downloadModel(modelUrl: String) = runCatching { @@ -210,35 +213,42 @@ internal class DefaultIdentityRepository @Inject constructor( private suspend fun executeRequestWithModelJsonParser( request: StripeRequest, - responseJsonParser: ModelJsonParser - ): Response = runCatching { - stripeNetworkClient.executeRequest( - request - ) - }.fold( - onSuccess = { response -> - if (response.isError) { - // TODO(ccen) Parse the response code and throw different exceptions - throw APIException( - stripeError = stripeErrorJsonParser.parse(response.responseJson()), - requestId = response.requestId?.value, - statusCode = response.code - ) - } else { - responseJsonParser.parse(response.responseJson()) ?: run { + responseJsonParser: ModelJsonParser, + onSuccessExecutionTimeBlock: (Long) -> Unit = {} + ): Response { + val started = Clock.markNow() + return runCatching { + stripeNetworkClient.executeRequest( + request + ) + }.fold( + onSuccess = { response -> + if (response.isError) { + // TODO(ccen) Parse the response code and throw different exceptions throw APIException( - message = "$responseJsonParser returns null for ${response.responseJson()}" + stripeError = stripeErrorJsonParser.parse(response.responseJson()), + requestId = response.requestId?.value, + statusCode = response.code ) + } else { + responseJsonParser.parse(response.responseJson())?.let { response -> + onSuccessExecutionTimeBlock(started.elapsedSince().inMilliseconds.toLong()) + response + } ?: run { + throw APIException( + message = "$responseJsonParser returns null for ${response.responseJson()}" + ) + } } + }, + onFailure = { + throw APIConnectionException( + "Failed to execute $request", + cause = it + ) } - }, - onFailure = { - throw APIConnectionException( - "Failed to execute $request", - cause = it - ) - } - ) + ) + } internal companion object { const val SUBMIT = "submit" diff --git a/identity/src/main/java/com/stripe/android/identity/networking/IdentityRepository.kt b/identity/src/main/java/com/stripe/android/identity/networking/IdentityRepository.kt index adb875bc670..881513cddc4 100644 --- a/identity/src/main/java/com/stripe/android/identity/networking/IdentityRepository.kt +++ b/identity/src/main/java/com/stripe/android/identity/networking/IdentityRepository.kt @@ -52,7 +52,8 @@ internal interface IdentityRepository { verificationId: String, ephemeralKey: String, imageFile: File, - filePurpose: StripeFilePurpose + filePurpose: StripeFilePurpose, + onSuccessExecutionTimeBlock: (Long) -> Unit = {} ): StripeFile @Throws( diff --git a/identity/src/main/java/com/stripe/android/identity/viewmodel/IdentityViewModel.kt b/identity/src/main/java/com/stripe/android/identity/viewmodel/IdentityViewModel.kt index 44be86bb3bd..299ee9af902 100644 --- a/identity/src/main/java/com/stripe/android/identity/viewmodel/IdentityViewModel.kt +++ b/identity/src/main/java/com/stripe/android/identity/viewmodel/IdentityViewModel.kt @@ -218,7 +218,8 @@ internal class IdentityViewModel @Inject constructor( uri: Uri, isFront: Boolean, docCapturePage: VerificationPageStaticContentDocumentCapturePage, - uploadMethod: UploadMethod + uploadMethod: UploadMethod, + scanType: IdentityScanState.ScanType ) { uploadDocumentImagesAndNotify( imageFile = @@ -235,7 +236,9 @@ internal class IdentityViewModel @Inject constructor( ), uploadMethod = uploadMethod, isHighRes = true, - isFront = isFront + isFront = isFront, + scanType = scanType, + compressionQuality = docCapturePage.highResImageCompressionQuality ) } @@ -272,7 +275,8 @@ internal class IdentityViewModel @Inject constructor( verificationPage.documentCapture, isHighRes = true, isFront = isFront, - scores + scores, + targetScanType ) // upload low res @@ -282,7 +286,8 @@ internal class IdentityViewModel @Inject constructor( verificationPage.documentCapture, isHighRes = false, isFront = isFront, - scores + scores, + targetScanType ) } is FaceDetectorOutput -> { @@ -323,7 +328,8 @@ internal class IdentityViewModel @Inject constructor( docCapturePage: VerificationPageStaticContentDocumentCapturePage, isHighRes: Boolean, isFront: Boolean, - scores: List + scores: List, + targetScanType: IdentityScanState.ScanType ) { identityIO.resizeBitmapAndCreateFileToUpload( bitmap = @@ -367,7 +373,14 @@ internal class IdentityViewModel @Inject constructor( uploadMethod = UploadMethod.AUTOCAPTURE, scores = scores, isHighRes = isHighRes, - isFront = isFront + isFront = isFront, + scanType = targetScanType, + compressionQuality = + if (isHighRes) { + docCapturePage.highResImageCompressionQuality + } else { + docCapturePage.lowResImageCompressionQuality + } ) } } @@ -388,18 +401,33 @@ internal class IdentityViewModel @Inject constructor( uploadMethod: UploadMethod, scores: List? = null, isHighRes: Boolean, - isFront: Boolean + isFront: Boolean, + scanType: IdentityScanState.ScanType, + compressionQuality: Float ) { viewModelScope.launch { runCatching { + var uploadTime = 0L identityRepository.uploadImage( verificationId = verificationArgs.verificationSessionId, ephemeralKey = verificationArgs.ephemeralKeySecret, imageFile = imageFile, - filePurpose = filePurpose - ) + filePurpose = filePurpose, + onSuccessExecutionTimeBlock = { uploadTime = it } + ) to uploadTime }.fold( - onSuccess = { uploadedStripeFile -> + onSuccess = { fileTimePair -> + identityRepository.sendAnalyticsRequest( + identityAnalyticsRequestFactory.imageUpload( + value = fileTimePair.second, + compressionQuality = compressionQuality, + scanType = scanType, + id = fileTimePair.first.id, + fileName = fileTimePair.first.filename, + fileSize = imageFile.length() / BYTES_IN_KB + ) + ) + updateAnalyticsState { oldState -> if (isFront) { oldState.copy( @@ -416,7 +444,7 @@ internal class IdentityViewModel @Inject constructor( isHighRes = isHighRes, isFront = isFront, newResult = UploadedResult( - uploadedStripeFile, + fileTimePair.first, scores, uploadMethod ) @@ -496,7 +524,12 @@ internal class IdentityViewModel @Inject constructor( StripeFilePurpose.fromCode(selfieCapturePage.filePurpose) ), isHighRes = isHighRes, - selfie = selfie + selfie = selfie, + compressionQuality = if (isHighRes) { + selfieCapturePage.highResImageCompressionQuality + } else { + selfieCapturePage.lowResImageCompressionQuality + } ) } } @@ -505,23 +538,37 @@ internal class IdentityViewModel @Inject constructor( imageFile: File, filePurpose: StripeFilePurpose, isHighRes: Boolean, - selfie: FaceDetectorTransitioner.Selfie + selfie: FaceDetectorTransitioner.Selfie, + compressionQuality: Float ) { viewModelScope.launch { runCatching { + var uploadTime = 0L identityRepository.uploadImage( verificationId = verificationArgs.verificationSessionId, ephemeralKey = verificationArgs.ephemeralKeySecret, imageFile = imageFile, - filePurpose = filePurpose - ) + filePurpose = filePurpose, + onSuccessExecutionTimeBlock = { uploadTime = it } + ) to uploadTime }.fold( - onSuccess = { uploadedStripeFile -> + onSuccess = { fileTimePair -> + identityRepository.sendAnalyticsRequest( + identityAnalyticsRequestFactory.imageUpload( + value = fileTimePair.second, + compressionQuality = compressionQuality, + scanType = IdentityScanState.ScanType.SELFIE, + id = fileTimePair.first.id, + fileName = fileTimePair.first.filename, + fileSize = imageFile.length() / BYTES_IN_KB + ) + ) + _selfieUploadedState.update { currentState -> currentState.update( isHighRes = isHighRes, newResult = UploadedResult( - uploadedStripeFile + fileTimePair.first ), selfie = selfie ) @@ -735,5 +782,6 @@ internal class IdentityViewModel @Inject constructor( val TAG: String = IdentityViewModel::class.java.simpleName const val FRONT = "front" const val BACK = "back" + const val BYTES_IN_KB = 1024 } } diff --git a/identity/src/test/java/com/stripe/android/identity/navigation/IdentityUploadFragmentTest.kt b/identity/src/test/java/com/stripe/android/identity/navigation/IdentityUploadFragmentTest.kt index c35e3c98881..3fd41b06d6f 100644 --- a/identity/src/test/java/com/stripe/android/identity/navigation/IdentityUploadFragmentTest.kt +++ b/identity/src/test/java/com/stripe/android/identity/navigation/IdentityUploadFragmentTest.kt @@ -558,7 +558,8 @@ class IdentityUploadFragmentTest { eq(DocumentUploadParam.UploadMethod.MANUALCAPTURE) } else { eq(DocumentUploadParam.UploadMethod.FILEUPLOAD) - } + }, + scanType = eq(scanType) ) assertThat(binding.selectFront.visibility).isEqualTo(View.GONE) assertThat(binding.progressCircularFront.visibility).isEqualTo(View.VISIBLE) @@ -573,7 +574,8 @@ class IdentityUploadFragmentTest { eq(DocumentUploadParam.UploadMethod.MANUALCAPTURE) } else { eq(DocumentUploadParam.UploadMethod.FILEUPLOAD) - } + }, + scanType = eq(scanType) ) assertThat(binding.selectBack.visibility).isEqualTo(View.GONE) assertThat(binding.progressCircularBack.visibility).isEqualTo(View.VISIBLE) diff --git a/identity/src/test/java/com/stripe/android/identity/navigation/PassportUploadFragmentTest.kt b/identity/src/test/java/com/stripe/android/identity/navigation/PassportUploadFragmentTest.kt index 817d535a7b6..8271ba7d055 100644 --- a/identity/src/test/java/com/stripe/android/identity/navigation/PassportUploadFragmentTest.kt +++ b/identity/src/test/java/com/stripe/android/identity/navigation/PassportUploadFragmentTest.kt @@ -35,6 +35,7 @@ import com.stripe.android.identity.networking.models.DocumentUploadParam import com.stripe.android.identity.networking.models.DocumentUploadParam.UploadMethod import com.stripe.android.identity.networking.models.VerificationPage import com.stripe.android.identity.networking.models.VerificationPageStaticContentDocumentCapturePage +import com.stripe.android.identity.states.IdentityScanState import com.stripe.android.identity.utils.ARG_SHOULD_SHOW_CHOOSE_PHOTO import com.stripe.android.identity.utils.ARG_SHOULD_SHOW_TAKE_PHOTO import com.stripe.android.identity.viewModelFactoryFor @@ -341,7 +342,8 @@ class PassportUploadFragmentTest { eq(UploadMethod.MANUALCAPTURE) } else { eq(UploadMethod.FILEUPLOAD) - } + }, + scanType = eq(IdentityScanState.ScanType.PASSPORT) ) assertThat(binding.selectFront.visibility).isEqualTo(View.GONE) assertThat(binding.progressCircularFront.visibility).isEqualTo(View.VISIBLE) diff --git a/identity/src/test/java/com/stripe/android/identity/viewmodel/IdentityViewModelTest.kt b/identity/src/test/java/com/stripe/android/identity/viewmodel/IdentityViewModelTest.kt index e5fbbda253a..e19db4ce3f9 100644 --- a/identity/src/test/java/com/stripe/android/identity/viewmodel/IdentityViewModelTest.kt +++ b/identity/src/test/java/com/stripe/android/identity/viewmodel/IdentityViewModelTest.kt @@ -10,6 +10,7 @@ import com.stripe.android.core.injection.DUMMY_INJECTOR_KEY import com.stripe.android.core.model.StripeFile import com.stripe.android.core.model.StripeFilePurpose import com.stripe.android.identity.IdentityVerificationSheetContract +import com.stripe.android.identity.analytics.IdentityAnalyticsRequestFactory import com.stripe.android.identity.camera.IdentityAggregator import com.stripe.android.identity.ml.AnalyzerInput import com.stripe.android.identity.ml.BoundingBox @@ -37,9 +38,11 @@ import org.junit.Test import org.junit.rules.TestRule import org.junit.runner.RunWith import org.mockito.kotlin.any +import org.mockito.kotlin.anyOrNull import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.same +import org.mockito.kotlin.times import org.mockito.kotlin.verify import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @@ -81,6 +84,8 @@ internal class IdentityViewModelTest { ) } + val mockIdentityAnalyticsRequestFactory = mock() + val viewModel = IdentityViewModel( IdentityVerificationSheetContract.Args( verificationSessionId = VERIFICATION_SESSION_ID, @@ -93,7 +98,7 @@ internal class IdentityViewModelTest { mockIdentityModelFetcher, mockIdentityIO, mock(), - mock(), + mockIdentityAnalyticsRequestFactory, mock(), mock(), mock(), @@ -101,13 +106,13 @@ internal class IdentityViewModelTest { ) private fun mockUploadSuccess() = runBlocking { - whenever(mockIdentityRepository.uploadImage(any(), any(), any(), any())).thenReturn( + whenever(mockIdentityRepository.uploadImage(any(), any(), any(), any(), any())).thenReturn( UPLOADED_STRIPE_FILE ) } private fun mockUploadFailure() = runBlocking { - whenever(mockIdentityRepository.uploadImage(any(), any(), any(), any())).thenThrow( + whenever(mockIdentityRepository.uploadImage(any(), any(), any(), any(), any())).thenThrow( UPLOADED_FAILURE_EXCEPTION ) } @@ -162,6 +167,14 @@ internal class IdentityViewModelTest { (FaceDetectorTransitioner.Selfie.BEST), (FaceDetectorTransitioner.Selfie.LAST) ).forEach { selfie -> + verify(mockIdentityAnalyticsRequestFactory, times(6)).imageUpload( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) listOf(true, false).forEach { isHighRes -> testUploadSelfieScanSuccessResult(selfie, isHighRes) } @@ -176,6 +189,14 @@ internal class IdentityViewModelTest { mockVerificationPage, IdentityScanState.ScanType.SELFIE ) + verify(mockIdentityAnalyticsRequestFactory, times(0)).imageUpload( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) listOf( (viewModel.selfieUploadState.value.firstHighResResult), @@ -283,7 +304,8 @@ internal class IdentityViewModelTest { mockUri, isFront, DOCUMENT_CAPTURE, - DocumentUploadParam.UploadMethod.FILEUPLOAD + DocumentUploadParam.UploadMethod.FILEUPLOAD, + IdentityScanState.ScanType.DL_FRONT ) verify(mockIdentityIO).resizeUriAndCreateFileToUpload( @@ -295,6 +317,15 @@ internal class IdentityViewModelTest { eq(HIGH_RES_COMPRESSION_QUALITY) ) + verify(mockIdentityAnalyticsRequestFactory).imageUpload( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() + ) + if (isFront) { viewModel.documentUploadState.value.frontHighResResult } else { @@ -467,7 +498,17 @@ internal class IdentityViewModelTest { mock(), isFront, DOCUMENT_CAPTURE, - DocumentUploadParam.UploadMethod.FILEUPLOAD + DocumentUploadParam.UploadMethod.FILEUPLOAD, + IdentityScanState.ScanType.DL_FRONT + ) + + verify(mockIdentityAnalyticsRequestFactory, times(0)).imageUpload( + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull(), + anyOrNull() ) if (isFront) {