Update the version of resource after updates are downloaded from the …

…server (#2272)

* Update the version of resource after updates are downloaded from the server

* Review changes: Added tests and refactored code

* Review comments updates

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

@@ -1,5 +1,5 @@
 /*
- * Copyright 2022 Google LLC
+ * Copyright 2022-2023 Google LLC
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -57,7 +57,7 @@ fun <R : Resource> getResourceClass(resourceType: String): Class<R> {
  return Class.forName(R4_RESOURCE_PACKAGE_PREFIX + className) as Class<R>
 }
 
-internal val Resource.versionId
+internal val Resource.versionId: String?
  get() = meta.versionId
 
 internal val Resource.lastUpdated

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

@@ -140,7 +140,7 @@ internal class DatabaseImpl(
  resources.forEach {
  val timeOfLocalChange = Instant.now()
  val oldResourceEntity = selectEntity(it.resourceType, it.logicalId)
- resourceDao.update(it, timeOfLocalChange)
+ resourceDao.applyLocalUpdate(it, timeOfLocalChange)
  localChangeDao.addUpdate(oldResourceEntity, it, timeOfLocalChange)
  }
  }

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

@@ -58,43 +58,69 @@ internal abstract class ResourceDao {
  lateinit var iParser: IParser
  lateinit var resourceIndexer: ResourceIndexer
 
- open suspend fun update(resource: Resource, timeOfLocalChange: Instant?) {
+ /**
+ * Updates the resource in the [ResourceEntity] and adds indexes as a result of changes made on
+ * device.
+ *
+ * @param [resource] the resource with local (on device) updates
+ * @param [timeOfLocalChange] time when the local change was made
+ */
+ suspend fun applyLocalUpdate(resource: Resource, timeOfLocalChange: Instant?) {
  getResourceEntity(resource.logicalId, resource.resourceType)?.let {
- // In case the resource has lastUpdated meta data, use it, otherwise use the old value.
- val lastUpdatedRemote: Date? = resource.meta.lastUpdated
  val entity =
  it.copy(
  serializedResource = iParser.encodeResourceToString(resource),
  lastUpdatedLocal = timeOfLocalChange,
- lastUpdatedRemote = lastUpdatedRemote?.toInstant() ?: it.lastUpdatedRemote,
  )
- // The foreign key in Index entity tables is set with cascade delete constraint and
- // insertResource has REPLACE conflict resolution. So, when we do an insert to update the
- // resource, it deletes old resource and corresponding index entities (based on foreign key
- // constrain) before inserting the new resource.
- insertResource(entity)
- val index =
- ResourceIndices.Builder(resourceIndexer.index(resource))
- .apply {
- timeOfLocalChange?.let {
- addDateTimeIndex(
- createLocalLastUpdatedIndex(
- resource.resourceType,
- InstantType(Date.from(timeOfLocalChange)),
- ),
- )
- }
- lastUpdatedRemote?.let { date ->
- addDateTimeIndex(createLastUpdatedIndex(resource.resourceType, InstantType(date)))
- }
- }
- .build()
- updateIndicesForResource(index, resource.resourceType, it.resourceUuid)
+ updateChanges(entity, resource)
  }
  ?: throw ResourceNotFoundException(resource.resourceType.name, resource.id)
  }
 
- open suspend fun insertAllRemote(resources: List<Resource>): List<UUID> {
+ /**
+ * Updates the resource in the [ResourceEntity] and adds indexes as a result of downloading the
+ * resource from server.
+ *
+ * @param [resource] the resource with the remote(server) updates
+ */
+ private suspend fun applyRemoteUpdate(resource: Resource) {
+ getResourceEntity(resource.logicalId, resource.resourceType)?.let {
+ val entity =
+ it.copy(
+ serializedResource = iParser.encodeResourceToString(resource),
+ lastUpdatedRemote = resource.meta.lastUpdated?.toInstant(),
+ versionId = resource.versionId,
+ )
+ updateChanges(entity, resource)
+ }
+ ?: throw ResourceNotFoundException(resource.resourceType.name, resource.id)
+ }
+
+ private suspend fun updateChanges(entity: ResourceEntity, resource: Resource) {
+ // The foreign key in Index entity tables is set with cascade delete constraint and
+ // insertResource has REPLACE conflict resolution. So, when we do an insert to update the
+ // resource, it deletes old resource and corresponding index entities (based on foreign key
+ // constrain) before inserting the new resource.
+ insertResource(entity)
+ val index =
+ ResourceIndices.Builder(resourceIndexer.index(resource))
+ .apply {
+ entity.lastUpdatedLocal?.let { instant ->
+ addDateTimeIndex(
+ createLocalLastUpdatedIndex(resource.resourceType, InstantType(Date.from(instant))),
+ )
+ }
+ entity.lastUpdatedRemote?.let { instant ->
+ addDateTimeIndex(
+ createLastUpdatedIndex(resource.resourceType, InstantType(Date.from(instant))),
+ )
+ }
+ }
+ .build()
+ updateIndicesForResource(index, resource.resourceType, entity.resourceUuid)
+ }
+
+ suspend fun insertAllRemote(resources: List<Resource>): List<UUID> {
  return resources.map { resource -> insertRemoteResource(resource) }
  }
 
@@ -189,7 +215,7 @@ internal abstract class ResourceDao {
  private suspend fun insertRemoteResource(resource: Resource): UUID {
  val existingResourceEntity = getResourceEntity(resource.logicalId, resource.resourceType)
  if (existingResourceEntity != null) {
- update(resource, existingResourceEntity.lastUpdatedLocal)
+ applyRemoteUpdate(resource)
  return existingResourceEntity.resourceUuid
  }
  return insertResource(resource, null)

engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt

@@ -24,6 +24,7 @@ import com.google.android.fhir.LocalChange.Type
 import com.google.android.fhir.LocalChangeToken
 import com.google.android.fhir.db.ResourceNotFoundException
 import com.google.android.fhir.get
+import com.google.android.fhir.lastUpdated
 import com.google.android.fhir.logicalId
 import com.google.android.fhir.search.LOCAL_LAST_UPDATED_PARAM
 import com.google.android.fhir.search.search
@@ -36,7 +37,9 @@ import com.google.android.fhir.sync.upload.UploadSyncResult
 import com.google.android.fhir.testing.assertResourceEquals
 import com.google.android.fhir.testing.assertResourceNotEquals
 import com.google.android.fhir.testing.readFromFile
+import com.google.android.fhir.versionId
 import com.google.common.truth.Truth.assertThat
+import java.time.Instant
 import java.util.Date
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.flowOf
@@ -592,6 +595,87 @@ class FhirEngineImplTest {
  assertResourceEquals(fhirEngine.get<Patient>("original-002"), localChange)
  }
 
+ @Test
+ fun `syncDownload ResourceEntity should have the latest versionId and lastUpdated from server`() =
+ runBlocking {
+ val originalPatient =
+ Patient().apply {
+ id = "original-002"
+ meta =
+ Meta().apply {
+ versionId = "1"
+ lastUpdated = Date.from(Instant.parse("2022-12-02T10:15:30.00Z"))
+ }
+ addName(
+ HumanName().apply {
+ family = "Stark"
+ addGiven("Tony")
+ },
+ )
+ }
+ // First sync
+ fhirEngine.syncDownload(AcceptLocalConflictResolver) { flowOf((listOf((originalPatient)))) }
+
+ val updatedPatient =
+ originalPatient.copy().apply {
+ meta =
+ Meta().apply {
+ versionId = "2"
+ lastUpdated = Date.from(Instant.parse("2022-12-03T10:15:30.00Z"))
+ }
+ addAddress(Address().apply { country = "USA" })
+ }
+
+ // Sync to get updates from server
+ fhirEngine.syncDownload(AcceptLocalConflictResolver) { flowOf((listOf(updatedPatient))) }
+
+ val result = services.database.selectEntity(ResourceType.Patient, "original-002")
+ assertThat(result.versionId).isEqualTo(updatedPatient.versionId)
+ assertThat(result.lastUpdatedRemote).isEqualTo(updatedPatient.lastUpdated)
+ }
+
+ @Test
+ fun `syncDownload LocalChangeEntity should have the latest versionId from server`() =
+ runBlocking {
+ val originalPatient =
+ Patient().apply {
+ id = "original-002"
+ meta =
+ Meta().apply {
+ versionId = "1"
+ lastUpdated = Date.from(Instant.parse("2022-12-02T10:15:30.00Z"))
+ }
+ addName(
+ HumanName().apply {
+ family = "Stark"
+ addGiven("Tony")
+ },
+ )
+ }
+ // First sync
+ fhirEngine.syncDownload(AcceptLocalConflictResolver) { flowOf((listOf((originalPatient)))) }
+
+ val localChange =
+ originalPatient.copy().apply { addAddress(Address().apply { city = "Malibu" }) }
+ fhirEngine.update(localChange)
+
+ val updatedPatient =
+ originalPatient.copy().apply {
+ meta =
+ Meta().apply {
+ versionId = "2"
+ lastUpdated = Date.from(Instant.parse("2022-12-03T10:15:30.00Z"))
+ }
+ addAddress(Address().apply { country = "USA" })
+ }
+
+ // Sync to get updates from server
+ fhirEngine.syncDownload(AcceptLocalConflictResolver) { flowOf((listOf(updatedPatient))) }
+
+ val result = fhirEngine.getLocalChanges(ResourceType.Patient, "original-002").first()
+ assertThat(result.versionId).isEqualTo(updatedPatient.versionId)
+ }
+
  @Test
  fun `create should allow patient search with LOCAL_LAST_UPDATED_PARAM`(): Unit = runBlocking {
  val patient = Patient().apply { id = "patient-id-create" }