From 19d708b973582989de8185b28a42702a46e25c26 Mon Sep 17 00:00:00 2001 From: "justin.fiedler" Date: Mon, 13 Nov 2023 20:22:48 +0000 Subject: [PATCH 1/3] fix: correctly format client_upload_time in HTTP request when minIdLength is set --- .../amplitude/core/utilities/HttpClient.kt | 8 +- .../core/utilities/HttpClientTest.kt | 108 ++++++++++++++++++ 2 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt diff --git a/core/src/main/java/com/amplitude/core/utilities/HttpClient.kt b/core/src/main/java/com/amplitude/core/utilities/HttpClient.kt index eecb0ea0..a7b977c4 100644 --- a/core/src/main/java/com/amplitude/core/utilities/HttpClient.kt +++ b/core/src/main/java/com/amplitude/core/utilities/HttpClient.kt @@ -85,7 +85,7 @@ internal class HttpClient( } internal fun getClientUploadTime(): String { - val currentTimeMillis = System.currentTimeMillis() + val currentTimeMillis = getCurrentTimeMillis() val sdf = SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'") sdf.timeZone = TimeZone.getTimeZone("UTC") return sdf.format(Date(currentTimeMillis)) @@ -95,6 +95,10 @@ internal class HttpClient( return configuration.minIdLength } + internal fun getCurrentTimeMillis(): Long { + return System.currentTimeMillis() + } + fun getInputStream(connection: HttpURLConnection): InputStream { return try { connection.inputStream @@ -149,7 +153,7 @@ abstract class Connection( if (minIdLength == null) { return "{\"api_key\":\"$apiKey\",\"client_upload_time\":\"$clientUploadTime\",\"events\":$events}" } - return "{\"api_key\":\"$apiKey\",\"client_upload_time\":$clientUploadTime,\"events\":$events,\"options\":{\"min_id_length\":$minIdLength}}" + return "{\"api_key\":\"$apiKey\",\"client_upload_time\":\"$clientUploadTime\",\"events\":$events,\"options\":{\"min_id_length\":$minIdLength}}" } } diff --git a/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt b/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt new file mode 100644 index 00000000..c9dbd077 --- /dev/null +++ b/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt @@ -0,0 +1,108 @@ +package com.amplitude.core.utilities + +import com.amplitude.core.Configuration +import com.amplitude.core.events.BaseEvent +import io.mockk.every +import io.mockk.spyk +import kotlinx.coroutines.ExperimentalCoroutinesApi +import okhttp3.mockwebserver.MockResponse +import okhttp3.mockwebserver.MockWebServer +import okhttp3.mockwebserver.RecordedRequest +import org.json.JSONObject +import org.junit.jupiter.api.AfterEach +import org.junit.jupiter.api.Assertions.assertEquals +import org.junit.jupiter.api.BeforeEach +import org.junit.jupiter.api.Test +import org.junit.jupiter.api.TestInstance +import java.util.concurrent.TimeUnit + +@TestInstance(TestInstance.Lifecycle.PER_CLASS) +class HttpClientTest { + private lateinit var server: MockWebServer + val apiKey = "API_KEY" + val clientUploadTimeMillis = 1699905773000L + val clientUploadTimeString = "2023-11-13T20:02:53.000Z" + + @ExperimentalCoroutinesApi + @BeforeEach + fun setup() { + server = MockWebServer() + server.start() + } + + @AfterEach + fun shutdown() { + server.shutdown() + } + + @Test + fun `test client_upload_time is set on the request`() { + server.enqueue(MockResponse().setBody("{\"code\": \"success\"}")) + + val config = Configuration( + apiKey = apiKey, + serverUrl = server.url("/").toString() + ) + val event = BaseEvent() + event.eventType = "test" + + val httpClient = spyk(HttpClient(config)); + every { httpClient.getCurrentTimeMillis() } returns clientUploadTimeMillis + + val connection = httpClient.upload() + System.currentTimeMillis() + + connection.outputStream?.let { + connection.setEvents(JSONUtil.eventsToString(listOf(event))) + // Upload the payloads. + connection.close() + } + + val request: RecordedRequest? = runRequest() + val body = request?.body?.readUtf8(); + val result = JSONObject(body) + + assertEquals(apiKey, result.getString("api_key")) + assertEquals(clientUploadTimeString, result.getString("client_upload_time")) + } + + @Test + fun `test client_upload_time is set correctly when minIdLength is set`() { + server.enqueue(MockResponse().setBody("{\"code\": \"success\"}")) + + val config = Configuration( + apiKey = apiKey, + serverUrl = server.url("/").toString(), + minIdLength = 3, + ) + val event = BaseEvent() + event.eventType = "test" + + val httpClient = spyk(HttpClient(config)); + every { httpClient.getCurrentTimeMillis() } returns clientUploadTimeMillis + + val connection = httpClient.upload() + System.currentTimeMillis() + + connection.outputStream?.let { + connection.setEvents(JSONUtil.eventsToString(listOf(event))) + // Upload the payloads. + connection.close() + } + + val request: RecordedRequest? = runRequest() + val body = request?.body?.readUtf8(); + val result = JSONObject(body) + + assertEquals(apiKey, result.getString("api_key")) + assertEquals(clientUploadTimeString, result.getString("client_upload_time")) + } + + private fun runRequest(): RecordedRequest? { + return try { + server.takeRequest(5, TimeUnit.SECONDS) + } catch (e: InterruptedException) { + null + } + } +} From fca04922594b95230542ae857aeb6d44e2a063f3 Mon Sep 17 00:00:00 2001 From: "justin.fiedler" Date: Mon, 13 Nov 2023 20:51:32 +0000 Subject: [PATCH 2/3] chore: lint fixes in HTTPClientTest --- .../kotlin/com/amplitude/core/utilities/HttpClientTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt b/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt index c9dbd077..9da1c652 100644 --- a/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt +++ b/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt @@ -46,7 +46,7 @@ class HttpClientTest { val event = BaseEvent() event.eventType = "test" - val httpClient = spyk(HttpClient(config)); + val httpClient = spyk(HttpClient(config)) every { httpClient.getCurrentTimeMillis() } returns clientUploadTimeMillis val connection = httpClient.upload() @@ -59,7 +59,7 @@ class HttpClientTest { } val request: RecordedRequest? = runRequest() - val body = request?.body?.readUtf8(); + val body = request?.body?.readUtf8() val result = JSONObject(body) assertEquals(apiKey, result.getString("api_key")) @@ -78,7 +78,7 @@ class HttpClientTest { val event = BaseEvent() event.eventType = "test" - val httpClient = spyk(HttpClient(config)); + val httpClient = spyk(HttpClient(config)) every { httpClient.getCurrentTimeMillis() } returns clientUploadTimeMillis val connection = httpClient.upload() @@ -91,7 +91,7 @@ class HttpClientTest { } val request: RecordedRequest? = runRequest() - val body = request?.body?.readUtf8(); + val body = request?.body?.readUtf8() val result = JSONObject(body) assertEquals(apiKey, result.getString("api_key")) From fdc2539e8e50871ba0aed0676c3e32fa8e03dd39 Mon Sep 17 00:00:00 2001 From: "justin.fiedler" Date: Mon, 13 Nov 2023 21:32:15 +0000 Subject: [PATCH 3/3] chore: code clean up from PR feedback --- .../com/amplitude/core/utilities/HttpClientTest.kt | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt b/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt index 9da1c652..191bf1a5 100644 --- a/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt +++ b/core/src/test/kotlin/com/amplitude/core/utilities/HttpClientTest.kt @@ -50,17 +50,14 @@ class HttpClientTest { every { httpClient.getCurrentTimeMillis() } returns clientUploadTimeMillis val connection = httpClient.upload() - System.currentTimeMillis() - connection.outputStream?.let { connection.setEvents(JSONUtil.eventsToString(listOf(event))) // Upload the payloads. connection.close() } - val request: RecordedRequest? = runRequest() - val body = request?.body?.readUtf8() - val result = JSONObject(body) + val request = runRequest() + val result = JSONObject(request?.body?.readUtf8()) assertEquals(apiKey, result.getString("api_key")) assertEquals(clientUploadTimeString, result.getString("client_upload_time")) @@ -82,17 +79,14 @@ class HttpClientTest { every { httpClient.getCurrentTimeMillis() } returns clientUploadTimeMillis val connection = httpClient.upload() - System.currentTimeMillis() - connection.outputStream?.let { connection.setEvents(JSONUtil.eventsToString(listOf(event))) // Upload the payloads. connection.close() } - val request: RecordedRequest? = runRequest() - val body = request?.body?.readUtf8() - val result = JSONObject(body) + val request = runRequest() + val result = JSONObject(request?.body?.readUtf8()) assertEquals(apiKey, result.getString("api_key")) assertEquals(clientUploadTimeString, result.getString("client_upload_time"))