From 2af6a5cc39d3c12b7ec1af5bdbee2497fc4aba0a Mon Sep 17 00:00:00 2001 From: Timothy Chow Date: Wed, 18 Sep 2024 14:43:54 -0500 Subject: [PATCH 1/2] Fix analytic event timestamps --- .../api/core/AnalyticsClient.kt | 26 ++----- .../api/core/AnalyticsEvent.kt | 4 +- .../api/core/BraintreeClient.kt | 24 ++++-- .../api/core/AnalyticsClientUnitTest.kt | 78 ++++++------------- .../api/core/BraintreeClientUnitTest.kt | 18 +++-- .../braintreepayments/api/sharedutils/Time.kt | 13 ++++ 6 files changed, 76 insertions(+), 87 deletions(-) create mode 100644 SharedUtils/src/main/java/com/braintreepayments/api/sharedutils/Time.kt diff --git a/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsClient.kt b/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsClient.kt index d22b528514..d1d54d13c8 100644 --- a/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsClient.kt +++ b/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsClient.kt @@ -7,6 +7,7 @@ import androidx.work.ExistingWorkPolicy import androidx.work.ListenableWorker import androidx.work.OneTimeWorkRequest import androidx.work.WorkManager +import com.braintreepayments.api.sharedutils.Time import org.json.JSONArray import org.json.JSONException import org.json.JSONObject @@ -20,7 +21,8 @@ internal class AnalyticsClient( private val analyticsDatabase: AnalyticsDatabase = AnalyticsDatabase.getInstance(context.applicationContext), private val workManager: WorkManager = WorkManager.getInstance(context.applicationContext), private val deviceInspector: DeviceInspector = DeviceInspector(), - private val analyticsParamRepository: AnalyticsParamRepository = AnalyticsParamRepository.instance + private val analyticsParamRepository: AnalyticsParamRepository = AnalyticsParamRepository.instance, + private val time: Time = Time() ) { private val applicationContext = context.applicationContext @@ -121,6 +123,7 @@ internal class AnalyticsClient( ) val analyticsRequest = createFPTIPayload(authorization, eventBlobs, metadata) + httpClient.post( FPTI_ANALYTICS_URL, analyticsRequest.toString(), @@ -137,27 +140,11 @@ internal class AnalyticsClient( } } - fun reportCrash( - context: Context?, - configuration: Configuration?, - integration: IntegrationType?, - authorization: Authorization? - ) { - reportCrash( - context, - configuration, - integration, - System.currentTimeMillis(), - authorization - ) - } - @VisibleForTesting fun reportCrash( context: Context?, configuration: Configuration?, integration: IntegrationType?, - timestamp: Long, authorization: Authorization? ) { if (authorization == null) { @@ -169,7 +156,10 @@ internal class AnalyticsClient( sessionId = analyticsParamRepository.sessionId, integration = integration ) - val event = AnalyticsEvent(name = "crash", timestamp = timestamp) + val event = AnalyticsEvent( + name = "crash", + timestamp = time.currentTime + ) val eventJSON = mapAnalyticsEventToFPTIEventJSON(event) val eventBlobs = listOf( AnalyticsEventBlob( diff --git a/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsEvent.kt b/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsEvent.kt index 0c038f0f33..689eede1bc 100644 --- a/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsEvent.kt +++ b/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsEvent.kt @@ -2,11 +2,11 @@ package com.braintreepayments.api.core internal data class AnalyticsEvent( val name: String, + val timestamp: Long, val payPalContextId: String? = null, val linkType: String? = null, val isVaultRequest: Boolean = false, val startTime: Long? = null, val endTime: Long? = null, - val endpoint: String? = null, - val timestamp: Long = System.currentTimeMillis(), + val endpoint: String? = null ) diff --git a/BraintreeCore/src/main/java/com/braintreepayments/api/core/BraintreeClient.kt b/BraintreeCore/src/main/java/com/braintreepayments/api/core/BraintreeClient.kt index 40508b36b4..f7e58e4e2f 100644 --- a/BraintreeCore/src/main/java/com/braintreepayments/api/core/BraintreeClient.kt +++ b/BraintreeCore/src/main/java/com/braintreepayments/api/core/BraintreeClient.kt @@ -8,6 +8,7 @@ import androidx.annotation.VisibleForTesting import com.braintreepayments.api.sharedutils.HttpResponseCallback import com.braintreepayments.api.sharedutils.HttpResponseTiming import com.braintreepayments.api.sharedutils.ManifestValidator +import com.braintreepayments.api.sharedutils.Time import org.json.JSONException import org.json.JSONObject @@ -38,6 +39,7 @@ class BraintreeClient @VisibleForTesting internal constructor( private val graphQLClient: BraintreeGraphQLClient, private val configurationLoader: ConfigurationLoader, private val manifestValidator: ManifestValidator, + private val time: Time, private val returnUrlScheme: String, private val braintreeDeepLinkReturnUrlScheme: String, /** @@ -50,7 +52,10 @@ class BraintreeClient @VisibleForTesting internal constructor( private var launchesBrowserSwitchAsNewTask: Boolean = false // NOTE: this constructor is used to make dependency injection easy - internal constructor(params: BraintreeClientParams) : this( + internal constructor( + params: BraintreeClientParams, + time: Time = Time() + ) : this( applicationContext = params.applicationContext, integrationType = params.integrationType, authorization = params.authorization, @@ -59,6 +64,7 @@ class BraintreeClient @VisibleForTesting internal constructor( graphQLClient = params.graphQLClient, configurationLoader = params.configurationLoader, manifestValidator = params.manifestValidator, + time = time, returnUrlScheme = params.returnUrlScheme, braintreeDeepLinkReturnUrlScheme = params.braintreeReturnUrlScheme, appLinkReturnUri = params.appLinkReturnUri @@ -132,15 +138,17 @@ class BraintreeClient @VisibleForTesting internal constructor( eventName: String, params: AnalyticsEventParams = AnalyticsEventParams() ) { + val timestamp = time.currentTime getConfiguration { configuration, _ -> val event = AnalyticsEvent( - eventName, - params.payPalContextId, - params.linkType, - params.isVaultRequest, - params.startTime, - params.endTime, - params.endpoint + name = eventName, + timestamp = timestamp, + payPalContextId = params.payPalContextId, + linkType = params.linkType, + isVaultRequest = params.isVaultRequest, + startTime = params.startTime, + endTime = params.endTime, + endpoint = params.endpoint, ) sendAnalyticsEvent(event, configuration, authorization) } diff --git a/BraintreeCore/src/test/java/com/braintreepayments/api/core/AnalyticsClientUnitTest.kt b/BraintreeCore/src/test/java/com/braintreepayments/api/core/AnalyticsClientUnitTest.kt index a2b29fbde8..b35689d130 100644 --- a/BraintreeCore/src/test/java/com/braintreepayments/api/core/AnalyticsClientUnitTest.kt +++ b/BraintreeCore/src/test/java/com/braintreepayments/api/core/AnalyticsClientUnitTest.kt @@ -8,6 +8,7 @@ import com.braintreepayments.api.core.AnalyticsClient.Companion.WORK_INPUT_KEY_S import com.braintreepayments.api.core.AnalyticsClient.Companion.WORK_NAME_ANALYTICS_UPLOAD import com.braintreepayments.api.core.Authorization.Companion.fromString import com.braintreepayments.api.core.Configuration.Companion.fromJson +import com.braintreepayments.api.sharedutils.Time import com.braintreepayments.api.testutils.Fixtures import io.mockk.* import org.json.JSONException @@ -30,6 +31,7 @@ class AnalyticsClientUnitTest { private lateinit var httpClient: BraintreeHttpClient private lateinit var deviceInspector: DeviceInspector private lateinit var analyticsParamRepository: AnalyticsParamRepository + private lateinit var time: Time private lateinit var eventName: String private lateinit var sessionId: String private lateinit var payPalContextId: String @@ -39,6 +41,8 @@ class AnalyticsClientUnitTest { private lateinit var analyticsDatabase: AnalyticsDatabase private lateinit var analyticsEventBlobDao: AnalyticsEventBlobDao + private lateinit var sut: AnalyticsClient + private var timestamp: Long = 0 @Before @@ -59,9 +63,22 @@ class AnalyticsClientUnitTest { analyticsDatabase = mockk(relaxed = true) analyticsEventBlobDao = mockk(relaxed = true) workManager = mockk(relaxed = true) + time = mockk(relaxed = true) every { analyticsDatabase.analyticsEventBlobDao() } returns analyticsEventBlobDao every { analyticsParamRepository.sessionId } returns sessionId + + every { time.currentTime } returns 123 + + sut = AnalyticsClient( + context = context, + httpClient = httpClient, + analyticsDatabase = analyticsDatabase, + workManager = workManager, + deviceInspector = deviceInspector, + analyticsParamRepository = analyticsParamRepository, + time = time + ) } @Test @@ -77,13 +94,7 @@ class AnalyticsClientUnitTest { } returns mockk() val event = AnalyticsEvent(eventName, timestamp = 123) - val sut = AnalyticsClient( - context = context, - httpClient = httpClient, - analyticsDatabase = analyticsDatabase, - workManager = workManager, - deviceInspector = deviceInspector - ) + sut.sendEvent(configuration, event, integration, authorization) val workSpec = workRequestSlot.captured.workSpec @@ -120,8 +131,7 @@ class AnalyticsClientUnitTest { isVaultRequest = true, timestamp = 456 ) - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) + sut.sendEvent(configuration, event, integration, authorization) val workSpec = workRequestSlot.captured.workSpec @@ -156,8 +166,6 @@ class AnalyticsClientUnitTest { .putString(WORK_INPUT_KEY_ANALYTICS_JSON, JSONObject().toString()) .putString(WORK_INPUT_KEY_SESSION_ID, JSONObject().toString()) .build() - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) val result = sut.performAnalyticsWrite(inputData) assertTrue(result is ListenableWorker.Result.Success) } @@ -165,8 +173,6 @@ class AnalyticsClientUnitTest { @Test fun writeAnalytics_whenAnalyticsJSONIsMissing_returnsSuccess() { val inputData = Data.Builder().build() - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) val result = sut.performAnalyticsWrite(inputData) assertTrue(result is ListenableWorker.Result.Failure) } @@ -181,8 +187,7 @@ class AnalyticsClientUnitTest { .putString(WORK_INPUT_KEY_ANALYTICS_JSON, json) .putString(WORK_INPUT_KEY_SESSION_ID, sessionId) .build() - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) + sut.performAnalyticsWrite(inputData) val blob = analyticsEventBlobSlot.captured @@ -238,8 +243,6 @@ class AnalyticsClientUnitTest { ) } - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) sut.performAnalyticsUpload(inputData) // language=JSON @@ -285,8 +288,6 @@ class AnalyticsClientUnitTest { .putString(AnalyticsClient.WORK_INPUT_KEY_INTEGRATION, integration.stringValue) .build() - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) val result = sut.performAnalyticsUpload(inputData) assertTrue(result is ListenableWorker.Result.Failure) @@ -303,8 +304,6 @@ class AnalyticsClientUnitTest { .putString(AnalyticsClient.WORK_INPUT_KEY_INTEGRATION, integration.stringValue) .build() - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) val result = sut.performAnalyticsUpload(inputData) assertTrue(result is ListenableWorker.Result.Failure) @@ -340,8 +339,6 @@ class AnalyticsClientUnitTest { val analyticsJSONSlot = slot() every { httpClient.post(any(), capture(analyticsJSONSlot), any(), any()) } - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) sut.performAnalyticsUpload(inputData) val analyticsJson = JSONObject(analyticsJSONSlot.captured) @@ -360,8 +357,6 @@ class AnalyticsClientUnitTest { .putString(AnalyticsClient.WORK_INPUT_KEY_INTEGRATION, integration.stringValue) .build() - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) val result = sut.performAnalyticsUpload(inputData) assertTrue(result is ListenableWorker.Result.Failure) @@ -378,8 +373,6 @@ class AnalyticsClientUnitTest { .putString(AnalyticsClient.WORK_INPUT_KEY_SESSION_ID, sessionId) .build() - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) val result = sut.performAnalyticsUpload(inputData) assertTrue(result is ListenableWorker.Result.Failure) @@ -410,8 +403,6 @@ class AnalyticsClientUnitTest { ) every { analyticsEventBlobDao.getBlobsBySessionId(sessionId) } returns blobs - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) sut.performAnalyticsUpload(inputData) verify { analyticsEventBlobDao.deleteEventBlobs(blobs) } @@ -442,8 +433,6 @@ class AnalyticsClientUnitTest { val httpError = Exception("error") every { httpClient.post(any(), any(), any(), any()) } throws httpError - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) val result = sut.performAnalyticsUpload(inputData) assertTrue(result is ListenableWorker.Result.Failure) } @@ -469,15 +458,7 @@ class AnalyticsClientUnitTest { ) } returns Unit - val sut = AnalyticsClient( - context = context, - httpClient = httpClient, - analyticsDatabase = analyticsDatabase, - workManager = workManager, - deviceInspector = deviceInspector, - analyticsParamRepository = analyticsParamRepository - ) - sut.reportCrash(context, configuration, integration, 123, authorization) + sut.reportCrash(context, configuration, integration, authorization) // language=JSON val expectedJSON = """ @@ -527,12 +508,10 @@ class AnalyticsClientUnitTest { deviceInspector.getDeviceMetadata(context, configuration, sessionId, integration) } returns metadata - val sut = - AnalyticsClient(context, httpClient, analyticsDatabase, workManager, deviceInspector) - val event = AnalyticsEvent(eventName) + val event = AnalyticsEvent(eventName, timestamp) sut.sendEvent(configuration, event, integration, authorization) - sut.reportCrash(context, configuration, integration, 123, null) + sut.reportCrash(context, configuration, integration, null) // or confirmVerified(httpClient) verify { httpClient wasNot Called } @@ -540,16 +519,7 @@ class AnalyticsClientUnitTest { @Test fun `sendEvent enqueues work to upload analytic events with sessionId in the name`() { - val sut = AnalyticsClient( - context = context, - httpClient = httpClient, - analyticsDatabase = analyticsDatabase, - workManager = workManager, - deviceInspector = deviceInspector, - analyticsParamRepository = analyticsParamRepository - ) - - sut.sendEvent(configuration, AnalyticsEvent("event-name"), integration, authorization) + sut.sendEvent(configuration, AnalyticsEvent("event-name", timestamp), integration, authorization) verify { workManager.enqueueUniqueWork( diff --git a/BraintreeCore/src/test/java/com/braintreepayments/api/core/BraintreeClientUnitTest.kt b/BraintreeCore/src/test/java/com/braintreepayments/api/core/BraintreeClientUnitTest.kt index 7619c4ea70..f94c38de78 100644 --- a/BraintreeCore/src/test/java/com/braintreepayments/api/core/BraintreeClientUnitTest.kt +++ b/BraintreeCore/src/test/java/com/braintreepayments/api/core/BraintreeClientUnitTest.kt @@ -7,10 +7,11 @@ import androidx.fragment.app.FragmentActivity import androidx.test.core.app.ApplicationProvider import androidx.work.testing.WorkManagerTestInitHelper import com.braintreepayments.api.BrowserSwitchClient -import com.braintreepayments.api.testutils.Fixtures import com.braintreepayments.api.sharedutils.HttpResponseCallback import com.braintreepayments.api.sharedutils.ManifestValidator import com.braintreepayments.api.sharedutils.NetworkResponseCallback +import com.braintreepayments.api.sharedutils.Time +import com.braintreepayments.api.testutils.Fixtures import io.mockk.* import org.json.JSONException import org.json.JSONObject @@ -326,14 +327,17 @@ class BraintreeClientUnitTest { .configuration(configuration) .build() + val time: Time = mockk() + every { time.currentTime } returns 123 + val params = createDefaultParams(configurationLoader) - val sut = BraintreeClient(params) + val sut = BraintreeClient(params, time) sut.sendAnalyticsEvent("event.started") verify { analyticsClient.sendEvent( configuration, - match { it.name == "event.started" }, + match { it.name == "event.started" && it.timestamp == 123L }, IntegrationType.CUSTOM, authorization ) @@ -393,8 +397,12 @@ class BraintreeClientUnitTest { fun returnUrlScheme_returnsUrlSchemeDefinedInConstructor() { val context = ApplicationProvider.getApplicationContext() val returnUrlScheme = "custom-url-scheme" - val sut = BraintreeClient(BraintreeOptions(context, authorization, returnUrlScheme = - returnUrlScheme)) + val sut = BraintreeClient( + BraintreeOptions( + context, authorization, returnUrlScheme = + returnUrlScheme + ) + ) assertEquals("custom-url-scheme", sut.getReturnUrlScheme()) } diff --git a/SharedUtils/src/main/java/com/braintreepayments/api/sharedutils/Time.kt b/SharedUtils/src/main/java/com/braintreepayments/api/sharedutils/Time.kt new file mode 100644 index 0000000000..a6b9857b73 --- /dev/null +++ b/SharedUtils/src/main/java/com/braintreepayments/api/sharedutils/Time.kt @@ -0,0 +1,13 @@ +package com.braintreepayments.api.sharedutils + +import androidx.annotation.RestrictTo + +@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) +class Time { + + /** + * Returns the current time in milliseconds + */ + val currentTime: Long + get() = System.currentTimeMillis() +} From 211f1837fd3eba3ee5d22b921213db61beafe1b2 Mon Sep 17 00:00:00 2001 From: Timothy Chow Date: Wed, 18 Sep 2024 14:52:39 -0500 Subject: [PATCH 2/2] Fix lint --- .../main/java/com/braintreepayments/api/core/AnalyticsClient.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsClient.kt b/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsClient.kt index d1d54d13c8..d8b19c1ba8 100644 --- a/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsClient.kt +++ b/BraintreeCore/src/main/java/com/braintreepayments/api/core/AnalyticsClient.kt @@ -123,7 +123,7 @@ internal class AnalyticsClient( ) val analyticsRequest = createFPTIPayload(authorization, eventBlobs, metadata) - + httpClient.post( FPTI_ANALYTICS_URL, analyticsRequest.toString(),