Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add ability to configure http stack #52

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package dev.androidx.ci.config

import com.google.auth.Credentials
import okhttp3.logging.HttpLoggingInterceptor

/**
* Common configuration for TestRunner.
Expand All @@ -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,
Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
@@ -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<Interceptor> = 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<Interceptor> {
val log4jLogger = loggerOf(logClass.java)
val loggingInterceptor = HttpLoggingInterceptor(
logger = {
log4jLogger.info(it)
}
)
loggingInterceptor.level = logLevel
return listOf(
loggingInterceptor
)
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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].
*/
Expand All @@ -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
Expand All @@ -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
Expand Down
27 changes: 7 additions & 20 deletions AndroidXCI/lib/src/main/kotlin/dev/androidx/ci/util/RetrofitExt.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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)
}
}
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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()
Expand Down Expand Up @@ -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<Interceptor> {
createInterceptorCalls.incrementAndGet()
return super.interceptors()
}
}
val createdTestAdapters = mutableListOf<TestHttpAdapter>()
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()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ internal class TestRunnerServicePlayground {
bucketName = "androidx-ftl-test-results",
bucketPath = "github-ci-action",
gcsResultPath = "yigit-local",
logHttpCalls = false
) as TestRunnerServiceImpl
}

Expand Down