Store remote version id for the resources and provide the version id …

…for local change entity (google#1127)

* Store remote version id for the resources and provide the version id for local change entity

* Review comment changes

* Added additional test case

* Updated the Database api to accept non-nullable version and lastupdate values

* review comment: updated variable names

engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt

@@ -26,20 +26,19 @@ import com.google.android.fhir.db.ResourceNotFoundException
 import com.google.android.fhir.db.impl.entities.LocalChangeEntity
 import com.google.android.fhir.logicalId
 import com.google.android.fhir.resource.TestingUtils
+import com.google.android.fhir.resource.versionId
 import com.google.android.fhir.search.Operation
 import com.google.android.fhir.search.Order
 import com.google.android.fhir.search.Search
 import com.google.android.fhir.search.StringFilterModifier
 import com.google.android.fhir.search.getQuery
 import com.google.android.fhir.search.has
-import com.google.android.fhir.sync.DataSource
 import com.google.common.truth.Truth.assertThat
 import java.math.BigDecimal
 import java.time.Instant
-import kotlin.collections.ArrayList
+import java.util.Date
 import kotlinx.coroutines.runBlocking
 import org.hl7.fhir.r4.model.Address
-import org.hl7.fhir.r4.model.Bundle
 import org.hl7.fhir.r4.model.CarePlan
 import org.hl7.fhir.r4.model.CodeableConcept
 import org.hl7.fhir.r4.model.Coding
@@ -50,8 +49,8 @@ import org.hl7.fhir.r4.model.DecimalType
 import org.hl7.fhir.r4.model.Enumerations
 import org.hl7.fhir.r4.model.HumanName
 import org.hl7.fhir.r4.model.Immunization
+import org.hl7.fhir.r4.model.Meta
 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.Practitioner
 import org.hl7.fhir.r4.model.Quantity
@@ -81,33 +80,6 @@ class DatabaseImplTest {
  /** Whether to run the test with encryption on or off. */
  @JvmField @Parameterized.Parameter(0) var encrypted: Boolean = false
 
- private val dataSource =
- object : DataSource {
-
- override suspend fun loadData(path: String): Bundle {
- return Bundle()
- }
-
- override suspend fun insert(
- resourceType: String,
- resourceId: String,
- payload: String
- ): Resource {
- return Patient()
- }
-
- override suspend fun update(
- resourceType: String,
- resourceId: String,
- payload: String
- ): OperationOutcome {
- return OperationOutcome()
- }
-
- override suspend fun delete(resourceType: String, resourceId: String): OperationOutcome {
- return OperationOutcome()
- }
- }
  private val context: Context = ApplicationProvider.getApplicationContext()
  private val services =
  FhirServices.builder(context)
@@ -231,6 +203,49 @@ class DatabaseImplTest {
  assertThat(payload).isEqualTo(patientString)
  }
 
+ @Test
+ fun update_remoteResourceWithLocalChange_shouldSaveVersionIdAndLastUpdated() = runBlocking {
+ val patient =
+ Patient().apply {
+ id = "remote-patient-1"
+ addName(
+ HumanName().apply {
+ family = "FamilyName"
+ addGiven("FirstName")
+ }
+ )
+ meta =
+ Meta().apply {
+ versionId = "remote-patient-1-version-001"
+ lastUpdated = Date()
+ }
+ }
+
+ database.insertRemote(patient)
+
+ val updatedPatient =
+ Patient().apply {
+ id = "remote-patient-1"
+ addName(
+ HumanName().apply {
+ family = "UpdatedFamilyName"
+ addGiven("UpdatedFirstName")
+ }
+ )
+ }
+ database.update(updatedPatient)
+
+ val selectedEntity = database.selectEntity(Patient::class.java, "remote-patient-1")
+ assertThat(selectedEntity.resourceId).isEqualTo("remote-patient-1")
+ assertThat(selectedEntity.versionId).isEqualTo(patient.meta.versionId)
+ assertThat(selectedEntity.lastUpdatedRemote).isEqualTo(patient.meta.lastUpdated.toInstant())
+
+ val squashedLocalChange =
+ database.getAllLocalChanges().first { it.localChange.resourceId == "remote-patient-1" }
+ assertThat(squashedLocalChange.localChange.resourceId).isEqualTo("remote-patient-1")
+ assertThat(squashedLocalChange.localChange.versionId).isEqualTo(patient.meta.versionId)
+ }
+
  @Test
  fun delete_shouldAddDeleteLocalChange() = runBlocking {
  database.delete(Patient::class.java, TEST_PATIENT_1_ID)
@@ -285,6 +300,64 @@ class DatabaseImplTest {
  .isTrue()
  }
 
+ @Test
+ fun insert_remoteResource_shouldSaveVersionIdAndLastUpdated() = runBlocking {
+ val patient =
+ Patient().apply {
+ id = "remote-patient-1"
+ meta =
+ Meta().apply {
+ versionId = "remote-patient-1-version-1"
+ lastUpdated = Date()
+ }
+ }
+ database.insertRemote(patient)
+ val selectedEntity = database.selectEntity(Patient::class.java, "remote-patient-1")
+ assertThat(selectedEntity.versionId).isEqualTo("remote-patient-1-version-1")
+ assertThat(selectedEntity.lastUpdatedRemote).isEqualTo(patient.meta.lastUpdated.toInstant())
+ }
+
+ @Test
+ fun insert_remoteResourceWithNoMeta_shouldSaveNullRemoteVersionAndLastUpdated() = runBlocking {
+ val patient = Patient().apply { id = "remote-patient-2" }
+ database.insertRemote(patient)
+ val selectedEntity = database.selectEntity(Patient::class.java, "remote-patient-2")
+ assertThat(selectedEntity.versionId).isNull()
+ assertThat(selectedEntity.lastUpdatedRemote).isNull()
+ }
+
+ @Test
+ fun insert_localResourceWithNoMeta_shouldSaveNullRemoteVersionAndLastUpdated() = runBlocking {
+ val patient = Patient().apply { id = "local-patient-2" }
+ database.insert(patient)
+ val selectedEntity = database.selectEntity(Patient::class.java, "local-patient-2")
+ assertThat(selectedEntity.versionId).isNull()
+ assertThat(selectedEntity.lastUpdatedRemote).isNull()
+ }
+
+ @Test
+ fun insert_localResourceWithNoMetaAndSync_shouldSaveRemoteVersionAndLastUpdated() = runBlocking {
+ val patient = Patient().apply { id = "remote-patient-3" }
+ val remoteMeta =
+ Meta().apply {
+ versionId = "remote-patient-3-version-001"
+ lastUpdated = Date()
+ }
+ database.insert(patient)
+ services.fhirEngine.syncUpload {
+ it.map {
+ it.token to
+ Patient().apply {
+ id = it.localChange.resourceId
+ meta = remoteMeta
+ }
+ }
+ }
+ val selectedEntity = database.selectEntity(Patient::class.java, "remote-patient-3")
+ assertThat(selectedEntity.versionId).isEqualTo(remoteMeta.versionId)
+ assertThat(selectedEntity.lastUpdatedRemote).isEqualTo(remoteMeta.lastUpdated.toInstant())
+ }
+
  @Test
  fun insertAll_remoteResources_shouldNotInsertAnyLocalChange() = runBlocking {
  val patient: Patient = testingUtils.readFromFile(Patient::class.java, "/date_test_patient.json")
@@ -318,35 +391,44 @@ class DatabaseImplTest {
 
  @Test
  fun updateTwice_remoteResource_readSquashedChanges_shouldReturnMergedPatch() = runBlocking {
+ val remoteMeta =
+ Meta().apply {
+ versionId = "patient-version-1"
+ lastUpdated = Date()
+ }
  var patient: Patient = testingUtils.readFromFile(Patient::class.java, "/date_test_patient.json")
+ patient.meta = remoteMeta
  database.insertRemote(patient)
  patient = testingUtils.readFromFile(Patient::class.java, "/update_test_patient_1.json")
  database.update(patient)
  patient = testingUtils.readFromFile(Patient::class.java, "/update_test_patient_2.json")
  database.update(patient)
  val updatePatch = testingUtils.readJsonArrayFromFile("/update_patch_2.json")
- val (_, resourceType, resourceId, _, type, payload) =
+ val (_, resourceType, resourceId, _, type, payload, versionId) =
  database
  .getAllLocalChanges()
  .single { it.localChange.resourceId.equals(patient.logicalId) }
  .localChange
  assertThat(type).isEqualTo(LocalChangeEntity.Type.UPDATE)
  assertThat(resourceId).isEqualTo(patient.logicalId)
  assertThat(resourceType).isEqualTo(patient.resourceType.name)
+ assertThat(resourceType).isEqualTo(patient.resourceType.name)
+ assertThat(versionId).isEqualTo(remoteMeta.versionId)
  testingUtils.assertJsonArrayEqualsIgnoringOrder(JSONArray(payload), updatePatch)
  }
 
  @Test
  fun delete_remoteResource_shouldReturnDeleteLocalChange() = runBlocking {
  database.insertRemote(TEST_PATIENT_2)
  database.delete(Patient::class.java, TEST_PATIENT_2_ID)
- val (_, resourceType, resourceId, _, type, payload) =
+ val (_, resourceType, resourceId, _, type, payload, versionId) =
  database.getAllLocalChanges().map { it.localChange }.single {
  it.resourceId.equals(TEST_PATIENT_2_ID)
  }
  assertThat(type).isEqualTo(LocalChangeEntity.Type.DELETE)
  assertThat(resourceId).isEqualTo(TEST_PATIENT_2_ID)
  assertThat(resourceType).isEqualTo(TEST_PATIENT_2.resourceType.name)
+ assertThat(versionId).isEqualTo(TEST_PATIENT_2.versionId)
  assertThat(payload).isEmpty()
  }
 
@@ -2562,6 +2644,7 @@ class DatabaseImplTest {
  val TEST_PATIENT_1 = Patient()
 
  init {
+ TEST_PATIENT_1.meta.id = "v1-of-patient1"
  TEST_PATIENT_1.setId(TEST_PATIENT_1_ID)
  TEST_PATIENT_1.setGender(Enumerations.AdministrativeGender.MALE)
  }
@@ -2570,6 +2653,7 @@ class DatabaseImplTest {
  val TEST_PATIENT_2 = Patient()
 
  init {
+ TEST_PATIENT_2.meta.id = "v1-of-patient2"
  TEST_PATIENT_2.setId(TEST_PATIENT_2_ID)
  TEST_PATIENT_2.setGender(Enumerations.AdministrativeGender.MALE)
  }

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

@@ -67,12 +67,13 @@ interface FhirEngine {
  * Synchronizes the [upload] result in the database. The database will be updated to reflect the
  * result of the [upload] operation.
  */
- suspend fun syncUpload(upload: (suspend (List<SquashedLocalChange>) -> List<LocalChangeToken>))
+ suspend fun syncUpload(
+ upload: (suspend (List<SquashedLocalChange>) -> List<Pair<LocalChangeToken, Resource>>)
+ )
 
  /**
  * Synchronizes the [download] result in the database. The database will be updated to reflect the
- * result of the [download] operation. [onPageDownloaded] is called with the resources after each
- * successful download of page.
+ * result of the [download] operation.
  */
  suspend fun syncDownload(download: suspend (SyncDownloadContext) -> Flow<List<Resource>>)
 

engine/src/main/java/com/google/android/fhir/db/Database.kt

@@ -19,8 +19,10 @@ package com.google.android.fhir.db
 import com.google.android.fhir.db.impl.dao.LocalChangeToken
 import com.google.android.fhir.db.impl.dao.SquashedLocalChange
 import com.google.android.fhir.db.impl.entities.LocalChangeEntity
+import com.google.android.fhir.db.impl.entities.ResourceEntity
 import com.google.android.fhir.db.impl.entities.SyncedResourceEntity
 import com.google.android.fhir.search.SearchQuery
+import java.time.Instant
 import org.hl7.fhir.r4.model.Resource
 import org.hl7.fhir.r4.model.ResourceType
 
@@ -50,6 +52,14 @@ internal interface Database {
  */
  suspend fun <R : Resource> update(resource: R)
 
+ /** Updates the `resource` meta in the FHIR resource database. */
+ suspend fun updateVersionIdAndLastUpdated(
+ resourceId: String,
+ resourceType: ResourceType,
+ versionId: String,
+ lastUpdated: Instant
+ )
+
  /**
  * Selects the FHIR resource of type `clazz` with `id`.
  *
@@ -59,6 +69,15 @@ internal interface Database {
  @Throws(ResourceNotFoundException::class)
  suspend fun <R : Resource> select(clazz: Class<R>, id: String): R
 
+ /**
+ * Selects the saved `ResourceEntity` of type `clazz` with `id`.
+ *
+ * @param <R> The resource type
+ * @throws ResourceNotFoundException if the resource is not found in the database
+ */
+ @Throws(ResourceNotFoundException::class)
+ suspend fun <R : Resource> selectEntity(clazz: Class<R>, id: String): ResourceEntity
+
  /**
  * Return the last update data of a resource based on the resource type. If no resource of
  * [resourceType] is inserted, return `null`.

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

@@ -29,10 +29,12 @@ import com.google.android.fhir.db.impl.dao.LocalChangeToken
 import com.google.android.fhir.db.impl.dao.LocalChangeUtils
 import com.google.android.fhir.db.impl.dao.SquashedLocalChange
 import com.google.android.fhir.db.impl.entities.LocalChangeEntity
+import com.google.android.fhir.db.impl.entities.ResourceEntity
 import com.google.android.fhir.db.impl.entities.SyncedResourceEntity
 import com.google.android.fhir.logicalId
 import com.google.android.fhir.resource.getResourceType
 import com.google.android.fhir.search.SearchQuery
+import java.time.Instant
 import org.hl7.fhir.r4.model.Resource
 import org.hl7.fhir.r4.model.ResourceType
 
@@ -111,9 +113,25 @@ internal class DatabaseImpl(
 
  override suspend fun <R : Resource> update(resource: R) {
  db.withTransaction {
- val oldResource = select(resource.javaClass, resource.logicalId)
+ val oldResourceEntity = selectEntity(resource.javaClass, resource.logicalId)
  resourceDao.update(resource)
- localChangeDao.addUpdate(oldResource, resource)
+ localChangeDao.addUpdate(oldResourceEntity, resource)
+ }
+ }
+
+ override suspend fun updateVersionIdAndLastUpdated(
+ resourceId: String,
+ resourceType: ResourceType,
+ versionId: String,
+ lastUpdated: Instant
+ ) {
+ db.withTransaction {
+ resourceDao.updateRemoteVersionIdAndLastUpdate(
+ resourceId,
+ resourceType,
+ versionId,
+ lastUpdated
+ )
  }
  }
 
@@ -143,9 +161,20 @@ internal class DatabaseImpl(
 
  override suspend fun <R : Resource> delete(clazz: Class<R>, id: String) {
  db.withTransaction {
+ val remoteVersionId: String? =
+ try {
+ selectEntity(clazz, id).versionId
+ } catch (e: ResourceNotFoundException) {
+ null
+ }
  val type = getResourceType(clazz)
  val rowsDeleted = resourceDao.deleteResource(resourceId = id, resourceType = type)
- if (rowsDeleted > 0) localChangeDao.addDelete(resourceId = id, resourceType = type)
+ if (rowsDeleted > 0)
+ localChangeDao.addDelete(
+ resourceId = id,
+ resourceType = type,
+ remoteVersionId = remoteVersionId
+ )
  }
  }
 
@@ -180,6 +209,14 @@ internal class DatabaseImpl(
  db.withTransaction { localChangeDao.discardLocalChanges(token) }
  }
 
+ override suspend fun <R : Resource> selectEntity(clazz: Class<R>, id: String): ResourceEntity {
+ return db.withTransaction {
+ val type = getResourceType(clazz)
+ resourceDao.getResourceEntity(resourceId = id, resourceType = type)
+ ?: throw ResourceNotFoundException(type.name, id)
+ }
+ }
+
  companion object {
  /**
  * The name for unencrypted database.