Fix wildcards in Retrofit services and increase read timeout (#40)

Fix issues found using the feature preview spec:
- Add `@JvmSuppressWildcards` to avoid runtime Retrofit errors. When
specs generate strongly typed code, such as
`kotlin.collections.List<ApiType>`, for Java interop this is
`java.util.List<? extends ApiType>` which is not allowed by Retrofit.
- Increase default read timeout and support overriding it, as some
endpoints would timeout with default 10s
- Also add full request logging if debugLoggingEnabled and fix
overriding `-Pversion`, which was always overwritten by the buildscipt

Author: gabrielfeo

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")
@@ -73,6 +72,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")
@@ -177,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")

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

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
     }
 
     /**

src/main/kotlin/com/gabrielfeo/gradle/enterprise/api/internal/OkHttpClient.kt

@@ -5,6 +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
 
@@ -15,22 +19,34 @@ 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())
     }
-    addInterceptor(HttpBearerAuth("bearer", options.gradleEnterpriseInstance.token()))
+}
+
+private fun OkHttpClient.Builder.addNetworkInterceptors(options: Options) {
     if (options.cache.cacheEnabled) {
         addNetworkInterceptor(buildCacheEnforcingInterceptor(options))
     }
-    build().apply {
-        options.httpClient.maxConcurrentRequests?.let {
-            dispatcher.maxRequests = it
-            dispatcher.maxRequestsPerHost = it
-        }
+    if (options.debugging.debugLoggingEnabled) {
+        addNetworkInterceptor(HttpLoggingInterceptor().apply { level = BODY })
     }
+    addNetworkInterceptor(HttpBearerAuth("bearer", options.gradleEnterpriseInstance.token()))
 }
 
 internal fun buildCache(

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<String, String?>,
         clientBuilder: OkHttpClient.Builder? = null,

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'")