Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for removing old indexes when resource is updated #1840

Merged
merged 5 commits into from
Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Fix for removing old indexes when resource is updated
  • Loading branch information
aditya-07 committed Feb 2, 2023
commit 6c953e59ec1cf2854fe3ea2ce7b374637656fe3b
Original file line number Diff line number Diff line change
Expand Up @@ -451,10 +451,13 @@ class DatabaseImplTest {
fun delete_nonExistent_shouldNotInsertLocalChange() = runBlocking {
database.delete(ResourceType.Patient, "nonexistent_patient")
assertThat(
database.getAllLocalChanges().map { it }.none {
it.localChange.type.equals(LocalChangeEntity.Type.DELETE) &&
it.localChange.resourceId.equals("nonexistent_patient")
}
database
.getAllLocalChanges()
.map { it }
.none {
it.localChange.type == LocalChangeEntity.Type.DELETE &&
it.localChange.resourceId == "nonexistent_patient"
}
)
.isTrue()
}
Expand All @@ -480,9 +483,10 @@ class DatabaseImplTest {
val patient: Patient = testingUtils.readFromFile(Patient::class.java, "/date_test_patient.json")
database.insertRemote(patient)
assertThat(
database.getAllLocalChanges().map { it }.none {
it.localChange.resourceId.equals(patient.logicalId)
}
database
.getAllLocalChanges()
.map { it }
.none { it.localChange.resourceId == patient.logicalId }
)
.isTrue()
}
Expand Down Expand Up @@ -532,15 +536,17 @@ class DatabaseImplTest {
}
database.insert(patient)
services.fhirEngine.syncUpload { it ->
it.first { it.resourceId == "remote-patient-3" }.let {
flowOf(
it.token to
Patient().apply {
id = it.resourceId
meta = remoteMeta
}
)
}
it
.first { it.resourceId == "remote-patient-3" }
.let {
flowOf(
it.token to
Patient().apply {
id = it.resourceId
meta = remoteMeta
}
)
}
}
val selectedEntity = database.selectEntity(ResourceType.Patient, "remote-patient-3")
assertThat(selectedEntity.versionId).isEqualTo(remoteMeta.versionId)
Expand All @@ -552,13 +558,92 @@ class DatabaseImplTest {
val patient: Patient = testingUtils.readFromFile(Patient::class.java, "/date_test_patient.json")
database.insertRemote(patient, TEST_PATIENT_2)
assertThat(
database.getAllLocalChanges().map { it }.none {
it.localChange.resourceId in listOf(patient.logicalId, TEST_PATIENT_2_ID)
}
database
.getAllLocalChanges()
.map { it }
.none { it.localChange.resourceId in listOf(patient.logicalId, TEST_PATIENT_2_ID) }
)
.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")
Expand Down Expand Up @@ -606,6 +691,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)
Expand Down Expand Up @@ -2547,7 +2671,8 @@ class DatabaseImplTest {
)

assertThat(
database.search<Patient>(
database
.search<Patient>(
Search(ResourceType.Patient)
.apply { sort(Patient.BIRTHDATE, Order.DESCENDING) }
.getQuery()
Expand All @@ -2574,7 +2699,8 @@ class DatabaseImplTest {
)

assertThat(
database.search<Patient>(
database
.search<Patient>(
Search(ResourceType.Patient)
.apply { sort(Patient.BIRTHDATE, Order.ASCENDING) }
.getQuery()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*
* Copyright 2021 Google LLC
* Copyright 2022 Google LLC
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -51,22 +51,9 @@ internal abstract class ResourceDao {
lateinit var iParser: IParser

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))
insertResource(entity)
aditya-07 marked this conversation as resolved.
Show resolved Hide resolved
val index = ResourceIndexer.index(resource)
updateIndicesForResource(index, entity, it.resourceUuid)
}
Expand Down Expand Up @@ -111,20 +98,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
Expand Down