From 857f112a3d56c084e56b1199378bfd22daa1f8a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=E2=89=A1ZRS?= <12814349+LZRS@users.noreply.github.com> Date: Tue, 7 May 2024 00:35:53 +0300 Subject: [PATCH 1/7] Add FhirEngine interface method 'withTransaction' To support performing FhirEngine actions as a single atomic transaction --- .../com/google/android/fhir/FhirEngine.kt | 6 ++ .../android/fhir/impl/FhirEngineImpl.kt | 4 + .../google/android/fhir/testing/Utilities.kt | 2 + .../android/fhir/impl/FhirEngineImplTest.kt | 81 +++++++++++++++++++ 4 files changed, 93 insertions(+) diff --git a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt index 45eb630896..c73aecfb16 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -212,6 +212,12 @@ interface FhirEngine { * back and no record is purged. */ suspend fun purge(type: ResourceType, ids: Set, forcePurge: Boolean = false) + + /** + * Adds support for performing actions on `FhirEngine` as a single atomic transaction where the + * entire set of changes succeed or fail as a single entity + */ + suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) } /** diff --git a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt index 4645c149bc..9827643b9f 100644 --- a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt @@ -106,6 +106,10 @@ internal class FhirEngineImpl(private val database: Database, private val contex } } + override suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) { + database.withTransaction { block.invoke(this@FhirEngineImpl) } + } + private suspend fun saveResolvedResourcesToDatabase(resolved: List?) { resolved?.let { database.deleteUpdates(it) diff --git a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt index 07c764241a..5be334b298 100644 --- a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt +++ b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt @@ -176,6 +176,8 @@ object TestFhirEngineImpl : FhirEngine { download().collect() } + override suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) {} + override suspend fun count(search: Search): Long { return 0 } diff --git a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt index 21d440a1ba..3f43ed9f94 100644 --- a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt @@ -22,6 +22,7 @@ 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.delete import com.google.android.fhir.get import com.google.android.fhir.lastUpdated import com.google.android.fhir.logicalId @@ -763,6 +764,86 @@ class FhirEngineImplTest { .inOrder() } + @Test + fun `withTransaction saves all changes successfully in order`() = runTest { + val patient01ID = "patient-01" + val patient01 = + Patient().apply { + id = patient01ID + gender = Enumerations.AdministrativeGender.FEMALE + } + val patient02ID = "patient-02" + val patient02 = + Patient().apply { + id = patient02ID + gender = Enumerations.AdministrativeGender.MALE + } + val patient03ID = "patient-03" + val patient03 = + Patient().apply { + id = patient03ID + gender = Enumerations.AdministrativeGender.FEMALE + } + + fhirEngine.create(patient01) + assertThat(fhirEngine.get(patient01ID).gender) + .isEqualTo(Enumerations.AdministrativeGender.FEMALE) + fhirEngine.withTransaction { + fhirEngine.create(patient02, patient03) + patient01.gender = Enumerations.AdministrativeGender.FEMALE + fhirEngine.update(patient01) + fhirEngine.delete(patient01ID) + } + assertResourceEquals(patient02, fhirEngine.get(patient02ID)) + assertResourceEquals(patient03, fhirEngine.get(patient03ID)) + assertThrows(ResourceNotFoundException::class.java) { + runBlocking { fhirEngine.get(patient01ID) } + } + } + + @Test + fun `withTransaction reverts all changes when an error occurs`() = runTest { + val patient01ID = "patient-01" + val patient01 = + Patient().apply { + id = patient01ID + gender = Enumerations.AdministrativeGender.FEMALE + } + val patient02ID = "patient-02" + val patient02 = + Patient().apply { + id = patient02ID + gender = Enumerations.AdministrativeGender.MALE + } + val patient03ID = "patient-03" + val patient03 = + Patient().apply { + id = patient03ID + gender = Enumerations.AdministrativeGender.FEMALE + } + val patient03Updated = + patient03.copy().apply { gender = Enumerations.AdministrativeGender.MALE } + + fhirEngine.create(patient03) + try { + fhirEngine.withTransaction { + this.create(patient01) + this.create(patient02) + this.update(patient03Updated) + this.get("non_existent_patient_id") // Force ResourceNotFoundException + } + } catch (_: ResourceNotFoundException) {} + + assertThrows(ResourceNotFoundException::class.java) { + runBlocking { fhirEngine.get(patient01ID) } + } + assertThrows(ResourceNotFoundException::class.java) { + runBlocking { fhirEngine.get(patient02ID) } + } + assertResourceEquals(patient03, fhirEngine.get(patient03ID)) + assertResourceNotEquals(patient03Updated, fhirEngine.get(patient03ID)) + } + companion object { private const val TEST_PATIENT_1_ID = "test_patient_1" private var TEST_PATIENT_1 = From 72be126aefe3c8bf52cefdf3ace40f0b4d8458c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=E2=89=A1ZRS?= <12814349+LZRS@users.noreply.github.com> Date: Sun, 26 May 2024 13:34:13 +0300 Subject: [PATCH 2/7] Only allow FHIREngine CRUD api for withTransaction --- .../com/google/android/fhir/FhirEngine.kt | 136 +++++++++--------- .../android/fhir/impl/FhirEngineImpl.kt | 3 +- .../google/android/fhir/testing/Utilities.kt | 3 +- .../android/fhir/impl/FhirEngineImplTest.kt | 3 +- 4 files changed, 75 insertions(+), 70 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt index c73aecfb16..e0cd278152 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -58,64 +58,7 @@ import org.hl7.fhir.r4.model.ResourceType * val fhirEngine = FhirEngineProvider.getInstance(this) * ``` */ -interface FhirEngine { - /** - * Creates one or more FHIR [Resource]s in the local storage. FHIR Engine requires all stored - * resources to have a logical [Resource.id]. If the `id` is specified in the resource passed to - * [create], the resource created in `FhirEngine` will have the same `id`. If no `id` is - * specified, `FhirEngine` will generate a UUID as that resource's `id` and include it in the - * returned list of IDs. - * - * @param resource The FHIR resources to create. - * @return A list of logical IDs of the newly created resources. - */ - suspend fun create(vararg resource: Resource): List - - /** - * Loads a FHIR resource given its [ResourceType] and logical ID. - * - * @param type The type of the resource to load. - * @param id The logical ID of the resource. - * @return The requested FHIR resource. - * @throws ResourceNotFoundException if the resource is not found. - */ - @Throws(ResourceNotFoundException::class) - suspend fun get(type: ResourceType, id: String): Resource - - /** - * Updates one or more FHIR [Resource]s in the local storage. - * - * @param resource The FHIR resources to update. - */ - suspend fun update(vararg resource: Resource) - - /** - * Removes a FHIR resource given its [ResourceType] and logical ID. - * - * @param type The type of the resource to delete. - * @param id The logical ID of the resource. - */ - suspend fun delete(type: ResourceType, id: String) - - /** - * Searches the database and returns a list of resources matching the [Search] specifications. - * - * Example: - * ``` - * fhirEngine.search { - * filter(Patient.GIVEN, { - * value = "Kiran" - * modifier = StringFilterModifier.MATCHES_EXACTLY - * }) - * } - * ``` - * - * @param search The search criteria to apply. - * @return A list of [SearchResult] objects containing the matching resources and any included - * references. - */ - suspend fun search(search: Search): List> - +interface FhirEngine : BaseFHIREngine { /** * Synchronizes upload results with the database. * @@ -151,14 +94,6 @@ interface FhirEngine { download: suspend () -> Flow>, ) - /** - * Returns the total count of entities available for the given [Search]. - * - * @param search The search criteria to apply. - * @return The total number of matching resources. - */ - suspend fun count(search: Search): Long - /** * Returns the timestamp when data was last synchronized, or `null` if no synchronization has * occurred yet. @@ -212,12 +147,79 @@ interface FhirEngine { * back and no record is purged. */ suspend fun purge(type: ResourceType, ids: Set, forcePurge: Boolean = false) +} + +interface BaseFHIREngine { + /** + * Creates one or more FHIR [Resource]s in the local storage. FHIR Engine requires all stored + * resources to have a logical [Resource.id]. If the `id` is specified in the resource passed to + * [create], the resource created in `FhirEngine` will have the same `id`. If no `id` is + * specified, `FhirEngine` will generate a UUID as that resource's `id` and include it in the + * returned list of IDs. + * + * @param resource The FHIR resources to create. + * @return A list of logical IDs of the newly created resources. + */ + suspend fun create(vararg resource: Resource): List + + /** + * Loads a FHIR resource given its [ResourceType] and logical ID. + * + * @param type The type of the resource to load. + * @param id The logical ID of the resource. + * @return The requested FHIR resource. + * @throws ResourceNotFoundException if the resource is not found. + */ + @Throws(ResourceNotFoundException::class) + suspend fun get(type: ResourceType, id: String): Resource + + /** + * Updates one or more FHIR [Resource]s in the local storage. + * + * @param resource The FHIR resources to update. + */ + suspend fun update(vararg resource: Resource) + + /** + * Removes a FHIR resource given its [ResourceType] and logical ID. + * + * @param type The type of the resource to delete. + * @param id The logical ID of the resource. + */ + suspend fun delete(type: ResourceType, id: String) + + /** + * Searches the database and returns a list of resources matching the [Search] specifications. + * + * Example: + * ``` + * fhirEngine.search { + * filter(Patient.GIVEN, { + * value = "Kiran" + * modifier = StringFilterModifier.MATCHES_EXACTLY + * }) + * } + * ``` + * + * @param search The search criteria to apply. + * @return A list of [SearchResult] objects containing the matching resources and any included + * references. + */ + suspend fun search(search: Search): List> + + /** + * Returns the total count of entities available for the given [Search]. + * + * @param search The search criteria to apply. + * @return The total number of matching resources. + */ + suspend fun count(search: Search): Long /** * Adds support for performing actions on `FhirEngine` as a single atomic transaction where the * entire set of changes succeed or fail as a single entity */ - suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) + suspend fun withTransaction(block: suspend BaseFHIREngine.() -> Unit) } /** diff --git a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt index 9827643b9f..47215d1532 100644 --- a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt @@ -17,6 +17,7 @@ package com.google.android.fhir.impl import android.content.Context +import com.google.android.fhir.BaseFHIREngine import com.google.android.fhir.FhirEngine import com.google.android.fhir.FhirEngineProvider import com.google.android.fhir.LocalChange @@ -106,7 +107,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex } } - override suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) { + override suspend fun withTransaction(block: suspend BaseFHIREngine.() -> Unit) { database.withTransaction { block.invoke(this@FhirEngineImpl) } } diff --git a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt index 5be334b298..c7aed249b6 100644 --- a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt +++ b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt @@ -20,6 +20,7 @@ import androidx.work.Data import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum import ca.uhn.fhir.parser.IParser +import com.google.android.fhir.BaseFHIREngine import com.google.android.fhir.FhirEngine import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken @@ -176,7 +177,7 @@ object TestFhirEngineImpl : FhirEngine { download().collect() } - override suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) {} + override suspend fun withTransaction(block: suspend BaseFHIREngine.() -> Unit) {} override suspend fun count(search: Search): Long { return 0 diff --git a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt index 3f43ed9f94..25e7725280 100644 --- a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt @@ -830,7 +830,8 @@ class FhirEngineImplTest { this.create(patient01) this.create(patient02) this.update(patient03Updated) - this.get("non_existent_patient_id") // Force ResourceNotFoundException + this.get(ResourceType.Patient, "non_existent_patient_id") + as Patient // Force ResourceNotFoundException } } catch (_: ResourceNotFoundException) {} From 868877f9c4af02605dc92a7a6291e03ed03acfe6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=E2=89=A1ZRS?= <12814349+LZRS@users.noreply.github.com> Date: Thu, 30 May 2024 21:01:46 +0300 Subject: [PATCH 3/7] Rename BaseFhirEngine to CrudFhirEngine --- engine/src/main/java/com/google/android/fhir/FhirEngine.kt | 6 +++--- .../java/com/google/android/fhir/impl/FhirEngineImpl.kt | 4 ++-- .../main/java/com/google/android/fhir/testing/Utilities.kt | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt index 7f4974eb25..53df1a2e11 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -58,7 +58,7 @@ import org.hl7.fhir.r4.model.ResourceType * val fhirEngine = FhirEngineProvider.getInstance(this) * ``` */ -interface FhirEngine : BaseFHIREngine { +interface FhirEngine : CrudFhirEngine { /** * Synchronizes upload results with the database. * @@ -149,7 +149,7 @@ interface FhirEngine : BaseFHIREngine { suspend fun purge(type: ResourceType, ids: Set, forcePurge: Boolean = false) } -interface BaseFHIREngine { +interface CrudFhirEngine { /** * Creates one or more FHIR [Resource]s in the local storage. FHIR Engine requires all stored * resources to have a logical [Resource.id]. If the `id` is specified in the resource passed to @@ -219,7 +219,7 @@ interface BaseFHIREngine { * Adds support for performing actions on `FhirEngine` as a single atomic transaction where the * entire set of changes succeed or fail as a single entity */ - suspend fun withTransaction(block: suspend BaseFHIREngine.() -> Unit) + suspend fun withTransaction(block: suspend CrudFhirEngine.() -> Unit) } /** diff --git a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt index 28ddf3a586..2d51ed27b2 100644 --- a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt @@ -17,7 +17,7 @@ package com.google.android.fhir.impl import android.content.Context -import com.google.android.fhir.BaseFHIREngine +import com.google.android.fhir.CrudFhirEngine import com.google.android.fhir.FhirEngine import com.google.android.fhir.FhirEngineProvider import com.google.android.fhir.LocalChange @@ -107,7 +107,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex } } - override suspend fun withTransaction(block: suspend BaseFHIREngine.() -> Unit) { + override suspend fun withTransaction(block: suspend CrudFhirEngine.() -> Unit) { database.withTransaction { block.invoke(this@FhirEngineImpl) } } diff --git a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt index b263392903..fbde51870e 100644 --- a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt +++ b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt @@ -20,7 +20,7 @@ import androidx.work.Data import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum import ca.uhn.fhir.parser.IParser -import com.google.android.fhir.BaseFHIREngine +import com.google.android.fhir.CrudFhirEngine import com.google.android.fhir.FhirEngine import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken @@ -177,7 +177,7 @@ object TestFhirEngineImpl : FhirEngine { download().collect() } - override suspend fun withTransaction(block: suspend BaseFHIREngine.() -> Unit) {} + override suspend fun withTransaction(block: suspend CrudFhirEngine.() -> Unit) {} override suspend fun count(search: Search): Long { return 0 From 75b41b513ba83514a13d07de480c966d4a931312 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=E2=89=A1ZRS?= <12814349+LZRS@users.noreply.github.com> Date: Thu, 30 May 2024 21:02:28 +0300 Subject: [PATCH 4/7] Move count method back to FhirEngine class --- .../java/com/google/android/fhir/FhirEngine.kt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt index 53df1a2e11..39778cdba6 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -94,6 +94,14 @@ interface FhirEngine : CrudFhirEngine { download: suspend () -> Flow>, ) + /** + * Returns the total count of entities available for the given [Search]. + * + * @param search The search criteria to apply. + * @return The total number of matching resources. + */ + suspend fun count(search: Search): Long + /** * Returns the timestamp when data was last synchronized, or `null` if no synchronization has * occurred yet. @@ -207,14 +215,6 @@ interface CrudFhirEngine { */ suspend fun search(search: Search): List> - /** - * Returns the total count of entities available for the given [Search]. - * - * @param search The search criteria to apply. - * @return The total number of matching resources. - */ - suspend fun count(search: Search): Long - /** * Adds support for performing actions on `FhirEngine` as a single atomic transaction where the * entire set of changes succeed or fail as a single entity From 892ed273c004e7799b469e1bb05e55a32d803d7c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=E2=89=A1ZRS?= <12814349+LZRS@users.noreply.github.com> Date: Fri, 14 Jun 2024 19:14:49 +0300 Subject: [PATCH 5/7] Update 'withTransaction' tests to better reflect need for atomic transactions --- .../android/fhir/impl/FhirEngineImplTest.kt | 139 ++++++++++++------ 1 file changed, 94 insertions(+), 45 deletions(-) diff --git a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt index 6f0ee53e13..98b05e8273 100644 --- a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt @@ -22,7 +22,6 @@ 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.delete import com.google.android.fhir.get import com.google.android.fhir.lastUpdated import com.google.android.fhir.logicalId @@ -48,13 +47,18 @@ import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.hl7.fhir.exceptions.FHIRException import org.hl7.fhir.r4.model.Address +import org.hl7.fhir.r4.model.Appointment 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.Encounter import org.hl7.fhir.r4.model.Enumerations import org.hl7.fhir.r4.model.HumanName import org.hl7.fhir.r4.model.Meta +import org.hl7.fhir.r4.model.Observation import org.hl7.fhir.r4.model.Patient +import org.hl7.fhir.r4.model.Practitioner +import org.hl7.fhir.r4.model.Reference import org.hl7.fhir.r4.model.ResourceType import org.junit.Assert.assertThrows import org.junit.Before @@ -792,33 +796,54 @@ class FhirEngineImplTest { id = patient01ID gender = Enumerations.AdministrativeGender.FEMALE } - val patient02ID = "patient-02" - val patient02 = - Patient().apply { - id = patient02ID - gender = Enumerations.AdministrativeGender.MALE + val patient01AppointmentID = "appointment-01" + val patient01Appointment = + Appointment().apply { + id = patient01AppointmentID + status = Appointment.AppointmentStatus.BOOKED + addParticipant( + Appointment.AppointmentParticipantComponent().apply { + actor = Reference("${patient01.resourceType}/$patient01ID") + }, + ) } - val patient03ID = "patient-03" - val patient03 = - Patient().apply { - id = patient03ID - gender = Enumerations.AdministrativeGender.FEMALE + fhirEngine.create(patient01, patient01Appointment) + + // Fulfill appointment with related Encounter/Observation + val patient01AppointmentEncounterID = "enc-01" + val patient01AppointmentEncounter = + Encounter().apply { + id = patient01AppointmentEncounterID + subject = Reference("${patient01.resourceType}/$patient01ID") + addAppointment(Reference("${patient01Appointment.resourceType}/$patient01AppointmentID")) } + val patient01AppointmentEncounterObservationID = "obs-01" + val patient01AppointmentEncounterObservation = + Observation().apply { + id = patient01AppointmentEncounterObservationID + encounter = + Reference( + "${patient01AppointmentEncounter.resourceType}/$patient01AppointmentEncounterID", + ) + } + val updatedAppointment = + patient01Appointment.copy().apply { status = Appointment.AppointmentStatus.FULFILLED } - fhirEngine.create(patient01) - assertThat(fhirEngine.get(patient01ID).gender) - .isEqualTo(Enumerations.AdministrativeGender.FEMALE) fhirEngine.withTransaction { - fhirEngine.create(patient02, patient03) - patient01.gender = Enumerations.AdministrativeGender.FEMALE - fhirEngine.update(patient01) - fhirEngine.delete(patient01ID) - } - assertResourceEquals(patient02, fhirEngine.get(patient02ID)) - assertResourceEquals(patient03, fhirEngine.get(patient03ID)) - assertThrows(ResourceNotFoundException::class.java) { - runBlocking { fhirEngine.get(patient01ID) } + this.create(patient01AppointmentEncounter, patient01AppointmentEncounterObservation) + this.update(updatedAppointment) } + + assertThat( + fhirEngine.get(patient01AppointmentEncounterID).appointmentFirstRep.reference, + ) + .isEqualTo("Appointment/$patient01AppointmentID") + assertThat( + fhirEngine.get(patient01AppointmentEncounterObservationID).encounter.reference, + ) + .isEqualTo("Encounter/$patient01AppointmentEncounterID") + assertThat(fhirEngine.get(patient01AppointmentID).status) + .isEqualTo(Appointment.AppointmentStatus.FULFILLED) } @Test @@ -829,40 +854,64 @@ class FhirEngineImplTest { id = patient01ID gender = Enumerations.AdministrativeGender.FEMALE } - val patient02ID = "patient-02" - val patient02 = - Patient().apply { - id = patient02ID - gender = Enumerations.AdministrativeGender.MALE + val patient01AppointmentID = "appointment-01" + val patient01Appointment = + Appointment().apply { + id = patient01AppointmentID + status = Appointment.AppointmentStatus.BOOKED + addParticipant( + Appointment.AppointmentParticipantComponent().apply { + actor = Reference("${patient01.resourceType}/$patient01ID") + }, + ) } - val patient03ID = "patient-03" - val patient03 = - Patient().apply { - id = patient03ID - gender = Enumerations.AdministrativeGender.FEMALE + fhirEngine.create(patient01, patient01Appointment) + + // Fulfill appointment with related Encounter/Observation + val patient01AppointmentEncounterID = "enc-01" + val patient01AppointmentEncounter = + Encounter().apply { + id = patient01AppointmentEncounterID + subject = Reference("${patient01.resourceType}/$patient01ID") + addAppointment(Reference("${patient01Appointment.resourceType}/$patient01AppointmentID")) + } + val patient01AppointmentEncounterObservationID = "obs-01" + val patient01AppointmentEncounterObservation = + Observation().apply { + id = patient01AppointmentEncounterObservationID + encounter = + Reference( + "${patient01AppointmentEncounter.resourceType}/$patient01AppointmentEncounterID", + ) } - val patient03Updated = - patient03.copy().apply { gender = Enumerations.AdministrativeGender.MALE } - fhirEngine.create(patient03) try { fhirEngine.withTransaction { - this.create(patient01) - this.create(patient02) - this.update(patient03Updated) - this.get(ResourceType.Patient, "non_existent_patient_id") - as Patient // Force ResourceNotFoundException + this.create(patient01AppointmentEncounter, patient01AppointmentEncounterObservation) + // Get non-existent practitioner to force ResourceNotFoundException + val nonExistentPractitioner = + this.get(ResourceType.Practitioner, "non_existent_practitioner_id") as Practitioner + val updatedAppointment = + patient01Appointment.copy().apply { + status = Appointment.AppointmentStatus.FULFILLED + addParticipant( + Appointment.AppointmentParticipantComponent().apply { + actor = Reference("Practitioner/${nonExistentPractitioner.logicalId}") + }, + ) + } + this.update(updatedAppointment) } } catch (_: ResourceNotFoundException) {} assertThrows(ResourceNotFoundException::class.java) { - runBlocking { fhirEngine.get(patient01ID) } + runBlocking { fhirEngine.get(patient01AppointmentEncounterID) } } assertThrows(ResourceNotFoundException::class.java) { - runBlocking { fhirEngine.get(patient02ID) } + runBlocking { fhirEngine.get(patient01AppointmentEncounterObservationID) } } - assertResourceEquals(patient03, fhirEngine.get(patient03ID)) - assertResourceNotEquals(patient03Updated, fhirEngine.get(patient03ID)) + assertThat(fhirEngine.get(patient01AppointmentID).status) + .isEqualTo(Appointment.AppointmentStatus.BOOKED) } companion object { From b520d30ff7ea2b02dddc03913f904636a1831d26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=E2=89=A1ZRS?= <12814349+LZRS@users.noreply.github.com> Date: Fri, 4 Oct 2024 17:06:21 +0300 Subject: [PATCH 6/7] Revert to using FhirEngine interface and removed the CRUDEngine interface --- .../com/google/android/fhir/FhirEngine.kt | 110 ++++++++---------- .../android/fhir/impl/FhirEngineImpl.kt | 3 +- .../google/android/fhir/testing/Utilities.kt | 3 +- 3 files changed, 51 insertions(+), 65 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt index 6ae329f53a..16553409bb 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngine.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngine.kt @@ -58,7 +58,54 @@ import org.hl7.fhir.r4.model.ResourceType * val fhirEngine = FhirEngineProvider.getInstance(this) * ``` */ -interface FhirEngine : CrudFhirEngine { +interface FhirEngine { + /** + * Creates one or more FHIR [Resource]s in the local storage. FHIR Engine requires all stored + * resources to have a logical [Resource.id]. If the `id` is specified in the resource passed to + * [create], the resource created in `FhirEngine` will have the same `id`. If no `id` is + * specified, `FhirEngine` will generate a UUID as that resource's `id` and include it in the + * returned list of IDs. + * + * @param resource The FHIR resources to create. + * @return A list of logical IDs of the newly created resources. + */ + suspend fun create(vararg resource: Resource): List + + /** + * Loads a FHIR resource given its [ResourceType] and logical ID. + * + * @param type The type of the resource to load. + * @param id The logical ID of the resource. + * @return The requested FHIR resource. + * @throws ResourceNotFoundException if the resource is not found. + */ + @Throws(ResourceNotFoundException::class) + suspend fun get(type: ResourceType, id: String): Resource + + /** + * Updates one or more FHIR [Resource]s in the local storage. + * + * @param resource The FHIR resources to update. + */ + suspend fun update(vararg resource: Resource) + + /** + * Removes a FHIR resource given its [ResourceType] and logical ID. + * + * @param type The type of the resource to delete. + * @param id The logical ID of the resource. + */ + suspend fun delete(type: ResourceType, id: String) + + /** + * Searches the database and returns a list of resources matching the [Search] specifications. + * + * @param search The search criteria to apply. + * @return A list of [SearchResult] objects containing the matching resources and any included + * references. + */ + suspend fun search(search: Search): List> + /** * Synchronizes upload results with the database. * @@ -155,71 +202,12 @@ interface FhirEngine : CrudFhirEngine { * back and no record is purged. */ suspend fun purge(type: ResourceType, ids: Set, forcePurge: Boolean = false) -} - -interface CrudFhirEngine { - /** - * Creates one or more FHIR [Resource]s in the local storage. FHIR Engine requires all stored - * resources to have a logical [Resource.id]. If the `id` is specified in the resource passed to - * [create], the resource created in `FhirEngine` will have the same `id`. If no `id` is - * specified, `FhirEngine` will generate a UUID as that resource's `id` and include it in the - * returned list of IDs. - * - * @param resource The FHIR resources to create. - * @return A list of logical IDs of the newly created resources. - */ - suspend fun create(vararg resource: Resource): List - - /** - * Loads a FHIR resource given its [ResourceType] and logical ID. - * - * @param type The type of the resource to load. - * @param id The logical ID of the resource. - * @return The requested FHIR resource. - * @throws ResourceNotFoundException if the resource is not found. - */ - @Throws(ResourceNotFoundException::class) - suspend fun get(type: ResourceType, id: String): Resource - - /** - * Updates one or more FHIR [Resource]s in the local storage. - * - * @param resource The FHIR resources to update. - */ - suspend fun update(vararg resource: Resource) - - /** - * Removes a FHIR resource given its [ResourceType] and logical ID. - * - * @param type The type of the resource to delete. - * @param id The logical ID of the resource. - */ - suspend fun delete(type: ResourceType, id: String) - - /** - * Searches the database and returns a list of resources matching the [Search] specifications. - * - * Example: - * ``` - * fhirEngine.search { - * filter(Patient.GIVEN, { - * value = "Kiran" - * modifier = StringFilterModifier.MATCHES_EXACTLY - * }) - * } - * ``` - * - * @param search The search criteria to apply. - * @return A list of [SearchResult] objects containing the matching resources and any included - * references. - */ - suspend fun search(search: Search): List> /** * Adds support for performing actions on `FhirEngine` as a single atomic transaction where the * entire set of changes succeed or fail as a single entity */ - suspend fun withTransaction(block: suspend CrudFhirEngine.() -> Unit) + suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) } /** diff --git a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt index ad52fc14e2..ac79292dd8 100644 --- a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt @@ -17,7 +17,6 @@ package com.google.android.fhir.impl import android.content.Context -import com.google.android.fhir.CrudFhirEngine import com.google.android.fhir.FhirEngine import com.google.android.fhir.FhirEngineProvider import com.google.android.fhir.LocalChange @@ -98,7 +97,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex } } - override suspend fun withTransaction(block: suspend CrudFhirEngine.() -> Unit) { + override suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) { database.withTransaction { block.invoke(this@FhirEngineImpl) } } diff --git a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt index 2ce2608999..5faf188a28 100644 --- a/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt +++ b/engine/src/main/java/com/google/android/fhir/testing/Utilities.kt @@ -20,7 +20,6 @@ import androidx.work.Data import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum import ca.uhn.fhir.parser.IParser -import com.google.android.fhir.CrudFhirEngine import com.google.android.fhir.FhirEngine import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken @@ -177,7 +176,7 @@ internal object TestFhirEngineImpl : FhirEngine { download().collect() } - override suspend fun withTransaction(block: suspend CrudFhirEngine.() -> Unit) {} + override suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) {} override suspend fun count(search: Search): Long { return 0 From faacaec5ae4bb9836e39be318225db9d3d7ae300 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=E2=89=A1ZRS?= <12814349+LZRS@users.noreply.github.com> Date: Wed, 9 Oct 2024 02:09:33 +0300 Subject: [PATCH 7/7] Simplify tests for fhirengine's withTransaction --- .../android/fhir/impl/FhirEngineImpl.kt | 2 +- .../android/fhir/impl/FhirEngineImplTest.kt | 142 +++++------------- 2 files changed, 41 insertions(+), 103 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt index ac79292dd8..d0a274313a 100644 --- a/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/impl/FhirEngineImpl.kt @@ -98,7 +98,7 @@ internal class FhirEngineImpl(private val database: Database, private val contex } override suspend fun withTransaction(block: suspend FhirEngine.() -> Unit) { - database.withTransaction { block.invoke(this@FhirEngineImpl) } + database.withTransaction { this.block() } } private suspend fun saveResolvedResourcesToDatabase(resolved: List?) { diff --git a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt index 98b05e8273..2864fc721e 100644 --- a/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt +++ b/engine/src/test/java/com/google/android/fhir/impl/FhirEngineImplTest.kt @@ -47,8 +47,8 @@ import kotlinx.coroutines.runBlocking import kotlinx.coroutines.test.runTest import org.hl7.fhir.exceptions.FHIRException import org.hl7.fhir.r4.model.Address -import org.hl7.fhir.r4.model.Appointment import org.hl7.fhir.r4.model.CanonicalType +import org.hl7.fhir.r4.model.CodeableConcept import org.hl7.fhir.r4.model.Coding import org.hl7.fhir.r4.model.DateTimeType import org.hl7.fhir.r4.model.Encounter @@ -57,7 +57,6 @@ import org.hl7.fhir.r4.model.HumanName import org.hl7.fhir.r4.model.Meta import org.hl7.fhir.r4.model.Observation import org.hl7.fhir.r4.model.Patient -import org.hl7.fhir.r4.model.Practitioner import org.hl7.fhir.r4.model.Reference import org.hl7.fhir.r4.model.ResourceType import org.junit.Assert.assertThrows @@ -789,129 +788,68 @@ class FhirEngineImplTest { } @Test - fun `withTransaction saves all changes successfully in order`() = runTest { - val patient01ID = "patient-01" - val patient01 = - Patient().apply { - id = patient01ID - gender = Enumerations.AdministrativeGender.FEMALE - } - val patient01AppointmentID = "appointment-01" - val patient01Appointment = - Appointment().apply { - id = patient01AppointmentID - status = Appointment.AppointmentStatus.BOOKED - addParticipant( - Appointment.AppointmentParticipantComponent().apply { - actor = Reference("${patient01.resourceType}/$patient01ID") - }, - ) - } - fhirEngine.create(patient01, patient01Appointment) - - // Fulfill appointment with related Encounter/Observation - val patient01AppointmentEncounterID = "enc-01" - val patient01AppointmentEncounter = - Encounter().apply { - id = patient01AppointmentEncounterID - subject = Reference("${patient01.resourceType}/$patient01ID") - addAppointment(Reference("${patient01Appointment.resourceType}/$patient01AppointmentID")) - } - val patient01AppointmentEncounterObservationID = "obs-01" - val patient01AppointmentEncounterObservation = - Observation().apply { - id = patient01AppointmentEncounterObservationID - encounter = - Reference( - "${patient01AppointmentEncounter.resourceType}/$patient01AppointmentEncounterID", - ) - } - val updatedAppointment = - patient01Appointment.copy().apply { status = Appointment.AppointmentStatus.FULFILLED } - + fun `withTransaction saves changes successfully`() = runTest { fhirEngine.withTransaction { - this.create(patient01AppointmentEncounter, patient01AppointmentEncounterObservation) - this.update(updatedAppointment) + val patient01 = + Patient().apply { + id = "patient-01" + gender = Enumerations.AdministrativeGender.FEMALE + } + this.create(patient01) + + val patient01Observation = + Observation().apply { + id = "patient-01-observation" + status = Observation.ObservationStatus.FINAL + code = CodeableConcept() + subject = Reference(patient01) + } + this.create(patient01Observation) } assertThat( - fhirEngine.get(patient01AppointmentEncounterID).appointmentFirstRep.reference, + fhirEngine.get("patient-01"), ) - .isEqualTo("Appointment/$patient01AppointmentID") + .isNotNull() + assertThat(fhirEngine.get("patient-01-observation")).isNotNull() assertThat( - fhirEngine.get(patient01AppointmentEncounterObservationID).encounter.reference, + fhirEngine.get("patient-01-observation").subject.reference, ) - .isEqualTo("Encounter/$patient01AppointmentEncounterID") - assertThat(fhirEngine.get(patient01AppointmentID).status) - .isEqualTo(Appointment.AppointmentStatus.FULFILLED) + .isEqualTo("Patient/patient-01") } @Test - fun `withTransaction reverts all changes when an error occurs`() = runTest { - val patient01ID = "patient-01" + fun `withTransaction rolls back changes when an error occurs`() = runTest { val patient01 = Patient().apply { - id = patient01ID + id = "patient-01" gender = Enumerations.AdministrativeGender.FEMALE } - val patient01AppointmentID = "appointment-01" - val patient01Appointment = - Appointment().apply { - id = patient01AppointmentID - status = Appointment.AppointmentStatus.BOOKED - addParticipant( - Appointment.AppointmentParticipantComponent().apply { - actor = Reference("${patient01.resourceType}/$patient01ID") - }, - ) - } - fhirEngine.create(patient01, patient01Appointment) - - // Fulfill appointment with related Encounter/Observation - val patient01AppointmentEncounterID = "enc-01" - val patient01AppointmentEncounter = - Encounter().apply { - id = patient01AppointmentEncounterID - subject = Reference("${patient01.resourceType}/$patient01ID") - addAppointment(Reference("${patient01Appointment.resourceType}/$patient01AppointmentID")) - } - val patient01AppointmentEncounterObservationID = "obs-01" - val patient01AppointmentEncounterObservation = - Observation().apply { - id = patient01AppointmentEncounterObservationID - encounter = - Reference( - "${patient01AppointmentEncounter.resourceType}/$patient01AppointmentEncounterID", - ) - } + + fhirEngine.create(patient01) try { fhirEngine.withTransaction { - this.create(patient01AppointmentEncounter, patient01AppointmentEncounterObservation) - // Get non-existent practitioner to force ResourceNotFoundException - val nonExistentPractitioner = - this.get(ResourceType.Practitioner, "non_existent_practitioner_id") as Practitioner - val updatedAppointment = - patient01Appointment.copy().apply { - status = Appointment.AppointmentStatus.FULFILLED - addParticipant( - Appointment.AppointmentParticipantComponent().apply { - actor = Reference("Practitioner/${nonExistentPractitioner.logicalId}") - }, - ) + val patientEncounter = + Encounter().apply { + id = "enc-01" + status = Encounter.EncounterStatus.FINISHED + class_ = Coding() + subject = Reference(patient01) } - this.update(updatedAppointment) + + this.create(patientEncounter) + + // Update encounter to reference non-existent subject to force ResourceNotFoundException + val nonExistentSubject = this.get(ResourceType.Patient, "non_existent_id") as Patient + patientEncounter.subject = Reference(nonExistentSubject) + this.update(patientEncounter) } } catch (_: ResourceNotFoundException) {} assertThrows(ResourceNotFoundException::class.java) { - runBlocking { fhirEngine.get(patient01AppointmentEncounterID) } - } - assertThrows(ResourceNotFoundException::class.java) { - runBlocking { fhirEngine.get(patient01AppointmentEncounterObservationID) } + runBlocking { fhirEngine.get("enc-01") } } - assertThat(fhirEngine.get(patient01AppointmentID).status) - .isEqualTo(Appointment.AppointmentStatus.BOOKED) } companion object {