From 0e9a0ebf967e5cd2a0c6e3665e11416269bba275 Mon Sep 17 00:00:00 2001 From: Nikita Ogorodnikov Date: Tue, 7 Feb 2023 15:20:10 +0100 Subject: [PATCH] RUMM-2952: Add Gzip support for mapping file upload --- .../gradle/plugin/DdMappingFileUploadTask.kt | 10 +- .../gradle/plugin/internal/OkHttpUploader.kt | 51 +++++++++- .../gradle/plugin/internal/Uploader.kt | 3 +- .../DdAndroidGradlePluginFunctionalTest.kt | 39 ++++++++ .../plugin/DdMappingFileUploadTaskTest.kt | 20 ++-- .../gradle/plugin/RecordedRequestAssert.kt | 15 ++- .../plugin/internal/OkHttpUploaderTest.kt | 95 ++++++++++++++++--- 7 files changed, 206 insertions(+), 27 deletions(-) diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt index 08bb05f4..306a08bf 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTask.kt @@ -12,6 +12,8 @@ import com.datadog.gradle.plugin.internal.DdAppIdentifier import com.datadog.gradle.plugin.internal.OkHttpUploader import com.datadog.gradle.plugin.internal.Uploader import org.gradle.api.DefaultTask +import org.gradle.api.provider.Provider +import org.gradle.api.provider.ProviderFactory import org.gradle.api.tasks.Input import org.gradle.api.tasks.InputFile import org.gradle.api.tasks.InputFiles @@ -30,6 +32,7 @@ import javax.inject.Inject */ open class DdMappingFileUploadTask @Inject constructor( + providerFactory: ProviderFactory, @get:Internal internal val repositoryDetector: RepositoryDetector ) : DefaultTask() { @@ -43,6 +46,9 @@ open class DdMappingFileUploadTask @get:Input var apiKey: String = "" + private val disableGzipOption: Provider = + providerFactory.gradleProperty(DISABLE_GZIP_GRADLE_PROPERTY) + /** * Source of the API key set: environment, gradle property, etc. */ @@ -166,7 +172,8 @@ open class DdMappingFileUploadTask version = versionName, variant = variantName ), - repositories.firstOrNull() + repositories.firstOrNull(), + !disableGzipOption.isPresent ) } @@ -364,5 +371,6 @@ open class DdMappingFileUploadTask private const val DATADOG_CI_API_KEY_PROPERTY = "apiKey" private const val DATADOG_CI_SITE_PROPERTY = "datadogSite" const val DATADOG_SITE = "DATADOG_SITE" + const val DISABLE_GZIP_GRADLE_PROPERTY = "dd-disable-gzip" } } 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 c1f36d00..378bc1a2 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 @@ -9,17 +9,23 @@ package com.datadog.gradle.plugin.internal import com.datadog.gradle.plugin.DatadogSite import com.datadog.gradle.plugin.DdAndroidGradlePlugin.Companion.LOGGER import com.datadog.gradle.plugin.RepositoryInfo +import okhttp3.MediaType 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.BufferedSink +import okio.GzipSink +import okio.buffer import org.json.JSONException import org.json.JSONObject import org.json.JSONTokener import java.io.File +import java.io.IOException import java.net.HttpURLConnection import java.util.concurrent.TimeUnit @@ -44,18 +50,29 @@ internal class OkHttpUploader : Uploader { repositoryFile: File?, apiKey: String, identifier: DdAppIdentifier, - repositoryInfo: RepositoryInfo? + repositoryInfo: RepositoryInfo?, + useGzip: Boolean ) { LOGGER.info("Uploading mapping file for $identifier (site=${site.domain}):\n") val body = createBody(site, identifier, mappingFile, repositoryFile, repositoryInfo) - val request = Request.Builder() + val requestBuilder = Request.Builder() .url(site.uploadEndpoint()) - .post(body) .header(HEADER_EVP_ORIGIN, "dd-sdk-android-gradle-plugin") .header(HEADER_EVP_ORIGIN_VERSION, VERSION) .header(HEADER_API_KEY, apiKey) - .build() + val request = if (useGzip) { + LOGGER.info("Creating request with GZIP encoding.") + requestBuilder + .post(body.gzip()) + .header(HEADER_CONTENT_ENCODING, ENCODING_GZIP) + .build() + } else { + LOGGER.info("Creating request without GZIP encoding.") + requestBuilder + .post(body) + .build() + } val call = client.newCall(request) val response = try { @@ -209,6 +226,30 @@ internal class OkHttpUploader : Uploader { // endregion + private fun RequestBody.gzip(): RequestBody { + val uncompressedBody = this + return object : RequestBody() { + override fun contentType(): MediaType? { + return uncompressedBody.contentType() + } + + override fun contentLength(): Long { + return -1 // We don't know the compressed length in advance! + } + + @Throws(IOException::class) + override fun writeTo(sink: BufferedSink) { + val gzipSink = GzipSink(sink).buffer() + uncompressedBody.writeTo(gzipSink) + gzipSink.close() + } + + override fun isOneShot(): Boolean { + return uncompressedBody.isOneShot() + } + } + } + companion object { // TODO add a plugin to automatically sync this with the `MavenConfig` value @@ -218,6 +259,8 @@ internal class OkHttpUploader : Uploader { internal const val HEADER_EVP_ORIGIN = "DD-EVP-ORIGIN" internal const val HEADER_EVP_ORIGIN_VERSION = "DD-EVP-ORIGIN-VERSION" internal const val HEADER_REQUEST_ID = "DD-REQUEST-ID" + internal const val HEADER_CONTENT_ENCODING = "Content-Encoding" + internal const val ENCODING_GZIP = "gzip" internal const val KEY_EVENT = "event" internal const val KEY_JVM_MAPPING_FILE = "jvm_mapping_file" diff --git a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/Uploader.kt b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/Uploader.kt index c2d8ac10..2c2aa23b 100644 --- a/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/Uploader.kt +++ b/dd-sdk-android-gradle-plugin/src/main/kotlin/com/datadog/gradle/plugin/internal/Uploader.kt @@ -19,6 +19,7 @@ internal interface Uploader { repositoryFile: File?, apiKey: String, identifier: DdAppIdentifier, - repositoryInfo: RepositoryInfo? + repositoryInfo: RepositoryInfo?, + useGzip: Boolean ) } diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt index 4b05cf0a..1608e667 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdAndroidGradlePluginFunctionalTest.kt @@ -476,6 +476,45 @@ internal class DdAndroidGradlePluginFunctionalTest { .buildAndFail() // Then + assertThat(result.output).contains("Creating request with GZIP encoding.") + assertThat(result.output).contains( + "Uploading mapping file for " + + "com.example.variants.$variantVersionName:1.0-$variantVersionName " + + "{variant:$variant} (site=datadoghq.com):" + ) + } + + @Test + fun `M try to upload the mapping file W upload { using a fake API_KEY, gzip disabled }`(forge: Forge) { + // Given + stubGradleBuildFromResourceFile( + "build_with_datadog_dep.gradle", + appBuildGradleFile + ) + val color = forge.anElementFrom(colors) + val version = forge.anElementFrom(versions) + val variantVersionName = version.lowercase() + val variant = "${version.lowercase()}$color" + val taskName = resolveUploadTask(variant) + + // When + // since there is no explicit dependency between assemble and upload tasks, Gradle may + // optimize the execution and run them in parallel, ignoring the order in the command + // line, so we do the explicit split + GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments(":samples:app:assembleRelease") + .withPluginClasspath(getTestConfigurationClasspath()) + .build() + + val result = GradleRunner.create() + .withProjectDir(testProjectDir) + .withArguments(taskName, "--info", "-PDD_API_KEY=fakekey", "-Pdd-disable-gzip") + .withPluginClasspath(getTestConfigurationClasspath()) + .buildAndFail() + + // Then + assertThat(result.output).contains("Creating request without GZIP encoding.") assertThat(result.output).contains( "Uploading mapping file for " + "com.example.variants.$variantVersionName:1.0-$variantVersionName " + diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt index 39a3c58b..714669e0 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/DdMappingFileUploadTaskTest.kt @@ -47,7 +47,7 @@ import java.io.File @ForgeConfiguration(Configurator::class) internal class DdMappingFileUploadTaskTest { - lateinit var testedTask: DdMappingFileUploadTask + private lateinit var testedTask: DdMappingFileUploadTask @TempDir lateinit var tempDir: File @@ -138,7 +138,8 @@ internal class DdMappingFileUploadTaskTest { version = fakeVersion, variant = fakeVariant ), - fakeRepoInfo + fakeRepoInfo, + useGzip = true ) assertThat(fakeRepositoryFile.readText()) .isEqualTo( @@ -184,7 +185,8 @@ internal class DdMappingFileUploadTaskTest { variant = fakeVariant ) ), - eq(fakeRepoInfo) + eq(fakeRepoInfo), + useGzip = eq(true) ) assertThat(lastValue).hasSameTextualContentAs( fileFromResourcesPath("mapping-with-aliases.txt") @@ -232,7 +234,8 @@ internal class DdMappingFileUploadTaskTest { variant = fakeVariant ) ), - eq(fakeRepoInfo) + eq(fakeRepoInfo), + useGzip = eq(true) ) assertThat(lastValue.readLines()).isEqualTo(expectedLines) } @@ -264,7 +267,8 @@ internal class DdMappingFileUploadTaskTest { version = fakeVersion, variant = fakeVariant ), - fakeRepoInfo + fakeRepoInfo, + useGzip = true ) assertThat(fakeRepositoryFile.readText()) .isEqualTo( @@ -297,7 +301,8 @@ internal class DdMappingFileUploadTaskTest { version = fakeVersion, variant = fakeVariant ), - null + null, + useGzip = true ) } @@ -391,7 +396,8 @@ internal class DdMappingFileUploadTaskTest { version = fakeVersion, variant = fakeVariant ), - fakeRepoInfo + fakeRepoInfo, + useGzip = true ) assertThat(fakeRepositoryFile.readText()) .isEqualTo( diff --git a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/RecordedRequestAssert.kt b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/RecordedRequestAssert.kt index 50149e9d..be23dcbe 100644 --- a/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/RecordedRequestAssert.kt +++ b/dd-sdk-android-gradle-plugin/src/test/kotlin/com/datadog/gradle/plugin/RecordedRequestAssert.kt @@ -7,6 +7,8 @@ package com.datadog.gradle.plugin import okhttp3.mockwebserver.RecordedRequest +import okio.GzipSource +import okio.buffer import org.assertj.core.api.AbstractObjectAssert import org.assertj.core.api.Assertions.assertThat @@ -16,7 +18,12 @@ internal class RecordedRequestAssert(actual: RecordedRequest?) : RecordedRequestAssert::class.java ) { - val bodyContentUtf8 = actual?.body?.readUtf8() + private val bodyContentUtf8 = if (actual?.getHeader("Content-Encoding") == "gzip") { + val gzipSource = GzipSource(actual.body) + gzipSource.buffer().readUtf8() + } else { + actual?.body?.readUtf8() + } fun containsFormData(name: String, value: String): RecordedRequestAssert { isNotNull() @@ -68,6 +75,12 @@ internal class RecordedRequestAssert(actual: RecordedRequest?) : return this } + fun doesNotHaveHeader(header: String): RecordedRequestAssert { + isNotNull + assertThat(actual.headers.names()).doesNotContain(header) + return this + } + companion object { fun assertThat(actual: RecordedRequest?): RecordedRequestAssert { return RecordedRequestAssert(actual) 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 75edd1e0..066a07f7 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 @@ -156,13 +156,63 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) // Then assertThat(mockWebServer.requestCount).isEqualTo(1) assertThat(dispatchedUploadRequest) .hasMethod("POST") + .hasHeaderValue("Content-Encoding", "gzip") + .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", + "jvm_mapping", + fakeMappingFileContent, + "text/plain" + ) + .containsMultipartFile( + "repository", + "repository", + fakeRepositoryFileContent, + "application/json" + ) + } + + @Test + fun `𝕄 upload proper request 𝕎 upload() { without gzip }`() { + // Given + mockUploadResponse = MockResponse() + .setResponseCode(HttpURLConnection.HTTP_OK) + .setBody("{}") + + // When + testedUploader.upload( + mockSite, + fakeMappingFile, + fakeRepositoryFile, + fakeApiKey, + fakeIdentifier, + fakeRepositoryInfo, + useGzip = false + ) + + // Then + assertThat(mockWebServer.requestCount).isEqualTo(1) + assertThat(dispatchedUploadRequest) + .hasMethod("POST") + .doesNotHaveHeader("Content-Encoding") .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) .containsMultipartFile( @@ -202,13 +252,15 @@ internal class OkHttpUploaderTest { null, fakeApiKey, fakeIdentifier, - null + null, + useGzip = true ) // Then assertThat(mockWebServer.requestCount).isEqualTo(1) assertThat(dispatchedUploadRequest) .hasMethod("POST") + .hasHeaderValue("Content-Encoding", "gzip") .containsMultipartFile( "event", "event", @@ -244,7 +296,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) } @@ -252,6 +305,7 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(1) assertThat(dispatchedUploadRequest) .hasMethod("POST") + .hasHeaderValue("Content-Encoding", "gzip") .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) .containsMultipartFile( @@ -298,7 +352,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) } @@ -306,6 +361,7 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(1) assertThat(dispatchedUploadRequest) .hasMethod("POST") + .hasHeaderValue("Content-Encoding", "gzip") .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) .containsMultipartFile( @@ -349,7 +405,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) } @@ -357,6 +414,7 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(2) assertThat(dispatchedUploadRequest) .hasMethod("POST") + .hasHeaderValue("Content-Encoding", "gzip") .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) .containsMultipartFile( @@ -416,7 +474,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) } @@ -424,6 +483,7 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(2) assertThat(dispatchedUploadRequest) .hasMethod("POST") + .hasHeaderValue("Content-Encoding", "gzip") .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) .containsMultipartFile( @@ -471,7 +531,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) } @@ -479,6 +540,7 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(2) assertThat(dispatchedUploadRequest) .hasMethod("POST") + .hasHeaderValue("Content-Encoding", "gzip") .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) .containsMultipartFile( @@ -525,7 +587,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) } @@ -533,6 +596,7 @@ internal class OkHttpUploaderTest { assertThat(mockWebServer.requestCount).isEqualTo(2) assertThat(dispatchedUploadRequest) .hasMethod("POST") + .hasHeaderValue("Content-Encoding", "gzip") .containsFormData("git_repository_url", fakeRepositoryInfo.url) .containsFormData("git_commit_sha", fakeRepositoryInfo.hash) .containsMultipartFile( @@ -580,7 +644,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) } assertThat(exception.message).isEqualTo( @@ -614,7 +679,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) } assertThat(exception.message).isEqualTo( @@ -648,7 +714,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) // Then @@ -678,7 +745,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) // Then @@ -704,7 +772,8 @@ internal class OkHttpUploaderTest { fakeRepositoryFile, fakeApiKey, fakeIdentifier, - fakeRepositoryInfo + fakeRepositoryInfo, + useGzip = true ) // Then