Review changes

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,

engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt

@@ -184,6 +184,9 @@ internal class FhirEngineImpl(private val database: Database, private val contex
  * 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 {

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,
 )

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

@@ -45,6 +45,42 @@ interface DownloadWorkManager {
  suspend fun processResponse(response: Resource): Collection<Resource>
 }
 
+/**
+ * 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<String, String>) {
  companion object {
  /** @return [UrlRequest] for a FHIR search [url]. */
@@ -60,15 +96,15 @@ sealed class Request(open val headers: Map<String, String>) {
  * A [url] based FHIR request to download resources from the server. e.g.
  * `Patient?given=valueGiven&family=valueFamily`
  */
-data class UrlRequest(val url: String, override val headers: Map<String, String> = emptyMap()) :
+data class UrlRequest
+internal constructor(val url: String, override val headers: Map<String, String> = 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,
- override val headers: Map<String, String> = emptyMap()
-) : Request(headers)
+data class BundleRequest
+internal constructor(val bundle: Bundle, override val headers: Map<String, String> = emptyMap()) :
+ Request(headers)

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)
 

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,
 ) {
 
  /**
@@ -60,11 +61,13 @@ internal abstract class HttpVerbBasedBundleEntryComponentGenerator(
  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}\""
- LocalChange.Type.INSERT -> {}
+ if (useETagForUpload) {
+ // 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 -> {}
+ }
  }
  }
 }

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

@@ -24,17 +24,17 @@ 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()
  .parseResource(localChange.payload)
  }
 }
 
-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
 }

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)
  }
  })

engine/src/test/java/com/google/android/fhir/sync/upload/TransactionBundleGeneratorTest.kt

@@ -228,4 +228,52 @@ class TransactionBundleGeneratorTest {
  .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 jsonParser = FhirContext.forCached(FhirVersionEnum.R4).newJsonParser()
+ val changes =
+ listOf(
+ LocalChangeEntity(
+ id = 1,
+ resourceType = ResourceType.Patient.name,
+ resourceId = "Patient-002",
+ type = Type.UPDATE,
+ payload =
+ LocalChangeUtils.diff(
+ jsonParser,
+ Patient().apply {
+ id = "Patient-002"
+ addName(
+ HumanName().apply {
+ addGiven("Jane")
+ family = "Doe"
+ }
+ )
+ },
+ Patient().apply {
+ id = "Patient-002"
+ addName(
+ HumanName().apply {
+ addGiven("Janet")
+ family = "Doe"
+ }
+ )
+ }
+ )
+ .toString(),
+ versionId = "v-p002-01"
+ )
+ .toLocalChange()
+ .apply { LocalChangeToken(listOf(2)) }
+ )
+ val generator = TransactionBundleGenerator.Factory.getDefault(useETagForUpload = false)
+ val result = generator.generate(changes.chunked(1))
+
+ assertThat(result).hasSize(1)
+ assertThat(result.first().first.type).isEqualTo(Bundle.BundleType.TRANSACTION)
+ assertThat(result.first().first.entry.first().request.method).isEqualTo(Bundle.HTTPVerb.PATCH)
+ assertThat(result.first().first.entry.first().request.ifMatch).isNull()
+ }
 }