Fix for removing old indexes when resource is updated (#1840)

* Fix for removing old indexes when resource is updated * Added comments
google · Feb 6, 2023 · c0d809c · c0d809c
1 parent fd6950d
commit c0d809c
Show file tree

Hide file tree

Showing 2 changed files with 126 additions and 32 deletions.
diff --git a/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt b/engine/src/androidTest/java/com/google/android/fhir/db/impl/DatabaseImplTest.kt
@@ -474,8 +474,8 @@ class DatabaseImplTest {
  .getAllLocalChanges()
  .map { it }
  .none {
- it.localChange.type.equals(LocalChangeEntity.Type.DELETE) &&
- it.localChange.resourceId.equals("nonexistent_patient")
+ it.localChange.type == LocalChangeEntity.Type.DELETE &&
+ it.localChange.resourceId == "nonexistent_patient"
  }
  )
  .isTrue()
@@ -505,7 +505,7 @@ class DatabaseImplTest {
  database
  .getAllLocalChanges()
  .map { it }
- .none { it.localChange.resourceId.equals(patient.logicalId) }
+ .none { it.localChange.resourceId == patient.logicalId }
  )
  .isTrue()
  }
@@ -585,6 +585,84 @@ class DatabaseImplTest {
  .isTrue()
  }
 
+ @Test
+ fun insert_should_remove_old_indexes() = runBlocking {
+ val patient =
+ Patient().apply {
+ id = "local-1"
+ addName(
+ HumanName().apply {
+ addGiven("Jane")
+ family = "Doe"
+ }
+ )
+ }
+
+ database.insert(patient)
+ val result =
+ database.search<Patient>(
+ Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery()
+ )
+ assertThat(result.size).isEqualTo(1)
+
+ val updatedPatient =
+ Patient().apply {
+ id = "local-1"
+ addName(
+ HumanName().apply {
+ addGiven("John")
+ family = "Doe"
+ }
+ )
+ }
+
+ database.insert(updatedPatient)
+ val updatedResult =
+ database.search<Patient>(
+ Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery()
+ )
+ assertThat(updatedResult.size).isEqualTo(0)
+ }
+
+ @Test
+ fun insertRemote_should_remove_old_indexes() = runBlocking {
+ val patient =
+ Patient().apply {
+ id = "local-1"
+ addName(
+ HumanName().apply {
+ addGiven("Jane")
+ family = "Doe"
+ }
+ )
+ }
+
+ database.insertRemote(patient)
+ val result =
+ database.search<Patient>(
+ Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery()
+ )
+ assertThat(result.size).isEqualTo(1)
+
+ val updatedPatient =
+ Patient().apply {
+ id = "local-1"
+ addName(
+ HumanName().apply {
+ addGiven("John")
+ family = "Doe"
+ }
+ )
+ }
+
+ database.insertRemote(updatedPatient)
+ val updatedResult =
+ database.search<Patient>(
+ Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery()
+ )
+ assertThat(updatedResult.size).isEqualTo(0)
+ }
+
  @Test
  fun update_remoteResource_readSquashedChanges_shouldReturnPatch() = runBlocking {
  val patient: Patient = testingUtils.readFromFile(Patient::class.java, "/date_test_patient.json")
@@ -632,6 +710,45 @@ class DatabaseImplTest {
  testingUtils.assertJsonArrayEqualsIgnoringOrder(JSONArray(payload), updatePatch)
  }
 
+ @Test
+ fun update_should_remove_old_indexes() = runBlocking {
+ val patient =
+ Patient().apply {
+ id = "local-1"
+ addName(
+ HumanName().apply {
+ addGiven("Jane")
+ family = "Doe"
+ }
+ )
+ }
+
+ database.insertRemote(patient)
+ val result =
+ database.search<Patient>(
+ Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery()
+ )
+ assertThat(result.size).isEqualTo(1)
+
+ val updatedPatient =
+ Patient().apply {
+ id = "local-1"
+ addName(
+ HumanName().apply {
+ addGiven("John")
+ family = "Doe"
+ }
+ )
+ }
+
+ database.update(updatedPatient)
+ val updatedResult =
+ database.search<Patient>(
+ Search(ResourceType.Patient).apply { filter(Patient.GIVEN, { value = "Jane" }) }.getQuery()
+ )
+ assertThat(updatedResult.size).isEqualTo(0)
+ }
+
  @Test
  fun delete_remoteResource_shouldReturnDeleteLocalChange() = runBlocking {
  database.insertRemote(TEST_PATIENT_2)

diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/ResourceDao.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/ResourceDao.kt
@@ -52,22 +52,13 @@ internal abstract class ResourceDao {
  lateinit var resourceIndexer: ResourceIndexer
 
  open suspend fun update(resource: Resource) {
- updateResource(
- resource.logicalId,
- resource.resourceType,
- iParser.encodeResourceToString(resource),
- )
  getResourceEntity(resource.logicalId, resource.resourceType)?.let {
- val entity =
- ResourceEntity(
- id = 0,
- resourceType = resource.resourceType,
- resourceUuid = it.resourceUuid,
- resourceId = resource.logicalId,
- serializedResource = iParser.encodeResourceToString(resource),
- versionId = it.versionId,
- lastUpdatedRemote = it.lastUpdatedRemote
- )
+ val entity = it.copy(serializedResource = iParser.encodeResourceToString(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 = resourceIndexer.index(resource)
  updateIndicesForResource(index, entity, it.resourceUuid)
  }
@@ -112,20 +103,6 @@ internal abstract class ResourceDao {
  @Insert(onConflict = OnConflictStrategy.REPLACE)
  abstract suspend fun insertPositionIndex(positionIndexEntity: PositionIndexEntity)
 
- @Query(
- """
- UPDATE ResourceEntity
- SET serializedResource = :serializedResource
- WHERE resourceId = :resourceId
- AND resourceType = :resourceType
- """
- )
- abstract suspend fun updateResource(
- resourceId: String,
- resourceType: ResourceType,
- serializedResource: String
- )
-
  @Query(
  """
  UPDATE ResourceEntity