Review changes

engine/src/main/java/com/google/android/fhir/FhirEngineProvider.kt

@@ -133,7 +133,7 @@ data class ServerConfiguration(
  val baseUrl: String,
  val networkConfiguration: NetworkConfiguration = NetworkConfiguration(),
  val authenticator: Authenticator? = null,
- val httpLogger: HttpLogger = HttpLogger.DEFAULT
+ val httpLogger: HttpLogger = HttpLogger.NONE
 )
 
 /** A configuration to provide the network connection parameters. */

engine/src/main/java/com/google/android/fhir/FhirServices.kt

@@ -51,11 +51,11 @@ internal data class FhirServices(
  enableEncryption = true
  }
 
- internal fun setDatabaseErrorStrategy(databaseErrorStrategy: DatabaseErrorStrategy) {
+ internal fun setDatabaseErrorStrategy(databaseErrorStrategy: DatabaseErrorStrategy) = apply {
  this.databaseErrorStrategy = databaseErrorStrategy
  }
 
- internal fun setServerConfiguration(serverConfiguration: ServerConfiguration) {
+ internal fun setServerConfiguration(serverConfiguration: ServerConfiguration) = apply {
  this.serverConfiguration = serverConfiguration
  }
 
@@ -71,10 +71,8 @@ internal data class FhirServices(
  val remoteDataSource =
  serverConfiguration?.let {
  RemoteFhirService.builder(it.baseUrl, it.networkConfiguration)
- .apply {
- setAuthenticator(it.authenticator)
- setHttpLogger(it.httpLogger)
- }
+ .setAuthenticator(it.authenticator)
+ .setHttpLogger(it.httpLogger)
  .build()
  }
  return FhirServices(

engine/src/main/java/com/google/android/fhir/sync/remote/FhirHttpLogger.kt

@@ -17,7 +17,6 @@
 package com.google.android.fhir.sync.remote
 
 import androidx.annotation.WorkerThread
-import timber.log.Timber
 
 /** Logger for the network communication between the engine and the remote server */
 class HttpLogger(val configuration: Configuration, @WorkerThread val log: (String) -> Unit) {
@@ -43,8 +42,5 @@ class HttpLogger(val configuration: Configuration, @WorkerThread val log: (Strin
  companion object {
  /** The logger will not log any data. */
  val NONE = HttpLogger(Configuration(Level.NONE)) {}
-
- /** The logger will only log [Level.BASIC] data onto the system logger. */
- val DEFAULT = HttpLogger(Configuration(Level.BASIC)) { Timber.tag("FHIR-Http-Logger").d(it) }
  }
 }

engine/src/main/java/com/google/android/fhir/sync/remote/MoreHttpLogger.kt

@@ -0,0 +1,33 @@
+/*
+ * Copyright 2022 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http:https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.android.fhir.sync.remote
+
+import okhttp3.logging.HttpLoggingInterceptor
+
+internal fun HttpLogger.toOkHttpLoggingInterceptor() =
+ HttpLoggingInterceptor(log).apply {
+ level = configuration.level.toOkhttpLogLevel()
+ configuration.headersToIgnore?.forEach { this.redactHeader(it) }
+ }
+
+private fun HttpLogger.Level.toOkhttpLogLevel() =
+ when (this) {
+ HttpLogger.Level.NONE -> HttpLoggingInterceptor.Level.NONE
+ HttpLogger.Level.BASIC -> HttpLoggingInterceptor.Level.BASIC
+ HttpLogger.Level.HEADERS -> HttpLoggingInterceptor.Level.HEADERS
+ HttpLogger.Level.BODY -> HttpLoggingInterceptor.Level.BODY
+ }

engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt

@@ -46,33 +46,21 @@ internal interface RemoteFhirService : DataSource {
  private var authenticator: Authenticator? = null
  private var httpLoggingInterceptor: HttpLoggingInterceptor? = null
 
- fun setAuthenticator(authenticator: Authenticator?) {
+ fun setAuthenticator(authenticator: Authenticator?) = apply {
  this.authenticator = authenticator
  }
 
- fun setHttpLogger(httpLogger: HttpLogger) {
- httpLoggingInterceptor =
- HttpLoggingInterceptor { httpLogger.log(it) }.apply {
- level = httpLogger.configuration.level.toOkhttpLogLevel()
- httpLogger.configuration.headersToIgnore?.forEach { this.redactHeader(it) }
- }
+ fun setHttpLogger(httpLogger: HttpLogger) = apply {
+ httpLoggingInterceptor = httpLogger.toOkHttpLoggingInterceptor()
  }
 
- private fun HttpLogger.Level.toOkhttpLogLevel() =
- when (this) {
- HttpLogger.Level.NONE -> HttpLoggingInterceptor.Level.NONE
- HttpLogger.Level.HEADERS -> HttpLoggingInterceptor.Level.HEADERS
- HttpLogger.Level.BASIC -> HttpLoggingInterceptor.Level.BASIC
- HttpLogger.Level.BODY -> HttpLoggingInterceptor.Level.BODY
- }
-
  fun build(): RemoteFhirService {
  val client =
  OkHttpClient.Builder()
+ .connectTimeout(networkConfiguration.connectionTimeOut, TimeUnit.SECONDS)
+ .readTimeout(networkConfiguration.readTimeOut, TimeUnit.SECONDS)
+ .writeTimeout(networkConfiguration.writeTimeOut, TimeUnit.SECONDS)
  .apply {
- connectTimeout(networkConfiguration.connectionTimeOut, TimeUnit.SECONDS)
- readTimeout(networkConfiguration.readTimeOut, TimeUnit.SECONDS)
- writeTimeout(networkConfiguration.writeTimeOut, TimeUnit.SECONDS)
  httpLoggingInterceptor?.let { addInterceptor(it) }
  authenticator?.let {
  addInterceptor(

engine/src/test/java/com/google/android/fhir/sync/remote/MoreHttpLoggerKtTest.kt

@@ -0,0 +1,64 @@
+/*
+ * Copyright 2022 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http:https://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package com.google.android.fhir.sync.remote
+
+import com.google.common.truth.Truth.assertThat
+import okhttp3.logging.HttpLoggingInterceptor
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.util.ReflectionHelpers
+
+@RunWith(RobolectricTestRunner::class)
+class MoreHttpLoggerKtTest {
+
+ @Test
+ fun `toOkHttpLoggingInterceptor HttpLoggingInterceptor Level should match provided HttpLogger Level`() {
+ assertThat(httpLogger(HttpLogger.Level.NONE).toOkHttpLoggingInterceptor().level)
+ .isEqualTo(HttpLoggingInterceptor.Level.NONE)
+ assertThat(httpLogger(HttpLogger.Level.BASIC).toOkHttpLoggingInterceptor().level)
+ .isEqualTo(HttpLoggingInterceptor.Level.BASIC)
+ assertThat(httpLogger(HttpLogger.Level.HEADERS).toOkHttpLoggingInterceptor().level)
+ .isEqualTo(HttpLoggingInterceptor.Level.HEADERS)
+ assertThat(httpLogger(HttpLogger.Level.BODY).toOkHttpLoggingInterceptor().level)
+ .isEqualTo(HttpLoggingInterceptor.Level.BODY)
+ }
+
+ @Test
+ fun `toOkHttpLoggingInterceptor headersToRedact should be empty when headersToIgnore is not provided`() {
+ assertThat(
+ httpLogger(HttpLogger.Level.BODY, emptyList()).toOkHttpLoggingInterceptor().redactHeaders
+ )
+ .isEmpty()
+ }
+
+ @Test
+ fun `toOkHttpLoggingInterceptor headersToIgnore should populate headersToRedact`() {
+ assertThat(
+ httpLogger(HttpLogger.Level.BODY, listOf("Authorization", "content-type"))
+ .toOkHttpLoggingInterceptor()
+ .redactHeaders
+ )
+ .containsExactly("Authorization", "content-type")
+ }
+
+ private fun httpLogger(level: HttpLogger.Level, headersToIgnore: List<String>? = null) =
+ HttpLogger(HttpLogger.Configuration(level, headersToIgnore)) {}
+
+ private val HttpLoggingInterceptor.redactHeaders: Collection<String>
+ get() = ReflectionHelpers.getField(this, "headersToRedact")
+}