Review comments: Cleaned up remotefhirservice interface

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

@@ -29,6 +29,7 @@ import com.google.android.fhir.index.ResourceIndexer
 import com.google.android.fhir.index.SearchParamDefinitionsProviderImpl
 import com.google.android.fhir.sync.DataSource
 import com.google.android.fhir.sync.remote.RemoteFhirService
+import com.google.android.fhir.sync.remote.RetrofitBasedRemoteDataSource
 import org.hl7.fhir.r4.model.SearchParameter
 import timber.log.Timber
 
@@ -81,10 +82,12 @@ internal data class FhirServices(
  val engine = FhirEngineImpl(database = db, context = context)
  val remoteDataSource =
  serverConfiguration?.let {
- RemoteFhirService.builder(it.baseUrl, it.networkConfiguration)
- .setAuthenticator(it.authenticator)
- .setHttpLogger(it.httpLogger)
- .build()
+ RetrofitBasedRemoteDataSource(
+ RemoteFhirService.builder(it.baseUrl, it.networkConfiguration)
+ .setAuthenticator(it.authenticator)
+ .setHttpLogger(it.httpLogger)
+ .build()
+ )
  }
  return FhirServices(
  fhirEngine = engine,

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

@@ -18,7 +18,6 @@ package com.google.android.fhir.sync.remote
 
 import com.google.android.fhir.NetworkConfiguration
 import com.google.android.fhir.sync.Authenticator
-import com.google.android.fhir.sync.DataSource
 import java.util.concurrent.TimeUnit
 import okhttp3.Interceptor
 import okhttp3.OkHttpClient
@@ -32,13 +31,11 @@ import retrofit2.http.POST
 import retrofit2.http.Url
 
 /** Interface to make http requests to the FHIR server. */
-internal interface RemoteFhirService : DataSource {
+internal interface RemoteFhirService {
 
- @GET override suspend fun download(@Url path: String): Resource
+ @GET suspend fun get(@Url path: String): Resource
 
- @POST(".") override suspend fun download(@Body bundle: Bundle): Resource
-
- @POST(".") override suspend fun upload(@Body bundle: Bundle): Resource
+ @POST(".") suspend fun post(@Body bundle: Bundle): Resource
 
  class Builder(
  private val baseUrl: String,

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

@@ -0,0 +1,29 @@
+/*
+ * 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
+ *
+ * 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.android.fhir.sync.DataSource
+import org.hl7.fhir.r4.model.Bundle
+
+internal class RetrofitBasedRemoteDataSource(private val remoteFhirService: RemoteFhirService) :
+ DataSource {
+ override suspend fun download(path: String) = remoteFhirService.get(path)
+
+ override suspend fun download(bundle: Bundle) = remoteFhirService.post(bundle)
+
+ override suspend fun upload(bundle: Bundle) = remoteFhirService.post(bundle)
+}

engine/src/test-common/java/com/google/android/fhir/resource/TestingUtils.kt

@@ -28,6 +28,7 @@ import com.google.android.fhir.sync.DataSource
 import com.google.android.fhir.sync.DownloadWorkManager
 import com.google.android.fhir.sync.Request
 import com.google.common.truth.Truth.assertThat
+import java.net.SocketTimeoutException
 import java.time.OffsetDateTime
 import java.util.Date
 import java.util.LinkedList
@@ -200,11 +201,11 @@ class TestingUtils constructor(private val iParser: IParser) {
  }
 
  override suspend fun download(bundle: Bundle): Resource {
- throw Exception("Posting Download Bundle failed...")
+ throw SocketTimeoutException("Posting Download Bundle failed...")
  }
 
  override suspend fun upload(bundle: Bundle): Resource {
- throw Exception("Posting Upload Bundle failed...")
+ throw SocketTimeoutException("Posting Upload Bundle failed...")
  }
  }
 

engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt → engine/src/test/java/com/google/android/fhir/sync/remote/RetrofitBasedRemoteDataSourceTest.kt

@@ -34,12 +34,14 @@ import org.junit.runner.RunWith
 import org.robolectric.RobolectricTestRunner
 
 @RunWith(RobolectricTestRunner::class)
-class RemoteFhirServiceTest {
+class RetrofitBasedRemoteDataSourceTest {
 
  private val mockWebServer = MockWebServer()
 
- private val apiService by lazy {
- RemoteFhirService.Builder(mockWebServer.url("/").toString(), NetworkConfiguration()).build()
+ private val retrofitBasedRemoteDataSource by lazy {
+ RetrofitBasedRemoteDataSource(
+ RemoteFhirService.Builder(mockWebServer.url("/").toString(), NetworkConfiguration()).build()
+ )
  }
 
  private val parser = FhirContext.forR4Cached().newJsonParser()
@@ -76,7 +78,7 @@ class RemoteFhirServiceTest {
  }
  mockWebServer.enqueue(mockResponse)
 
- val result = apiService.download("Patient/patient-001")
+ val result = retrofitBasedRemoteDataSource.download("Patient/patient-001")
 
  // No exception should occur
  assertThat(result).isInstanceOf(Patient::class.java)
@@ -104,7 +106,7 @@ class RemoteFhirServiceTest {
  type = Bundle.BundleType.TRANSACTION
  }
 
- val result = apiService.upload(request)
+ val result = retrofitBasedRemoteDataSource.upload(request)
 
  // No exception has occurred
  assertThat(result).isInstanceOf(Bundle::class.java)
@@ -132,7 +134,7 @@ class RemoteFhirServiceTest {
  type = Bundle.BundleType.TRANSACTION
  }
 
- val result = apiService.upload(request)
+ val result = retrofitBasedRemoteDataSource.upload(request)
 
  assertThat(result).isInstanceOf(Bundle::class.java)
  assertThat((result as Bundle).type).isEqualTo(Bundle.BundleType.TRANSACTIONRESPONSE)