Adding resource UUID in LocalChangeEntity

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

@@ -467,6 +467,21 @@ class DatabaseImplTest {
       .isTrue()
   }
 
+  @Test
+  fun insert_existingRemoteResource_shouldNotChangeResourceEntityUuidOrId() = runBlocking {
+    val patient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json")
+    database.insertRemote(patient)
+    val patientEntityAfterFirstRemoteSync =
+      database.selectEntity(ResourceType.Patient, patient.logicalId)
+    database.insertRemote(patient)
+    val patientEntityAfterSecondRemoteSync =
+      database.selectEntity(ResourceType.Patient, patient.logicalId)
+    assertThat(patientEntityAfterSecondRemoteSync.resourceUuid)
+      .isEqualTo(patientEntityAfterFirstRemoteSync.resourceUuid)
+    assertThat(patientEntityAfterSecondRemoteSync.id)
+      .isEqualTo(patientEntityAfterFirstRemoteSync.id)
+  }
+
   @Test
   fun insert_remoteResource_shouldSaveVersionIdAndLastUpdated() = runBlocking {
     val patient =

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

@@ -163,7 +163,7 @@ class ResourceDatabaseMigrationTest {
       close()
     }
 
-    // Re-open the database with version 4 and provide MIGRATION_3_4 as the migration process.
+    // Re-open the database with version 5 and provide MIGRATION_4_5 as the migration process.
     helper.runMigrationsAndValidate(DB_NAME, 5, true, MIGRATION_4_5)
 
     val retrievedTask: String?
@@ -213,7 +213,7 @@ class ResourceDatabaseMigrationTest {
     val localChangeEntityCorruptedTimeStamp: Long
 
     getMigratedRoomDatabase().apply {
-      retrievedTask = this.resourceDao().getResource(taskId, ResourceType.Task)
+       retrievedTask = this.resourceDao().getResource(taskId, ResourceType.Task)
       resourceEntityLastUpdatedLocal =
         query("Select lastUpdatedLocal from ResourceEntity", null).let {
           it.moveToFirst()
@@ -235,13 +235,72 @@ class ResourceDatabaseMigrationTest {
     assertThat(Instant.ofEpochMilli(localChangeEntityCorruptedTimeStamp)).isEqualTo(Instant.EPOCH)
   }
 
+  @Test
+  fun migrate6To7_should_execute_with_no_exception(): Unit = runBlocking {
+    val taskId = "bed-net-001"
+    val taskResourceUuid = "e2c79e28-ed4d-4029-a12c-108d1eb5bedb"
+    val bedNetTask: String =
+      Task()
+        .apply {
+          id = taskId
+          description = "Issue bed net"
+          meta.lastUpdated = Date()
+        }
+        .let { iParser.encodeResourceToString(it) }
+
+    helper.createDatabase(DB_NAME, 6).apply {
+      val date = Date()
+      execSQL(
+        "INSERT INTO ResourceEntity (resourceUuid, resourceType, resourceId, serializedResource, lastUpdatedLocal) VALUES ('$taskResourceUuid', 'Task', '$taskId', '$bedNetTask', '${DbTypeConverters.instantToLong(date.toInstant())}' );",
+      )
+
+      execSQL(
+        "INSERT INTO LocalChangeEntity (resourceType, resourceId, timestamp, type, payload) VALUES ('Task', '$taskId', '${date.toTimeZoneString()}', '${DbTypeConverters.localChangeTypeToInt(LocalChangeEntity.Type.INSERT)}', '$bedNetTask'  );",
+      )
+      close()
+    }
+
+    helper.runMigrationsAndValidate(DB_NAME, 7, true, MIGRATION_6_7)
+
+    val retrievedTaskResourceId: String?
+    val retrievedTaskResourceUuid: String?
+    val localChangeResourceUuid: String?
+    val localChangeResourceId: String?
+
+    getMigratedRoomDatabase().apply {
+      query("SELECT resourceId, resourceUuid FROM ResourceEntity", null).let {
+        it.moveToFirst()
+        retrievedTaskResourceId = it.getString(0)
+        retrievedTaskResourceUuid = String(it.getBlob(1), Charsets.UTF_8)
+      }
+
+      query("SELECT resourceId,resourceUuid FROM LocalChangeEntity", null).let {
+        it.moveToFirst()
+        localChangeResourceId = it.getString(0)
+        localChangeResourceUuid = String(it.getBlob(1), Charsets.UTF_8)
+      }
+
+      openHelper.writableDatabase.close()
+    }
+
+    assertThat(retrievedTaskResourceUuid).isEqualTo(localChangeResourceUuid)
+    assertThat(localChangeResourceId).isEqualTo(retrievedTaskResourceId)
+  }
+
   private fun getMigratedRoomDatabase(): ResourceDatabase =
     Room.databaseBuilder(
         InstrumentationRegistry.getInstrumentation().targetContext,
         ResourceDatabase::class.java,
         DB_NAME,
       )
-      .addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6)
+      .addMigrations(
+        MIGRATION_1_2,
+        MIGRATION_2_3,
+        MIGRATION_3_4,
+        MIGRATION_4_5,
+        MIGRATION_5_6,
+        MIGRATION_6_7,
+      )
       .build()
 
   companion object {

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

@@ -95,7 +95,14 @@ internal class DatabaseImpl(
             }
           }
 
-          addMigrations(MIGRATION_1_2, MIGRATION_2_3, MIGRATION_3_4, MIGRATION_4_5, MIGRATION_5_6)
+          addMigrations(
+            MIGRATION_1_2,
+            MIGRATION_2_3,
+            MIGRATION_3_4,
+            MIGRATION_4_5,
+            MIGRATION_5_6,
+            MIGRATION_6_7,
+          )
         }
         .build()
   }
@@ -115,8 +122,10 @@ internal class DatabaseImpl(
       logicalIds.addAll(
         resource.map {
           val timeOfLocalChange = Instant.now()
-          localChangeDao.addInsert(it, timeOfLocalChange)
-          resourceDao.insertLocalResource(it, timeOfLocalChange)
+          val resourceId = resourceDao.insertLocalResource(it, timeOfLocalChange)
+          val resourceEntity = selectEntity(it.resourceType, it.logicalId)
+          localChangeDao.addInsert(it, resourceEntity.resourceUuid, timeOfLocalChange)
+          resourceId
         },
       )
     }
@@ -169,19 +178,16 @@ internal class DatabaseImpl(
 
   override suspend fun delete(type: ResourceType, id: String) {
     db.withTransaction {
-      val remoteVersionId: String? =
-        try {
-          selectEntity(type, id).versionId
-        } catch (e: ResourceNotFoundException) {
-          null
+      resourceDao.getResourceEntity(id, type)?.let {
+        val rowsDeleted = resourceDao.deleteResource(resourceId = id, resourceType = type)
+        if (rowsDeleted > 0) {
+          localChangeDao.addDelete(
+            resourceId = id,
+            resourceType = type,
+            resourceUuid = it.resourceUuid,
+            remoteVersionId = it.versionId,
+          )
         }
-      val rowsDeleted = resourceDao.deleteResource(resourceId = id, resourceType = type)
-      if (rowsDeleted > 0) {
-        localChangeDao.addDelete(
-          resourceId = id,
-          resourceType = type,
-          remoteVersionId = remoteVersionId,
-        )
       }
     }
   }

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

@@ -50,7 +50,7 @@ import com.google.android.fhir.db.impl.entities.UriIndexEntity
       LocalChangeEntity::class,
       PositionIndexEntity::class,
     ],
-  version = 6,
+  version = 7,
   exportSchema = true,
 )
 @TypeConverters(DbTypeConverters::class)
@@ -127,3 +127,25 @@ val MIGRATION_5_6 =
       )
     }
   }
+
+/** Add column resourceUuid in [LocalChangeEntity] */
+val MIGRATION_6_7 =
+  object : Migration(6, 7) {
+    override fun migrate(database: SupportSQLiteDatabase) {
+      database.execSQL(
+        "CREATE TABLE IF NOT EXISTS `_new_LocalChangeEntity` (`id` INTEGER PRIMARY KEY AUTOINCREMENT NOT NULL, `resourceType` TEXT NOT NULL, `resourceId` TEXT NOT NULL, `resourceUuid` BLOB NOT NULL, `timestamp` INTEGER NOT NULL, `type` INTEGER NOT NULL, `payload` TEXT NOT NULL, `versionId` TEXT)",
+      )
+      database.execSQL(
+        "INSERT INTO `_new_LocalChangeEntity` (`id`,`resourceType`,`resourceId`,`resourceUuid`,`timestamp`,`type`,`payload`,`versionId`) " +
+          "SELECT localChange.id, localChange.resourceType, localChange.resourceId, resource.resourceUuid, localChange.timestamp, localChange.type, localChange.payload, localChange.versionId FROM `LocalChangeEntity` localChange LEFT JOIN ResourceEntity resource ON localChange.resourceId= resource.resourceId",
+      )
+      database.execSQL("DROP TABLE `LocalChangeEntity`")
+      database.execSQL("ALTER TABLE `_new_LocalChangeEntity` RENAME TO `LocalChangeEntity`")
+      database.execSQL(
+        "CREATE INDEX IF NOT EXISTS `index_LocalChangeEntity_resourceType_resourceId` ON `LocalChangeEntity` (`resourceType`, `resourceId`)",
+      )
+      database.execSQL(
+        "CREATE INDEX IF NOT EXISTS `index_LocalChangeEntity_resourceType_resourceUuid` ON `LocalChangeEntity` (`resourceType`, `resourceUuid`)",
+      )
+    }
+  }

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

@@ -32,6 +32,7 @@ import com.google.android.fhir.logicalId
 import com.google.android.fhir.versionId
 import java.time.Instant
 import java.util.Date
+import java.util.UUID
 import org.hl7.fhir.r4.model.Resource
 import org.hl7.fhir.r4.model.ResourceType
 import org.json.JSONArray
@@ -51,7 +52,7 @@ internal abstract class LocalChangeDao {
   @Insert abstract suspend fun addLocalChange(localChangeEntity: LocalChangeEntity)
 
   @Transaction
-  open suspend fun addInsert(resource: Resource, timeOfLocalChange: Instant) {
+  open suspend fun addInsert(resource: Resource, resourceUuid: UUID, timeOfLocalChange: Instant) {
     val resourceId = resource.logicalId
     val resourceType = resource.resourceType
     val resourceString = iParser.encodeResourceToString(resource)
@@ -61,6 +62,7 @@ internal abstract class LocalChangeDao {
         id = 0,
         resourceType = resourceType.name,
         resourceId = resourceId,
+        resourceUuid = resourceUuid,
         timestamp = timeOfLocalChange,
         type = Type.INSERT,
         payload = resourceString,
@@ -95,6 +97,7 @@ internal abstract class LocalChangeDao {
         id = 0,
         resourceType = resourceType.name,
         resourceId = resourceId,
+        resourceUuid = oldEntity.resourceUuid,
         timestamp = timeOfLocalChange,
         type = Type.UPDATE,
         payload = jsonDiff.toString(),
@@ -103,12 +106,18 @@ internal abstract class LocalChangeDao {
     )
   }
 
-  suspend fun addDelete(resourceId: String, resourceType: ResourceType, remoteVersionId: String?) {
+  suspend fun addDelete(
+    resourceId: String,
+    resourceUuid: UUID,
+    resourceType: ResourceType,
+    remoteVersionId: String?,
+  ) {
     addLocalChange(
       LocalChangeEntity(
         id = 0,
         resourceType = resourceType.name,
         resourceId = resourceId,
+        resourceUuid = resourceUuid,
         timestamp = Date().toInstant(),
         type = Type.DELETE,
         payload = "",

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

@@ -58,7 +58,7 @@ internal abstract class ResourceDao {
   lateinit var iParser: IParser
   lateinit var resourceIndexer: ResourceIndexer
 
-  open suspend fun update(resource: Resource, timeOfLocalChange: Instant) {
+  open suspend fun update(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
@@ -76,12 +76,14 @@ internal abstract class ResourceDao {
       val index =
         ResourceIndices.Builder(resourceIndexer.index(resource))
           .apply {
-            addDateTimeIndex(
-              createLocalLastUpdatedIndex(
-                resource.resourceType,
-                InstantType(Date.from(timeOfLocalChange)),
-              ),
-            )
+            timeOfLocalChange?.let {
+              addDateTimeIndex(
+                createLocalLastUpdatedIndex(
+                  resource.resourceType,
+                  InstantType(Date.from(timeOfLocalChange)),
+                ),
+              )
+            }
             lastUpdatedRemote?.let { date ->
               addDateTimeIndex(createLastUpdatedIndex(resource.resourceType, InstantType(date)))
             }
@@ -183,11 +185,14 @@ internal abstract class ResourceDao {
 
   // Since the insert removes any old indexes and lastUpdatedLocal (data not contained in resource
   // itself), we extract the lastUpdatedLocal if any and then set it back again.
-  private suspend fun insertRemoteResource(resource: Resource) =
-    insertResource(
-      resource,
-      getResourceEntity(resource.logicalId, resource.resourceType)?.lastUpdatedLocal,
-    )
+  private suspend fun insertRemoteResource(resource: Resource): String {
+    val existingResourceEntity = getResourceEntity(resource.logicalId, resource.resourceType)
+    if (existingResourceEntity != null) {
+      update(resource, existingResourceEntity.lastUpdatedLocal)
+      return resource.id
+    }
+    return insertResource(resource, null)
+  }
 
   private suspend fun insertResource(resource: Resource, lastUpdatedLocal: Instant?): String {
     val resourceUuid = UUID.randomUUID()

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

@@ -20,6 +20,7 @@ import androidx.room.Entity
 import androidx.room.Index
 import androidx.room.PrimaryKey
 import java.time.Instant
+import java.util.UUID
 
 /**
  * When a local change to a resource happens, the lastUpdated timestamp in [ResourceEntity] is
@@ -55,11 +56,18 @@ import java.time.Instant
  *
  * ] For resource that is fully synced with server this table should not have any rows.
  */
-@Entity(indices = [Index(value = ["resourceType", "resourceId"])])
+@Entity(
+  indices =
+    [
+      Index(value = ["resourceType", "resourceId"]),
+      Index(value = ["resourceType", "resourceUuid"]),
+    ],
+)
 internal data class LocalChangeEntity(
   @PrimaryKey(autoGenerate = true) val id: Long,
   val resourceType: String,
   val resourceId: String,
+  val resourceUuid: UUID,
   val timestamp: Instant,
   val type: Type,
   val payload: String,

engine/src/test/java/com/google/android/fhir/LocalChangeTest.kt

@@ -22,6 +22,7 @@ import ca.uhn.fhir.parser.IParser
 import com.google.android.fhir.db.impl.entities.LocalChangeEntity
 import com.google.common.truth.Truth.assertThat
 import java.time.Instant
+import java.util.UUID
 import junit.framework.TestCase
 import kotlinx.coroutines.runBlocking
 import org.hl7.fhir.r4.model.HumanName
@@ -42,6 +43,7 @@ class LocalChangeTest : TestCase() {
       LocalChangeEntity(
         id = 1,
         resourceType = ResourceType.Patient.name,
+        resourceUuid = UUID.randomUUID(),
         resourceId = "Patient-001",
         type = LocalChangeEntity.Type.INSERT,
         payload =

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

@@ -25,6 +25,7 @@ import com.google.android.fhir.toLocalChange
 import com.google.common.truth.Truth.assertThat
 import java.net.ConnectException
 import java.time.Instant
+import java.util.UUID
 import kotlinx.coroutines.flow.toList
 import kotlinx.coroutines.runBlocking
 import org.hl7.fhir.r4.model.Bundle
@@ -133,6 +134,7 @@ class UploaderImplTest {
         LocalChangeEntity(
             id = 1,
             resourceType = ResourceType.Patient.name,
+            resourceUuid = UUID.randomUUID(),
             resourceId = "Patient-001",
             type = LocalChangeEntity.Type.INSERT,
             payload =