Allow DownloadWorkManager to have Bundle based requests along with ur…

…l based ones. (#1887)

* Added Request types to DownloadWorkManager for it to allow requests other than url based ones

* Added additional api to datasource  and updated tests.

* Updated comments.

* Updated code

* Review Comments: Updated DownloaderImpl tests

* Review comments: Cleaned up remotefhirservice interface

* Spotless

* Review comment: Refactor datasource implementation

demo/src/main/java/com/google/android/fhir/demo/data/TimestampBasedDownloadWorkManagerImpl.kt

@@ -18,6 +18,7 @@ package com.google.android.fhir.demo.data
 
 import com.google.android.fhir.demo.DemoDataStore
 import com.google.android.fhir.sync.DownloadWorkManager
+import com.google.android.fhir.sync.Request
 import com.google.android.fhir.sync.SyncDataParams
 import java.time.ZoneId
 import java.time.format.DateTimeFormatter
@@ -43,15 +44,15 @@ class TimestampBasedDownloadWorkManagerImpl(private val dataStore: DemoDataStore
  )
  )
 
- override suspend fun getNextRequestUrl(): String? {
+ override suspend fun getNextRequest(): Request? {
  var url = urls.poll() ?: return null
 
  val resourceTypeToDownload =
  ResourceType.fromCode(url.findAnyOf(resourceTypeList, ignoreCase = true)!!.second)
  dataStore.getLasUpdateTimestamp(resourceTypeToDownload)?.let {
  url = affixLastUpdatedTimestamp(url, it)
  }
- return url
+ return Request.of(url)
  }
 
  override suspend fun getSummaryRequestUrls(): Map<ResourceType, String> {

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

@@ -28,7 +28,8 @@ import com.google.android.fhir.impl.FhirEngineImpl
 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.FhirHttpDataSource
+import com.google.android.fhir.sync.remote.RetrofitHttpService
 import org.hl7.fhir.r4.model.SearchParameter
 import timber.log.Timber
 
@@ -81,10 +82,13 @@ 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()
+ FhirHttpDataSource(
+ fhirHttpService =
+ RetrofitHttpService.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/DataSource.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright 2021 Google LLC
+ * 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.
@@ -17,6 +17,7 @@
 package com.google.android.fhir.sync
 
 import org.hl7.fhir.r4.model.Bundle
+import org.hl7.fhir.r4.model.Bundle.BundleType
 import org.hl7.fhir.r4.model.OperationOutcome
 import org.hl7.fhir.r4.model.Resource
 
@@ -31,6 +32,13 @@ internal interface DataSource {
  */
  suspend fun download(path: String): Resource
 
+ /**
+ * @return [Bundle] on a successful operation, [OperationOutcome] otherwise. Call this api with
+ * the [Bundle] that contains individual requests bundled together to be downloaded from the
+ * server. (e.g. https://www.hl7.org/fhir/bundle-request-medsallergies.json.html)
+ */
+ suspend fun download(bundle: Bundle): Resource
+
  /**
  * @return [Bundle] of type [BundleType.TRANSACTIONRESPONSE] for a successful operation,
  * [OperationOutcome] otherwise. Call this api with the [Bundle] that needs to be uploaded to the

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

@@ -16,6 +16,7 @@
 
 package com.google.android.fhir.sync
 
+import org.hl7.fhir.r4.model.Bundle
 import org.hl7.fhir.r4.model.Resource
 import org.hl7.fhir.r4.model.ResourceType
 
@@ -30,7 +31,7 @@ interface DownloadWorkManager {
  * Returns the URL for the next download request, or `null` if there is no more download request
  * to be issued.
  */
- suspend fun getNextRequestUrl(): String?
+ suspend fun getNextRequest(): Request?
 
  /* TODO: Generalize the DownloadWorkManager API to not sequentially download resource by type (https://github.com/google/android-fhir/issues/1884) */
  /**
@@ -43,3 +44,26 @@ interface DownloadWorkManager {
  */
  suspend fun processResponse(response: Resource): Collection<Resource>
 }
+
+sealed class Request {
+ companion object {
+ /** @return [UrlRequest] for a FHIR search [url]. */
+ fun of(url: String) = UrlRequest(url)
+
+ /** @return [BundleRequest] for a FHIR search [bundle]. */
+ fun of(bundle: Bundle) = BundleRequest(bundle)
+ }
+}
+
+/**
+ * A [url] based FHIR request to download resources from the server. e.g.
+ * `Patient?given=valueGiven&family=valueFamily`
+ */
+data class UrlRequest(val url: String) : Request()
+
+/**
+ * A [bundle] based FHIR request to download resources from the server. For an example, see
+ * [bundle-request-medsallergies.json](https://www.hl7.org/fhir/bundle-request-medsallergies.json.html)
+ * .
+ */
+data class BundleRequest(val bundle: Bundle) : Request()

engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt

@@ -16,11 +16,14 @@
 
 package com.google.android.fhir.sync.download
 
+import com.google.android.fhir.sync.BundleRequest
 import com.google.android.fhir.sync.DataSource
 import com.google.android.fhir.sync.DownloadState
 import com.google.android.fhir.sync.DownloadWorkManager
 import com.google.android.fhir.sync.Downloader
+import com.google.android.fhir.sync.Request
 import com.google.android.fhir.sync.ResourceSyncException
+import com.google.android.fhir.sync.UrlRequest
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.flow
 import org.hl7.fhir.r4.model.Bundle
@@ -41,45 +44,50 @@ internal class DownloaderImpl(
 
  override suspend fun download(): Flow<DownloadState> = flow {
  var resourceTypeToDownload: ResourceType = ResourceType.Bundle
-
  // download count summary of all resources for progress i.e. <type, total, completed>
- val progressSummary =
- downloadWorkManager
- .getSummaryRequestUrls()
- .map { summary ->
- summary.key to
- runCatching { dataSource.download(summary.value) }
- .onFailure { Timber.e(it) }
- .getOrNull()
- .takeIf { it is Bundle }
- ?.let { (it as Bundle).total }
- }
- .also { Timber.i("Download summary " + it.joinToString()) }
- .toMap()
-
- val total = progressSummary.values.sumOf { it ?: 0 }
- var completed = 0
-
- emit(DownloadState.Started(resourceTypeToDownload, total))
-
- var url = downloadWorkManager.getNextRequestUrl()
- while (url != null) {
+ val totalResourcesToDownloadCount = getProgressSummary().values.sumOf { it ?: 0 }
+ emit(DownloadState.Started(resourceTypeToDownload, totalResourcesToDownloadCount))
+ var downloadedResourcesCount = 0
+ var request = downloadWorkManager.getNextRequest()
+ while (request != null) {
  try {
- resourceTypeToDownload =
- ResourceType.fromCode(url.findAnyOf(resourceTypeList, ignoreCase = true)!!.second)
-
- emit(
- downloadWorkManager.processResponse(dataSource.download(url!!)).toList().let {
- completed += it.size
- DownloadState.Success(it, total, completed)
- }
- )
+ resourceTypeToDownload = request.toResourceType()
+ downloadWorkManager.processResponse(download(request)).toList().let {
+ downloadedResourcesCount += it.size
+ emit(DownloadState.Success(it, totalResourcesToDownloadCount, downloadedResourcesCount))
+ }
  } catch (exception: Exception) {
  Timber.e(exception)
  emit(DownloadState.Failure(ResourceSyncException(resourceTypeToDownload, exception)))
  }
-
- url = downloadWorkManager.getNextRequestUrl()
+ request = downloadWorkManager.getNextRequest()
  }
  }
+
+ private suspend fun download(request: Request) =
+ when (request) {
+ is UrlRequest -> dataSource.download(request.url)
+ is BundleRequest -> dataSource.download(request.bundle)
+ }
+
+ private fun Request.toResourceType() =
+ when (this) {
+ is UrlRequest ->
+ ResourceType.fromCode(url.findAnyOf(resourceTypeList, ignoreCase = true)!!.second)
+ is BundleRequest -> ResourceType.Bundle
+ }
+
+ private suspend fun getProgressSummary() =
+ downloadWorkManager
+ .getSummaryRequestUrls()
+ .map { summary ->
+ summary.key to
+ runCatching { dataSource.download(summary.value) }
+ .onFailure { Timber.e(it) }
+ .getOrNull()
+ .takeIf { it is Bundle }
+ ?.let { (it as Bundle).total }
+ }
+ .also { Timber.i("Download summary " + it.joinToString()) }
+ .toMap()
 }

engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt

@@ -19,6 +19,7 @@ package com.google.android.fhir.sync.download
 import com.google.android.fhir.sync.DownloadWorkManager
 import com.google.android.fhir.sync.GREATER_THAN_PREFIX
 import com.google.android.fhir.sync.ParamMap
+import com.google.android.fhir.sync.Request
 import com.google.android.fhir.sync.SyncDataParams
 import com.google.android.fhir.sync.concatParams
 import com.google.android.fhir.toTimeZoneString
@@ -43,15 +44,15 @@ class ResourceParamsBasedDownloadWorkManager(
  private val resourcesToDownloadWithSearchParams = LinkedList(syncParams.entries)
  private val urlOfTheNextPagesToDownloadForAResource = LinkedList<String>()
 
- override suspend fun getNextRequestUrl(): String? {
+ override suspend fun getNextRequest(): Request? {
  if (urlOfTheNextPagesToDownloadForAResource.isNotEmpty())
- return urlOfTheNextPagesToDownloadForAResource.poll()
+ return urlOfTheNextPagesToDownloadForAResource.poll()?.let { Request.of(it) }
 
  return resourcesToDownloadWithSearchParams.poll()?.let { (resourceType, params) ->
  val newParams =
  params.toMutableMap().apply { putAll(getLastUpdatedParam(resourceType, params, context)) }
 
- "${resourceType.name}?${newParams.concatParams()}"
+ Request.of("${resourceType.name}?${newParams.concatParams()}")
  }
  }
 

engine/src/main/java/com/google/android/fhir/sync/remote/FhirHttpDataSource.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 com.google.android.fhir.sync.DataSource
+import org.hl7.fhir.r4.model.Bundle
+
+/**
+ * Implementation of [DataSource] to sync data with the FHIR server using HTTP method calls.
+ * @param fhirHttpService Http service to make requests to the server.
+ */
+internal class FhirHttpDataSource(private val fhirHttpService: FhirHttpService) : DataSource {
+
+ override suspend fun download(path: String) = fhirHttpService.get(path)
+
+ override suspend fun download(bundle: Bundle) = fhirHttpService.post(bundle)
+
+ override suspend fun upload(bundle: Bundle) = fhirHttpService.post(bundle)
+}

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

@@ -0,0 +1,38 @@
+/*
+ * 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 org.hl7.fhir.r4.model.Bundle
+import org.hl7.fhir.r4.model.OperationOutcome
+import org.hl7.fhir.r4.model.Resource
+
+/** Interface to make HTTP requests to the FHIR server. */
+internal interface FhirHttpService {
+
+ /**
+ * Makes a HTTP-GET method request to the server.
+ * @return The server may return a particular [Resource], [Bundle] or [OperationOutcome] based on
+ * the request processing.
+ */
+ suspend fun get(path: String): Resource
+
+ /**
+ * Makes a HTTP-POST method request to the server with the [Bundle] as request-body.
+ * @return The server may return [Bundle] or [OperationOutcome] based on the request processing.
+ */
+ suspend fun post(bundle: Bundle): Resource
+}

engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt → engine/src/main/java/com/google/android/fhir/sync/remote/RetrofitHttpService.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
@@ -31,12 +30,12 @@ import retrofit2.http.GET
 import retrofit2.http.POST
 import retrofit2.http.Url
 
-/** Interface to make http requests to the FHIR server. */
-internal interface RemoteFhirService : DataSource {
+/** Retrofit service to make http requests to the FHIR server. */
+internal interface RetrofitHttpService : FhirHttpService {
 
- @GET override suspend fun download(@Url path: String): Resource
+ @GET override suspend fun get(@Url path: String): Resource
 
- @POST(".") override suspend fun upload(@Body bundle: Bundle): Resource
+ @POST(".") override suspend fun post(@Body bundle: Bundle): Resource
 
  class Builder(
  private val baseUrl: String,
@@ -53,7 +52,7 @@ internal interface RemoteFhirService : DataSource {
  httpLoggingInterceptor = httpLogger.toOkHttpLoggingInterceptor()
  }
 
- fun build(): RemoteFhirService {
+ fun build(): RetrofitHttpService {
  val client =
  OkHttpClient.Builder()
  .connectTimeout(networkConfiguration.connectionTimeOut, TimeUnit.SECONDS)
@@ -82,7 +81,7 @@ internal interface RemoteFhirService : DataSource {
  .client(client)
  .addConverterFactory(FhirConverterFactory.create())
  .build()
- .create(RemoteFhirService::class.java)
+ .create(RetrofitHttpService::class.java)
  }
  }