diff --git a/demo/src/main/java/com/google/android/fhir/demo/FhirApplication.kt b/demo/src/main/java/com/google/android/fhir/demo/FhirApplication.kt index a5f0511f63..cfb30758c7 100644 --- a/demo/src/main/java/com/google/android/fhir/demo/FhirApplication.kt +++ b/demo/src/main/java/com/google/android/fhir/demo/FhirApplication.kt @@ -30,6 +30,7 @@ import com.google.android.fhir.demo.data.FhirSyncWorker import com.google.android.fhir.search.search import com.google.android.fhir.sync.Sync import com.google.android.fhir.sync.remote.HttpLogger +import org.hl7.fhir.r4.model.Patient import timber.log.Timber class FhirApplication : Application(), DataCaptureConfig.Provider { @@ -45,6 +46,7 @@ class FhirApplication : Application(), DataCaptureConfig.Provider { if (BuildConfig.DEBUG) { Timber.plant(Timber.DebugTree()) } + Patient.IDENTIFIER FhirEngineProvider.init( FhirEngineConfiguration( enableEncryptionIfSupported = true, diff --git a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt index 93c9c800cd..4092f9e3a8 100644 --- a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt @@ -161,7 +161,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex database.updateVersionIdAndLastUpdated( id, type, - response.etag, + getVersionFromETag(response.etag), response.lastModified.toInstant() ) } @@ -179,6 +179,20 @@ internal class FhirEngineImpl(private val database: Database, private val contex } } + /** + * FHIR uses weak ETag that look something like W/"MTY4NDMyODE2OTg3NDUyNTAwMA", so we need to + * extract version from it. See https://hl7.org/fhir/http.html#Http-Headers. + */ + private fun getVersionFromETag(eTag: String) = + // The server should always return a weak etag that starts with W, but if it server returns a + // strong tag, we store it as-is. The http-headers for conditional upload like if-match will + // always add value as a weak tag. + if (eTag.startsWith("W/")) { + eTag.split("\"")[1] + } else { + eTag + } + /** * May return a Pair of versionId and resource type extracted from the * [Bundle.BundleEntryResponseComponent.location]. diff --git a/engine/src/main/java/com/google/android/fhir/sync/Config.kt b/engine/src/main/java/com/google/android/fhir/sync/Config.kt index fbc3282b14..87c0678b3b 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/Config.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/Config.kt @@ -112,12 +112,19 @@ data class BackoffCriteria( /** * Configuration for max number of resources to be uploaded in a Bundle.The default size is - * [DEFAULT_BUNDLE_SIZE]. + * [DEFAULT_BUNDLE_SIZE]. The application developer may also configure if the eTag should be used + * for edit and delete requests during the upload. Default is to use the eTag. */ data class UploadConfiguration( /** * Number of [Resource]s to be added in a singe [Bundle] for upload and default is * [DEFAULT_BUNDLE_SIZE] */ - val uploadBundleSize: Int = DEFAULT_BUNDLE_SIZE + val uploadBundleSize: Int = DEFAULT_BUNDLE_SIZE, + + /** + * Use if-match http header with e-tag for upload requests. See ETag + * [section](https://hl7.org/fhir/http.html#Http-Headers) for more details. + */ + val useETagForUpload: Boolean = true, ) 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 3e0e1b9d6e..e760f2e0cd 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 @@ -26,23 +26,13 @@ import org.hl7.fhir.r4.model.Resource * operations are [Bundle] based to optimize network traffic. */ internal interface DataSource { - /** - * @return [Bundle] of type [BundleType.SEARCHSET] for a successful operation, [OperationOutcome] - * otherwise. Call this api with the relative path of the resource search url to be downloaded. - */ - 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] on a successful operation, [OperationOutcome] otherwise. */ + suspend fun download(request: Request): 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 * server. */ - suspend fun upload(bundle: Bundle): Resource + suspend fun upload(request: BundleRequest): Resource } 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 0d18cddafa..4ba6c037ec 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 @@ -45,13 +45,50 @@ interface DownloadWorkManager { suspend fun processResponse(response: Resource): Collection } -sealed class Request { +/** + * Structure represents a request that can be made to download resources from the FHIR server. The + * request may contain http headers for conditional requests for getting precise results. + * + * Implementations of [Request] are [UrlRequest] and [BundleRequest] and the application developers + * may choose the appropriate [Request.of] companion functions to create request objects. + * + * **UrlRequest** + * + * The application developer may use a request like below to get an update on Patient/123 since it + * was last downloaded. + * ``` + * Request.of("/Patient/123", mapOf("If-Modified-Since" to "knownLastUpdatedOfPatient123")) + * ``` + * **BundleRequest** + * + * The application developer may use a request like below to download multiple resources in a single + * shot. + * + * ``` + * Request.of(Bundle().apply { + * addEntry(Bundle.BundleEntryComponent().apply { + * request = Bundle.BundleEntryRequestComponent().apply { + * url = "Patient/123" + * method = Bundle.HTTPVerb.GET + * } + * }) + * addEntry(Bundle.BundleEntryComponent().apply { + * request = Bundle.BundleEntryRequestComponent().apply { + * url = "Patient/124" + * method = Bundle.HTTPVerb.GET + * } + * }) + * }) + * ``` + */ +sealed class Request(open val headers: Map) { companion object { /** @return [UrlRequest] for a FHIR search [url]. */ - fun of(url: String) = UrlRequest(url) + fun of(url: String, headers: Map = emptyMap()) = UrlRequest(url, headers) /** @return [BundleRequest] for a FHIR search [bundle]. */ - fun of(bundle: Bundle) = BundleRequest(bundle) + fun of(bundle: Bundle, headers: Map = emptyMap()) = + BundleRequest(bundle, headers) } } @@ -59,11 +96,15 @@ sealed class Request { * 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() +data class UrlRequest +internal constructor(val url: String, override val headers: Map = emptyMap()) : + Request(headers) /** * 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() +data class BundleRequest +internal constructor(val bundle: Bundle, override val headers: Map = emptyMap()) : + Request(headers) diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt index 501acc3fb7..aece04e854 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt @@ -89,18 +89,20 @@ abstract class FhirSyncWorker(appContext: Context, workerParams: WorkerParameter Timber.v("Subscribed to flow for progress") val result = - FhirSynchronizer( - applicationContext, - getFhirEngine(), - BundleUploader( - dataSource, - TransactionBundleGenerator.getDefault(), - LocalChangesPaginator.create(getUploadConfiguration()) - ), - DownloaderImpl(dataSource, getDownloadWorkManager()), - getConflictResolver() - ) - .apply { subscribe(flow) } + with(getUploadConfiguration()) { + FhirSynchronizer( + applicationContext, + getFhirEngine(), + BundleUploader( + dataSource, + TransactionBundleGenerator.getDefault(useETagForUpload), + LocalChangesPaginator.create(this) + ), + DownloaderImpl(dataSource, getDownloadWorkManager()), + getConflictResolver() + ) + .apply { subscribe(flow) } + } .synchronize() val output = buildWorkData(result) 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 2e01932f09..b2718535f1 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 @@ -52,7 +52,7 @@ internal class DownloaderImpl( while (request != null) { try { resourceTypeToDownload = request.toResourceType() - downloadWorkManager.processResponse(download(request)).toList().let { + downloadWorkManager.processResponse(dataSource.download(request)).toList().let { downloadedResourcesCount += it.size emit(DownloadState.Success(it, totalResourcesToDownloadCount, downloadedResourcesCount)) } @@ -64,12 +64,6 @@ internal class DownloaderImpl( } } - 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 -> @@ -82,7 +76,7 @@ internal class DownloaderImpl( .getSummaryRequestUrls() .map { summary -> summary.key to - runCatching { dataSource.download(summary.value) } + runCatching { dataSource.download(Request.of(summary.value)) } .onFailure { Timber.e(it) } .getOrNull() .takeIf { it is Bundle } 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 index 9e03956017..b6b13adb02 100644 --- 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 @@ -16,8 +16,10 @@ package com.google.android.fhir.sync.remote +import com.google.android.fhir.sync.BundleRequest import com.google.android.fhir.sync.DataSource -import org.hl7.fhir.r4.model.Bundle +import com.google.android.fhir.sync.Request +import com.google.android.fhir.sync.UrlRequest /** * Implementation of [DataSource] to sync data with the FHIR server using HTTP method calls. @@ -25,9 +27,12 @@ import org.hl7.fhir.r4.model.Bundle */ internal class FhirHttpDataSource(private val fhirHttpService: FhirHttpService) : DataSource { - override suspend fun download(path: String) = fhirHttpService.get(path) + override suspend fun download(request: Request) = + when (request) { + is UrlRequest -> fhirHttpService.get(request.url, request.headers) + is BundleRequest -> fhirHttpService.post(request.bundle, request.headers) + } - override suspend fun download(bundle: Bundle) = fhirHttpService.post(bundle) - - override suspend fun upload(bundle: Bundle) = fhirHttpService.post(bundle) + override suspend fun upload(request: BundleRequest) = + fhirHttpService.post(request.bundle, request.headers) } 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 index 95f0feba76..70c37d9c00 100644 --- 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 @@ -28,11 +28,11 @@ internal interface FhirHttpService { * @return The server may return a particular [Resource], [Bundle] or [OperationOutcome] based on * the request processing. */ - suspend fun get(path: String): Resource + suspend fun get(path: String, headers: Map): 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 + suspend fun post(bundle: Bundle, headers: Map): Resource } diff --git a/engine/src/main/java/com/google/android/fhir/sync/remote/RetrofitHttpService.kt b/engine/src/main/java/com/google/android/fhir/sync/remote/RetrofitHttpService.kt index bcba4f24c5..f4478b9e89 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/remote/RetrofitHttpService.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/remote/RetrofitHttpService.kt @@ -27,15 +27,18 @@ import org.hl7.fhir.r4.model.Resource import retrofit2.Retrofit import retrofit2.http.Body import retrofit2.http.GET +import retrofit2.http.HeaderMap import retrofit2.http.POST import retrofit2.http.Url /** Retrofit service to make http requests to the FHIR server. */ internal interface RetrofitHttpService : FhirHttpService { - @GET override suspend fun get(@Url path: String): Resource + @GET + override suspend fun get(@Url path: String, @HeaderMap headers: Map): Resource - @POST(".") override suspend fun post(@Body bundle: Bundle): Resource + @POST(".") + override suspend fun post(@Body bundle: Bundle, @HeaderMap headers: Map): Resource class Builder( private val baseUrl: String, diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/BundleUploader.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/BundleUploader.kt index 43c021dca1..7a5182afdc 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/BundleUploader.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/BundleUploader.kt @@ -19,6 +19,7 @@ package com.google.android.fhir.sync.upload import com.google.android.fhir.LocalChange import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.sync.DataSource +import com.google.android.fhir.sync.Request import com.google.android.fhir.sync.ResourceSyncException import com.google.android.fhir.sync.UploadResult import com.google.android.fhir.sync.Uploader @@ -47,7 +48,7 @@ internal class BundleUploader( bundleGenerator.generate(localChangesPaginator.page(localChanges)).forEach { (bundle, localChangeTokens) -> try { - val response = dataSource.upload(bundle) + val response = dataSource.upload(Request.of(bundle)) completed += bundle.entry.size emit(getUploadResult(response, localChangeTokens, total, completed)) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/HttpVerbBasedBundleEntryComponentGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/HttpVerbBasedBundleEntryComponentGenerator.kt index 43a1b0b2f1..3483ff4bfd 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/HttpVerbBasedBundleEntryComponentGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/HttpVerbBasedBundleEntryComponentGenerator.kt @@ -32,7 +32,8 @@ import org.hl7.fhir.r4.model.UriType * more info regarding the supported [Bundle.HTTPVerb]. */ internal abstract class HttpVerbBasedBundleEntryComponentGenerator( - private val httpVerb: Bundle.HTTPVerb + private val httpVerb: Bundle.HTTPVerb, + private val useETagForUpload: Boolean, ) { /** @@ -56,7 +57,17 @@ internal abstract class HttpVerbBasedBundleEntryComponentGenerator( private fun getEntryRequest(localChange: LocalChange) = Bundle.BundleEntryRequestComponent( - Enumeration(Bundle.HTTPVerbEnumFactory()).apply { value = httpVerb }, - UriType("${localChange.resourceType}/${localChange.resourceId}") - ) + Enumeration(Bundle.HTTPVerbEnumFactory()).apply { value = httpVerb }, + UriType("${localChange.resourceType}/${localChange.resourceId}") + ) + .apply { + if (useETagForUpload && !localChange.versionId.isNullOrEmpty()) { + // FHIR supports weak Etag, See ETag section https://hl7.org/fhir/http.html#Http-Headers + when (localChange.type) { + LocalChange.Type.UPDATE, + LocalChange.Type.DELETE -> ifMatch = "W/\"${localChange.versionId}\"" + LocalChange.Type.INSERT -> {} + } + } + } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/HttpVerbBasedBundleEntryComponentImplementations.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/HttpVerbBasedBundleEntryComponentImplementations.kt index 71d8f3bafe..05eb72c68d 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/HttpVerbBasedBundleEntryComponentImplementations.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/HttpVerbBasedBundleEntryComponentImplementations.kt @@ -24,8 +24,8 @@ import org.hl7.fhir.instance.model.api.IBaseResource import org.hl7.fhir.r4.model.Binary import org.hl7.fhir.r4.model.Bundle -internal object HttpPutForCreateEntryComponentGenerator : - HttpVerbBasedBundleEntryComponentGenerator(Bundle.HTTPVerb.PUT) { +internal class HttpPutForCreateEntryComponentGenerator(useETagForUpload: Boolean) : + HttpVerbBasedBundleEntryComponentGenerator(Bundle.HTTPVerb.PUT, useETagForUpload) { override fun getEntryResource(localChange: LocalChange): IBaseResource { return FhirContext.forCached(FhirVersionEnum.R4) .newJsonParser() @@ -33,8 +33,8 @@ internal object HttpPutForCreateEntryComponentGenerator : } } -internal object HttpPatchForUpdateEntryComponentGenerator : - HttpVerbBasedBundleEntryComponentGenerator(Bundle.HTTPVerb.PATCH) { +internal class HttpPatchForUpdateEntryComponentGenerator(useETagForUpload: Boolean) : + HttpVerbBasedBundleEntryComponentGenerator(Bundle.HTTPVerb.PATCH, useETagForUpload) { override fun getEntryResource(localChange: LocalChange): IBaseResource { return Binary().apply { contentType = ContentTypes.APPLICATION_JSON_PATCH @@ -43,7 +43,7 @@ internal object HttpPatchForUpdateEntryComponentGenerator : } } -internal object HttpDeleteEntryComponentGenerator : - HttpVerbBasedBundleEntryComponentGenerator(Bundle.HTTPVerb.DELETE) { +internal class HttpDeleteEntryComponentGenerator(useETagForUpload: Boolean) : + HttpVerbBasedBundleEntryComponentGenerator(Bundle.HTTPVerb.DELETE, useETagForUpload) { override fun getEntryResource(localChange: LocalChange): IBaseResource? = null } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/TransactionBundleGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/TransactionBundleGenerator.kt index 74ca94d926..add93a77a7 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/TransactionBundleGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/TransactionBundleGenerator.kt @@ -51,7 +51,8 @@ internal open class TransactionBundleGenerator( companion object Factory { - fun getDefault() = PutForCreateAndPatchForUpdateBasedTransactionGenerator + fun getDefault(useETagForUpload: Boolean = true) = + PutForCreateAndPatchForUpdateBasedTransactionGenerator(useETagForUpload) /** * Returns a [TransactionBundleGenerator] based on the provided [Bundle.HTTPVerb]s for creating @@ -60,13 +61,14 @@ internal open class TransactionBundleGenerator( */ fun getGenerator( httpVerbToUseForCreate: Bundle.HTTPVerb, - httpVerbToUseForUpdate: Bundle.HTTPVerb + httpVerbToUseForUpdate: Bundle.HTTPVerb, + useETagForUpload: Boolean, ): TransactionBundleGenerator { return if (httpVerbToUseForCreate == Bundle.HTTPVerb.PUT && httpVerbToUseForUpdate == Bundle.HTTPVerb.PATCH ) { - PutForCreateAndPatchForUpdateBasedTransactionGenerator + PutForCreateAndPatchForUpdateBasedTransactionGenerator(useETagForUpload) } else { throw IllegalArgumentException( "Engine currently supports creation using [PUT] and updates using [PATCH]" @@ -76,11 +78,11 @@ internal open class TransactionBundleGenerator( } } -internal object PutForCreateAndPatchForUpdateBasedTransactionGenerator : +internal class PutForCreateAndPatchForUpdateBasedTransactionGenerator(useETagForUpload: Boolean) : TransactionBundleGenerator({ type -> when (type) { - Type.INSERT -> HttpPutForCreateEntryComponentGenerator - Type.UPDATE -> HttpPatchForUpdateEntryComponentGenerator - Type.DELETE -> HttpDeleteEntryComponentGenerator + Type.INSERT -> HttpPutForCreateEntryComponentGenerator(useETagForUpload) + Type.UPDATE -> HttpPatchForUpdateEntryComponentGenerator(useETagForUpload) + Type.DELETE -> HttpDeleteEntryComponentGenerator(useETagForUpload) } }) 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 e2cde8ad94..3c4132fba0 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 @@ -24,10 +24,12 @@ import com.google.android.fhir.FhirEngine import com.google.android.fhir.LocalChange import com.google.android.fhir.db.impl.dao.LocalChangeToken import com.google.android.fhir.search.Search +import com.google.android.fhir.sync.BundleRequest 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.android.fhir.sync.UrlRequest import com.google.common.truth.Truth.assertThat import java.net.SocketTimeoutException import java.time.OffsetDateTime @@ -93,15 +95,13 @@ fun readJsonArrayFromFile(filename: String): JSONArray { object TestDataSourceImpl : DataSource { - override suspend fun download(path: String): Resource { - return Bundle().apply { type = Bundle.BundleType.SEARCHSET } - } - - override suspend fun download(bundle: Bundle): Resource { - return Bundle().apply { type = Bundle.BundleType.BATCHRESPONSE } - } + override suspend fun download(request: Request) = + when (request) { + is UrlRequest -> Bundle().apply { type = Bundle.BundleType.SEARCHSET } + is BundleRequest -> Bundle().apply { type = Bundle.BundleType.BATCHRESPONSE } + } - override suspend fun upload(bundle: Bundle): Resource { + override suspend fun upload(request: BundleRequest): Resource { return Bundle().apply { type = Bundle.BundleType.TRANSACTIONRESPONSE } } } @@ -177,32 +177,28 @@ object TestFhirEngineImpl : FhirEngine { object TestFailingDatasource : DataSource { - override suspend fun download(path: String): Resource { - val allowedChars = ('A'..'Z') + ('a'..'z') + ('0'..'9') - // data size exceeding the bytes acceptable by WorkManager serializer - val dataSize = Data.MAX_DATA_BYTES + 1 - val hugeStackTraceMessage = (1..dataSize).map { allowedChars.random() }.joinToString("") - throw Exception(hugeStackTraceMessage) - } - - override suspend fun download(bundle: Bundle): Resource { - throw SocketTimeoutException("Posting Download Bundle failed...") - } - - override suspend fun upload(bundle: Bundle): Resource { + override suspend fun download(request: Request) = + when (request) { + is UrlRequest -> { + val allowedChars = ('A'..'Z') + ('a'..'z') + ('0'..'9') + // data size exceeding the bytes acceptable by WorkManager serializer + val dataSize = Data.MAX_DATA_BYTES + 1 + val hugeStackTraceMessage = (1..dataSize).map { allowedChars.random() }.joinToString("") + throw Exception(hugeStackTraceMessage) + } + is BundleRequest -> throw SocketTimeoutException("Posting Download Bundle failed...") + } + + override suspend fun upload(request: BundleRequest): Resource { throw SocketTimeoutException("Posting Upload Bundle failed...") } } class BundleDataSource(val onPostBundle: suspend (Bundle) -> Resource) : DataSource { - override suspend fun download(path: String): Resource { - TODO("Not yet implemented") - } - - override suspend fun download(bundle: Bundle): Resource { + override suspend fun download(request: Request): Resource { TODO("Not yet implemented") } - override suspend fun upload(bundle: Bundle) = onPostBundle(bundle) + override suspend fun upload(request: BundleRequest) = onPostBundle(request.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 a09f78d79d..c479ea0b3e 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 @@ -17,10 +17,12 @@ package com.google.android.fhir.sync.download import com.google.android.fhir.logicalId +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.Request +import com.google.android.fhir.sync.UrlRequest import com.google.common.truth.Truth.assertThat import java.util.LinkedList import java.util.Queue @@ -55,7 +57,7 @@ class DownloaderImplTest { val testDataSource: DataSource = object : DataSource { - override suspend fun download(path: String): Resource { + private fun download(path: String): Resource { return when (path) { "Patient" -> Bundle().apply { @@ -92,7 +94,7 @@ class DownloaderImplTest { } } - override suspend fun download(bundle: Bundle): Resource { + private fun download(bundle: Bundle): Resource { return Bundle().apply { type = Bundle.BundleType.BATCHRESPONSE addEntry( @@ -116,7 +118,13 @@ class DownloaderImplTest { } } - override suspend fun upload(bundle: Bundle): Resource { + override suspend fun download(request: Request) = + when (request) { + is UrlRequest -> download(request.url) + is BundleRequest -> download(request.bundle) + } + + override suspend fun upload(request: BundleRequest): Resource { throw UnsupportedOperationException() } } @@ -148,7 +156,7 @@ class DownloaderImplTest { val testDataSource: DataSource = object : DataSource { - override suspend fun download(path: String): Resource { + private fun download(path: String): Resource { return when (path) { "Patient" -> Bundle().apply { @@ -185,7 +193,7 @@ class DownloaderImplTest { } } - override suspend fun download(bundle: Bundle): Resource { + private fun download(bundle: Bundle): Resource { return Bundle().apply { type = Bundle.BundleType.BATCHRESPONSE addEntry( @@ -209,7 +217,13 @@ class DownloaderImplTest { } } - override suspend fun upload(bundle: Bundle): Resource { + override suspend fun download(request: Request) = + when (request) { + is UrlRequest -> download(request.url) + is BundleRequest -> download(request.bundle) + } + + override suspend fun upload(request: BundleRequest): Resource { throw UnsupportedOperationException() } } diff --git a/engine/src/test/java/com/google/android/fhir/sync/remote/RetrofitHttpServiceTest.kt b/engine/src/test/java/com/google/android/fhir/sync/remote/RetrofitHttpServiceTest.kt index 678b8a1b0a..0b0c7f0ea7 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/remote/RetrofitHttpServiceTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/remote/RetrofitHttpServiceTest.kt @@ -76,8 +76,10 @@ class RetrofitHttpServiceTest { } mockWebServer.enqueue(mockResponse) - val result = retrofitHttpService.get("Patient/patient-001") - + val result = + retrofitHttpService.get("Patient/patient-001", mapOf("If-Match" to "randomResourceVersionID")) + val serverRequest = mockWebServer.takeRequest() + assertThat(serverRequest.headers).contains("If-Match" to "randomResourceVersionID") // No exception should occur assertThat(result).isInstanceOf(Patient::class.java) } @@ -104,8 +106,9 @@ class RetrofitHttpServiceTest { type = Bundle.BundleType.TRANSACTION } - val result = retrofitHttpService.post(request) - + val result = retrofitHttpService.post(request, mapOf("If-Match" to "randomResourceVersionID")) + val serverRequest = mockWebServer.takeRequest() + assertThat(serverRequest.headers["If-Match"]).isEqualTo("randomResourceVersionID") // No exception has occurred assertThat(result).isInstanceOf(Bundle::class.java) } @@ -132,7 +135,7 @@ class RetrofitHttpServiceTest { type = Bundle.BundleType.TRANSACTION } - val result = retrofitHttpService.post(request) + val result = retrofitHttpService.post(request, emptyMap()) assertThat(result).isInstanceOf(Bundle::class.java) assertThat((result as Bundle).type).isEqualTo(Bundle.BundleType.TRANSACTIONRESPONSE) diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/TransactionBundleGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/TransactionBundleGeneratorTest.kt index 9e5019cb53..0f65598248 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/TransactionBundleGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/TransactionBundleGeneratorTest.kt @@ -184,7 +184,8 @@ class TransactionBundleGeneratorTest { ) } ) - .toString() + .toString(), + versionId = "v-p002-01" ) .toLocalChange() .apply { LocalChangeToken(listOf(2)) }, @@ -204,7 +205,8 @@ class TransactionBundleGeneratorTest { } ) } - ) + ), + versionId = "v-p003-01" ) .toLocalChange() .apply { LocalChangeToken(listOf(3)) } @@ -221,5 +223,83 @@ class TransactionBundleGeneratorTest { assertThat(result.map { it.first.entry.first().request.method }) .containsExactly(Bundle.HTTPVerb.PUT, Bundle.HTTPVerb.PATCH, Bundle.HTTPVerb.DELETE) .inOrder() + // Insert has no etag related header, update and delete have etag in the request. + assertThat(result.map { it.first.entry.first().request.ifMatch }) + .containsExactly(null, "W/\"v-p002-01\"", "W/\"v-p003-01\"") + .inOrder() } + + @Test + fun `generate() should return Bundle Entry without if-match when useETagForUpload is false`() = + runBlocking { + val changes = + listOf( + LocalChangeEntity( + id = 1, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Type.UPDATE, + payload = "[]", + versionId = "patient-002-version-1" + ) + .toLocalChange() + ) + val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = false) + val result = generator.generate(changes.chunked(1)) + + assertThat(result.first().first.entry.first().request.ifMatch).isNull() + } + + @Test + fun `generate() should return Bundle Entry with if-match when useETagForUpload is true`() = + runBlocking { + val changes = + listOf( + LocalChangeEntity( + id = 1, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Type.UPDATE, + payload = "[]", + versionId = "patient-002-version-1" + ) + .toLocalChange() + ) + val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = true) + val result = generator.generate(changes.chunked(1)) + + assertThat(result.first().first.entry.first().request.ifMatch) + .isEqualTo("W/\"patient-002-version-1\"") + } + + @Test + fun `generate() should return Bundle Entry without if-match when the LocalChangeEntity has no versionId`() = + runBlocking { + val changes = + listOf( + LocalChangeEntity( + id = 1, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-002", + type = Type.UPDATE, + payload = "[]", + versionId = "" + ) + .toLocalChange(), + LocalChangeEntity( + id = 1, + resourceType = ResourceType.Patient.name, + resourceId = "Patient-003", + type = Type.UPDATE, + payload = "[]", + versionId = null + ) + .toLocalChange() + ) + val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = true) + val result = generator.generate(changes.chunked(2)) + + assertThat(result.first().first.entry[0].request.ifMatch).isNull() + assertThat(result.first().first.entry[1].request.ifMatch).isNull() + } }