diff --git a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/config/Config.kt b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/config/Config.kt index 0744345..68cc0c8 100644 --- a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/config/Config.kt +++ b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/config/Config.kt @@ -17,7 +17,6 @@ package dev.androidx.ci.config import com.google.auth.Credentials -import okhttp3.logging.HttpLoggingInterceptor /** * Common configuration for TestRunner. @@ -43,7 +42,7 @@ internal class Config { class FirebaseTestLab( val gcp: Gcp, val endPoint: String = "https://testing.googleapis.com/v1/", - val httpLogLevel: HttpLoggingInterceptor.Level = HttpLoggingInterceptor.Level.NONE, + val httpConfigAdapterFactory: HttpConfigAdapter.Factory = HttpConfigAdapter.Factory.NoOp ) class Datastore( val gcp: Gcp, @@ -58,7 +57,7 @@ internal class Config { class ToolsResult( val gcp: Gcp, val endPoint: String = "https://toolresults.googleapis.com/toolresults/v1beta3/", - val httpLogLevel: HttpLoggingInterceptor.Level = HttpLoggingInterceptor.Level.NONE, + val httpConfigAdapterFactory: HttpConfigAdapter.Factory = HttpConfigAdapter.Factory.NoOp ) class Gcp( diff --git a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/config/HttpConfigAdapter.kt b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/config/HttpConfigAdapter.kt new file mode 100644 index 0000000..4f52b6d --- /dev/null +++ b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/config/HttpConfigAdapter.kt @@ -0,0 +1,62 @@ +package dev.androidx.ci.config + +import okhttp3.Interceptor +import okhttp3.logging.HttpLoggingInterceptor +import org.apache.logging.log4j.kotlin.loggerOf +import kotlin.reflect.KClass + +/** + * Configuration for OkHttp & Retrofit. + */ +interface HttpConfigAdapter { + fun createCallFactory( + delegate: okhttp3.Call.Factory + ): okhttp3.Call.Factory = delegate + + fun interceptors(): List = emptyList() + + fun interface Factory { + /** + * Creates an [HttpConfigAdapter] for the given [klass]. + * + * @param klass The class which is creating the Retrofit / OkHttp clients. + * @return A HttpConfigAdapter that will be used to configure the network stack. + */ + fun create(klass: KClass<*>): HttpConfigAdapter + + class Logging( + private val logLevel: HttpLoggingInterceptor.Level + ) : Factory { + override fun create(klass: KClass<*>): HttpConfigAdapter { + return LoggingConfig( + logClass = klass, + logLevel = logLevel + ) + } + } + + object NoOp : Factory { + override fun create(klass: KClass<*>): HttpConfigAdapter { + return object : HttpConfigAdapter {} + } + } + } + + class LoggingConfig( + private val logClass: KClass<*>, + private val logLevel: HttpLoggingInterceptor.Level + ) : HttpConfigAdapter { + override fun interceptors(): List { + val log4jLogger = loggerOf(logClass.java) + val loggingInterceptor = HttpLoggingInterceptor( + logger = { + log4jLogger.info(it) + } + ) + loggingInterceptor.level = logLevel + return listOf( + loggingInterceptor + ) + } + } +} diff --git a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/firebase/FirebaseTestLabApi.kt b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/firebase/FirebaseTestLabApi.kt index b2edc4e..ef3aab5 100644 --- a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/firebase/FirebaseTestLabApi.kt +++ b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/firebase/FirebaseTestLabApi.kt @@ -25,7 +25,7 @@ import dev.androidx.ci.generated.ftl.TestEnvironmentCatalog import dev.androidx.ci.generated.ftl.TestMatrix import dev.androidx.ci.util.Retry import dev.androidx.ci.util.RetryCallAdapterFactory -import dev.androidx.ci.util.withLog4J +import dev.androidx.ci.util.withConfig import dev.zacsweers.moshix.reflect.MetadataKotlinJsonAdapterFactory import okhttp3.OkHttpClient import retrofit2.Retrofit @@ -68,7 +68,10 @@ internal interface FirebaseTestLabApi { fun build( config: Config.FirebaseTestLab ): FirebaseTestLabApi { - val client = OkHttpClient.Builder().authenticateWith( + val httpAdapter = config.httpConfigAdapterFactory.create(FirebaseTestLabApi::class) + val client = OkHttpClient.Builder().withConfig( + httpAdapter + ).authenticateWith( config.gcp ).addInterceptor { val newBuilder = it.request().newBuilder() @@ -81,10 +84,7 @@ internal interface FirebaseTestLabApi { it.proceed( newBuilder.build() ) - }.withLog4J( - level = config.httpLogLevel, - klass = FirebaseTestLabApi::class - ).build() + }.build() val moshi = Moshi.Builder() .add(MetadataKotlinJsonAdapterFactory()) .build() @@ -93,6 +93,7 @@ internal interface FirebaseTestLabApi { .baseUrl(config.endPoint) .addConverterFactory(MoshiConverterFactory.create(moshi).asLenient()) .addCallAdapterFactory(RetryCallAdapterFactory.GLOBAL) + .callFactory(httpAdapter.createCallFactory(client)) .build() .create(FirebaseTestLabApi::class.java) } diff --git a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/firebase/ToolsResultApi.kt b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/firebase/ToolsResultApi.kt index 058a1cd..d1a9b9f 100644 --- a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/firebase/ToolsResultApi.kt +++ b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/firebase/ToolsResultApi.kt @@ -23,7 +23,7 @@ import dev.androidx.ci.generated.testResults.ListHistoriesResponse import dev.androidx.ci.generated.testResults.ListStepsResponse import dev.androidx.ci.util.Retry import dev.androidx.ci.util.RetryCallAdapterFactory -import dev.androidx.ci.util.withLog4J +import dev.androidx.ci.util.withConfig import dev.zacsweers.moshix.reflect.MetadataKotlinJsonAdapterFactory import okhttp3.OkHttpClient import retrofit2.Retrofit @@ -65,7 +65,10 @@ internal interface ToolsResultApi { fun build( config: Config.ToolsResult ): ToolsResultApi { - val client = OkHttpClient.Builder().authenticateWith( + val httpAdapter = config.httpConfigAdapterFactory.create(ToolsResultApi::class) + val client = OkHttpClient.Builder().withConfig( + httpAdapter + ).authenticateWith( config.gcp ).addInterceptor { val newBuilder = it.request().newBuilder() @@ -78,10 +81,7 @@ internal interface ToolsResultApi { it.proceed( newBuilder.build() ) - }.withLog4J( - level = config.httpLogLevel, - klass = ToolsResultApi::class - ).build() + }.build() val moshi = Moshi.Builder() .add(MetadataKotlinJsonAdapterFactory()) .build() @@ -90,6 +90,7 @@ internal interface ToolsResultApi { .baseUrl(config.endPoint) .addConverterFactory(MoshiConverterFactory.create(moshi).asLenient()) .addCallAdapterFactory(RetryCallAdapterFactory.GLOBAL) + .callFactory(httpAdapter.createCallFactory(client)) .build() .create(ToolsResultApi::class.java) } diff --git a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestRunnerService.kt b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestRunnerService.kt index 9de3f6f..4e32b6e 100644 --- a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestRunnerService.kt +++ b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/testRunner/TestRunnerService.kt @@ -3,6 +3,7 @@ package dev.androidx.ci.testRunner import com.google.auth.Credentials import dev.androidx.ci.config.Config import dev.androidx.ci.config.Config.Datastore.Companion.AOSP_OBJECT_KIND +import dev.androidx.ci.config.HttpConfigAdapter import dev.androidx.ci.datastore.DatastoreApi import dev.androidx.ci.firebase.FirebaseTestLabApi import dev.androidx.ci.firebase.ToolsResultApi @@ -18,7 +19,6 @@ import dev.androidx.ci.testRunner.vo.RemoteApk import dev.androidx.ci.testRunner.vo.UploadedApk import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.Dispatchers -import okhttp3.logging.HttpLoggingInterceptor import java.io.InputStream interface TestRunnerService { @@ -119,10 +119,9 @@ interface TestRunnerService { */ gcsResultPath: String, /** - * If enabled, HTTP requests will also be logged. Keep in mind, they might include - * sensitive data. + * If provided, the given factory will be used to configure Retrofit & OkHttp. */ - logHttpCalls: Boolean = false, + httpConfigAdapterFactory: HttpConfigAdapter.Factory = HttpConfigAdapter.Factory.NoOp, /** * The coroutine dispatcher to use for IO operations. Defaults to [Dispatchers.IO]. */ @@ -134,11 +133,6 @@ interface TestRunnerService { */ testRunDataStoreObjectKind: String = AOSP_OBJECT_KIND ): TestRunnerService { - val httpLogLevel = if (logHttpCalls) { - HttpLoggingInterceptor.Level.BODY - } else { - HttpLoggingInterceptor.Level.NONE - } val gcpConfig = Config.Gcp( credentials = credentials, projectId = firebaseProjectId @@ -162,14 +156,14 @@ interface TestRunnerService { toolsResultApi = ToolsResultApi.build( config = Config.ToolsResult( gcp = gcpConfig, - httpLogLevel = httpLogLevel, + httpConfigAdapterFactory = httpConfigAdapterFactory ) ), firebaseProjectId = firebaseProjectId, firebaseTestLabApi = FirebaseTestLabApi.build( config = Config.FirebaseTestLab( gcp = gcpConfig, - httpLogLevel = httpLogLevel, + httpConfigAdapterFactory = httpConfigAdapterFactory ) ), gcsResultPath = gcsResultPath diff --git a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/util/RetrofitExt.kt b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/util/RetrofitExt.kt index 06d39cf..9e4fb3e 100644 --- a/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/util/RetrofitExt.kt +++ b/AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/util/RetrofitExt.kt @@ -16,15 +16,13 @@ package dev.androidx.ci.util +import dev.androidx.ci.config.HttpConfigAdapter import kotlinx.coroutines.GlobalScope import kotlinx.coroutines.delay import kotlinx.coroutines.launch import okhttp3.OkHttpClient import okhttp3.Request -import okhttp3.logging.HttpLoggingInterceptor import okio.Timeout -import org.apache.logging.log4j.kotlin.logger -import org.apache.logging.log4j.kotlin.loggerOf import retrofit2.Call import retrofit2.CallAdapter import retrofit2.Callback @@ -33,7 +31,6 @@ import retrofit2.Retrofit import java.lang.reflect.Type import java.util.concurrent.TimeUnit import java.util.concurrent.atomic.AtomicBoolean -import kotlin.reflect.KClass @Target(AnnotationTarget.FUNCTION) internal annotation class Retry(val times: Int = 3) @@ -183,22 +180,12 @@ internal class RetryCallAdapterFactory( } /** - * Add a log4j logger for the given level to all requests. + * Configures the OkHttp builder with the given [config]. */ -internal fun OkHttpClient.Builder.withLog4J( - level: HttpLoggingInterceptor.Level, - klass: KClass<*> -): OkHttpClient.Builder { - return if (level == HttpLoggingInterceptor.Level.NONE) { - this - } else { - val log4jLogger = loggerOf(klass.java) - val loggingInterceptor = HttpLoggingInterceptor( - logger = { - log4jLogger.info(it) - } - ) - loggingInterceptor.level = level - this.addInterceptor(loggingInterceptor) +internal fun OkHttpClient.Builder.withConfig( + config: HttpConfigAdapter +): OkHttpClient.Builder = apply { + config.interceptors().forEach { + addInterceptor(it) } } diff --git a/AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServiceImplTest.kt b/AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServiceImplTest.kt index ae357a0..8677e2c 100644 --- a/AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServiceImplTest.kt +++ b/AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServiceImplTest.kt @@ -1,7 +1,11 @@ package dev.androidx.ci.testRunner +import com.google.cloud.NoCredentials import com.google.common.truth.Truth.assertThat +import dev.androidx.ci.config.HttpConfigAdapter import dev.androidx.ci.fake.FakeBackend +import dev.androidx.ci.firebase.FirebaseTestLabApi +import dev.androidx.ci.firebase.ToolsResultApi import dev.androidx.ci.generated.ftl.ClientInfo import dev.androidx.ci.generated.ftl.ClientInfoDetail import dev.androidx.ci.generated.ftl.EnvironmentMatrix @@ -23,8 +27,12 @@ import dev.androidx.ci.generated.testResults.ToolOutputReference import dev.androidx.ci.testRunner.vo.DeviceSetup import dev.androidx.ci.util.sha256 import kotlinx.coroutines.runBlocking +import okhttp3.Call +import okhttp3.Interceptor import org.junit.Test import java.util.UUID +import java.util.concurrent.atomic.AtomicInteger +import kotlin.reflect.KClass class TestRunnerServiceImplTest { private val fakeBackend = FakeBackend() @@ -808,6 +816,52 @@ class TestRunnerServiceImplTest { ) } + @Test + fun createWithHttpAdapter() { + class TestHttpAdapter( + val klass: KClass<*> + ) : HttpConfigAdapter { + var createCallFactoryCalls = AtomicInteger(0) + var createInterceptorCalls = AtomicInteger(0) + override fun createCallFactory(delegate: Call.Factory): Call.Factory { + createCallFactoryCalls.incrementAndGet() + return super.createCallFactory(delegate) + } + + override fun interceptors(): List { + createInterceptorCalls.incrementAndGet() + return super.interceptors() + } + } + val createdTestAdapters = mutableListOf() + val adapterFactory = HttpConfigAdapter.Factory { + TestHttpAdapter(it).also { + createdTestAdapters.add(it) + } + } + TestRunnerService.create( + credentials = NoCredentials.getInstance(), + firebaseProjectId = fakeBackend.firebaseProjectId, + bucketName = "some-bucket", + bucketPath = "some-path", + gcsResultPath = "a-result-path", + httpConfigAdapterFactory = adapterFactory + ) + assertThat( + createdTestAdapters + ).hasSize(2) + createdTestAdapters.forEach { + assertThat(it.createCallFactoryCalls.get()).isEqualTo(1) + assertThat(it.createInterceptorCalls.get()).isEqualTo(1) + } + assertThat( + createdTestAdapters.map { it.klass } + ).containsExactly( + ToolsResultApi::class, + FirebaseTestLabApi::class + ) + } + private fun TestRunnerService.ResultFileResource.readFully() = openInputStream().use { it.readAllBytes() } diff --git a/AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServicePlayground.kt b/AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServicePlayground.kt index 570dde2..eee19f4 100644 --- a/AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServicePlayground.kt +++ b/AndroidXCI/lib/src/test/kotlin/dev/androidx/ci/testRunner/TestRunnerServicePlayground.kt @@ -17,7 +17,6 @@ internal class TestRunnerServicePlayground { bucketName = "androidx-ftl-test-results", bucketPath = "github-ci-action", gcsResultPath = "yigit-local", - logHttpCalls = false ) as TestRunnerServiceImpl }