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

Start saving local lastUpdated to resources for search operations. #2030

Merged
merged 17 commits into from
Jul 21, 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
Prev Previous commit
Next Next commit
Review comments: Udpated tests
  • Loading branch information
aditya-07 committed Jul 20, 2023
commit 96edd1d4de33418cde972e65b42572158cee1e30
9 changes: 0 additions & 9 deletions engine/src/main/java/com/google/android/fhir/db/Database.kt
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,8 @@

package com.google.android.fhir.db

import androidx.annotation.VisibleForTesting
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.DateTimeIndexEntity
import com.google.android.fhir.db.impl.entities.LocalChangeEntity
import com.google.android.fhir.db.impl.entities.ResourceEntity
import com.google.android.fhir.search.SearchQuery
Expand Down Expand Up @@ -147,11 +145,4 @@ internal interface Database {
* delete resource entry from LocalChangeEntity table.
*/
suspend fun purge(type: ResourceType, id: String, forcePurge: Boolean = false)

/** @return DateTimeIndexEntities for the resource. */
@VisibleForTesting
suspend fun getDateTimeIndexEntities(
resourceId: String,
resourceType: ResourceType
): List<DateTimeIndexEntity>
}
Original file line number Diff line number Diff line change
Expand Up @@ -280,12 +280,6 @@ internal class DatabaseImpl(
}
}

override suspend fun getDateTimeIndexEntities(resourceId: String, resourceType: ResourceType) =
resourceDao.getResourceEntity(resourceId, resourceType)?.let {
resourceDao.getDateTimeIndexes(it.resourceUuid)
}
?: emptyList()

companion object {
/**
* The name for unencrypted database.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ internal object LocalChangeUtils {
resourceType = second.resourceType,
type = type,
payload = payload,
versionId = second.versionId
versionId = second.versionId,
timestamp = second.timestamp
)
}

Expand Down Expand Up @@ -136,9 +137,11 @@ internal object LocalChangeUtils {
with(JSONArray(jsonDiff.toString())) {
val ignorePaths = setOf("/meta", "/text")
return@with JSONArray(
(0 until length()).map { optJSONObject(it) }.filterNot { jsonObject ->
ignorePaths.any { jsonObject.optString("path").startsWith(it) }
}
(0 until length())
.map { optJSONObject(it) }
.filterNot { jsonObject ->
ignorePaths.any { jsonObject.optString("path").startsWith(it) }
}
)
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,9 +125,6 @@ internal abstract class ResourceDao {
@Insert(onConflict = OnConflictStrategy.REPLACE)
abstract suspend fun insertPositionIndex(positionIndexEntity: PositionIndexEntity)

@Query("select * from DateTimeIndexEntity where resourceUuid = :resourceUuid ")
abstract suspend fun getDateTimeIndexes(resourceUuid: UUID): List<DateTimeIndexEntity>

@Query(
"""
UPDATE ResourceEntity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,15 @@
package com.google.android.fhir.impl

import androidx.test.core.app.ApplicationProvider
import ca.uhn.fhir.rest.param.ParamPrefixEnum
import com.google.android.fhir.FhirServices.Companion.builder
import com.google.android.fhir.LocalChange
import com.google.android.fhir.LocalChange.Type
import com.google.android.fhir.db.ResourceNotFoundException
import com.google.android.fhir.db.impl.dao.LocalChangeToken
import com.google.android.fhir.get
import com.google.android.fhir.logicalId
import com.google.android.fhir.search.LOCAL_LAST_UPDATED
import com.google.android.fhir.search.LOCAL_LAST_UPDATED_PARAM
import com.google.android.fhir.search.search
import com.google.android.fhir.sync.AcceptLocalConflictResolver
import com.google.android.fhir.sync.AcceptRemoteConflictResolver
Expand All @@ -41,6 +42,7 @@ import org.hl7.fhir.exceptions.FHIRException
import org.hl7.fhir.r4.model.Address
import org.hl7.fhir.r4.model.CanonicalType
import org.hl7.fhir.r4.model.Coding
import org.hl7.fhir.r4.model.DateTimeType
import org.hl7.fhir.r4.model.Enumerations
import org.hl7.fhir.r4.model.HumanName
import org.hl7.fhir.r4.model.Meta
Expand Down Expand Up @@ -560,40 +562,65 @@ class FhirEngineImplTest {
}

@Test
fun create_should_save_lastUpdated_Indexes(): Unit = runBlocking {
fun `create should allow patient search with LOCAL_LAST_UPDATED_PARAM`(): Unit = runBlocking {
val patient = Patient().apply { id = "patient-id-create" }
fhirEngine.create(patient)
val indexes =
services.database.getDateTimeIndexEntities("patient-id-create", ResourceType.Patient)
assertThat(indexes.map { it.index.name }).contains(LOCAL_LAST_UPDATED)
}
val localChangeTimestamp =
fhirEngine.getLocalChange(ResourceType.Patient, "patient-id-create")!!.timestamp

@Test
fun update_should_save_lastUpdated_Indexes() = runBlocking {
val patient = Patient().apply { id = "patient-id-update" }
fhirEngine.create(patient)
val indexesWhenCreated =
services.database.getDateTimeIndexEntities("patient-id-update", ResourceType.Patient)
val patientUpdate =
Patient().apply {
id = "patient-id-update"
addName(
HumanName().apply {
addGiven("John")
family = "Doe"
val result =
fhirEngine.search<Patient> {
filter(
LOCAL_LAST_UPDATED_PARAM,
{
value = of(DateTimeType(localChangeTimestamp))
prefix = ParamPrefixEnum.EQUAL
}
)
}

fhirEngine.update(patientUpdate)
val indexesWhenUpdated =
services.database.getDateTimeIndexEntities("patient-id-update", ResourceType.Patient)

assertThat(indexesWhenUpdated.map { it.index.name }).contains(LOCAL_LAST_UPDATED)
assertThat(indexesWhenUpdated.first { it.index.name == LOCAL_LAST_UPDATED }.index.from)
.isGreaterThan(indexesWhenCreated.first { it.index.name == LOCAL_LAST_UPDATED }.index.from)
assertThat(result).isNotEmpty()
assertThat(result.map { it.logicalId }).containsExactly("patient-id-create").inOrder()
}

@Test
fun `update should allow patient search with LOCAL_LAST_UPDATED_PARAM and update local entity`() =
runBlocking {
val patient = Patient().apply { id = "patient-id-update" }
fhirEngine.create(patient)
val localChangeTimestampWhenCreated =
fhirEngine.getLocalChange(ResourceType.Patient, "patient-id-update")!!.timestamp
val patientUpdate =
Patient().apply {
id = "patient-id-update"
addName(
HumanName().apply {
addGiven("John")
family = "Doe"
}
)
}
fhirEngine.update(patientUpdate)
val localChangeTimestampWhenUpdated =
fhirEngine.getLocalChange(ResourceType.Patient, "patient-id-update")!!.timestamp

val result =
fhirEngine.search<Patient> {
filter(
LOCAL_LAST_UPDATED_PARAM,
{
value = of(DateTimeType(localChangeTimestampWhenUpdated))
prefix = ParamPrefixEnum.EQUAL
}
)
}

assertThat(DateTimeType(localChangeTimestampWhenUpdated).value)
.isGreaterThan(DateTimeType(localChangeTimestampWhenCreated).value)
assertThat(result).isNotEmpty()
assertThat(result.map { it.logicalId }).containsExactly("patient-id-update").inOrder()
}

companion object {
private const val TEST_PATIENT_1_ID = "test_patient_1"
private var TEST_PATIENT_1 =
Expand Down
Loading