diff --git a/demo/src/main/java/com/google/android/fhir/demo/data/TimestampBasedDownloadWorkManagerImpl.kt b/demo/src/main/java/com/google/android/fhir/demo/data/TimestampBasedDownloadWorkManagerImpl.kt index ba7ab25867..1d7e18b84a 100644 --- a/demo/src/main/java/com/google/android/fhir/demo/data/TimestampBasedDownloadWorkManagerImpl.kt +++ b/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,7 +44,7 @@ class TimestampBasedDownloadWorkManagerImpl(private val dataStore: DemoDataStore ) ) - override suspend fun getNextRequestUrl(): String? { + override suspend fun getNextRequest(): Request? { var url = urls.poll() ?: return null val resourceTypeToDownload = @@ -51,7 +52,7 @@ class TimestampBasedDownloadWorkManagerImpl(private val dataStore: DemoDataStore dataStore.getLasUpdateTimestamp(resourceTypeToDownload)?.let { url = affixLastUpdatedTimestamp(url, it) } - return url + return Request.of(url) } override suspend fun getSummaryRequestUrls(): Map { diff --git a/engine/src/main/java/com/google/android/fhir/FhirServices.kt b/engine/src/main/java/com/google/android/fhir/FhirServices.kt index eb11475d81..6827e82bcf 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirServices.kt +++ b/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, diff --git a/engine/src/main/java/com/google/android/fhir/sync/DataSource.kt b/engine/src/main/java/com/google/android/fhir/sync/DataSource.kt index 0abdf129df..3e0e1b9d6e 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/DataSource.kt +++ b/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 diff --git a/engine/src/main/java/com/google/android/fhir/sync/DownloadWorkManager.kt b/engine/src/main/java/com/google/android/fhir/sync/DownloadWorkManager.kt index adbb253735..0d18cddafa 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/DownloadWorkManager.kt +++ b/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 } + +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() diff --git a/engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt b/engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt index d8692c0a44..2e01932f09 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/download/DownloaderImpl.kt +++ b/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 = flow { var resourceTypeToDownload: ResourceType = ResourceType.Bundle - // download count summary of all resources for progress i.e. - 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() } diff --git a/engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt b/engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt index 091439d686..7d54ed7cb6 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManager.kt +++ b/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() - 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()}") } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/remote/FhirHttpDataSource.kt b/engine/src/main/java/com/google/android/fhir/sync/remote/FhirHttpDataSource.kt new file mode 100644 index 0000000000..9e03956017 --- /dev/null +++ b/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://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) +} diff --git a/engine/src/main/java/com/google/android/fhir/sync/remote/FhirHttpService.kt b/engine/src/main/java/com/google/android/fhir/sync/remote/FhirHttpService.kt new file mode 100644 index 0000000000..95f0feba76 --- /dev/null +++ b/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://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 +} diff --git a/engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt b/engine/src/main/java/com/google/android/fhir/sync/remote/RetrofitHttpService.kt similarity index 88% rename from engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt rename to engine/src/main/java/com/google/android/fhir/sync/remote/RetrofitHttpService.kt index 3bdfd0cbec..c178241cd1 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/remote/RemoteFhirService.kt +++ b/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) } } diff --git a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt index 6f24fae0f5..e2cde8ad94 100644 --- a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt +++ b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt @@ -27,7 +27,9 @@ import com.google.android.fhir.search.Search import com.google.android.fhir.sync.ConflictResolver 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 @@ -95,6 +97,10 @@ object TestDataSourceImpl : DataSource { return Bundle().apply { type = Bundle.BundleType.SEARCHSET } } + override suspend fun download(bundle: Bundle): Resource { + return Bundle().apply { type = Bundle.BundleType.BATCHRESPONSE } + } + override suspend fun upload(bundle: Bundle): Resource { return Bundle().apply { type = Bundle.BundleType.TRANSACTIONRESPONSE } } @@ -105,7 +111,7 @@ open class TestDownloadManagerImpl( ) : DownloadWorkManager { private val urls = LinkedList(queries) - override suspend fun getNextRequestUrl(): String? = urls.poll() + override suspend fun getNextRequest(): Request? = urls.poll()?.let { Request.of(it) } override suspend fun getSummaryRequestUrls() = queries .stream() @@ -179,8 +185,12 @@ object TestFailingDatasource : DataSource { throw Exception(hugeStackTraceMessage) } + override suspend fun download(bundle: Bundle): Resource { + throw SocketTimeoutException("Posting Download Bundle failed...") + } + override suspend fun upload(bundle: Bundle): Resource { - throw Exception("Posting Bundle failed...") + throw SocketTimeoutException("Posting Upload Bundle failed...") } } @@ -190,5 +200,9 @@ class BundleDataSource(val onPostBundle: suspend (Bundle) -> Resource) : DataSou TODO("Not yet implemented") } + override suspend fun download(bundle: Bundle): Resource { + TODO("Not yet implemented") + } + override suspend fun upload(bundle: Bundle) = onPostBundle(bundle) } diff --git a/engine/src/test/java/com/google/android/fhir/sync/download/DownloaderImplTest.kt b/engine/src/test/java/com/google/android/fhir/sync/download/DownloaderImplTest.kt index 5ef2d833c8..a09f78d79d 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/download/DownloaderImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/download/DownloaderImplTest.kt @@ -16,15 +16,24 @@ package com.google.android.fhir.sync.download +import com.google.android.fhir.logicalId 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.Request import com.google.common.truth.Truth.assertThat -import java.net.UnknownHostException +import java.util.LinkedList +import java.util.Queue +import kotlinx.coroutines.flow.collectIndexed import kotlinx.coroutines.runBlocking +import org.hl7.fhir.exceptions.FHIRException import org.hl7.fhir.r4.model.Bundle +import org.hl7.fhir.r4.model.Condition +import org.hl7.fhir.r4.model.Encounter import org.hl7.fhir.r4.model.Observation import org.hl7.fhir.r4.model.OperationOutcome import org.hl7.fhir.r4.model.Patient +import org.hl7.fhir.r4.model.Reference import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType import org.junit.Test @@ -34,277 +43,230 @@ import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class DownloaderImplTest { - val searchPageParamToSearchResponseBundleMap = - mapOf( - "patient-page1" to - Bundle().apply { - type = Bundle.BundleType.SEARCHSET - addLink( - Bundle.BundleLinkComponent().apply { - relation = "next" - url = "url-to-server/patient-page2" - } - ) - addEntry(Bundle.BundleEntryComponent().setResource(Patient().apply { id = "Patient-1" })) - }, - "patient-page2" to - Bundle().apply { - type = Bundle.BundleType.SEARCHSET - addEntry(Bundle.BundleEntryComponent().setResource(Patient().apply { id = "Patient-2" })) - }, - "observation-page1" to - Bundle().apply { - type = Bundle.BundleType.SEARCHSET - addLink( - Bundle.BundleLinkComponent().apply { - relation = "next" - url = "url-to-server/observation-page2" - } - ) - addEntry( - Bundle.BundleEntryComponent().setResource(Observation().apply { id = "Observation-1" }) - ) - }, - "observation-page2" to - Bundle().apply { - type = Bundle.BundleType.SEARCHSET - addEntry( - Bundle.BundleEntryComponent().setResource(Observation().apply { id = "Observation-2" }) - ) - } - ) - @Test - fun `downloader with patient and observations should download successfully`() = runBlocking { - val downloader = - DownloaderImpl( - object : DataSource { - override suspend fun download(path: String): Resource { - return when { - path.contains("summary") -> Bundle().apply { total = 1 } - path.contains("patient-page1") -> - searchPageParamToSearchResponseBundleMap["patient-page1"]!! - path.contains("patient-page2") -> - searchPageParamToSearchResponseBundleMap["patient-page2"]!! - path.contains("observation-page1") -> - searchPageParamToSearchResponseBundleMap["observation-page1"]!! - path.contains("observation-page2") -> - searchPageParamToSearchResponseBundleMap["observation-page2"]!! - else -> OperationOutcome() - } - } - - override suspend fun upload(bundle: Bundle): Resource { - TODO("Not yet implemented") - } - }, - ResourceParamsBasedDownloadWorkManager( - mapOf( - ResourceType.Patient to mapOf("param" to "patient-page1"), - ResourceType.Observation to mapOf("param" to "observation-page1") - ), - NoOpResourceParamsBasedDownloadWorkManagerContext - ) + fun `downloader should download all the requests even when some fail`() = runBlocking { + val requests = + listOf( + Request.of("Patient"), + Request.of("Encounter"), + Request.of("Medication/med-123-that-fails"), + Request.of(bundleOf("Observation/ob-123", "Condition/con-123")) ) - val result = mutableListOf() - downloader.download().collect { result.add(it) } - - assertThat(result.filterIsInstance()) - .containsExactly( - DownloadState.Started(ResourceType.Bundle, 2), // 1 patient and 1 observation - ) - - assertThat( - result.filterIsInstance().flatMap { it.resources }.map { it.id } - ) - .containsExactly("Patient-1", "Patient-2", "Observation-1", "Observation-2") - .inOrder() - } - - @Test - fun `downloader with patient and observations should return failure in case of server or network error`() = - runBlocking { - val downloader = - DownloaderImpl( - object : DataSource { - override suspend fun download(path: String): Resource { - return when { - path.contains("summary") -> Bundle().apply { total = 1 } - path.contains("patient-page1") -> - searchPageParamToSearchResponseBundleMap["patient-page1"]!! - path.contains("patient-page2") -> - OperationOutcome().apply { - addIssue( - OperationOutcome.OperationOutcomeIssueComponent().apply { - diagnostics = "Server couldn't fulfil the request." + val testDataSource: DataSource = + object : DataSource { + override suspend fun download(path: String): Resource { + return when (path) { + "Patient" -> + Bundle().apply { + type = Bundle.BundleType.SEARCHSET + addEntry( + Bundle.BundleEntryComponent().apply { + resource = Patient().apply { id = "pa-123" } + } + ) + } + "Encounter" -> + Bundle().apply { + type = Bundle.BundleType.SEARCHSET + addEntry( + Bundle.BundleEntryComponent().apply { + resource = + Encounter().apply { + id = "en-123" + subject = Reference("Patient/pa-123") } - ) } - path.contains("observation-page1") -> - searchPageParamToSearchResponseBundleMap["observation-page1"]!! - path.contains("observation-page2") -> - throw UnknownHostException( - "Url host can't be found. Check if device is connected to internet." - ) - else -> OperationOutcome() + ) } - } - - override suspend fun upload(bundle: Bundle): Resource { - TODO("Upload not tested in this path") - } - }, - ResourceParamsBasedDownloadWorkManager( - mapOf( - ResourceType.Patient to mapOf("param" to "patient-page1"), - ResourceType.Observation to mapOf("param" to "observation-page1") - ), - NoOpResourceParamsBasedDownloadWorkManagerContext - ) - ) - - val result = mutableListOf() - downloader.download().collect { result.add(it) } - - assertThat(result.filterIsInstance()) - .containsExactly( - DownloadState.Started(ResourceType.Bundle, 2), // 1 patient and 1 observation - ) - - assertThat(result.filterIsInstance()).hasSize(2) - - assertThat(result.filterIsInstance().map { it.syncError.resourceType }) - .containsExactly(ResourceType.Patient, ResourceType.Observation) - .inOrder() - assertThat( - result.filterIsInstance().map { it.syncError.exception.message } - ) - .containsExactly( - "Server couldn't fulfil the request.", - "Url host can't be found. Check if device is connected to internet." - ) - .inOrder() - } + "Medication/med-123-that-fails" -> + OperationOutcome().apply { + addIssue( + OperationOutcome.OperationOutcomeIssueComponent().apply { + severity = OperationOutcome.IssueSeverity.FATAL + diagnostics = "Resource not found." + } + ) + } + else -> OperationOutcome() + } + } - @Test - fun `downloader with patient and observations should continue to download observations if patient download fail`() = - runBlocking { - val downloader = - DownloaderImpl( - object : DataSource { - override suspend fun download(path: String): Resource { - return when { - path.contains("summary") -> Bundle().apply { total = 1 } - path.contains("patient-page1") || path.contains("patient-page2") -> - OperationOutcome().apply { - addIssue( - OperationOutcome.OperationOutcomeIssueComponent().apply { - diagnostics = "Server couldn't fulfil the request." - } - ) + override suspend fun download(bundle: Bundle): Resource { + return Bundle().apply { + type = Bundle.BundleType.BATCHRESPONSE + addEntry( + Bundle.BundleEntryComponent().apply { + resource = + Observation().apply { + id = "ob-123" + subject = Reference("Patient/pq-123") } - path.contains("observation-page1") -> - searchPageParamToSearchResponseBundleMap["observation-page1"]!! - path.contains("observation-page2") -> - searchPageParamToSearchResponseBundleMap["observation-page2"]!! - else -> OperationOutcome() } - } + ) + addEntry( + Bundle.BundleEntryComponent().apply { + resource = + Condition().apply { + id = "con-123" + subject = Reference("Patient/pq-123") + } + } + ) + } + } - override suspend fun upload(bundle: Bundle): Resource { - TODO("Not yet implemented") - } - }, - ResourceParamsBasedDownloadWorkManager( - mapOf( - ResourceType.Patient to mapOf("param" to "patient-page1"), - ResourceType.Observation to mapOf("param" to "observation-page1") - ), - NoOpResourceParamsBasedDownloadWorkManagerContext - ) - ) + override suspend fun upload(bundle: Bundle): Resource { + throw UnsupportedOperationException() + } + } - val result = mutableListOf() - downloader.download().collect { result.add(it) } + val downloader = DownloaderImpl(testDataSource, TestDownloadWorkManager(requests)) - assertThat(result.filterIsInstance()) - .containsExactly( - DownloadState.Started(ResourceType.Bundle, 2), // 1 patient and 1 observation - ) + val result = mutableListOf() + downloader.download().collectIndexed { _, value -> + if (value is DownloadState.Success) { + result.addAll(value.resources) + } + } - assertThat(result.filterIsInstance().map { it.syncError.resourceType }) - .containsExactly(ResourceType.Patient) + assertThat(result.map { it.logicalId }) + .containsExactly("pa-123", "en-123", "ob-123", "con-123") + .inOrder() + } - assertThat( - result - .filterIsInstance() - .flatMap { it.resources } - .filterIsInstance() + @Test + fun `downloader should emit all the states for requests whether they pass or fail`() = + runBlocking { + val requests = + listOf( + Request.of("Patient"), + Request.of("Encounter"), + Request.of("Medication/med-123-that-fails"), + Request.of(bundleOf("Observation/ob-123", "Condition/con-123")) ) - .hasSize(2) - } - @Test - fun `downloader should emit Started state`() = runBlocking { - val downloader = - DownloaderImpl( + val testDataSource: DataSource = object : DataSource { override suspend fun download(path: String): Resource { - return when { - path.contains("patient-page1") -> - searchPageParamToSearchResponseBundleMap["patient-page1"]!! + return when (path) { + "Patient" -> + Bundle().apply { + type = Bundle.BundleType.SEARCHSET + addEntry( + Bundle.BundleEntryComponent().apply { + resource = Patient().apply { id = "pa-123" } + } + ) + } + "Encounter" -> + Bundle().apply { + type = Bundle.BundleType.SEARCHSET + addEntry( + Bundle.BundleEntryComponent().apply { + resource = + Encounter().apply { + id = "en-123" + subject = Reference("Patient/pa-123") + } + } + ) + } + "Medication/med-123-that-fails" -> + OperationOutcome().apply { + addIssue( + OperationOutcome.OperationOutcomeIssueComponent().apply { + severity = OperationOutcome.IssueSeverity.FATAL + diagnostics = "Resource not found." + } + ) + } else -> OperationOutcome() } } + override suspend fun download(bundle: Bundle): Resource { + return Bundle().apply { + type = Bundle.BundleType.BATCHRESPONSE + addEntry( + Bundle.BundleEntryComponent().apply { + resource = + Observation().apply { + id = "ob-123" + subject = Reference("Patient/pq-123") + } + } + ) + addEntry( + Bundle.BundleEntryComponent().apply { + resource = + Condition().apply { + id = "con-123" + subject = Reference("Patient/pq-123") + } + } + ) + } + } + override suspend fun upload(bundle: Bundle): Resource { throw UnsupportedOperationException() } - }, - ResourceParamsBasedDownloadWorkManager( - mapOf(ResourceType.Patient to mapOf("param" to "patient-page1")), - NoOpResourceParamsBasedDownloadWorkManagerContext + } + val downloader = DownloaderImpl(testDataSource, TestDownloadWorkManager(requests)) + + val result = mutableListOf() + downloader.download().collectIndexed { _, value -> result.add(value) } + + assertThat(result.map { it::class.java }) + .containsExactly( + DownloadState.Started::class.java, + DownloadState.Success::class.java, + DownloadState.Success::class.java, + DownloadState.Failure::class.java, + DownloadState.Success::class.java ) - ) + .inOrder() - val result = mutableListOf() - downloader.download().collect { value -> result.add(value) } + assertThat(result.filterIsInstance().map { it.completed }) + .containsExactly(1, 2, 4) + .inOrder() + } - assertThat(result.first()).isInstanceOf(DownloadState.Started::class.java) - } + companion object { - @Test - fun `downloader should emit Success state`() = runBlocking { - val downloader = - DownloaderImpl( - object : DataSource { - override suspend fun download(path: String): Resource { - return when { - path.contains("patient-page1") -> - searchPageParamToSearchResponseBundleMap["patient-page1"]!!.apply { total = 1 } - else -> OperationOutcome() + private fun bundleOf(vararg getRequest: String) = + Bundle().apply { + type = Bundle.BundleType.BATCH + getRequest.forEach { + addEntry( + Bundle.BundleEntryComponent().apply { + request = + Bundle.BundleEntryRequestComponent().apply { + method = Bundle.HTTPVerb.GET + url = it + } } - } + ) + } + } + } +} - override suspend fun upload(bundle: Bundle): Resource { - throw UnsupportedOperationException() - } - }, - ResourceParamsBasedDownloadWorkManager( - mapOf(ResourceType.Patient to mapOf("param" to "patient-page1")), - NoOpResourceParamsBasedDownloadWorkManagerContext - ) - ) +class TestDownloadWorkManager(requests: List) : DownloadWorkManager { + private val queue: Queue = LinkedList(requests) - val result = mutableListOf() - downloader.download().collect { value -> result.add(value) } + override suspend fun getNextRequest(): Request? = queue.poll() - assertThat(result.first()).isInstanceOf(DownloadState.Started::class.java) - assertThat(result.elementAt(1)).isInstanceOf(DownloadState.Success::class.java) + override suspend fun getSummaryRequestUrls() = emptyMap() - val success = result.elementAt(1) as DownloadState.Success - assertThat(success.total).isEqualTo(1) - assertThat(success.completed).isEqualTo(1) + override suspend fun processResponse(response: Resource): Collection { + if (response is OperationOutcome) { + throw FHIRException(response.issueFirstRep.diagnostics) + } + if (response is Bundle) { + return response.entry.map { it.resource } + } + return emptyList() } } diff --git a/engine/src/test/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManagerTest.kt b/engine/src/test/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManagerTest.kt index d0c814810d..86bfe0b975 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManagerTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/download/ResourceParamsBasedDownloadWorkManagerTest.kt @@ -18,6 +18,7 @@ package com.google.android.fhir.sync.download import com.google.android.fhir.logicalId import com.google.android.fhir.sync.SyncDataParams +import com.google.android.fhir.sync.UrlRequest import com.google.common.truth.Truth.assertThat import kotlinx.coroutines.test.runBlockingTest import org.hl7.fhir.exceptions.FHIRException @@ -47,7 +48,7 @@ class ResourceParamsBasedDownloadWorkManagerTest { val urlsToDownload = mutableListOf() do { - val url = downloadManager.getNextRequestUrl() + val url = downloadManager.getNextRequest()?.let { (it as UrlRequest).url } if (url != null) { urlsToDownload.add(url) } @@ -71,7 +72,7 @@ class ResourceParamsBasedDownloadWorkManagerTest { val urlsToDownload = mutableListOf() do { - val url = downloadManager.getNextRequestUrl() + val url = downloadManager.getNextRequest()?.let { (it as UrlRequest).url } if (url != null) { urlsToDownload.add(url) } @@ -111,7 +112,7 @@ class ResourceParamsBasedDownloadWorkManagerTest { mapOf(ResourceType.Patient to emptyMap()), TestResourceParamsBasedDownloadWorkManagerContext("2022-06-28") ) - val url = downloadManager.getNextRequestUrl() + val url = downloadManager.getNextRequest()?.let { (it as UrlRequest).url } assertThat(url).isEqualTo("Patient?_sort=_lastUpdated&_lastUpdated=gt2022-06-28") } @@ -129,7 +130,7 @@ class ResourceParamsBasedDownloadWorkManagerTest { ), TestResourceParamsBasedDownloadWorkManagerContext("2022-07-07") ) - val url = downloadManager.getNextRequestUrl() + val url = downloadManager.getNextRequest()?.let { (it as UrlRequest).url } assertThat(url).isEqualTo("Patient?_lastUpdated=2022-06-28&_sort=status") } @@ -141,7 +142,7 @@ class ResourceParamsBasedDownloadWorkManagerTest { mapOf(ResourceType.Patient to mapOf(SyncDataParams.LAST_UPDATED_KEY to "gt2022-06-28")), TestResourceParamsBasedDownloadWorkManagerContext("2022-07-07") ) - val url = downloadManager.getNextRequestUrl() + val url = downloadManager.getNextRequest()?.let { (it as UrlRequest).url } assertThat(url).isEqualTo("Patient?_lastUpdated=gt2022-06-28&_sort=_lastUpdated") } @@ -153,7 +154,7 @@ class ResourceParamsBasedDownloadWorkManagerTest { mapOf(ResourceType.Patient to mapOf(Patient.ADDRESS_CITY.paramName to "NAIROBI")), NoOpResourceParamsBasedDownloadWorkManagerContext ) - val actual = downloadManager.getNextRequestUrl() + val actual = downloadManager.getNextRequest()?.let { (it as UrlRequest).url } assertThat(actual).isEqualTo("Patient?address-city=NAIROBI&_sort=_lastUpdated") } @@ -165,7 +166,7 @@ class ResourceParamsBasedDownloadWorkManagerTest { mapOf(ResourceType.Patient to mapOf(Patient.ADDRESS_CITY.paramName to "NAIROBI")), TestResourceParamsBasedDownloadWorkManagerContext("") ) - val actual = downloadManager.getNextRequestUrl() + val actual = downloadManager.getNextRequest()?.let { (it as UrlRequest).url } assertThat(actual).isEqualTo("Patient?address-city=NAIROBI&_sort=_lastUpdated") } diff --git a/engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt b/engine/src/test/java/com/google/android/fhir/sync/remote/RetrofitHttpServiceTest.kt similarity index 92% rename from engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt rename to engine/src/test/java/com/google/android/fhir/sync/remote/RetrofitHttpServiceTest.kt index b2d3ac27e7..678b8a1b0a 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/remote/RemoteFhirServiceTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/remote/RetrofitHttpServiceTest.kt @@ -34,12 +34,12 @@ import org.junit.runner.RunWith import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) -class RemoteFhirServiceTest { +class RetrofitHttpServiceTest { private val mockWebServer = MockWebServer() - private val apiService by lazy { - RemoteFhirService.Builder(mockWebServer.url("/").toString(), NetworkConfiguration()).build() + private val retrofitHttpService by lazy { + RetrofitHttpService.Builder(mockWebServer.url("/").toString(), NetworkConfiguration()).build() } private val parser = FhirContext.forR4Cached().newJsonParser() @@ -76,7 +76,7 @@ class RemoteFhirServiceTest { } mockWebServer.enqueue(mockResponse) - val result = apiService.download("Patient/patient-001") + val result = retrofitHttpService.get("Patient/patient-001") // No exception should occur assertThat(result).isInstanceOf(Patient::class.java) @@ -104,7 +104,7 @@ class RemoteFhirServiceTest { type = Bundle.BundleType.TRANSACTION } - val result = apiService.upload(request) + val result = retrofitHttpService.post(request) // No exception has occurred assertThat(result).isInstanceOf(Bundle::class.java) @@ -132,7 +132,7 @@ class RemoteFhirServiceTest { type = Bundle.BundleType.TRANSACTION } - val result = apiService.upload(request) + val result = retrofitHttpService.post(request) assertThat(result).isInstanceOf(Bundle::class.java) assertThat((result as Bundle).type).isEqualTo(Bundle.BundleType.TRANSACTIONRESPONSE)