From 5812ebed7ab10c76356d5cd4908cea5a3aa9bbdf Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 20 Apr 2023 20:34:11 +0100 Subject: [PATCH 1/4] SuppressJvmWilcards in Retrofit interfaces --- build.gradle.kts | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/build.gradle.kts b/build.gradle.kts index 5dad7be4..48ec8ed3 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -73,6 +73,24 @@ tasks.openApiGenerate.configure { ) } } + // Add @JvmSuppressWildcards to avoid square/retrofit#3275 + doLast { + val apiFile = File( + outputDir.get(), + "src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/GradleEnterpriseApi.kt", + ) + ant.withGroovyBuilder { + "replaceregexp"( + "file" to apiFile, + "match" to "interface GradleEnterpriseApi", + "replace" to """ + @JvmSuppressWildcards + interface GradleEnterpriseApi + """.trimIndent(), + "flags" to "m", + ) + } + } // Workaround for properties generated with `arrayListOf(null,null)` as default value doLast { val srcDir = File(outputDir.get(), "src/main/kotlin") From a3010095934dcd11dc765a39f903ce0af0d20c5f Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 20 Apr 2023 20:52:51 +0100 Subject: [PATCH 2/4] Fix overriding version property with -P --- build.gradle.kts | 1 - gradle.properties | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/build.gradle.kts b/build.gradle.kts index 48ec8ed3..75c3d8ae 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -11,7 +11,6 @@ plugins { } group = "com.github.gabrielfeo" -version = "SNAPSHOT" val repoUrl = "https://github.com/gabrielfeo/gradle-enterprise-api-kotlin" val localSpecPath = providers.gradleProperty("localSpecPath") diff --git a/gradle.properties b/gradle.properties index 5f4ea183..9f186e02 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,4 +1,5 @@ org.gradle.jvmargs=-Xmx5g org.gradle.caching=true +version=SNAPSHOT # Must be later than 2022.1 gradle.enterprise.version=2022.4 From 3289b7c61d92c4e86fcd90141a377dcc61900879 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 20 Apr 2023 21:20:53 +0100 Subject: [PATCH 3/4] Add full request logging --- build.gradle.kts | 1 + .../gradle/enterprise/api/internal/OkHttpClient.kt | 7 ++++++- .../gabrielfeo/gradle/enterprise/api/OkHttpClientTest.kt | 9 ++++++++- .../com/gabrielfeo/gradle/enterprise/api/OptionsTest.kt | 9 +++++++++ 4 files changed, 24 insertions(+), 2 deletions(-) diff --git a/build.gradle.kts b/build.gradle.kts index 75c3d8ae..54f845ed 100644 --- a/build.gradle.kts +++ b/build.gradle.kts @@ -194,6 +194,7 @@ dependencies { api("com.squareup.moshi:moshi:1.14.0") implementation("com.squareup.moshi:moshi-kotlin:1.14.0") api("com.squareup.okhttp3:okhttp:4.10.0") + implementation("com.squareup.okhttp3:logging-interceptor:4.10.0") api("com.squareup.retrofit2:retrofit:2.9.0") implementation("com.squareup.retrofit2:converter-moshi:2.9.0") implementation("com.squareup.retrofit2:converter-scalars:2.9.0") diff --git a/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt b/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt index af867ea6..084d56d4 100644 --- a/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt +++ b/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt @@ -5,6 +5,8 @@ import com.gabrielfeo.gradle.enterprise.api.internal.auth.HttpBearerAuth import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheEnforcingInterceptor import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheHitLoggingInterceptor import okhttp3.Cache +import okhttp3.logging.HttpLoggingInterceptor +import okhttp3.logging.HttpLoggingInterceptor.Level.BODY import java.util.logging.Level import java.util.logging.Logger @@ -21,10 +23,13 @@ internal fun buildOkHttpClient( if (options.debugging.debugLoggingEnabled && options.cache.cacheEnabled) { addInterceptor(CacheHitLoggingInterceptor()) } - addInterceptor(HttpBearerAuth("bearer", options.gradleEnterpriseInstance.token())) if (options.cache.cacheEnabled) { addNetworkInterceptor(buildCacheEnforcingInterceptor(options)) } + if (options.debugging.debugLoggingEnabled) { + addNetworkInterceptor(HttpLoggingInterceptor().apply { level = BODY }) + } + addNetworkInterceptor(HttpBearerAuth("bearer", options.gradleEnterpriseInstance.token())) build().apply { options.httpClient.maxConcurrentRequests?.let { dispatcher.maxRequests = it diff --git a/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OkHttpClientTest.kt b/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OkHttpClientTest.kt index 34d5df8f..c69b86d6 100644 --- a/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OkHttpClientTest.kt +++ b/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OkHttpClientTest.kt @@ -15,7 +15,7 @@ class OkHttpClientTest { @Test fun `Adds authentication`() { val client = buildClient() - assertTrue(client.interceptors.any { it is HttpBearerAuth }) + assertTrue(client.networkInterceptors.any { it is HttpBearerAuth }) } @Test @@ -74,6 +74,13 @@ class OkHttpClientTest { assertNull(client.cache) } + @Test + fun `Increases read timeout`() { + val client = buildClient() + val defaultTimeout = OkHttpClient.Builder().build().readTimeoutMillis + assertTrue(client.readTimeoutMillis > defaultTimeout) + } + private fun buildClient( vararg envVars: Pair, clientBuilder: OkHttpClient.Builder? = null, diff --git a/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OptionsTest.kt b/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OptionsTest.kt index 57621326..dd2a1c99 100644 --- a/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OptionsTest.kt +++ b/src/test/kotlin/com/gabrielfeo/gradle/enterprise/api/OptionsTest.kt @@ -95,6 +95,15 @@ class OptionsTest { ) } + @Test + fun `Given timeout set in env, readTimeoutMillis returns env value`() { + val options = Options( + FakeEnv("GRADLE_ENTERPRISE_API_READ_TIMEOUT_MILLIS" to "100000"), + FakeKeychain(), + ) + assertEquals(100_000L, options.httpClient.readTimeoutMillis) + } + private fun Regex.assertMatches(vararg values: String) { values.forEach { assertTrue(matches(it), "/$pattern/ doesn't match '$it'") From d9be3b97cfa7e7f7a39199439c0acd14fa9b3705 Mon Sep 17 00:00:00 2001 From: Gabriel Feo Date: Thu, 20 Apr 2023 21:29:28 +0100 Subject: [PATCH 4/4] Increase default read timeout --- .../gradle/enterprise/api/Options.kt | 9 ++++++++ .../enterprise/api/internal/OkHttpClient.kt | 23 ++++++++++++++----- 2 files changed, 26 insertions(+), 6 deletions(-) diff --git a/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/Options.kt b/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/Options.kt index d0ea2e02..33cba6b5 100644 --- a/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/Options.kt +++ b/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/Options.kt @@ -91,6 +91,15 @@ class Options internal constructor( */ var maxConcurrentRequests = env["GRADLE_ENTERPRISE_API_MAX_CONCURRENT_REQUESTS"]?.toInt() + + /** + * Timeout for reading an API response, used for [OkHttpClient.readTimeoutMillis]. + * By default, uses environment variable `GRADLE_ENTERPRISE_API_READ_TIMEOUT_MILLIS` + * or 60_000. Keep in mind that GE API responses can be big and slow to send depending on + * the endpoint. + */ + var readTimeoutMillis: Long = env["GRADLE_ENTERPRISE_API_READ_TIMEOUT_MILLIS"]?.toLong() + ?: 60_000L } /** diff --git a/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt b/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt index 084d56d4..936ef50c 100644 --- a/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt +++ b/src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt @@ -5,8 +5,10 @@ import com.gabrielfeo.gradle.enterprise.api.internal.auth.HttpBearerAuth import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheEnforcingInterceptor import com.gabrielfeo.gradle.enterprise.api.internal.caching.CacheHitLoggingInterceptor import okhttp3.Cache +import okhttp3.OkHttpClient import okhttp3.logging.HttpLoggingInterceptor import okhttp3.logging.HttpLoggingInterceptor.Level.BODY +import java.time.Duration import java.util.logging.Level import java.util.logging.Logger @@ -17,12 +19,27 @@ internal val okHttpClient by lazy { internal fun buildOkHttpClient( options: Options, ) = with(options.httpClient.clientBuilder()) { + readTimeout(Duration.ofMillis(options.httpClient.readTimeoutMillis)) if (options.cache.cacheEnabled) { cache(buildCache(options)) } + addInterceptors(options) + addNetworkInterceptors(options) + build().apply { + options.httpClient.maxConcurrentRequests?.let { + dispatcher.maxRequests = it + dispatcher.maxRequestsPerHost = it + } + } +} + +private fun OkHttpClient.Builder.addInterceptors(options: Options) { if (options.debugging.debugLoggingEnabled && options.cache.cacheEnabled) { addInterceptor(CacheHitLoggingInterceptor()) } +} + +private fun OkHttpClient.Builder.addNetworkInterceptors(options: Options) { if (options.cache.cacheEnabled) { addNetworkInterceptor(buildCacheEnforcingInterceptor(options)) } @@ -30,12 +47,6 @@ internal fun buildOkHttpClient( addNetworkInterceptor(HttpLoggingInterceptor().apply { level = BODY }) } addNetworkInterceptor(HttpBearerAuth("bearer", options.gradleEnterpriseInstance.token())) - build().apply { - options.httpClient.maxConcurrentRequests?.let { - dispatcher.maxRequests = it - dispatcher.maxRequestsPerHost = it - } - } } internal fun buildCache(