From c43b4113494b9f11b5922841205eb77fcd7c363c Mon Sep 17 00:00:00 2001 From: Xavier Gouchet Date: Tue, 19 Jul 2022 17:08:42 +0200 Subject: [PATCH 1/3] RUMM-2342 Add support to v2 request --- .../gradle/plugin/internal/OkHttpUploader.kt | 42 +++++++++---- .../plugin/internal/OkHttpUploaderTest.kt | 63 +++++++++++++------ 2 files changed, 75 insertions(+), 30 deletions(-) diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt index 97ad3568..eff24354 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt @@ -8,16 +8,22 @@ package com.datadog.gradle.plugin.internal import com.datadog.gradle.plugin.DdAndroidGradlePlugin.Companion.LOGGER import com.datadog.gradle.plugin.RepositoryInfo -import java.io.File -import java.lang.IllegalStateException -import java.net.HttpURLConnection -import java.util.concurrent.TimeUnit import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.MultipartBody import okhttp3.OkHttpClient import okhttp3.Request +import okhttp3.RequestBody import okhttp3.RequestBody.Companion.asRequestBody +import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.Response +import okio.Buffer +import org.json.JSONObject +import java.io.File +import java.io.IOException +import java.net.HttpURLConnection +import java.nio.charset.Charset +import java.util.Locale +import java.util.concurrent.TimeUnit internal class OkHttpUploader : Uploader { @@ -30,6 +36,7 @@ internal class OkHttpUploader : Uploader { .Builder() .callTimeout(NETWORK_TIMEOUT_MS, TimeUnit.MILLISECONDS) .writeTimeout(NETWORK_TIMEOUT_MS, TimeUnit.MILLISECONDS) + .connectTimeout(NETWORK_TIMEOUT_MS, TimeUnit.MILLISECONDS) .build() @Suppress("TooGenericExceptionCaught") @@ -81,12 +88,16 @@ internal class OkHttpUploader : Uploader { .format(mappingFile.absolutePath) ) } + + val eventJson = JSONObject() + eventJson.put("version", identifier.version) + eventJson.put("service", identifier.serviceName) + eventJson.put("variant", identifier.variant) + eventJson.put("type", TYPE_JVM_MAPPING_FILE) + val builder = MultipartBody.Builder() builder.setType(MultipartBody.FORM) - .addFormDataPart("version", identifier.version) - .addFormDataPart("service", identifier.serviceName) - .addFormDataPart("variant", identifier.variant) - .addFormDataPart("type", TYPE_JVM_MAPPING_FILE) + .addFormDataPart("event", "event", eventJson.toString(0).toRequestBody(MEDIA_TYPE_JSON)) .addFormDataPart("jvm_mapping_file", mappingFile.name, mappingFileBody) if (repositoryFile != null) { @@ -116,12 +127,19 @@ internal class OkHttpUploader : Uploader { ) statusCode == HttpURLConnection.HTTP_FORBIDDEN -> throw IllegalStateException( "Unable to upload mapping file for $identifier; " + - "verify that you're using a valid API Key" + "verify that you're using a valid API Key" ) - statusCode >= HttpURLConnection.HTTP_BAD_REQUEST -> throw IllegalStateException( - "Unable to upload mapping file for $identifier; " + - "it can be because the mapping file already exist for this version" + statusCode == HttpURLConnection.HTTP_CLIENT_TIMEOUT -> throw RuntimeException( + "Unable to upload mapping file for $identifier because of a request timeout; " + + "check your network connection" ) + statusCode >= HttpURLConnection.HTTP_BAD_REQUEST -> { + throw IllegalStateException( + "Unable to upload mapping file for $identifier ($statusCode); " + + "it can be because the mapping file already exist for this version.\n" + + "${response.body?.string()}" + ) + } } } diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt index a1747ccb..e35ccaab 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt @@ -105,19 +105,26 @@ internal class OkHttpUploaderTest { } @Test - fun `M use an OkHttpClient W callTimeout { 45 seconds }`() { + fun `M set client callTimeout W init()`() { assertThat(testedUploader.client.callTimeoutMillis).isEqualTo( OkHttpUploader.NETWORK_TIMEOUT_MS.toInt() ) } @Test - fun `M use an OkHttpClient W writeTimeout { 45 seconds }`() { + fun `M set client writeTimeout W init()`() { assertThat(testedUploader.client.writeTimeoutMillis).isEqualTo( OkHttpUploader.NETWORK_TIMEOUT_MS.toInt() ) } + @Test + fun `M set client connectTimeout W init()`() { + assertThat(testedUploader.client.connectTimeoutMillis).isEqualTo( + OkHttpUploader.NETWORK_TIMEOUT_MS.toInt() + ) + } + @Test fun `𝕄 upload proper request 𝕎 upload()`() { // Given @@ -139,12 +146,17 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(1) assertThat(dispatchedRequest) .hasMethod("POST") - .containsFormData("version", fakeIdentifier.version) - .containsFormData("service", fakeIdentifier.serviceName) - .containsFormData("variant", fakeIdentifier.variant) - .containsFormData("type", OkHttpUploader.TYPE_JVM_MAPPING_FILE) .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) + .containsMultipartFile( + "event", + "event", + "{\"service\":\"${fakeIdentifier.serviceName}\"," + + "\"variant\":\"${fakeIdentifier.variant}\"," + + "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + + "\"version\":\"${fakeIdentifier.version}\"}", + "application/json; charset=utf-8" + ) .containsMultipartFile( "jvm_mapping_file", fakeMappingFileName, @@ -180,10 +192,15 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(1) assertThat(dispatchedRequest) .hasMethod("POST") - .containsFormData("version", fakeIdentifier.version) - .containsFormData("service", fakeIdentifier.serviceName) - .containsFormData("variant", fakeIdentifier.variant) - .containsFormData("type", OkHttpUploader.TYPE_JVM_MAPPING_FILE) + .containsMultipartFile( + "event", + "event", + "{\"service\":\"${fakeIdentifier.serviceName}\"," + + "\"variant\":\"${fakeIdentifier.variant}\"," + + "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + + "\"version\":\"${fakeIdentifier.version}\"}", + "application/json; charset=utf-8" + ) .containsMultipartFile( "jvm_mapping_file", fakeMappingFileName, @@ -218,12 +235,17 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(1) assertThat(dispatchedRequest) .hasMethod("POST") - .containsFormData("version", fakeIdentifier.version) - .containsFormData("service", fakeIdentifier.serviceName) - .containsFormData("variant", fakeIdentifier.variant) - .containsFormData("type", OkHttpUploader.TYPE_JVM_MAPPING_FILE) .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) + .containsMultipartFile( + "event", + "event", + "{\"service\":\"${fakeIdentifier.serviceName}\"," + + "\"variant\":\"${fakeIdentifier.variant}\"," + + "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + + "\"version\":\"${fakeIdentifier.version}\"}", + "application/json; charset=utf-8" + ) .containsMultipartFile( "jvm_mapping_file", fakeMappingFileName, @@ -267,12 +289,17 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(1) assertThat(dispatchedRequest) .hasMethod("POST") - .containsFormData("version", fakeIdentifier.version) - .containsFormData("service", fakeIdentifier.serviceName) - .containsFormData("variant", fakeIdentifier.variant) - .containsFormData("type", OkHttpUploader.TYPE_JVM_MAPPING_FILE) .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) + .containsMultipartFile( + "event", + "event", + "{\"service\":\"${fakeIdentifier.serviceName}\"," + + "\"variant\":\"${fakeIdentifier.variant}\"," + + "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + + "\"version\":\"${fakeIdentifier.version}\"}", + "application/json; charset=utf-8" + ) .containsMultipartFile( "jvm_mapping_file", fakeMappingFileName, From f7426c497b791b2cd8591891f662c94611396ad9 Mon Sep 17 00:00:00 2001 From: Xavier Gouchet Date: Wed, 20 Jul 2022 14:41:33 +0200 Subject: [PATCH 2/3] RUMM-2342 Update the upload timeout settings --- .../com/datadog/gradle/plugin/internal/OkHttpUploader.kt | 7 +------ .../datadog/gradle/plugin/internal/OkHttpUploaderTest.kt | 6 ++---- 2 files changed, 3 insertions(+), 10 deletions(-) diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt index eff24354..85cc2b1b 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt @@ -12,17 +12,12 @@ import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.MultipartBody import okhttp3.OkHttpClient import okhttp3.Request -import okhttp3.RequestBody import okhttp3.RequestBody.Companion.asRequestBody import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.Response -import okio.Buffer import org.json.JSONObject import java.io.File -import java.io.IOException import java.net.HttpURLConnection -import java.nio.charset.Charset -import java.util.Locale import java.util.concurrent.TimeUnit internal class OkHttpUploader : Uploader { @@ -34,7 +29,7 @@ internal class OkHttpUploader : Uploader { internal val client get() = OkHttpClient .Builder() - .callTimeout(NETWORK_TIMEOUT_MS, TimeUnit.MILLISECONDS) + .callTimeout(0, TimeUnit.MILLISECONDS) // unlimited .writeTimeout(NETWORK_TIMEOUT_MS, TimeUnit.MILLISECONDS) .connectTimeout(NETWORK_TIMEOUT_MS, TimeUnit.MILLISECONDS) .build() diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt index e35ccaab..42b130af 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt @@ -105,10 +105,8 @@ internal class OkHttpUploaderTest { } @Test - fun `M set client callTimeout W init()`() { - assertThat(testedUploader.client.callTimeoutMillis).isEqualTo( - OkHttpUploader.NETWORK_TIMEOUT_MS.toInt() - ) + fun `M set unlimited client callTimeout W init()`() { + assertThat(testedUploader.client.callTimeoutMillis).isEqualTo(0) } @Test From a411dd9c9dab06a4cc158e240ca2613fb645ab13 Mon Sep 17 00:00:00 2001 From: Xavier Gouchet Date: Wed, 20 Jul 2022 14:46:00 +0200 Subject: [PATCH 3/3] RUMM-2342 use the parameter name as filename for v2 --- .../gradle/plugin/internal/OkHttpUploader.kt | 37 +++++++++++++----- .../plugin/internal/OkHttpUploaderTest.kt | 38 +++++++++---------- 2 files changed, 46 insertions(+), 29 deletions(-) diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt index 85cc2b1b..677bb086 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploader.kt @@ -8,6 +8,9 @@ package com.datadog.gradle.plugin.internal import com.datadog.gradle.plugin.DdAndroidGradlePlugin.Companion.LOGGER import com.datadog.gradle.plugin.RepositoryInfo +import java.io.File +import java.net.HttpURLConnection +import java.util.concurrent.TimeUnit import okhttp3.MediaType.Companion.toMediaTypeOrNull import okhttp3.MultipartBody import okhttp3.OkHttpClient @@ -16,9 +19,6 @@ import okhttp3.RequestBody.Companion.asRequestBody import okhttp3.RequestBody.Companion.toRequestBody import okhttp3.Response import org.json.JSONObject -import java.io.File -import java.net.HttpURLConnection -import java.util.concurrent.TimeUnit internal class OkHttpUploader : Uploader { @@ -92,12 +92,24 @@ internal class OkHttpUploader : Uploader { val builder = MultipartBody.Builder() builder.setType(MultipartBody.FORM) - .addFormDataPart("event", "event", eventJson.toString(0).toRequestBody(MEDIA_TYPE_JSON)) - .addFormDataPart("jvm_mapping_file", mappingFile.name, mappingFileBody) + .addFormDataPart( + KEY_EVENT, + KEY_EVENT, + eventJson.toString(0).toRequestBody(MEDIA_TYPE_JSON) + ) + .addFormDataPart( + KEY_JVM_MAPPING_FILE, + KEY_JVM_MAPPING, + mappingFileBody + ) if (repositoryFile != null) { val repositoryFileBody = repositoryFile.asRequestBody(MEDIA_TYPE_JSON) - builder.addFormDataPart("repository", repositoryFile.name, repositoryFileBody) + builder.addFormDataPart( + KEY_REPOSITORY, + KEY_REPOSITORY, + repositoryFileBody + ) } if (repositoryInfo != null) { builder.addFormDataPart("git_repository_url", repositoryInfo.url) @@ -122,17 +134,17 @@ internal class OkHttpUploader : Uploader { ) statusCode == HttpURLConnection.HTTP_FORBIDDEN -> throw IllegalStateException( "Unable to upload mapping file for $identifier; " + - "verify that you're using a valid API Key" + "verify that you're using a valid API Key" ) statusCode == HttpURLConnection.HTTP_CLIENT_TIMEOUT -> throw RuntimeException( "Unable to upload mapping file for $identifier because of a request timeout; " + - "check your network connection" + "check your network connection" ) statusCode >= HttpURLConnection.HTTP_BAD_REQUEST -> { throw IllegalStateException( "Unable to upload mapping file for $identifier ($statusCode); " + - "it can be because the mapping file already exist for this version.\n" + - "${response.body?.string()}" + "it can be because the mapping file already exist for this version.\n" + + "${response.body?.string()}" ) } } @@ -150,6 +162,11 @@ internal class OkHttpUploader : Uploader { internal const val HEADER_EVP_ORIGIN_VERSION = "DD-EVP-ORIGIN-VERSION" internal const val HEADER_REQUEST_ID = "DD-REQUEST-ID" + internal const val KEY_EVENT = "event" + internal const val KEY_JVM_MAPPING_FILE = "jvm_mapping_file" + internal const val KEY_JVM_MAPPING = "jvm_mapping" + internal const val KEY_REPOSITORY = "repository" + internal val NETWORK_TIMEOUT_MS = TimeUnit.SECONDS.toMillis(45) internal val MEDIA_TYPE_TXT = "text/plain".toMediaTypeOrNull() internal val MEDIA_TYPE_JSON = "application/json".toMediaTypeOrNull() diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt index 42b130af..f755bd20 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/internal/OkHttpUploaderTest.kt @@ -150,20 +150,20 @@ internal class OkHttpUploaderTest { "event", "event", "{\"service\":\"${fakeIdentifier.serviceName}\"," + - "\"variant\":\"${fakeIdentifier.variant}\"," + - "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + - "\"version\":\"${fakeIdentifier.version}\"}", + "\"variant\":\"${fakeIdentifier.variant}\"," + + "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + + "\"version\":\"${fakeIdentifier.version}\"}", "application/json; charset=utf-8" ) .containsMultipartFile( "jvm_mapping_file", - fakeMappingFileName, + "jvm_mapping", fakeMappingFileContent, "text/plain" ) .containsMultipartFile( "repository", - fakeRepositoryFileName, + "repository", fakeRepositoryFileContent, "application/json" ) @@ -194,14 +194,14 @@ internal class OkHttpUploaderTest { "event", "event", "{\"service\":\"${fakeIdentifier.serviceName}\"," + - "\"variant\":\"${fakeIdentifier.variant}\"," + - "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + - "\"version\":\"${fakeIdentifier.version}\"}", + "\"variant\":\"${fakeIdentifier.variant}\"," + + "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + + "\"version\":\"${fakeIdentifier.version}\"}", "application/json; charset=utf-8" ) .containsMultipartFile( "jvm_mapping_file", - fakeMappingFileName, + "jvm_mapping", fakeMappingFileContent, "text/plain" ) @@ -239,20 +239,20 @@ internal class OkHttpUploaderTest { "event", "event", "{\"service\":\"${fakeIdentifier.serviceName}\"," + - "\"variant\":\"${fakeIdentifier.variant}\"," + - "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + - "\"version\":\"${fakeIdentifier.version}\"}", + "\"variant\":\"${fakeIdentifier.variant}\"," + + "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + + "\"version\":\"${fakeIdentifier.version}\"}", "application/json; charset=utf-8" ) .containsMultipartFile( "jvm_mapping_file", - fakeMappingFileName, + "jvm_mapping", fakeMappingFileContent, "text/plain" ) .containsMultipartFile( "repository", - fakeRepositoryFileName, + "repository", fakeRepositoryFileContent, "application/json" ) @@ -293,20 +293,20 @@ internal class OkHttpUploaderTest { "event", "event", "{\"service\":\"${fakeIdentifier.serviceName}\"," + - "\"variant\":\"${fakeIdentifier.variant}\"," + - "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + - "\"version\":\"${fakeIdentifier.version}\"}", + "\"variant\":\"${fakeIdentifier.variant}\"," + + "\"type\":\"${OkHttpUploader.TYPE_JVM_MAPPING_FILE}\"," + + "\"version\":\"${fakeIdentifier.version}\"}", "application/json; charset=utf-8" ) .containsMultipartFile( "jvm_mapping_file", - fakeMappingFileName, + "jvm_mapping", fakeMappingFileContent, "text/plain" ) .containsMultipartFile( "repository", - fakeRepositoryFileName, + "repository", fakeRepositoryFileContent, "application/json" )