Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for custom headers in the download http requests and use ETags for upload #2009

Merged
merged 8 commits into from
Jun 11, 2023
Next Next commit
Added header and etag support
  • Loading branch information
aditya-07 committed May 18, 2023
commit 14e12e15a8bdca943951ebaccef2924c25de2f7a
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
}
Expand All @@ -179,6 +179,17 @@ 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.
*/
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
private fun getVersionFromETag(eTag: String) =
if (eTag.startsWith("W/")) {
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
eTag.split("\"")[1]
} else {
eTag
}

/**
* May return a Pair of versionId and resource type extracted from the
* [Bundle.BundleEntryResponseComponent.location].
Expand Down
16 changes: 3 additions & 13 deletions engine/src/main/java/com/google/android/fhir/sync/DataSource.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,25 +45,30 @@ interface DownloadWorkManager {
suspend fun processResponse(response: Resource): Collection<Resource>
}

sealed class Request {
sealed class Request(open val headers: Map<String, String>) {
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
companion object {
/** @return [UrlRequest] for a FHIR search [url]. */
fun of(url: String) = UrlRequest(url)
fun of(url: String, headers: Map<String, String> = emptyMap()) = UrlRequest(url, headers)

/** @return [BundleRequest] for a FHIR search [bundle]. */
fun of(bundle: Bundle) = BundleRequest(bundle)
fun of(bundle: Bundle, headers: Map<String, String> = emptyMap()) =
BundleRequest(bundle, headers)
}
}

/**
* 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(val url: String, override val headers: Map<String, String> = emptyMap()) :
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
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(
val bundle: Bundle,
override val headers: Map<String, String> = emptyMap()
) : Request(headers)
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
Expand All @@ -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 ->
Expand All @@ -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 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,23 @@

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.
* @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(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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, 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
suspend fun post(bundle: Bundle, headers: Map<String, String>): Resource
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String, String>): Resource

@POST(".") override suspend fun post(@Body bundle: Bundle): Resource
@POST(".")
override suspend fun post(@Body bundle: Bundle, @HeaderMap headers: Map<String, String>): Resource

class Builder(
private val baseUrl: String,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,15 @@ 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 {
// 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}\""
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
LocalChange.Type.INSERT -> {}
}
}
}
50 changes: 23 additions & 27 deletions engine/src/main/java/com/google/android/fhir/testing/Utilities.kt
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 }
}
}
Expand Down Expand Up @@ -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)
}
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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()
}
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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(
Expand All @@ -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()
}
}
Expand Down
Loading
Loading