From ea57ccb4bf3e5cc0b1319261eb6c2aa6475f5c9f Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Thu, 15 Feb 2024 13:22:39 +0530 Subject: [PATCH 1/8] Updated PerResourcePatchGenerator to return ordererd PatchMapping to avoid referential integrity issues. --- .../upload/patch/PerResourcePatchGenerator.kt | 102 +- .../patch/PerResourcePatchGeneratorTest.kt | 941 ++++++++++++++++++ 2 files changed, 1041 insertions(+), 2 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt index 1ca70b7f43..1b517674e6 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt @@ -33,7 +33,12 @@ import com.google.android.fhir.LocalChange.Type internal object PerResourcePatchGenerator : PatchGenerator { override fun generate(localChanges: List): List { - return localChanges + return generateSquashedChangesMapping(localChanges).orderByReferences() + } + + @androidx.annotation.VisibleForTesting + internal fun generateSquashedChangesMapping(localChanges: List) = + localChanges .groupBy { it.resourceType to it.resourceId } .values .mapNotNull { resourceLocalChanges -> @@ -44,7 +49,6 @@ internal object PerResourcePatchGenerator : PatchGenerator { ) } } - } private fun mergeLocalChangesForSingleResource(localChanges: List): Patch? { // TODO (maybe this should throw exception when two entities don't have the same versionID) @@ -139,3 +143,97 @@ internal object PerResourcePatchGenerator : PatchGenerator { return objectMapper.writeValueAsString(mergedNode) } } + +private typealias Node = String + +/** + * @return - A ordered list of the [PatchMapping]s based on the references to other [PatchMapping] + * if the mappings are acyclic + * - throws [IllegalStateException] otherwise + */ +@androidx.annotation.VisibleForTesting +internal fun List.orderByReferences(): List { + // if the resource A has outgoing references (B,C) and these referenced resources are getting + // created on device, + // then these referenced resources (B,C) needs to go before the resource A so that referential + // integrity is retained. + val resourceIdToPatchMapping = associateBy { patchMapping -> + "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}" + } + + val adjacencyList = + createReferenceAdjacencyList( + resourceIdToPatchMapping.keys + .filter { resourceIdToPatchMapping[it]?.generatedPatch?.type == Patch.Type.INSERT } + .toSet(), + ) + return createTopologicalOrderedList(adjacencyList).mapNotNull { resourceIdToPatchMapping[it] } +} + +/** + * @return A map of [PatchMapping] to all the outgoing references to the other [PatchMapping]s of + * type [Patch.Type.INSERT] . + */ +@androidx.annotation.VisibleForTesting +internal fun List.createReferenceAdjacencyList( + insertResourceIds: Set, +): Map> { + val adjacencyList = mutableMapOf>() + forEach { patchMapping -> + adjacencyList[ + "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}", + ] = patchMapping.findOutgoingReferences().filter { insertResourceIds.contains(it) } + } + return adjacencyList +} + +// if the outgoing reference is to a resource that's just an update and not create, then don't link +// to it. This may make the sub graphs smaller and also help avoid cyclic dependencies. +@androidx.annotation.VisibleForTesting +internal fun PatchMapping.findOutgoingReferences(): List { + val references = mutableListOf() + when (generatedPatch.type) { + Patch.Type.INSERT -> { + JsonLoader.fromString(generatedPatch.payload).search(references) + } + Patch.Type.UPDATE -> { + JsonLoader.fromString(generatedPatch.payload).forEach { + if (it.get("path").asText().endsWith("/reference")) { + references.add(it.get("value").asText()) + } else { + it.get("value").search(references) + } + } + } + Patch.Type.DELETE -> { + // do nothing + } + } + return references +} + +private fun JsonNode.search(reference: MutableList) { + if (has("reference") && get("reference").isTextual) { + reference.add(get("reference").asText()) + } + elements().forEach { it.search(reference) } +} + +@androidx.annotation.VisibleForTesting +internal fun createTopologicalOrderedList(adjacencyList: Map>): List { + val stack = ArrayDeque() + val visited = mutableSetOf() + val currentPath = mutableSetOf() + + fun dfs(key: String) { + check(currentPath.add(key)) { "Detected a cycle." } + if (visited.add(key)) { + adjacencyList[key]?.forEach { dfs(it) } + stack.addFirst(key) + } + currentPath.remove(key) + } + + adjacencyList.keys.forEach { dfs(it) } + return stack.reversed() +} diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt index 5d8c8b0016..a8be90e4bf 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt @@ -32,10 +32,17 @@ import com.google.android.fhir.versionId import com.google.common.truth.Truth.assertThat import java.time.Instant import java.util.Date +import java.util.LinkedList import java.util.UUID +import org.hl7.fhir.r4.model.CarePlan +import org.hl7.fhir.r4.model.Encounter +import org.hl7.fhir.r4.model.Group 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.Reference +import org.hl7.fhir.r4.model.RelatedPerson import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType import org.json.JSONArray @@ -516,4 +523,938 @@ class PerResourcePatchGeneratorTest { timestamp = Instant.now(), ) } + + @Test + fun test_observation_findOutgoingReferences() { + val observation = + FhirContext.forR4Cached() + .newJsonParser() + .parseResource( + """ + { + "resourceType" : "Observation", + "id" : "f206", + "text" : { + "status" : "generated", + "div" : "

Generated Narrative: Observation

Resource Observation "f206"

status: final

code: Blood culture (acmelabs.org#104177; LOINC#600-7 "Bacteria identified in Blood by Culture")

subject: Patient/f201: Roel "Roel"

issued: 11 Mar 2013, 8:28:00 pm

performer: Practitioner/f202: Luigi Maas "Luigi Maas"

value: Staphylococcus aureus (SNOMED CT#3092008)

interpretation: Positive (ObservationInterpretation#POS)

method: Blood culture for bacteria, including anaerobic screen (SNOMED CT#104177005)

" + }, + "status" : "final", + "code" : { + "coding" : [{ + "system" : "http://acmelabs.org", + "code" : "104177", + "display" : "Blood culture" + }, + { + "system" : "http://loinc.org", + "code" : "600-7", + "display" : "Bacteria identified in Blood by Culture" + }] + }, + "subject" : { + "reference" : "Patient/f201", + "display" : "Roel" + }, + "issued" : "2013-03-11T10:28:00+01:00", + "performer" : [{ + "reference" : "Practitioner/f202", + "display" : "Luigi Maas" + }], + "valueCodeableConcept" : { + "coding" : [{ + "system" : "http://snomed.info/sct", + "code" : "3092008", + "display" : "Staphylococcus aureus" + }] + }, + "interpretation" : [{ + "coding" : [{ + "system" : "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation", + "code" : "POS" + }] + }], + "method" : { + "coding" : [{ + "system" : "http://snomed.info/sct", + "code" : "104177005", + "display" : "Blood culture for bacteria, including anaerobic screen" + }] + } + } + """ + .trimIndent(), + ) as Resource + + val result = + PerResourcePatchGenerator.generateSquashedChangesMapping( + listOf(createInsertLocalChange(observation)), + ) + .single() + .findOutgoingReferences() + assertThat(result).containsExactly("Patient/f201", "Practitioner/f202") + } + + @Test + fun test_group_findOutgoingReferences() { + val group = + FhirContext.forR4Cached() + .newJsonParser() + .parseResource( + """ + { + "resourceType" : "Group", + "id" : "102", + "text" : { + "status" : "additional", + "div" : "
\n

Selected Patients

\n
    \n
  • Patient Donald DUCK @ Acme Healthcare, Inc. MR = 654321
    • \n
  • Patient Donald D DUCK @ Acme Healthcare, Inc. MR = 123456
    • \n
  • Patient Simon Notsowell @ Acme Healthcare, Inc. MR = 123457, DECEASED
    • \n
  • Patient Sandy Notsowell @ Acme Healthcare, Inc. MR = 123458, DECEASED
    • \n
\n
" + }, + "type" : "person", + "membership" : "enumerated", + "member" : [{ + "entity" : { + "reference" : "Patient/pat1" + }, + "period" : { + "start" : "2014-10-08" + } + }, + { + "entity" : { + "reference" : "Patient/pat2" + }, + "period" : { + "start" : "2015-04-02" + }, + "inactive" : true + }, + { + "entity" : { + "reference" : "Patient/pat3" + }, + "period" : { + "start" : "2015-08-06" + } + }, + { + "entity" : { + "reference" : "Patient/pat4" + }, + "period" : { + "start" : "2015-08-06" + } + }], + "characteristic" : [{ + "code" : { + "coding" : [{ + "system" : "http://example.org", + "code" : "attributed-to" + }], + "text" : "Patients primarily attributed to" + }, + "valueReference" : { + "reference" : "Practitioner/123" + }, + "exclude" : false + }] +} + """ + .trimIndent(), + ) as Resource + + val result = + PerResourcePatchGenerator.generateSquashedChangesMapping( + listOf(createInsertLocalChange(group)), + ) + .single() + .findOutgoingReferences() + assertThat(result) + .containsExactly( + "Patient/pat1", + "Patient/pat2", + "Patient/pat3", + "Patient/pat4", + "Practitioner/123", + ) + } + + @Test + fun test_careplan_insertpatch_findOutgoingReferences() { + val group = + FhirContext.forR4Cached() + .newJsonParser() + .parseResource( + """{ + "resourceType": "CarePlan", + "id": "gpvisit", + "text": { + "status": "additional", + "div": "
\n

Represents the flow of a patient within a practice. The plan is created when\n they arrive and represents the 'care' of the patient over the course of that encounter.\n They first see the nurse for basic observations (BP, pulse, temp) then the doctor for\n the consultation and finally the nurse again for a tetanus immunization. As the plan is\n updated (e.g. a new activity added), different versions of the plan exist, and workflow timings\n for reporting can be gained by examining the plan history. This example is the version after\n seeing the doctor, and waiting for the nurse.The plan can either be created 'ad hoc' and modified as\n the parient progresses, or start with a standard template (which can, of course, be altered to suit the patient.

\n
" + }, + "contained": [ + { + "resourceType": "Condition", + "id": "p1", + "clinicalStatus": { + "coding": [ + { + "system": "http://terminology.hl7.org/CodeSystem/condition-clinical", + "code": "active" + } + ] + }, + "verificationStatus": { + "coding": [ + { + "system": "http://terminology.hl7.org/CodeSystem/condition-ver-status", + "code": "confirmed" + } + ] + }, + "code": { + "text": "Overseas encounter" + }, + "subject": { + "reference": "Patient/100", + "display": "Peter James Chalmers" + } + }, + { + "resourceType": "CareTeam", + "id": "careteam", + "participant": [ + { + "id": "part1", + "role": [ + { + "coding": [ + { + "system": "http://example.org/local", + "code": "nur" + } + ], + "text": "nurse" + } + ], + "member": { + "reference": "Practitioner/13", + "display": "Nurse Nancy" + } + }, + { + "id": "part2", + "role": [ + { + "coding": [ + { + "system": "http://example.org/local", + "code": "doc" + } + ], + "text": "doctor" + } + ], + "member": { + "reference": "Practitioner/14", + "display": "Doctor Dave" + } + } + ] + }, + { + "resourceType": "Goal", + "id": "goal", + "lifecycleStatus": "planned", + "description": { + "text": "Complete consultation" + }, + "subject": { + "reference": "Patient/100", + "display": "Peter James Chalmers" + } + } + ], + "status": "active", + "intent": "plan", + "subject": { + "reference": "Patient/100", + "display": "Peter James Chalmers" + }, + "period": { + "start": "2013-01-01T10:30:00+00:00" + }, + "careTeam": [ + { + "reference": "#careteam" + } + ], + "addresses": [ + { + "reference": "#p1", + "display": "obesity" + } + ], + "goal": [ + { + "reference": "#goal" + } + ], + "activity": [ + { + "outcomeReference": [ + { + "reference": "Encounter/example" + } + ], + "detail": { + "kind": "Appointment", + "code": { + "coding": [ + { + "system": "http://example.org/local", + "code": "nursecon" + } + ], + "text": "Nurse Consultation" + }, + "status": "completed", + "doNotPerform": false, + "scheduledPeriod": { + "start": "2013-01-01T10:38:00+00:00", + "end": "2013-01-01T10:50:00+00:00" + }, + "performer": [ + { + "reference": "Practitioner/13", + "display": "Nurse Nancy" + } + ] + } + }, + { + "detail": { + "kind": "Appointment", + "code": { + "coding": [ + { + "system": "http://example.org/local", + "code": "doccon" + } + ], + "text": "Doctor Consultation" + }, + "status": "scheduled", + "doNotPerform": false, + "performer": [ + { + "reference": "Practitioner/14", + "display": "Doctor Dave" + } + ] + } + } + ] +} + """ + .trimIndent(), + ) as Resource + + val result = + PerResourcePatchGenerator.generateSquashedChangesMapping( + listOf(createInsertLocalChange(group)), + ) + .single() + .findOutgoingReferences() + assertThat(result) + .containsExactly( + "Patient/100", + "Practitioner/13", + "Practitioner/14", + "Patient/100", + "Patient/100", + "#careteam", + "#p1", + "#goal", + "Encounter/example", + "Practitioner/13", + "Practitioner/14", + ) + .inOrder() + } + + @Test + fun test_careplan_updatepatch_findOutgoingReferences() { + val group = + FhirContext.forR4Cached() + .newJsonParser() + .parseResource( + """{ + "resourceType": "CarePlan", + "id": "gpvisit", + "text": { + "status": "additional", + "div": "
\n

Represents the flow of a patient within a practice. The plan is created when\n they arrive and represents the 'care' of the patient over the course of that encounter.\n They first see the nurse for basic observations (BP, pulse, temp) then the doctor for\n the consultation and finally the nurse again for a tetanus immunization. As the plan is\n updated (e.g. a new activity added), different versions of the plan exist, and workflow timings\n for reporting can be gained by examining the plan history. This example is the version after\n seeing the doctor, and waiting for the nurse.The plan can either be created 'ad hoc' and modified as\n the parient progresses, or start with a standard template (which can, of course, be altered to suit the patient.

\n
" + }, + "contained": [ + { + "resourceType": "Condition", + "id": "p1", + "clinicalStatus": { + "coding": [ + { + "system": "http://terminology.hl7.org/CodeSystem/condition-clinical", + "code": "active" + } + ] + }, + "verificationStatus": { + "coding": [ + { + "system": "http://terminology.hl7.org/CodeSystem/condition-ver-status", + "code": "confirmed" + } + ] + }, + "code": { + "text": "Overseas encounter" + }, + "subject": { + "reference": "Patient/100", + "display": "Peter James Chalmers" + } + }, + { + "resourceType": "CareTeam", + "id": "careteam", + "participant": [ + { + "id": "part1", + "role": [ + { + "coding": [ + { + "system": "http://example.org/local", + "code": "nur" + } + ], + "text": "nurse" + } + ], + "member": { + "reference": "Practitioner/13", + "display": "Nurse Nancy" + } + }, + { + "id": "part2", + "role": [ + { + "coding": [ + { + "system": "http://example.org/local", + "code": "doc" + } + ], + "text": "doctor" + } + ], + "member": { + "reference": "Practitioner/14", + "display": "Doctor Dave" + } + } + ] + }, + { + "resourceType": "Goal", + "id": "goal", + "lifecycleStatus": "planned", + "description": { + "text": "Complete consultation" + }, + "subject": { + "reference": "Patient/100", + "display": "Peter James Chalmers" + } + } + ], + "status": "active", + "intent": "plan", + "subject": { + "reference": "Patient/100", + "display": "Peter James Chalmers" + }, + "period": { + "start": "2013-01-01T10:30:00+00:00" + }, + "careTeam": [ + { + "reference": "#careteam" + } + ], + "addresses": [ + { + "reference": "#p1", + "display": "obesity" + } + ], + "goal": [ + { + "reference": "#goal" + } + ], + "activity": [ + { + "outcomeReference": [ + { + "reference": "Encounter/example" + } + ], + "detail": { + "kind": "Appointment", + "code": { + "coding": [ + { + "system": "http://example.org/local", + "code": "nursecon" + } + ], + "text": "Nurse Consultation" + }, + "status": "completed", + "doNotPerform": false, + "scheduledPeriod": { + "start": "2013-01-01T10:38:00+00:00", + "end": "2013-01-01T10:50:00+00:00" + }, + "performer": [ + { + "reference": "Practitioner/13", + "display": "Nurse Nancy" + } + ] + } + }, + { + "detail": { + "kind": "Appointment", + "code": { + "coding": [ + { + "system": "http://example.org/local", + "code": "doccon" + } + ], + "text": "Doctor Consultation" + }, + "status": "scheduled", + "doNotPerform": false, + "performer": [ + { + "reference": "Practitioner/14", + "display": "Doctor Dave" + } + ] + } + } + ] +} + """ + .trimIndent(), + ) as Resource + + val result = + PerResourcePatchGenerator.generateSquashedChangesMapping( + listOf(createUpdateLocalChange(CarePlan().apply { id = "gpvisit" }, group, 1)), + ) + .single() + .also { println(it) } + .findOutgoingReferences() + assertThat(result) + .containsExactly( + "Patient/100", + "Practitioner/13", + "Practitioner/14", + "Patient/100", + "Patient/100", + "#careteam", + "#p1", + "#goal", + "Encounter/example", + "Practitioner/13", + "Practitioner/14", + ) + } + + @Test + fun test_createReferenceAdjacencyList() { + val localChanges = LinkedList() + + var group = + Group() + .apply { + id = "group-1" + type = Group.GroupType.PERSON + } + .also { localChanges.add(createInsertLocalChange(it)) } + + // pa-01 + Patient().apply { id = "patient-1" }.also { localChanges.add(createInsertLocalChange(it)) } + Encounter() + .apply { + id = "encounter-1" + subject = Reference("Patient/patient-1") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Observation() + .apply { + id = "observation-1" + subject = Reference("Patient/patient-1") + encounter = Reference("Encounter/encounter-1") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 1)) } + + // pa-02 + Patient().apply { id = "patient-2" }.also { localChanges.add(createInsertLocalChange(it)) } + Encounter() + .apply { + id = "encounter-2" + subject = Reference("Patient/patient-2") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Observation() + .apply { + id = "observation-2" + subject = Reference("Patient/patient-2") + encounter = Reference("Encounter/encounter-2") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 2)) } + + // pa-03 + Patient().apply { id = "patient-3" }.also { localChanges.add(createInsertLocalChange(it)) } + + Encounter() + .apply { + id = "encounter-3" + subject = Reference("Patient/patient-3") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Observation() + .apply { + id = "observation-3" + subject = Reference("Patient/patient-3") + encounter = Reference("Encounter/encounter-3") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 3)) } + + val result = + PerResourcePatchGenerator.generateSquashedChangesMapping(localChanges) + .createReferenceAdjacencyList( + localChanges.map { "${it.resourceType}/${it.resourceId}" }.toSet(), + ) + + assertThat(result) + .isEqualTo( + mutableMapOf( + "Group/group-1" to listOf("Patient/patient-1", "Patient/patient-2", "Patient/patient-3"), + "Patient/patient-1" to emptyList(), + "Patient/patient-2" to emptyList(), + "Patient/patient-3" to emptyList(), + "Encounter/encounter-1" to listOf("Patient/patient-1"), + "Encounter/encounter-2" to listOf("Patient/patient-2"), + "Encounter/encounter-3" to listOf("Patient/patient-3"), + "Observation/observation-1" to listOf("Patient/patient-1", "Encounter/encounter-1"), + "Observation/observation-2" to listOf("Patient/patient-2", "Encounter/encounter-2"), + "Observation/observation-3" to listOf("Patient/patient-3", "Encounter/encounter-3"), + ), + ) + } + + @Test + fun test_createReferenceAdjacencyList_WithUpdate() { + val localChanges = LinkedList() + + var group = + Group() + .apply { + id = "group-1" + type = Group.GroupType.PERSON + } + .also { localChanges.add(createInsertLocalChange(it)) } + + // pa-01 + Patient() + .apply { id = "patient-1" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + 1, + ), + ) + } + Encounter() + .apply { + id = "encounter-1" + subject = Reference("Patient/patient-1") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Observation() + .apply { + id = "observation-1" + subject = Reference("Patient/patient-1") + encounter = Reference("Encounter/encounter-1") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 1)) } + + // pa-02 + Patient() + .apply { id = "patient-2" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + 1, + ), + ) + } + Encounter() + .apply { + id = "encounter-2" + subject = Reference("Patient/patient-2") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Observation() + .apply { + id = "observation-2" + subject = Reference("Patient/patient-2") + encounter = Reference("Encounter/encounter-2") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 2)) } + + // pa-03 + Patient() + .apply { id = "patient-3" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + 1, + ), + ) + } + + Encounter() + .apply { + id = "encounter-3" + subject = Reference("Patient/patient-3") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Observation() + .apply { + id = "observation-3" + subject = Reference("Patient/patient-3") + encounter = Reference("Encounter/encounter-3") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 3)) } + + val result = + PerResourcePatchGenerator.generateSquashedChangesMapping(localChanges) + .createReferenceAdjacencyList( + localChanges + .filter { it.type == LocalChange.Type.INSERT } + .map { "${it.resourceType}/${it.resourceId}" } + .toSet(), + ) + + assertThat(result) + .isEqualTo( + mutableMapOf( + "Group/group-1" to emptyList(), + "Patient/patient-1" to emptyList(), + "Patient/patient-2" to emptyList(), + "Patient/patient-3" to emptyList(), + "Encounter/encounter-1" to emptyList(), + "Encounter/encounter-2" to emptyList(), + "Encounter/encounter-3" to emptyList(), + "Observation/observation-1" to listOf("Encounter/encounter-1"), + "Observation/observation-2" to listOf("Encounter/encounter-2"), + "Observation/observation-3" to listOf("Encounter/encounter-3"), + ), + ) + } + + @Test + fun `generate with acyclic references should return the list in topological order`() { + val localChanges = LinkedList() + + var group = + Group() + .apply { + id = "group-1" + type = Group.GroupType.PERSON + } + .also { localChanges.add(createInsertLocalChange(it)) } + + // pa-01 + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 1)) } + + Observation() + .apply { + id = "observation-1" + subject = Reference("Patient/patient-1") + encounter = Reference("Encounter/encounter-1") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Encounter() + .apply { + id = "encounter-1" + subject = Reference("Patient/patient-1") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Patient().apply { id = "patient-1" }.also { localChanges.add(createInsertLocalChange(it)) } + + // pa-02 + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 2)) } + + Observation() + .apply { + id = "observation-2" + subject = Reference("Patient/patient-2") + encounter = Reference("Encounter/encounter-2") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Encounter() + .apply { + id = "encounter-2" + subject = Reference("Patient/patient-2") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Patient().apply { id = "patient-2" }.also { localChanges.add(createInsertLocalChange(it)) } + + // pa-03 + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } + .also { localChanges.add(createUpdateLocalChange(group, it, 3)) } + + Observation() + .apply { + id = "observation-3" + subject = Reference("Patient/patient-3") + encounter = Reference("Encounter/encounter-3") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Encounter() + .apply { + id = "encounter-3" + subject = Reference("Patient/patient-3") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + Patient().apply { id = "patient-3" }.also { localChanges.add(createInsertLocalChange(it)) } + + val result = PerResourcePatchGenerator.generate(localChanges) + assertThat(result.map { it.generatedPatch.resourceId }) + .containsExactly( + "patient-1", + "patient-2", + "patient-3", + "group-1", + "encounter-1", + "observation-1", + "encounter-2", + "observation-2", + "encounter-3", + "observation-3", + ) + .inOrder() + } + + @Test + fun `generate with cyclic references should throw exception`() { + val localChanges = LinkedList() + + Patient() + .apply { + id = "patient-1" + addLink( + Patient.PatientLinkComponent().apply { other = Reference("RelatedPerson/related-1") }, + ) + } + .also { localChanges.add(createInsertLocalChange(it)) } + + RelatedPerson() + .apply { + id = "related-1" + patient = Reference("Patient/patient-1") + } + .also { localChanges.add(createInsertLocalChange(it)) } + + val errorMessage = + Assert.assertThrows(IllegalStateException::class.java) { + PerResourcePatchGenerator.generate(localChanges) + } + .localizedMessage + + assertThat(errorMessage).isEqualTo("Detected a cycle.") + } } From 24a120eae34a04b020983bef9a1eb375df17bb85 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Thu, 15 Feb 2024 15:03:47 +0530 Subject: [PATCH 2/8] Fixed the failing test by fixing the palyload json format --- .../src/main/java/com/google/android/fhir/testing/Utilities.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 18f3b624e9..12ddd1fa07 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 @@ -191,7 +191,7 @@ object TestFhirEngineImpl : FhirEngine { LocalChange( resourceType = type.name, resourceId = id, - payload = "{ 'resourceType' : '$type', 'id' : '$id' }", + payload = """{ "resourceType" : "$type", "id" : "$id" }""", token = LocalChangeToken(listOf()), type = LocalChange.Type.INSERT, timestamp = Instant.now(), From 558d6c5cba1fd11e076f3d108f8a5ae3d4a8c440 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Thu, 22 Feb 2024 23:29:57 +0530 Subject: [PATCH 3/8] Review Comments: Refactored code to find references using LocalChangeResourceReferenceEntity --- engine/build.gradle.kts | 1 + .../google/android/fhir/FhirEngineProvider.kt | 6 + .../com/google/android/fhir/db/Database.kt | 15 + .../android/fhir/db/impl/DatabaseImpl.kt | 13 + .../fhir/db/impl/dao/LocalChangeDao.kt | 13 +- .../android/fhir/sync/FhirSyncWorker.kt | 6 +- .../fhir/sync/upload/patch/PatchGenerator.kt | 8 +- .../upload/patch/PerChangePatchGenerator.kt | 4 +- .../upload/patch/PerResourcePatchGenerator.kt | 57 +- .../google/android/fhir/testing/Utilities.kt | 2 +- .../android/fhir/sync/upload/UploaderTest.kt | 25 +- .../patch/PerResourcePatchGeneratorTest.kt | 1505 +++++++---------- 12 files changed, 770 insertions(+), 885 deletions(-) diff --git a/engine/build.gradle.kts b/engine/build.gradle.kts index ae44fe7a0c..afa565f71f 100644 --- a/engine/build.gradle.kts +++ b/engine/build.gradle.kts @@ -140,6 +140,7 @@ dependencies { testImplementation(Dependencies.AndroidxTest.core) testImplementation(Dependencies.AndroidxTest.workTestingRuntimeKtx) testImplementation(Dependencies.Kotlin.kotlinCoroutinesTest) + testImplementation(Dependencies.Kotlin.kotlinTestJunit) testImplementation(Dependencies.junit) testImplementation(Dependencies.jsonAssert) testImplementation(Dependencies.mockitoInline) diff --git a/engine/src/main/java/com/google/android/fhir/FhirEngineProvider.kt b/engine/src/main/java/com/google/android/fhir/FhirEngineProvider.kt index b3775f8165..2077576f73 100644 --- a/engine/src/main/java/com/google/android/fhir/FhirEngineProvider.kt +++ b/engine/src/main/java/com/google/android/fhir/FhirEngineProvider.kt @@ -18,6 +18,7 @@ package com.google.android.fhir import android.content.Context import com.google.android.fhir.DatabaseErrorStrategy.UNSPECIFIED +import com.google.android.fhir.db.Database import com.google.android.fhir.sync.DataSource import com.google.android.fhir.sync.FhirDataStore import com.google.android.fhir.sync.HttpAuthenticator @@ -67,6 +68,11 @@ object FhirEngineProvider { return getOrCreateFhirService(context).fhirDataStore } + @Synchronized + internal fun getFhirDatabase(context: Context): Database { + return getOrCreateFhirService(context).database + } + @Synchronized private fun getOrCreateFhirService(context: Context): FhirServices { if (fhirServices == null) { diff --git a/engine/src/main/java/com/google/android/fhir/db/Database.kt b/engine/src/main/java/com/google/android/fhir/db/Database.kt index 83157a35ba..5b813027b2 100644 --- a/engine/src/main/java/com/google/android/fhir/db/Database.kt +++ b/engine/src/main/java/com/google/android/fhir/db/Database.kt @@ -185,9 +185,24 @@ internal interface Database { * will delete resource entry from LocalChangeEntity table. */ suspend fun purge(type: ResourceType, id: String, forcePurge: Boolean = false) + + /** + * @return List of [LocalChangeResourceReference] associated with the [LocalChangeEntity.id]s. A + * single [LocalChangeEntity] may have one or more [LocalChangeResourceReference] associated + * with it. + */ + suspend fun getLocalChangeResourceReferences( + localChangeIds: List, + ): List } data class ResourceWithUUID( val uuid: UUID, val resource: R, ) + +data class LocalChangeResourceReference( + val localChangeId: Long, + val resourceReferenceValue: String, + val resourceReferencePath: String?, +) diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt index af044b4f63..9acdf95ea5 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/DatabaseImpl.kt @@ -26,6 +26,7 @@ import ca.uhn.fhir.util.FhirTerser import com.google.android.fhir.DatabaseErrorStrategy import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken +import com.google.android.fhir.db.LocalChangeResourceReference import com.google.android.fhir.db.ResourceNotFoundException import com.google.android.fhir.db.ResourceWithUUID import com.google.android.fhir.db.impl.DatabaseImpl.Companion.UNENCRYPTED_DATABASE_NAME @@ -408,6 +409,18 @@ internal class DatabaseImpl( } } + override suspend fun getLocalChangeResourceReferences( + localChangeIds: List, + ): List { + return localChangeDao.getReferencesForLocalChanges(localChangeIds).map { + LocalChangeResourceReference( + it.localChangeId, + it.resourceReferenceValue, + it.resourceReferencePath, + ) + } + } + companion object { /** * The name for unencrypted database. diff --git a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt index 43eac00442..664977fadb 100644 --- a/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt +++ b/engine/src/main/java/com/google/android/fhir/db/impl/dao/LocalChangeDao.kt @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2023-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -348,6 +348,17 @@ internal abstract class LocalChangeDao { localChangeId: Long, ): List + @Query( + """ + SELECT * + FROM LocalChangeResourceReferenceEntity + WHERE localChangeId = (:localChangeId) + """, + ) + abstract suspend fun getReferencesForLocalChanges( + localChangeId: List, + ): List + @Insert(onConflict = OnConflictStrategy.REPLACE) abstract suspend fun insertLocalChangeResourceReferences( resourceReferences: List, diff --git a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt b/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt index bc07ddc12a..eaac92d0b8 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/FhirSyncWorker.kt @@ -76,7 +76,11 @@ abstract class FhirSyncWorker(appContext: Context, workerParams: WorkerParameter UploadConfiguration( Uploader( dataSource = dataSource, - patchGenerator = PatchGeneratorFactory.byMode(getUploadStrategy().patchGeneratorMode), + patchGenerator = + PatchGeneratorFactory.byMode( + getUploadStrategy().patchGeneratorMode, + FhirEngineProvider.getFhirDatabase(applicationContext), + ), requestGenerator = UploadRequestGeneratorFactory.byMode(getUploadStrategy().requestGeneratorMode), ), diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt index 325378d242..27e27580f3 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2023-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -17,6 +17,7 @@ package com.google.android.fhir.sync.upload.patch import com.google.android.fhir.LocalChange +import com.google.android.fhir.db.Database /** * Generates [Patch]es from [LocalChange]s and output [List<[PatchMapping]>] to keep a mapping of @@ -34,16 +35,17 @@ internal interface PatchGenerator { * NOTE: different implementations may have requirements on the size of [localChanges] and output * certain numbers of [Patch]es. */ - fun generate(localChanges: List): List + suspend fun generate(localChanges: List): List } internal object PatchGeneratorFactory { fun byMode( mode: PatchGeneratorMode, + database: Database, ): PatchGenerator = when (mode) { is PatchGeneratorMode.PerChange -> PerChangePatchGenerator - is PatchGeneratorMode.PerResource -> PerResourcePatchGenerator + is PatchGeneratorMode.PerResource -> PerResourcePatchGenerator(database) } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt index 153314e118..8c787f644e 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerChangePatchGenerator.kt @@ -1,5 +1,5 @@ /* - * Copyright 2023 Google LLC + * Copyright 2023-2024 Google LLC * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -25,7 +25,7 @@ import com.google.android.fhir.LocalChange * maintain an audit trail. */ internal object PerChangePatchGenerator : PatchGenerator { - override fun generate(localChanges: List): List = + override suspend fun generate(localChanges: List): List = localChanges.map { PatchMapping( localChanges = listOf(it), diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt index 1b517674e6..c05f8c0f19 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt @@ -22,6 +22,8 @@ import com.github.fge.jackson.JsonLoader import com.github.fge.jsonpatch.JsonPatch import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChange.Type +import com.google.android.fhir.db.Database +import com.google.android.fhir.db.LocalChangeResourceReference /** * Generates a [Patch] for all [LocalChange]es made to a single FHIR resource. @@ -30,10 +32,10 @@ import com.google.android.fhir.LocalChange.Type * maintain an audit trail, but instead, multiple changes made to the same FHIR resource on the * client can be recorded as a single change on the server. */ -internal object PerResourcePatchGenerator : PatchGenerator { +internal class PerResourcePatchGenerator(val database: Database) : PatchGenerator { - override fun generate(localChanges: List): List { - return generateSquashedChangesMapping(localChanges).orderByReferences() + override suspend fun generate(localChanges: List): List { + return generateSquashedChangesMapping(localChanges).orderByReferences(database) } @androidx.annotation.VisibleForTesting @@ -152,7 +154,7 @@ private typealias Node = String * - throws [IllegalStateException] otherwise */ @androidx.annotation.VisibleForTesting -internal fun List.orderByReferences(): List { +internal suspend fun List.orderByReferences(database: Database): List { // if the resource A has outgoing references (B,C) and these referenced resources are getting // created on device, // then these referenced resources (B,C) needs to go before the resource A so that referential @@ -161,11 +163,18 @@ internal fun List.orderByReferences(): List { "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}" } + // get references for all the local changes + val localChangeIdToReferenceMap: Map> = + database + .getLocalChangeResourceReferences(flatMap { it.localChanges.flatMap { it.token.ids } }) + .groupBy { it.localChangeId } + val adjacencyList = - createReferenceAdjacencyList( + createAdjacencyListForCreateReferences( resourceIdToPatchMapping.keys .filter { resourceIdToPatchMapping[it]?.generatedPatch?.type == Patch.Type.INSERT } .toSet(), + localChangeIdToReferenceMap, ) return createTopologicalOrderedList(adjacencyList).mapNotNull { resourceIdToPatchMapping[it] } } @@ -175,33 +184,36 @@ internal fun List.orderByReferences(): List { * type [Patch.Type.INSERT] . */ @androidx.annotation.VisibleForTesting -internal fun List.createReferenceAdjacencyList( +internal fun List.createAdjacencyListForCreateReferences( insertResourceIds: Set, + localChangeIdToReferenceMap: Map>, ): Map> { val adjacencyList = mutableMapOf>() forEach { patchMapping -> adjacencyList[ "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}", - ] = patchMapping.findOutgoingReferences().filter { insertResourceIds.contains(it) } + ] = + patchMapping.findOutgoingReferences(localChangeIdToReferenceMap).filter { + insertResourceIds.contains(it) + } } return adjacencyList } // if the outgoing reference is to a resource that's just an update and not create, then don't link // to it. This may make the sub graphs smaller and also help avoid cyclic dependencies. -@androidx.annotation.VisibleForTesting -internal fun PatchMapping.findOutgoingReferences(): List { - val references = mutableListOf() +private fun PatchMapping.findOutgoingReferences( + localChangeIdToReferenceMap: Map>, +): Set { + val references = mutableSetOf() when (generatedPatch.type) { - Patch.Type.INSERT -> { - JsonLoader.fromString(generatedPatch.payload).search(references) - } - Patch.Type.UPDATE -> { - JsonLoader.fromString(generatedPatch.payload).forEach { - if (it.get("path").asText().endsWith("/reference")) { - references.add(it.get("value").asText()) - } else { - it.get("value").search(references) + Patch.Type.INSERT, + Patch.Type.UPDATE, -> { + localChanges.forEach { localChange -> + localChange.token.ids.forEach { id -> + localChangeIdToReferenceMap[id]?.let { + references.addAll(it.map { it.resourceReferenceValue }) + } } } } @@ -212,13 +224,6 @@ internal fun PatchMapping.findOutgoingReferences(): List { return references } -private fun JsonNode.search(reference: MutableList) { - if (has("reference") && get("reference").isTextual) { - reference.add(get("reference").asText()) - } - elements().forEach { it.search(reference) } -} - @androidx.annotation.VisibleForTesting internal fun createTopologicalOrderedList(adjacencyList: Map>): List { val stack = ArrayDeque() 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 12ddd1fa07..48b83f3f5e 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 @@ -192,7 +192,7 @@ object TestFhirEngineImpl : FhirEngine { resourceType = type.name, resourceId = id, payload = """{ "resourceType" : "$type", "id" : "$id" }""", - token = LocalChangeToken(listOf()), + token = LocalChangeToken(listOf(1)), type = LocalChange.Type.INSERT, timestamp = Instant.now(), ), diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderTest.kt index 9d21c9db34..feffe91d7c 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/UploaderTest.kt @@ -20,8 +20,10 @@ import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken +import com.google.android.fhir.db.Database import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.logicalId +import com.google.android.fhir.sync.upload.patch.PatchGenerator import com.google.android.fhir.sync.upload.patch.PatchGeneratorFactory import com.google.android.fhir.sync.upload.patch.PatchGeneratorMode import com.google.android.fhir.sync.upload.request.UploadRequestGeneratorFactory @@ -42,13 +44,32 @@ import org.hl7.fhir.r4.model.OperationOutcome import org.hl7.fhir.r4.model.Patient import org.hl7.fhir.r4.model.ResourceType import org.hl7.fhir.r4.model.codesystems.HttpVerb +import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any +import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class UploaderTest { + @Mock private lateinit var database: Database + private lateinit var perResourcePatchGenerator: PatchGenerator + private lateinit var perChangePatchGenerator: PatchGenerator + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + runTest { whenever(database.getLocalChangeResourceReferences(any())).thenReturn(emptyList()) } + + perResourcePatchGenerator = + PatchGeneratorFactory.byMode(PatchGeneratorMode.PerResource, database) + perChangePatchGenerator = PatchGeneratorFactory.byMode(PatchGeneratorMode.PerChange, database) + } + @Test fun `bundle upload for per resource patch should output responses mapped correctly to the local changes`() = runTest { @@ -513,9 +534,7 @@ class UploaderTest { } companion object { - private val perResourcePatchGenerator = - PatchGeneratorFactory.byMode(PatchGeneratorMode.PerResource) - private val perChangePatchGenerator = PatchGeneratorFactory.byMode(PatchGeneratorMode.PerChange) + private val urlUploadRequestGenerator = UploadRequestGeneratorFactory.byMode( UploadRequestGeneratorMode.UrlRequest(HttpVerb.PUT, HttpVerb.PATCH), diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt index a8be90e4bf..329641b5a8 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt @@ -20,6 +20,8 @@ import ca.uhn.fhir.context.FhirContext import ca.uhn.fhir.context.FhirVersionEnum import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken +import com.google.android.fhir.db.Database +import com.google.android.fhir.db.LocalChangeResourceReference import com.google.android.fhir.db.impl.dao.diff import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.logicalId @@ -34,7 +36,9 @@ import java.time.Instant import java.util.Date import java.util.LinkedList import java.util.UUID -import org.hl7.fhir.r4.model.CarePlan +import kotlin.random.Random +import kotlin.test.assertFailsWith +import kotlinx.coroutines.test.runTest import org.hl7.fhir.r4.model.Encounter import org.hl7.fhir.r4.model.Group import org.hl7.fhir.r4.model.HumanName @@ -46,20 +50,34 @@ import org.hl7.fhir.r4.model.RelatedPerson import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType import org.json.JSONArray -import org.junit.Assert +import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any +import org.mockito.kotlin.whenever import org.robolectric.RobolectricTestRunner @RunWith(RobolectricTestRunner::class) class PerResourcePatchGeneratorTest { + @Mock private lateinit var database: Database + private lateinit var patchGenerator: PerResourcePatchGenerator + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + runTest { whenever(database.getLocalChangeResourceReferences(any())).thenReturn(emptyList()) } + patchGenerator = PerResourcePatchGenerator(database) + } + @Test - fun `should generate a single insert patch if the resource is inserted`() { + fun `should generate a single insert patch if the resource is inserted`() = runTest { val patient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") val insertionLocalChange = createInsertLocalChange(patient) - val patches = PerResourcePatchGenerator.generate(listOf(insertionLocalChange)) + val patches = patchGenerator.generate(listOf(insertionLocalChange)) with(patches.single()) { with(generatedPatch) { @@ -77,7 +95,7 @@ class PerResourcePatchGeneratorTest { } @Test - fun `should generate a single update patch if the resource is updated`() { + fun `should generate a single update patch if the resource is updated`() = runTest { val remoteMeta = Meta().apply { versionId = "patient-version-1" @@ -90,7 +108,7 @@ class PerResourcePatchGeneratorTest { val updateLocalChange1 = createUpdateLocalChange(remotePatient, updatedPatient1, 1L) val updatePatch = readJsonArrayFromFile("/update_patch_1.json") - val patches = PerResourcePatchGenerator.generate(listOf(updateLocalChange1)) + val patches = patchGenerator.generate(listOf(updateLocalChange1)) with(patches.single()) { with(generatedPatch) { @@ -109,7 +127,7 @@ class PerResourcePatchGeneratorTest { } @Test - fun `should generate a single delete patch if the resource is deleted`() { + fun `should generate a single delete patch if the resource is deleted`() = runTest { val remoteMeta = Meta().apply { versionId = "patient-version-1" @@ -119,7 +137,7 @@ class PerResourcePatchGeneratorTest { remotePatient.meta = remoteMeta val deleteLocalChange = createDeleteLocalChange(remotePatient, 3L) - val patches = PerResourcePatchGenerator.generate(listOf(deleteLocalChange)) + val patches = patchGenerator.generate(listOf(deleteLocalChange)) with(patches.single()) { with(generatedPatch) { @@ -138,15 +156,14 @@ class PerResourcePatchGeneratorTest { } @Test - fun `should generate a single insert patch if the resource is inserted and updated`() { + fun `should generate a single insert patch if the resource is inserted and updated`() = runTest { val patient: Patient = readFromFile(Patient::class.java, "/date_test_patient.json") val insertionLocalChange = createInsertLocalChange(patient) val updatedPatient = readFromFile(Patient::class.java, "/update_test_patient_1.json") val updateLocalChange = createUpdateLocalChange(patient, updatedPatient, 1L) val patientString = jsonParser.encodeResourceToString(updatedPatient) - val patches = - PerResourcePatchGenerator.generate(listOf(insertionLocalChange, updateLocalChange)) + val patches = patchGenerator.generate(listOf(insertionLocalChange, updateLocalChange)) with(patches.single()) { with(generatedPatch) { @@ -164,7 +181,7 @@ class PerResourcePatchGeneratorTest { } @Test - fun `should generate no patch if the resource is inserted and deleted`() { + fun `should generate no patch if the resource is inserted and deleted`() = runTest { val changes = listOf( LocalChangeEntity( @@ -203,13 +220,13 @@ class PerResourcePatchGeneratorTest { .toLocalChange() .apply { LocalChangeToken(listOf(2)) }, ) - val patchToUpload = PerResourcePatchGenerator.generate(changes) + val patchToUpload = patchGenerator.generate(changes) assertThat(patchToUpload).isEmpty() } @Test - fun `should generate no patch if the resource is inserted, updated, and deleted`() { + fun `should generate no patch if the resource is inserted, updated, and deleted`() = runTest { val changes = listOf( LocalChangeEntity( @@ -281,13 +298,13 @@ class PerResourcePatchGeneratorTest { .toLocalChange() .apply { LocalChangeToken(listOf(3)) }, ) - val patchToUpload = PerResourcePatchGenerator.generate(changes) + val patchToUpload = patchGenerator.generate(changes) assertThat(patchToUpload).isEmpty() } @Test - fun `should generate a single update patch if the resource is updated twice`() { + fun `should generate a single update patch if the resource is updated twice`() = runTest { val remoteMeta = Meta().apply { versionId = "patient-version-1" @@ -303,7 +320,7 @@ class PerResourcePatchGeneratorTest { val updateLocalChange2 = createUpdateLocalChange(updatedPatient1, updatedPatient2, 2L) val updatePatch = readJsonArrayFromFile("/update_patch_2.json") - val patches = PerResourcePatchGenerator.generate(listOf(updateLocalChange1, updateLocalChange2)) + val patches = patchGenerator.generate(listOf(updateLocalChange1, updateLocalChange2)) with(patches.single()) { with(generatedPatch) { @@ -322,44 +339,44 @@ class PerResourcePatchGeneratorTest { } @Test - fun `should generate a single update patch with three elements of two adds and one remove`() { - val expectedPatch = readJsonArrayFromFile("/update_careplan_patch.json") - val updatePatch1 = readJsonArrayFromFile("/update_careplan_patch_1.json") - val updatePatch2 = readJsonArrayFromFile("/update_careplan_patch_2.json") - - val updatedLocalChange1 = - LocalChange( - resourceType = "CarePlan", - resourceId = "131b5257-a8b3-435a-8cb3-4cb1296be24a", - type = LocalChange.Type.UPDATE, - payload = updatePatch1.toString(), - timestamp = Instant.now(), - token = LocalChangeToken(listOf(1)), - ) + fun `should generate a single update patch with three elements of two adds and one remove`() = + runTest { + val expectedPatch = readJsonArrayFromFile("/update_careplan_patch.json") + val updatePatch1 = readJsonArrayFromFile("/update_careplan_patch_1.json") + val updatePatch2 = readJsonArrayFromFile("/update_careplan_patch_2.json") + + val updatedLocalChange1 = + LocalChange( + resourceType = "CarePlan", + resourceId = "131b5257-a8b3-435a-8cb3-4cb1296be24a", + type = LocalChange.Type.UPDATE, + payload = updatePatch1.toString(), + timestamp = Instant.now(), + token = LocalChangeToken(listOf(1)), + ) - val updatedLocalChange2 = - LocalChange( - resourceType = "CarePlan", - resourceId = "131b5257-a8b3-435a-8cb3-4cb1296be24a", - type = LocalChange.Type.UPDATE, - payload = updatePatch2.toString(), - timestamp = Instant.now(), - token = LocalChangeToken(listOf(1)), - ) + val updatedLocalChange2 = + LocalChange( + resourceType = "CarePlan", + resourceId = "131b5257-a8b3-435a-8cb3-4cb1296be24a", + type = LocalChange.Type.UPDATE, + payload = updatePatch2.toString(), + timestamp = Instant.now(), + token = LocalChangeToken(listOf(1)), + ) - val patches = - PerResourcePatchGenerator.generate(listOf(updatedLocalChange1, updatedLocalChange2)) + val patches = patchGenerator.generate(listOf(updatedLocalChange1, updatedLocalChange2)) - with(patches.single().generatedPatch) { - assertThat(type).isEqualTo(Patch.Type.UPDATE) - assertThat(resourceId).isEqualTo("131b5257-a8b3-435a-8cb3-4cb1296be24a") - assertThat(resourceType).isEqualTo("CarePlan") - assertJsonArrayEqualsIgnoringOrder(JSONArray(payload), expectedPatch) + with(patches.single().generatedPatch) { + assertThat(type).isEqualTo(Patch.Type.UPDATE) + assertThat(resourceId).isEqualTo("131b5257-a8b3-435a-8cb3-4cb1296be24a") + assertThat(resourceType).isEqualTo("CarePlan") + assertJsonArrayEqualsIgnoringOrder(JSONArray(payload), expectedPatch) + } } - } @Test - fun `should generate a single delete patch if the resource is updated and deleted`() { + fun `should generate a single delete patch if the resource is updated and deleted`() = runTest { val remoteMeta = Meta().apply { versionId = "patient-version-1" @@ -376,7 +393,7 @@ class PerResourcePatchGeneratorTest { val deleteLocalChange = createDeleteLocalChange(updatedPatient2, 3L) val patches = - PerResourcePatchGenerator.generate( + patchGenerator.generate( listOf(updateLocalChange1, updateLocalChange2, deleteLocalChange), ) @@ -397,7 +414,7 @@ class PerResourcePatchGeneratorTest { } @Test - fun `should throw an error if a change is done after a resource is deleted locally`() { + fun `should throw an error if a change is done after a resource is deleted locally`() = runTest { val changes = listOf( LocalChangeEntity( @@ -425,16 +442,14 @@ class PerResourcePatchGeneratorTest { ) val errorMessage = - Assert.assertThrows(IllegalArgumentException::class.java) { - PerResourcePatchGenerator.generate(changes) - } + assertFailsWith { patchGenerator.generate(changes) } .localizedMessage assertThat(errorMessage).isEqualTo("Changes after deletion of resource are not permitted") } @Test - fun `should throw an error if a change is done before a resource is created locally`() { + fun `should throw an error if a change is done before a resource is created locally`() = runTest { val changes = listOf( LocalChangeEntity( @@ -473,11 +488,8 @@ class PerResourcePatchGeneratorTest { .toLocalChange() .apply { LocalChangeToken(listOf(2)) }, ) - val errorMessage = - Assert.assertThrows(IllegalArgumentException::class.java) { - PerResourcePatchGenerator.generate(changes) - } + assertFailsWith { patchGenerator.generate(changes) } .localizedMessage assertThat(errorMessage).isEqualTo("Changes before creation of resource are not permitted") @@ -500,14 +512,14 @@ class PerResourcePatchGeneratorTest { ) } - private fun createInsertLocalChange(entity: Resource): LocalChange { + private fun createInsertLocalChange(entity: Resource, currentChangeId: Long = 1): LocalChange { return LocalChange( resourceId = entity.logicalId, resourceType = entity.resourceType.name, type = LocalChange.Type.INSERT, payload = jsonParser.encodeResourceToString(entity), versionId = entity.versionId, - token = LocalChangeToken(listOf(1L)), + token = LocalChangeToken(listOf(currentChangeId)), timestamp = Instant.now(), ) } @@ -525,812 +537,478 @@ class PerResourcePatchGeneratorTest { } @Test - fun test_observation_findOutgoingReferences() { - val observation = - FhirContext.forR4Cached() - .newJsonParser() - .parseResource( - """ - { - "resourceType" : "Observation", - "id" : "f206", - "text" : { - "status" : "generated", - "div" : "

Generated Narrative: Observation

Resource Observation "f206"

status: final

code: Blood culture (acmelabs.org#104177; LOINC#600-7 "Bacteria identified in Blood by Culture")

subject: Patient/f201: Roel "Roel"

issued: 11 Mar 2013, 8:28:00 pm

performer: Practitioner/f202: Luigi Maas "Luigi Maas"

value: Staphylococcus aureus (SNOMED CT#3092008)

interpretation: Positive (ObservationInterpretation#POS)

method: Blood culture for bacteria, including anaerobic screen (SNOMED CT#104177005)

" - }, - "status" : "final", - "code" : { - "coding" : [{ - "system" : "http://acmelabs.org", - "code" : "104177", - "display" : "Blood culture" - }, - { - "system" : "http://loinc.org", - "code" : "600-7", - "display" : "Bacteria identified in Blood by Culture" - }] - }, - "subject" : { - "reference" : "Patient/f201", - "display" : "Roel" - }, - "issued" : "2013-03-11T10:28:00+01:00", - "performer" : [{ - "reference" : "Practitioner/f202", - "display" : "Luigi Maas" - }], - "valueCodeableConcept" : { - "coding" : [{ - "system" : "http://snomed.info/sct", - "code" : "3092008", - "display" : "Staphylococcus aureus" - }] - }, - "interpretation" : [{ - "coding" : [{ - "system" : "http://terminology.hl7.org/CodeSystem/v3-ObservationInterpretation", - "code" : "POS" - }] - }], - "method" : { - "coding" : [{ - "system" : "http://snomed.info/sct", - "code" : "104177005", - "display" : "Blood culture for bacteria, including anaerobic screen" - }] - } - } - """ - .trimIndent(), - ) as Resource - - val result = - PerResourcePatchGenerator.generateSquashedChangesMapping( - listOf(createInsertLocalChange(observation)), - ) - .single() - .findOutgoingReferences() - assertThat(result).containsExactly("Patient/f201", "Practitioner/f202") - } - - @Test - fun test_group_findOutgoingReferences() { - val group = - FhirContext.forR4Cached() - .newJsonParser() - .parseResource( - """ - { - "resourceType" : "Group", - "id" : "102", - "text" : { - "status" : "additional", - "div" : "
\n

Selected Patients

\n
    \n
  • Patient Donald DUCK @ Acme Healthcare, Inc. MR = 654321
    • \n
  • Patient Donald D DUCK @ Acme Healthcare, Inc. MR = 123456
    • \n
  • Patient Simon Notsowell @ Acme Healthcare, Inc. MR = 123457, DECEASED
    • \n
  • Patient Sandy Notsowell @ Acme Healthcare, Inc. MR = 123458, DECEASED
    • \n
\n
" - }, - "type" : "person", - "membership" : "enumerated", - "member" : [{ - "entity" : { - "reference" : "Patient/pat1" - }, - "period" : { - "start" : "2014-10-08" - } - }, - { - "entity" : { - "reference" : "Patient/pat2" - }, - "period" : { - "start" : "2015-04-02" - }, - "inactive" : true - }, - { - "entity" : { - "reference" : "Patient/pat3" - }, - "period" : { - "start" : "2015-08-06" - } - }, - { - "entity" : { - "reference" : "Patient/pat4" - }, - "period" : { - "start" : "2015-08-06" - } - }], - "characteristic" : [{ - "code" : { - "coding" : [{ - "system" : "http://example.org", - "code" : "attributed-to" - }], - "text" : "Patients primarily attributed to" - }, - "valueReference" : { - "reference" : "Practitioner/123" - }, - "exclude" : false - }] -} - """ - .trimIndent(), - ) as Resource - - val result = - PerResourcePatchGenerator.generateSquashedChangesMapping( - listOf(createInsertLocalChange(group)), - ) - .single() - .findOutgoingReferences() - assertThat(result) - .containsExactly( - "Patient/pat1", - "Patient/pat2", - "Patient/pat3", - "Patient/pat4", - "Practitioner/123", - ) - } - - @Test - fun test_careplan_insertpatch_findOutgoingReferences() { - val group = - FhirContext.forR4Cached() - .newJsonParser() - .parseResource( - """{ - "resourceType": "CarePlan", - "id": "gpvisit", - "text": { - "status": "additional", - "div": "
\n

Represents the flow of a patient within a practice. The plan is created when\n they arrive and represents the 'care' of the patient over the course of that encounter.\n They first see the nurse for basic observations (BP, pulse, temp) then the doctor for\n the consultation and finally the nurse again for a tetanus immunization. As the plan is\n updated (e.g. a new activity added), different versions of the plan exist, and workflow timings\n for reporting can be gained by examining the plan history. This example is the version after\n seeing the doctor, and waiting for the nurse.The plan can either be created 'ad hoc' and modified as\n the parient progresses, or start with a standard template (which can, of course, be altered to suit the patient.

\n
" - }, - "contained": [ - { - "resourceType": "Condition", - "id": "p1", - "clinicalStatus": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/condition-clinical", - "code": "active" - } - ] - }, - "verificationStatus": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/condition-ver-status", - "code": "confirmed" - } - ] - }, - "code": { - "text": "Overseas encounter" - }, - "subject": { - "reference": "Patient/100", - "display": "Peter James Chalmers" - } - }, - { - "resourceType": "CareTeam", - "id": "careteam", - "participant": [ - { - "id": "part1", - "role": [ - { - "coding": [ - { - "system": "http://example.org/local", - "code": "nur" - } - ], - "text": "nurse" - } - ], - "member": { - "reference": "Practitioner/13", - "display": "Nurse Nancy" - } - }, - { - "id": "part2", - "role": [ - { - "coding": [ - { - "system": "http://example.org/local", - "code": "doc" - } - ], - "text": "doctor" - } - ], - "member": { - "reference": "Practitioner/14", - "display": "Doctor Dave" + fun `createReferenceAdjacencyList with local changes for only new resources should only have edges to inserted resources`() = + runTest { + val localChanges = LinkedList() + val localChangeResourceReferences = mutableListOf() + + var group = + Group() + .apply { + id = "group-1" + type = Group.GroupType.PERSON } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + // pa-01 + Patient() + .apply { id = "patient-1" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + Encounter() + .apply { + id = "encounter-1" + subject = Reference("Patient/patient-1") } - ] - }, - { - "resourceType": "Goal", - "id": "goal", - "lifecycleStatus": "planned", - "description": { - "text": "Complete consultation" - }, - "subject": { - "reference": "Patient/100", - "display": "Peter James Chalmers" - } - } - ], - "status": "active", - "intent": "plan", - "subject": { - "reference": "Patient/100", - "display": "Peter James Chalmers" - }, - "period": { - "start": "2013-01-01T10:30:00+00:00" - }, - "careTeam": [ - { - "reference": "#careteam" - } - ], - "addresses": [ - { - "reference": "#p1", - "display": "obesity" - } - ], - "goal": [ - { - "reference": "#goal" - } - ], - "activity": [ - { - "outcomeReference": [ - { - "reference": "Encounter/example" + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Encounter.subject", + ), + ) } - ], - "detail": { - "kind": "Appointment", - "code": { - "coding": [ - { - "system": "http://example.org/local", - "code": "nursecon" - } - ], - "text": "Nurse Consultation" - }, - "status": "completed", - "doNotPerform": false, - "scheduledPeriod": { - "start": "2013-01-01T10:38:00+00:00", - "end": "2013-01-01T10:50:00+00:00" - }, - "performer": [ - { - "reference": "Practitioner/13", - "display": "Nurse Nancy" - } - ] - } - }, - { - "detail": { - "kind": "Appointment", - "code": { - "coding": [ - { - "system": "http://example.org/local", - "code": "doccon" - } - ], - "text": "Doctor Consultation" - }, - "status": "scheduled", - "doNotPerform": false, - "performer": [ - { - "reference": "Practitioner/14", - "display": "Doctor Dave" - } - ] - } - } - ] -} - """ - .trimIndent(), - ) as Resource - val result = - PerResourcePatchGenerator.generateSquashedChangesMapping( - listOf(createInsertLocalChange(group)), - ) - .single() - .findOutgoingReferences() - assertThat(result) - .containsExactly( - "Patient/100", - "Practitioner/13", - "Practitioner/14", - "Patient/100", - "Patient/100", - "#careteam", - "#p1", - "#goal", - "Encounter/example", - "Practitioner/13", - "Practitioner/14", - ) - .inOrder() - } - - @Test - fun test_careplan_updatepatch_findOutgoingReferences() { - val group = - FhirContext.forR4Cached() - .newJsonParser() - .parseResource( - """{ - "resourceType": "CarePlan", - "id": "gpvisit", - "text": { - "status": "additional", - "div": "
\n

Represents the flow of a patient within a practice. The plan is created when\n they arrive and represents the 'care' of the patient over the course of that encounter.\n They first see the nurse for basic observations (BP, pulse, temp) then the doctor for\n the consultation and finally the nurse again for a tetanus immunization. As the plan is\n updated (e.g. a new activity added), different versions of the plan exist, and workflow timings\n for reporting can be gained by examining the plan history. This example is the version after\n seeing the doctor, and waiting for the nurse.The plan can either be created 'ad hoc' and modified as\n the parient progresses, or start with a standard template (which can, of course, be altered to suit the patient.

\n
" - }, - "contained": [ - { - "resourceType": "Condition", - "id": "p1", - "clinicalStatus": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/condition-clinical", - "code": "active" - } - ] - }, - "verificationStatus": { - "coding": [ - { - "system": "http://terminology.hl7.org/CodeSystem/condition-ver-status", - "code": "confirmed" - } - ] - }, - "code": { - "text": "Overseas encounter" - }, - "subject": { - "reference": "Patient/100", - "display": "Peter James Chalmers" - } - }, - { - "resourceType": "CareTeam", - "id": "careteam", - "participant": [ - { - "id": "part1", - "role": [ - { - "coding": [ - { - "system": "http://example.org/local", - "code": "nur" - } - ], - "text": "nurse" - } - ], - "member": { - "reference": "Practitioner/13", - "display": "Nurse Nancy" - } - }, - { - "id": "part2", - "role": [ - { - "coding": [ - { - "system": "http://example.org/local", - "code": "doc" - } - ], - "text": "doctor" - } - ], - "member": { - "reference": "Practitioner/14", - "display": "Doctor Dave" - } + Observation() + .apply { + id = "observation-1" + subject = Reference("Patient/patient-1") + encounter = Reference("Encounter/encounter-1") } - ] - }, - { - "resourceType": "Goal", - "id": "goal", - "lifecycleStatus": "planned", - "description": { - "text": "Complete consultation" - }, - "subject": { - "reference": "Patient/100", - "display": "Peter James Chalmers" - } - } - ], - "status": "active", - "intent": "plan", - "subject": { - "reference": "Patient/100", - "display": "Peter James Chalmers" - }, - "period": { - "start": "2013-01-01T10:30:00+00:00" - }, - "careTeam": [ - { - "reference": "#careteam" - } - ], - "addresses": [ - { - "reference": "#p1", - "display": "obesity" - } - ], - "goal": [ - { - "reference": "#goal" - } - ], - "activity": [ - { - "outcomeReference": [ - { - "reference": "Encounter/example" + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-1", + "Observation.encounter", + ), + ) } - ], - "detail": { - "kind": "Appointment", - "code": { - "coding": [ - { - "system": "http://example.org/local", - "code": "nursecon" - } - ], - "text": "Nurse Consultation" - }, - "status": "completed", - "doNotPerform": false, - "scheduledPeriod": { - "start": "2013-01-01T10:38:00+00:00", - "end": "2013-01-01T10:50:00+00:00" - }, - "performer": [ - { - "reference": "Practitioner/13", - "display": "Nurse Nancy" - } - ] - } - }, - { - "detail": { - "kind": "Appointment", - "code": { - "coding": [ - { - "system": "http://example.org/local", - "code": "doccon" - } - ], - "text": "Doctor Consultation" - }, - "status": "scheduled", - "doNotPerform": false, - "performer": [ - { - "reference": "Practitioner/14", - "display": "Doctor Dave" - } - ] - } - } - ] -} - """ - .trimIndent(), - ) as Resource - - val result = - PerResourcePatchGenerator.generateSquashedChangesMapping( - listOf(createUpdateLocalChange(CarePlan().apply { id = "gpvisit" }, group, 1)), - ) - .single() - .also { println(it) } - .findOutgoingReferences() - assertThat(result) - .containsExactly( - "Patient/100", - "Practitioner/13", - "Practitioner/14", - "Patient/100", - "Patient/100", - "#careteam", - "#p1", - "#goal", - "Encounter/example", - "Practitioner/13", - "Practitioner/14", - ) - } - @Test - fun test_createReferenceAdjacencyList() { - val localChanges = LinkedList() + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Group.member", + ), + ) + } - var group = - Group() + // pa-02 + Patient() + .apply { id = "patient-2" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + Encounter() .apply { - id = "group-1" - type = Group.GroupType.PERSON + id = "encounter-2" + subject = Reference("Patient/patient-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Encounter.subject", + ), + ) } - .also { localChanges.add(createInsertLocalChange(it)) } - - // pa-01 - Patient().apply { id = "patient-1" }.also { localChanges.add(createInsertLocalChange(it)) } - Encounter() - .apply { - id = "encounter-1" - subject = Reference("Patient/patient-1") - } - .also { localChanges.add(createInsertLocalChange(it)) } - - Observation() - .apply { - id = "observation-1" - subject = Reference("Patient/patient-1") - encounter = Reference("Encounter/encounter-1") - } - .also { localChanges.add(createInsertLocalChange(it)) } - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 1)) } + Observation() + .apply { + id = "observation-2" + subject = Reference("Patient/patient-2") + encounter = Reference("Encounter/encounter-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-2", + "Observation.encounter", + ), + ) + } - // pa-02 - Patient().apply { id = "patient-2" }.also { localChanges.add(createInsertLocalChange(it)) } - Encounter() - .apply { - id = "encounter-2" - subject = Reference("Patient/patient-2") - } - .also { localChanges.add(createInsertLocalChange(it)) } + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Group.member", + ), + ) + } - Observation() - .apply { - id = "observation-2" - subject = Reference("Patient/patient-2") - encounter = Reference("Encounter/encounter-2") - } - .also { localChanges.add(createInsertLocalChange(it)) } + // pa-03 + Patient() + .apply { id = "patient-3" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 2)) } + Encounter() + .apply { + id = "encounter-3" + subject = Reference("Patient/patient-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Encounter.subject", + ), + ) + } - // pa-03 - Patient().apply { id = "patient-3" }.also { localChanges.add(createInsertLocalChange(it)) } + Observation() + .apply { + id = "observation-3" + subject = Reference("Patient/patient-3") + encounter = Reference("Encounter/encounter-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-3", + "Observation.encounter", + ), + ) + } - Encounter() - .apply { - id = "encounter-3" - subject = Reference("Patient/patient-3") - } - .also { localChanges.add(createInsertLocalChange(it)) } + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Group.member", + ), + ) + } - Observation() - .apply { - id = "observation-3" - subject = Reference("Patient/patient-3") - encounter = Reference("Encounter/encounter-3") - } - .also { localChanges.add(createInsertLocalChange(it)) } + whenever(database.getLocalChangeResourceReferences(any())) + .thenReturn(localChangeResourceReferences) - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 3)) } + val result = + patchGenerator + .generateSquashedChangesMapping(localChanges) + .createAdjacencyListForCreateReferences( + localChanges.map { "${it.resourceType}/${it.resourceId}" }.toSet(), + localChangeResourceReferences.groupBy { it.localChangeId }, + ) - val result = - PerResourcePatchGenerator.generateSquashedChangesMapping(localChanges) - .createReferenceAdjacencyList( - localChanges.map { "${it.resourceType}/${it.resourceId}" }.toSet(), + assertThat(result) + .isEqualTo( + mutableMapOf( + "Group/group-1" to + listOf("Patient/patient-1", "Patient/patient-2", "Patient/patient-3"), + "Patient/patient-1" to emptyList(), + "Patient/patient-2" to emptyList(), + "Patient/patient-3" to emptyList(), + "Encounter/encounter-1" to listOf("Patient/patient-1"), + "Encounter/encounter-2" to listOf("Patient/patient-2"), + "Encounter/encounter-3" to listOf("Patient/patient-3"), + "Observation/observation-1" to listOf("Patient/patient-1", "Encounter/encounter-1"), + "Observation/observation-2" to listOf("Patient/patient-2", "Encounter/encounter-2"), + "Observation/observation-3" to listOf("Patient/patient-3", "Encounter/encounter-3"), + ), ) - - assertThat(result) - .isEqualTo( - mutableMapOf( - "Group/group-1" to listOf("Patient/patient-1", "Patient/patient-2", "Patient/patient-3"), - "Patient/patient-1" to emptyList(), - "Patient/patient-2" to emptyList(), - "Patient/patient-3" to emptyList(), - "Encounter/encounter-1" to listOf("Patient/patient-1"), - "Encounter/encounter-2" to listOf("Patient/patient-2"), - "Encounter/encounter-3" to listOf("Patient/patient-3"), - "Observation/observation-1" to listOf("Patient/patient-1", "Encounter/encounter-1"), - "Observation/observation-2" to listOf("Patient/patient-2", "Encounter/encounter-2"), - "Observation/observation-3" to listOf("Patient/patient-3", "Encounter/encounter-3"), - ), - ) - } + } @Test - fun test_createReferenceAdjacencyList_WithUpdate() { - val localChanges = LinkedList() + fun `createReferenceAdjacencyList with local changes for new and old resources should only have edges to inserted resources`() = + runTest { + val localChanges = mutableListOf() + val localChangeResourceReferences = mutableListOf() + + var group = + Group() + .apply { + id = "group-1" + type = Group.GroupType.PERSON + } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + // pa-01 + Patient() + .apply { id = "patient-1" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + Random.nextLong(), + ), + ) + } - var group = - Group() + Encounter() .apply { - id = "group-1" - type = Group.GroupType.PERSON + id = "encounter-1" + subject = Reference("Patient/patient-1") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Encounter.subject", + ), + ) } - .also { localChanges.add(createInsertLocalChange(it)) } - // pa-01 - Patient() - .apply { id = "patient-1" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - 1, - ), - ) - } - Encounter() - .apply { - id = "encounter-1" - subject = Reference("Patient/patient-1") - } - .also { localChanges.add(createInsertLocalChange(it)) } + Observation() + .apply { + id = "observation-1" + subject = Reference("Patient/patient-1") + encounter = Reference("Encounter/encounter-1") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-1", + "Observation.encounter", + ), + ) + } - Observation() - .apply { - id = "observation-1" - subject = Reference("Patient/patient-1") - encounter = Reference("Encounter/encounter-1") - } - .also { localChanges.add(createInsertLocalChange(it)) } + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Group.member", + ), + ) + } - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 1)) } + // pa-02 + Patient() + .apply { id = "patient-2" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + Random.nextLong(), + ), + ) + } + Encounter() + .apply { + id = "encounter-2" + subject = Reference("Patient/patient-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Encounter.subject", + ), + ) + } - // pa-02 - Patient() - .apply { id = "patient-2" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - 1, - ), - ) - } - Encounter() - .apply { - id = "encounter-2" - subject = Reference("Patient/patient-2") - } - .also { localChanges.add(createInsertLocalChange(it)) } + Observation() + .apply { + id = "observation-2" + subject = Reference("Patient/patient-2") + encounter = Reference("Encounter/encounter-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-2", + "Observation.encounter", + ), + ) + } - Observation() - .apply { - id = "observation-2" - subject = Reference("Patient/patient-2") - encounter = Reference("Encounter/encounter-2") - } - .also { localChanges.add(createInsertLocalChange(it)) } + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Group.member", + ), + ) + } - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 2)) } + // pa-03 + Patient() + .apply { id = "patient-3" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + Random.nextLong(), + ), + ) + } - // pa-03 - Patient() - .apply { id = "patient-3" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - 1, - ), - ) - } + Encounter() + .apply { + id = "encounter-3" + subject = Reference("Patient/patient-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Encounter.subject", + ), + ) + } - Encounter() - .apply { - id = "encounter-3" - subject = Reference("Patient/patient-3") - } - .also { localChanges.add(createInsertLocalChange(it)) } + Observation() + .apply { + id = "observation-3" + subject = Reference("Patient/patient-3") + encounter = Reference("Encounter/encounter-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-3", + "Observation.encounter", + ), + ) + } - Observation() - .apply { - id = "observation-3" - subject = Reference("Patient/patient-3") - encounter = Reference("Encounter/encounter-3") - } - .also { localChanges.add(createInsertLocalChange(it)) } + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Group.member", + ), + ) + } - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 3)) } - - val result = - PerResourcePatchGenerator.generateSquashedChangesMapping(localChanges) - .createReferenceAdjacencyList( - localChanges - .filter { it.type == LocalChange.Type.INSERT } - .map { "${it.resourceType}/${it.resourceId}" } - .toSet(), - ) + whenever(database.getLocalChangeResourceReferences(any())) + .thenReturn(localChangeResourceReferences) + + val result = + patchGenerator + .generateSquashedChangesMapping(localChanges) + .createAdjacencyListForCreateReferences( + localChanges + .filter { it.type == LocalChange.Type.INSERT } + .map { "${it.resourceType}/${it.resourceId}" } + .toSet(), + localChangeResourceReferences.groupBy { it.localChangeId }, + ) - assertThat(result) - .isEqualTo( - mutableMapOf( - "Group/group-1" to emptyList(), - "Patient/patient-1" to emptyList(), - "Patient/patient-2" to emptyList(), - "Patient/patient-3" to emptyList(), - "Encounter/encounter-1" to emptyList(), - "Encounter/encounter-2" to emptyList(), - "Encounter/encounter-3" to emptyList(), - "Observation/observation-1" to listOf("Encounter/encounter-1"), - "Observation/observation-2" to listOf("Encounter/encounter-2"), - "Observation/observation-3" to listOf("Encounter/encounter-3"), - ), - ) - } + assertThat(result) + .isEqualTo( + mutableMapOf( + "Group/group-1" to emptyList(), + "Patient/patient-1" to emptyList(), + "Patient/patient-2" to emptyList(), + "Patient/patient-3" to emptyList(), + "Encounter/encounter-1" to emptyList(), + "Encounter/encounter-2" to emptyList(), + "Encounter/encounter-3" to emptyList(), + "Observation/observation-1" to listOf("Encounter/encounter-1"), + "Observation/observation-2" to listOf("Encounter/encounter-2"), + "Observation/observation-3" to listOf("Encounter/encounter-3"), + ), + ) + } @Test - fun `generate with acyclic references should return the list in topological order`() { + fun `generate with acyclic references should return the list in topological order`() = runTest { val localChanges = LinkedList() + val localChangeResourceReferences = mutableListOf() var group = Group() @@ -1338,14 +1016,23 @@ class PerResourcePatchGeneratorTest { id = "group-1" type = Group.GroupType.PERSON } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } // pa-01 group = group .copy() .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 1)) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Group.member", + ), + ) + } Observation() .apply { @@ -1353,23 +1040,59 @@ class PerResourcePatchGeneratorTest { subject = Reference("Patient/patient-1") encounter = Reference("Encounter/encounter-1") } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-1", + "Observation.encounter", + ), + ) + } Encounter() .apply { id = "encounter-1" subject = Reference("Patient/patient-1") } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Encounter.subject", + ), + ) + } - Patient().apply { id = "patient-1" }.also { localChanges.add(createInsertLocalChange(it)) } + Patient() + .apply { id = "patient-1" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } // pa-02 group = group .copy() .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 2)) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Group.member", + ), + ) + } Observation() .apply { @@ -1377,23 +1100,59 @@ class PerResourcePatchGeneratorTest { subject = Reference("Patient/patient-2") encounter = Reference("Encounter/encounter-2") } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-2", + "Observation.encounter", + ), + ) + } Encounter() .apply { id = "encounter-2" subject = Reference("Patient/patient-2") } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Encounter.subject", + ), + ) + } - Patient().apply { id = "patient-2" }.also { localChanges.add(createInsertLocalChange(it)) } + Patient() + .apply { id = "patient-2" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } // pa-03 group = group .copy() .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { localChanges.add(createUpdateLocalChange(group, it, 3)) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Group.member", + ), + ) + } Observation() .apply { @@ -1401,18 +1160,48 @@ class PerResourcePatchGeneratorTest { subject = Reference("Patient/patient-3") encounter = Reference("Encounter/encounter-3") } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-3", + "Observation.encounter", + ), + ) + } Encounter() .apply { id = "encounter-3" subject = Reference("Patient/patient-3") } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Encounter.subject", + ), + ) + } + + Patient() + .apply { id = "patient-3" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - Patient().apply { id = "patient-3" }.also { localChanges.add(createInsertLocalChange(it)) } + whenever(database.getLocalChangeResourceReferences(any())) + .thenReturn(localChangeResourceReferences) - val result = PerResourcePatchGenerator.generate(localChanges) + val result = patchGenerator.generate(localChanges) assertThat(result.map { it.generatedPatch.resourceId }) .containsExactly( "patient-1", @@ -1430,8 +1219,9 @@ class PerResourcePatchGeneratorTest { } @Test - fun `generate with cyclic references should throw exception`() { + fun `generate with cyclic references should throw exception`() = runTest { val localChanges = LinkedList() + val localChangeResourceReferences = mutableListOf() Patient() .apply { @@ -1440,19 +1230,38 @@ class PerResourcePatchGeneratorTest { Patient.PatientLinkComponent().apply { other = Reference("RelatedPerson/related-1") }, ) } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "RelatedPerson/related-1", + "Patient.other", + ), + ) + } RelatedPerson() .apply { id = "related-1" patient = Reference("Patient/patient-1") } - .also { localChanges.add(createInsertLocalChange(it)) } + .also { + localChanges.add(createInsertLocalChange(it)) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "RelatedPerson.patient", + ), + ) + } + + whenever(database.getLocalChangeResourceReferences(any())) + .thenReturn(localChangeResourceReferences) val errorMessage = - Assert.assertThrows(IllegalStateException::class.java) { - PerResourcePatchGenerator.generate(localChanges) - } + assertFailsWith { patchGenerator.generate(localChanges) } .localizedMessage assertThat(errorMessage).isEqualTo("Detected a cycle.") From 75693e6d695c148b0e7d59cb2bc594f82cdcbaf2 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Tue, 5 Mar 2024 19:41:36 +0530 Subject: [PATCH 4/8] Review comments: Refactored code --- .../upload/patch/PerResourcePatchGenerator.kt | 99 +-- .../patch/PerResourcePatchGeneratorTest.kt | 739 ------------------ 2 files changed, 1 insertion(+), 837 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt index c05f8c0f19..c0e515d72c 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt @@ -23,7 +23,7 @@ import com.github.fge.jsonpatch.JsonPatch import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChange.Type import com.google.android.fhir.db.Database -import com.google.android.fhir.db.LocalChangeResourceReference +import com.google.android.fhir.sync.upload.patch.PatchOrdering.orderByReferences /** * Generates a [Patch] for all [LocalChange]es made to a single FHIR resource. @@ -145,100 +145,3 @@ internal class PerResourcePatchGenerator(val database: Database) : PatchGenerato return objectMapper.writeValueAsString(mergedNode) } } - -private typealias Node = String - -/** - * @return - A ordered list of the [PatchMapping]s based on the references to other [PatchMapping] - * if the mappings are acyclic - * - throws [IllegalStateException] otherwise - */ -@androidx.annotation.VisibleForTesting -internal suspend fun List.orderByReferences(database: Database): List { - // if the resource A has outgoing references (B,C) and these referenced resources are getting - // created on device, - // then these referenced resources (B,C) needs to go before the resource A so that referential - // integrity is retained. - val resourceIdToPatchMapping = associateBy { patchMapping -> - "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}" - } - - // get references for all the local changes - val localChangeIdToReferenceMap: Map> = - database - .getLocalChangeResourceReferences(flatMap { it.localChanges.flatMap { it.token.ids } }) - .groupBy { it.localChangeId } - - val adjacencyList = - createAdjacencyListForCreateReferences( - resourceIdToPatchMapping.keys - .filter { resourceIdToPatchMapping[it]?.generatedPatch?.type == Patch.Type.INSERT } - .toSet(), - localChangeIdToReferenceMap, - ) - return createTopologicalOrderedList(adjacencyList).mapNotNull { resourceIdToPatchMapping[it] } -} - -/** - * @return A map of [PatchMapping] to all the outgoing references to the other [PatchMapping]s of - * type [Patch.Type.INSERT] . - */ -@androidx.annotation.VisibleForTesting -internal fun List.createAdjacencyListForCreateReferences( - insertResourceIds: Set, - localChangeIdToReferenceMap: Map>, -): Map> { - val adjacencyList = mutableMapOf>() - forEach { patchMapping -> - adjacencyList[ - "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}", - ] = - patchMapping.findOutgoingReferences(localChangeIdToReferenceMap).filter { - insertResourceIds.contains(it) - } - } - return adjacencyList -} - -// if the outgoing reference is to a resource that's just an update and not create, then don't link -// to it. This may make the sub graphs smaller and also help avoid cyclic dependencies. -private fun PatchMapping.findOutgoingReferences( - localChangeIdToReferenceMap: Map>, -): Set { - val references = mutableSetOf() - when (generatedPatch.type) { - Patch.Type.INSERT, - Patch.Type.UPDATE, -> { - localChanges.forEach { localChange -> - localChange.token.ids.forEach { id -> - localChangeIdToReferenceMap[id]?.let { - references.addAll(it.map { it.resourceReferenceValue }) - } - } - } - } - Patch.Type.DELETE -> { - // do nothing - } - } - return references -} - -@androidx.annotation.VisibleForTesting -internal fun createTopologicalOrderedList(adjacencyList: Map>): List { - val stack = ArrayDeque() - val visited = mutableSetOf() - val currentPath = mutableSetOf() - - fun dfs(key: String) { - check(currentPath.add(key)) { "Detected a cycle." } - if (visited.add(key)) { - adjacencyList[key]?.forEach { dfs(it) } - stack.addFirst(key) - } - currentPath.remove(key) - } - - adjacencyList.keys.forEach { dfs(it) } - return stack.reversed() -} diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt index 329641b5a8..8116d329cc 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt @@ -21,7 +21,6 @@ import ca.uhn.fhir.context.FhirVersionEnum import com.google.android.fhir.LocalChange import com.google.android.fhir.LocalChangeToken import com.google.android.fhir.db.Database -import com.google.android.fhir.db.LocalChangeResourceReference import com.google.android.fhir.db.impl.dao.diff import com.google.android.fhir.db.impl.entities.LocalChangeEntity import com.google.android.fhir.logicalId @@ -34,19 +33,12 @@ import com.google.android.fhir.versionId import com.google.common.truth.Truth.assertThat import java.time.Instant import java.util.Date -import java.util.LinkedList import java.util.UUID -import kotlin.random.Random import kotlin.test.assertFailsWith import kotlinx.coroutines.test.runTest -import org.hl7.fhir.r4.model.Encounter -import org.hl7.fhir.r4.model.Group 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.Reference -import org.hl7.fhir.r4.model.RelatedPerson import org.hl7.fhir.r4.model.Resource import org.hl7.fhir.r4.model.ResourceType import org.json.JSONArray @@ -535,735 +527,4 @@ class PerResourcePatchGeneratorTest { timestamp = Instant.now(), ) } - - @Test - fun `createReferenceAdjacencyList with local changes for only new resources should only have edges to inserted resources`() = - runTest { - val localChanges = LinkedList() - val localChangeResourceReferences = mutableListOf() - - var group = - Group() - .apply { - id = "group-1" - type = Group.GroupType.PERSON - } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-01 - Patient() - .apply { id = "patient-1" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - Encounter() - .apply { - id = "encounter-1" - subject = Reference("Patient/patient-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-1" - subject = Reference("Patient/patient-1") - encounter = Reference("Encounter/encounter-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-1", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Group.member", - ), - ) - } - - // pa-02 - Patient() - .apply { id = "patient-2" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - Encounter() - .apply { - id = "encounter-2" - subject = Reference("Patient/patient-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-2" - subject = Reference("Patient/patient-2") - encounter = Reference("Encounter/encounter-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-2", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Group.member", - ), - ) - } - - // pa-03 - Patient() - .apply { id = "patient-3" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - Encounter() - .apply { - id = "encounter-3" - subject = Reference("Patient/patient-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-3" - subject = Reference("Patient/patient-3") - encounter = Reference("Encounter/encounter-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-3", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Group.member", - ), - ) - } - - whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(localChangeResourceReferences) - - val result = - patchGenerator - .generateSquashedChangesMapping(localChanges) - .createAdjacencyListForCreateReferences( - localChanges.map { "${it.resourceType}/${it.resourceId}" }.toSet(), - localChangeResourceReferences.groupBy { it.localChangeId }, - ) - - assertThat(result) - .isEqualTo( - mutableMapOf( - "Group/group-1" to - listOf("Patient/patient-1", "Patient/patient-2", "Patient/patient-3"), - "Patient/patient-1" to emptyList(), - "Patient/patient-2" to emptyList(), - "Patient/patient-3" to emptyList(), - "Encounter/encounter-1" to listOf("Patient/patient-1"), - "Encounter/encounter-2" to listOf("Patient/patient-2"), - "Encounter/encounter-3" to listOf("Patient/patient-3"), - "Observation/observation-1" to listOf("Patient/patient-1", "Encounter/encounter-1"), - "Observation/observation-2" to listOf("Patient/patient-2", "Encounter/encounter-2"), - "Observation/observation-3" to listOf("Patient/patient-3", "Encounter/encounter-3"), - ), - ) - } - - @Test - fun `createReferenceAdjacencyList with local changes for new and old resources should only have edges to inserted resources`() = - runTest { - val localChanges = mutableListOf() - val localChangeResourceReferences = mutableListOf() - - var group = - Group() - .apply { - id = "group-1" - type = Group.GroupType.PERSON - } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-01 - Patient() - .apply { id = "patient-1" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - Random.nextLong(), - ), - ) - } - - Encounter() - .apply { - id = "encounter-1" - subject = Reference("Patient/patient-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-1" - subject = Reference("Patient/patient-1") - encounter = Reference("Encounter/encounter-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-1", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Group.member", - ), - ) - } - - // pa-02 - Patient() - .apply { id = "patient-2" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - Random.nextLong(), - ), - ) - } - Encounter() - .apply { - id = "encounter-2" - subject = Reference("Patient/patient-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-2" - subject = Reference("Patient/patient-2") - encounter = Reference("Encounter/encounter-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-2", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Group.member", - ), - ) - } - - // pa-03 - Patient() - .apply { id = "patient-3" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - Random.nextLong(), - ), - ) - } - - Encounter() - .apply { - id = "encounter-3" - subject = Reference("Patient/patient-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-3" - subject = Reference("Patient/patient-3") - encounter = Reference("Encounter/encounter-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-3", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Group.member", - ), - ) - } - - whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(localChangeResourceReferences) - - val result = - patchGenerator - .generateSquashedChangesMapping(localChanges) - .createAdjacencyListForCreateReferences( - localChanges - .filter { it.type == LocalChange.Type.INSERT } - .map { "${it.resourceType}/${it.resourceId}" } - .toSet(), - localChangeResourceReferences.groupBy { it.localChangeId }, - ) - - assertThat(result) - .isEqualTo( - mutableMapOf( - "Group/group-1" to emptyList(), - "Patient/patient-1" to emptyList(), - "Patient/patient-2" to emptyList(), - "Patient/patient-3" to emptyList(), - "Encounter/encounter-1" to emptyList(), - "Encounter/encounter-2" to emptyList(), - "Encounter/encounter-3" to emptyList(), - "Observation/observation-1" to listOf("Encounter/encounter-1"), - "Observation/observation-2" to listOf("Encounter/encounter-2"), - "Observation/observation-3" to listOf("Encounter/encounter-3"), - ), - ) - } - - @Test - fun `generate with acyclic references should return the list in topological order`() = runTest { - val localChanges = LinkedList() - val localChangeResourceReferences = mutableListOf() - - var group = - Group() - .apply { - id = "group-1" - type = Group.GroupType.PERSON - } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-01 - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Group.member", - ), - ) - } - - Observation() - .apply { - id = "observation-1" - subject = Reference("Patient/patient-1") - encounter = Reference("Encounter/encounter-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-1", - "Observation.encounter", - ), - ) - } - - Encounter() - .apply { - id = "encounter-1" - subject = Reference("Patient/patient-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Encounter.subject", - ), - ) - } - - Patient() - .apply { id = "patient-1" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-02 - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Group.member", - ), - ) - } - - Observation() - .apply { - id = "observation-2" - subject = Reference("Patient/patient-2") - encounter = Reference("Encounter/encounter-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-2", - "Observation.encounter", - ), - ) - } - - Encounter() - .apply { - id = "encounter-2" - subject = Reference("Patient/patient-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Encounter.subject", - ), - ) - } - - Patient() - .apply { id = "patient-2" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-03 - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Group.member", - ), - ) - } - - Observation() - .apply { - id = "observation-3" - subject = Reference("Patient/patient-3") - encounter = Reference("Encounter/encounter-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-3", - "Observation.encounter", - ), - ) - } - - Encounter() - .apply { - id = "encounter-3" - subject = Reference("Patient/patient-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Encounter.subject", - ), - ) - } - - Patient() - .apply { id = "patient-3" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(localChangeResourceReferences) - - val result = patchGenerator.generate(localChanges) - assertThat(result.map { it.generatedPatch.resourceId }) - .containsExactly( - "patient-1", - "patient-2", - "patient-3", - "group-1", - "encounter-1", - "observation-1", - "encounter-2", - "observation-2", - "encounter-3", - "observation-3", - ) - .inOrder() - } - - @Test - fun `generate with cyclic references should throw exception`() = runTest { - val localChanges = LinkedList() - val localChangeResourceReferences = mutableListOf() - - Patient() - .apply { - id = "patient-1" - addLink( - Patient.PatientLinkComponent().apply { other = Reference("RelatedPerson/related-1") }, - ) - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "RelatedPerson/related-1", - "Patient.other", - ), - ) - } - - RelatedPerson() - .apply { - id = "related-1" - patient = Reference("Patient/patient-1") - } - .also { - localChanges.add(createInsertLocalChange(it)) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "RelatedPerson.patient", - ), - ) - } - - whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(localChangeResourceReferences) - - val errorMessage = - assertFailsWith { patchGenerator.generate(localChanges) } - .localizedMessage - - assertThat(errorMessage).isEqualTo("Detected a cycle.") - } } From b120218d678139576ac57cda8cd312a32b7e6d0e Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Tue, 5 Mar 2024 19:41:56 +0530 Subject: [PATCH 5/8] Added files --- .../fhir/sync/upload/patch/PatchOrdering.kt | 124 +++ .../sync/upload/patch/PatchOrderingTest.kt | 825 ++++++++++++++++++ 2 files changed, 949 insertions(+) create mode 100644 engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt create mode 100644 engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt new file mode 100644 index 0000000000..e4cc6317de --- /dev/null +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt @@ -0,0 +1,124 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.fhir.sync.upload.patch + +import androidx.annotation.VisibleForTesting +import com.google.android.fhir.db.Database +import com.google.android.fhir.db.LocalChangeResourceReference + +private typealias Node = String + +/** Orders the [PatchMapping]s to maintain referential integrity during upload. */ +object PatchOrdering { + + /** + * @return - A ordered list of the [PatchMapping]s based on the references to other [PatchMapping] + * if the mappings are acyclic + * - throws [IllegalStateException] otherwise + */ + @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) + internal suspend fun List.orderByReferences( + database: Database, + ): List { + // if the resource A has outgoing references (B,C) and these referenced resources are getting + // created on device, + // then these referenced resources (B,C) needs to go before the resource A so that referential + // integrity is retained. + val resourceIdToPatchMapping = associateBy { patchMapping -> + "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}" + } + + // get references for all the local changes + val localChangeIdToReferenceMap: Map> = + database + .getLocalChangeResourceReferences(flatMap { it.localChanges.flatMap { it.token.ids } }) + .groupBy { it.localChangeId } + + val adjacencyList = + createAdjacencyListForCreateReferences( + resourceIdToPatchMapping.keys + .filter { resourceIdToPatchMapping[it]?.generatedPatch?.type == Patch.Type.INSERT } + .toSet(), + localChangeIdToReferenceMap, + ) + return createTopologicalOrderedList(adjacencyList).mapNotNull { resourceIdToPatchMapping[it] } + } + + /** + * @return A map of [PatchMapping] to all the outgoing references to the other [PatchMapping]s of + * type [Patch.Type.INSERT] . + */ + @VisibleForTesting + internal fun List.createAdjacencyListForCreateReferences( + insertResourceIds: Set, + localChangeIdToReferenceMap: Map>, + ): Map> { + val adjacencyList = mutableMapOf>() + forEach { patchMapping -> + adjacencyList[ + "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}", + ] = + patchMapping.findOutgoingReferences(localChangeIdToReferenceMap).filter { + insertResourceIds.contains(it) + } + } + return adjacencyList + } + + // if the outgoing reference is to a resource that's just an update and not create, then don't + // link + // to it. This may make the sub graphs smaller and also help avoid cyclic dependencies. + private fun PatchMapping.findOutgoingReferences( + localChangeIdToReferenceMap: Map>, + ): Set { + val references = mutableSetOf() + when (generatedPatch.type) { + Patch.Type.INSERT, + Patch.Type.UPDATE, -> { + localChanges.forEach { localChange -> + localChange.token.ids.forEach { id -> + localChangeIdToReferenceMap[id]?.let { + references.addAll(it.map { it.resourceReferenceValue }) + } + } + } + } + Patch.Type.DELETE -> { + // do nothing + } + } + return references + } + + private fun createTopologicalOrderedList(adjacencyList: Map>): List { + val stack = ArrayDeque() + val visited = mutableSetOf() + val currentPath = mutableSetOf() + + fun dfs(key: String) { + check(currentPath.add(key)) { "Detected a cycle." } + if (visited.add(key)) { + adjacencyList[key]?.forEach { dfs(it) } + stack.addFirst(key) + } + currentPath.remove(key) + } + + adjacencyList.keys.forEach { dfs(it) } + return stack.reversed() + } +} diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt new file mode 100644 index 0000000000..b898dc1f48 --- /dev/null +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt @@ -0,0 +1,825 @@ +/* + * Copyright 2024 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.google.android.fhir.sync.upload.patch + +import com.google.android.fhir.LocalChange +import com.google.android.fhir.LocalChangeToken +import com.google.android.fhir.db.Database +import com.google.android.fhir.db.LocalChangeResourceReference +import com.google.android.fhir.db.impl.dao.diff +import com.google.android.fhir.logicalId +import com.google.android.fhir.sync.upload.patch.PatchOrdering.createAdjacencyListForCreateReferences +import com.google.android.fhir.testing.jsonParser +import com.google.android.fhir.versionId +import com.google.common.truth.Truth +import com.google.common.truth.Truth.assertThat +import java.time.Instant +import java.util.LinkedList +import kotlin.random.Random +import kotlin.test.assertFailsWith +import kotlinx.coroutines.test.runTest +import org.hl7.fhir.r4.model.Encounter +import org.hl7.fhir.r4.model.Group +import org.hl7.fhir.r4.model.Observation +import org.hl7.fhir.r4.model.Patient +import org.hl7.fhir.r4.model.Reference +import org.hl7.fhir.r4.model.RelatedPerson +import org.hl7.fhir.r4.model.Resource +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.mockito.Mock +import org.mockito.MockitoAnnotations +import org.mockito.kotlin.any +import org.mockito.kotlin.whenever +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class PatchOrderingTest { + + @Mock private lateinit var database: Database + private lateinit var patchGenerator: PerResourcePatchGenerator + + @Before + fun setUp() { + MockitoAnnotations.openMocks(this) + patchGenerator = PerResourcePatchGenerator(database) + } + + @Test + fun `createReferenceAdjacencyList with local changes for only new resources should only have edges to inserted resources`() = + runTest { + val localChanges = LinkedList() + val localChangeResourceReferences = mutableListOf() + + var group = + Group() + .apply { + id = "group-1" + type = Group.GroupType.PERSON + } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + // pa-01 + Patient() + .apply { id = "patient-1" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + Encounter() + .apply { + id = "encounter-1" + subject = Reference("Patient/patient-1") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Encounter.subject", + ), + ) + } + + Observation() + .apply { + id = "observation-1" + subject = Reference("Patient/patient-1") + encounter = Reference("Encounter/encounter-1") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-1", + "Observation.encounter", + ), + ) + } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Group.member", + ), + ) + } + + // pa-02 + Patient() + .apply { id = "patient-2" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + Encounter() + .apply { + id = "encounter-2" + subject = Reference("Patient/patient-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Encounter.subject", + ), + ) + } + + Observation() + .apply { + id = "observation-2" + subject = Reference("Patient/patient-2") + encounter = Reference("Encounter/encounter-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-2", + "Observation.encounter", + ), + ) + } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Group.member", + ), + ) + } + + // pa-03 + Patient() + .apply { id = "patient-3" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + Encounter() + .apply { + id = "encounter-3" + subject = Reference("Patient/patient-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Encounter.subject", + ), + ) + } + + Observation() + .apply { + id = "observation-3" + subject = Reference("Patient/patient-3") + encounter = Reference("Encounter/encounter-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-3", + "Observation.encounter", + ), + ) + } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Group.member", + ), + ) + } + + whenever(database.getLocalChangeResourceReferences(any())) + .thenReturn(localChangeResourceReferences) + + val result = + patchGenerator + .generateSquashedChangesMapping(localChanges) + .createAdjacencyListForCreateReferences( + localChanges.map { "${it.resourceType}/${it.resourceId}" }.toSet(), + localChangeResourceReferences.groupBy { it.localChangeId }, + ) + + assertThat(result) + .isEqualTo( + mutableMapOf( + "Group/group-1" to + listOf("Patient/patient-1", "Patient/patient-2", "Patient/patient-3"), + "Patient/patient-1" to emptyList(), + "Patient/patient-2" to emptyList(), + "Patient/patient-3" to emptyList(), + "Encounter/encounter-1" to listOf("Patient/patient-1"), + "Encounter/encounter-2" to listOf("Patient/patient-2"), + "Encounter/encounter-3" to listOf("Patient/patient-3"), + "Observation/observation-1" to listOf("Patient/patient-1", "Encounter/encounter-1"), + "Observation/observation-2" to listOf("Patient/patient-2", "Encounter/encounter-2"), + "Observation/observation-3" to listOf("Patient/patient-3", "Encounter/encounter-3"), + ), + ) + } + + @Test + fun `createReferenceAdjacencyList with local changes for new and old resources should only have edges to inserted resources`() = + runTest { + val localChanges = mutableListOf() + val localChangeResourceReferences = mutableListOf() + + var group = + Group() + .apply { + id = "group-1" + type = Group.GroupType.PERSON + } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + // pa-01 + Patient() + .apply { id = "patient-1" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + Random.nextLong(), + ), + ) + } + + Encounter() + .apply { + id = "encounter-1" + subject = Reference("Patient/patient-1") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Encounter.subject", + ), + ) + } + + Observation() + .apply { + id = "observation-1" + subject = Reference("Patient/patient-1") + encounter = Reference("Encounter/encounter-1") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-1", + "Observation.encounter", + ), + ) + } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Group.member", + ), + ) + } + + // pa-02 + Patient() + .apply { id = "patient-2" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + Random.nextLong(), + ), + ) + } + Encounter() + .apply { + id = "encounter-2" + subject = Reference("Patient/patient-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Encounter.subject", + ), + ) + } + + Observation() + .apply { + id = "observation-2" + subject = Reference("Patient/patient-2") + encounter = Reference("Encounter/encounter-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-2", + "Observation.encounter", + ), + ) + } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Group.member", + ), + ) + } + + // pa-03 + Patient() + .apply { id = "patient-3" } + .also { + localChanges.add( + createUpdateLocalChange( + it, + it.copy().apply { active = true }, + Random.nextLong(), + ), + ) + } + + Encounter() + .apply { + id = "encounter-3" + subject = Reference("Patient/patient-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Encounter.subject", + ), + ) + } + + Observation() + .apply { + id = "observation-3" + subject = Reference("Patient/patient-3") + encounter = Reference("Encounter/encounter-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-3", + "Observation.encounter", + ), + ) + } + + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Group.member", + ), + ) + } + + whenever(database.getLocalChangeResourceReferences(any())) + .thenReturn(localChangeResourceReferences) + + val result = + patchGenerator + .generateSquashedChangesMapping(localChanges) + .createAdjacencyListForCreateReferences( + localChanges + .filter { it.type == LocalChange.Type.INSERT } + .map { "${it.resourceType}/${it.resourceId}" } + .toSet(), + localChangeResourceReferences.groupBy { it.localChangeId }, + ) + + assertThat(result) + .isEqualTo( + mutableMapOf( + "Group/group-1" to emptyList(), + "Patient/patient-1" to emptyList(), + "Patient/patient-2" to emptyList(), + "Patient/patient-3" to emptyList(), + "Encounter/encounter-1" to emptyList(), + "Encounter/encounter-2" to emptyList(), + "Encounter/encounter-3" to emptyList(), + "Observation/observation-1" to listOf("Encounter/encounter-1"), + "Observation/observation-2" to listOf("Encounter/encounter-2"), + "Observation/observation-3" to listOf("Encounter/encounter-3"), + ), + ) + } + + @Test + fun `generate with acyclic references should return the list in topological order`() = runTest { + val localChanges = LinkedList() + val localChangeResourceReferences = mutableListOf() + + var group = + Group() + .apply { + id = "group-1" + type = Group.GroupType.PERSON + } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + // pa-01 + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Group.member", + ), + ) + } + + Observation() + .apply { + id = "observation-1" + subject = Reference("Patient/patient-1") + encounter = Reference("Encounter/encounter-1") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-1", + "Observation.encounter", + ), + ) + } + + Encounter() + .apply { + id = "encounter-1" + subject = Reference("Patient/patient-1") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Encounter.subject", + ), + ) + } + + Patient() + .apply { id = "patient-1" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + // pa-02 + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Group.member", + ), + ) + } + + Observation() + .apply { + id = "observation-2" + subject = Reference("Patient/patient-2") + encounter = Reference("Encounter/encounter-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-2", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-2", + "Observation.encounter", + ), + ) + } + + Encounter() + .apply { + id = "encounter-2" + subject = Reference("Patient/patient-2") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "Encounter.subject", + ), + ) + } + + Patient() + .apply { id = "patient-2" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + // pa-03 + group = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Group.member", + ), + ) + } + + Observation() + .apply { + id = "observation-3" + subject = Reference("Patient/patient-3") + encounter = Reference("Encounter/encounter-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Encounter/encounter-3", + "Observation.encounter", + ), + ) + } + + Encounter() + .apply { + id = "encounter-3" + subject = Reference("Patient/patient-3") + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-3", + "Encounter.subject", + ), + ) + } + + Patient() + .apply { id = "patient-3" } + .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + + whenever(database.getLocalChangeResourceReferences(any())) + .thenReturn(localChangeResourceReferences) + + val result = patchGenerator.generate(localChanges) + Truth.assertThat(result.map { it.generatedPatch.resourceId }) + .containsExactly( + "patient-1", + "patient-2", + "patient-3", + "group-1", + "encounter-1", + "observation-1", + "encounter-2", + "observation-2", + "encounter-3", + "observation-3", + ) + .inOrder() + } + + @Test + fun `generate with cyclic references should throw exception`() = runTest { + val localChanges = LinkedList() + val localChangeResourceReferences = mutableListOf() + + Patient() + .apply { + id = "patient-1" + addLink( + Patient.PatientLinkComponent().apply { other = Reference("RelatedPerson/related-1") }, + ) + } + .also { + localChanges.add(createInsertLocalChange(it, Random.nextLong())) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "RelatedPerson/related-1", + "Patient.other", + ), + ) + } + + RelatedPerson() + .apply { + id = "related-1" + patient = Reference("Patient/patient-1") + } + .also { + localChanges.add(createInsertLocalChange(it)) + localChangeResourceReferences.add( + LocalChangeResourceReference( + localChanges.last().token.ids.first(), + "Patient/patient-1", + "RelatedPerson.patient", + ), + ) + } + + whenever(database.getLocalChangeResourceReferences(any())) + .thenReturn(localChangeResourceReferences) + + val errorMessage = + assertFailsWith { patchGenerator.generate(localChanges) } + .localizedMessage + + assertThat(errorMessage).isEqualTo("Detected a cycle.") + } + + companion object { + + private fun createUpdateLocalChange( + oldEntity: Resource, + updatedResource: Resource, + currentChangeId: Long, + ): LocalChange { + val jsonDiff = diff(jsonParser, oldEntity, updatedResource) + return LocalChange( + resourceId = oldEntity.logicalId, + resourceType = oldEntity.resourceType.name, + type = LocalChange.Type.UPDATE, + payload = jsonDiff.toString(), + versionId = oldEntity.versionId, + token = LocalChangeToken(listOf(currentChangeId + 1)), + timestamp = Instant.now(), + ) + } + + private fun createInsertLocalChange(entity: Resource, currentChangeId: Long = 1): LocalChange { + return LocalChange( + resourceId = entity.logicalId, + resourceType = entity.resourceType.name, + type = LocalChange.Type.INSERT, + payload = jsonParser.encodeResourceToString(entity), + versionId = entity.versionId, + token = LocalChangeToken(listOf(currentChangeId)), + timestamp = Instant.now(), + ) + } + } +} From cdc2f208a0a62845788d83b2ab5f3976fdd7b426 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Tue, 5 Mar 2024 19:54:52 +0530 Subject: [PATCH 6/8] Review comments: Changes to cache PerResourcePatchGenerator instance --- .../fhir/sync/upload/patch/PatchGenerator.kt | 2 +- .../upload/patch/PerResourcePatchGenerator.kt | 19 ++++++++++++++++++- .../patch/PerResourcePatchGeneratorTest.kt | 2 +- 3 files changed, 20 insertions(+), 3 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt index 27e27580f3..1a52da87c8 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchGenerator.kt @@ -45,7 +45,7 @@ internal object PatchGeneratorFactory { ): PatchGenerator = when (mode) { is PatchGeneratorMode.PerChange -> PerChangePatchGenerator - is PatchGeneratorMode.PerResource -> PerResourcePatchGenerator(database) + is PatchGeneratorMode.PerResource -> PerResourcePatchGenerator.with(database) } } diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt index c0e515d72c..84d6e323a9 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt @@ -32,7 +32,8 @@ import com.google.android.fhir.sync.upload.patch.PatchOrdering.orderByReferences * maintain an audit trail, but instead, multiple changes made to the same FHIR resource on the * client can be recorded as a single change on the server. */ -internal class PerResourcePatchGenerator(val database: Database) : PatchGenerator { +internal class PerResourcePatchGenerator private constructor(val database: Database) : + PatchGenerator { override suspend fun generate(localChanges: List): List { return generateSquashedChangesMapping(localChanges).orderByReferences(database) @@ -144,4 +145,20 @@ internal class PerResourcePatchGenerator(val database: Database) : PatchGenerato mergedOperations.values.flatten().forEach(mergedNode::add) return objectMapper.writeValueAsString(mergedNode) } + + companion object { + + private lateinit var _instance: PerResourcePatchGenerator + + @Synchronized + fun with(database: Database): PerResourcePatchGenerator { + if (!::_instance.isInitialized) { + _instance = PerResourcePatchGenerator(database) + } else if (_instance.database != database) { + _instance = PerResourcePatchGenerator(database) + } + + return _instance + } + } } diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt index 8116d329cc..4860b66cdc 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGeneratorTest.kt @@ -61,7 +61,7 @@ class PerResourcePatchGeneratorTest { fun setUp() { MockitoAnnotations.openMocks(this) runTest { whenever(database.getLocalChangeResourceReferences(any())).thenReturn(emptyList()) } - patchGenerator = PerResourcePatchGenerator(database) + patchGenerator = PerResourcePatchGenerator.with(database) } @Test From 575875d2d2478e24ffcc4daa9267056650cadcfb Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Tue, 5 Mar 2024 19:56:14 +0530 Subject: [PATCH 7/8] spotless --- .../google/android/fhir/sync/upload/patch/PatchOrderingTest.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt index b898dc1f48..1e86df2aaf 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt @@ -57,7 +57,7 @@ class PatchOrderingTest { @Before fun setUp() { MockitoAnnotations.openMocks(this) - patchGenerator = PerResourcePatchGenerator(database) + patchGenerator = PerResourcePatchGenerator.with(database) } @Test From 59b7f36f5d7d19af450abaa35e1f582a746daad9 Mon Sep 17 00:00:00 2001 From: Aditya Khajuria Date: Fri, 8 Mar 2024 04:18:08 +0530 Subject: [PATCH 8/8] Review comments: Simplified tests and added kdocs --- .../fhir/sync/upload/patch/PatchOrdering.kt | 74 +- .../upload/patch/PerResourcePatchGenerator.kt | 2 +- .../sync/upload/patch/PatchOrderingTest.kt | 776 ++++-------------- 3 files changed, 216 insertions(+), 636 deletions(-) diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt index e4cc6317de..c76fb46751 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PatchOrdering.kt @@ -22,39 +22,55 @@ import com.google.android.fhir.db.LocalChangeResourceReference private typealias Node = String -/** Orders the [PatchMapping]s to maintain referential integrity during upload. */ -object PatchOrdering { +/** + * Orders the [PatchMapping]s to maintain referential integrity during upload. + * + * ``` + * Encounter().apply { + * id = "encounter-1" + * subject = Reference("Patient/patient-1") + * } + * + * Observation().apply { + * id = "observation-1" + * subject = Reference("Patient/patient-1") + * encounter = Reference("Encounter/encounter-1") + * } + * ``` + * * The Encounter has an outgoing reference to Patient and the Observation has outgoing references + * to Patient and the Encounter. + * * Now, to maintain the referential integrity of the resources during the upload, + * `Encounter/encounter-1` must go before the `Observation/observation-1`, irrespective of the + * order in which the Encounter and Observation were added to the database. + */ +internal object PatchOrdering { + + private val PatchMapping.resourceTypeAndId: String + get() = "${generatedPatch.resourceType}/${generatedPatch.resourceId}" /** + * Order the [PatchMapping] so that if the resource A has outgoing references {B,C} (CREATE) and + * {D} (UPDATE), then B,C needs to go before the resource A so that referential integrity is + * retained. Order of D shouldn't matter for the purpose of referential integrity. + * * @return - A ordered list of the [PatchMapping]s based on the references to other [PatchMapping] * if the mappings are acyclic * - throws [IllegalStateException] otherwise */ - @VisibleForTesting(otherwise = VisibleForTesting.PROTECTED) - internal suspend fun List.orderByReferences( + suspend fun List.orderByReferences( database: Database, ): List { - // if the resource A has outgoing references (B,C) and these referenced resources are getting - // created on device, - // then these referenced resources (B,C) needs to go before the resource A so that referential - // integrity is retained. - val resourceIdToPatchMapping = associateBy { patchMapping -> - "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}" - } + val resourceIdToPatchMapping = associateBy { patchMapping -> patchMapping.resourceTypeAndId } - // get references for all the local changes - val localChangeIdToReferenceMap: Map> = + /* Get LocalChangeResourceReferences for all the local changes. A single LocalChange may have + multiple LocalChangeResourceReference, one for each resource reference in the + LocalChange.payload.*/ + val localChangeIdToResourceReferenceMap: Map> = database .getLocalChangeResourceReferences(flatMap { it.localChanges.flatMap { it.token.ids } }) .groupBy { it.localChangeId } - val adjacencyList = - createAdjacencyListForCreateReferences( - resourceIdToPatchMapping.keys - .filter { resourceIdToPatchMapping[it]?.generatedPatch?.type == Patch.Type.INSERT } - .toSet(), - localChangeIdToReferenceMap, - ) + val adjacencyList = createAdjacencyListForCreateReferences(localChangeIdToResourceReferenceMap) return createTopologicalOrderedList(adjacencyList).mapNotNull { resourceIdToPatchMapping[it] } } @@ -64,24 +80,26 @@ object PatchOrdering { */ @VisibleForTesting internal fun List.createAdjacencyListForCreateReferences( - insertResourceIds: Set, localChangeIdToReferenceMap: Map>, ): Map> { val adjacencyList = mutableMapOf>() + /* if the outgoing reference is to a resource that's just an update and not create, then don't + link to it. This may make the sub graphs smaller and also help avoid cyclic dependencies.*/ + val resourceIdsOfInsertTypeLocalChanges = + asSequence() + .filter { it.generatedPatch.type == Patch.Type.INSERT } + .map { it.resourceTypeAndId } + .toSet() + forEach { patchMapping -> - adjacencyList[ - "${patchMapping.generatedPatch.resourceType}/${patchMapping.generatedPatch.resourceId}", - ] = + adjacencyList[patchMapping.resourceTypeAndId] = patchMapping.findOutgoingReferences(localChangeIdToReferenceMap).filter { - insertResourceIds.contains(it) + resourceIdsOfInsertTypeLocalChanges.contains(it) } } return adjacencyList } - // if the outgoing reference is to a resource that's just an update and not create, then don't - // link - // to it. This may make the sub graphs smaller and also help avoid cyclic dependencies. private fun PatchMapping.findOutgoingReferences( localChangeIdToReferenceMap: Map>, ): Set { diff --git a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt index 84d6e323a9..0eaa944008 100644 --- a/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt +++ b/engine/src/main/java/com/google/android/fhir/sync/upload/patch/PerResourcePatchGenerator.kt @@ -148,7 +148,7 @@ internal class PerResourcePatchGenerator private constructor(val database: Datab companion object { - private lateinit var _instance: PerResourcePatchGenerator + @Volatile private lateinit var _instance: PerResourcePatchGenerator @Synchronized fun with(database: Database): PerResourcePatchGenerator { diff --git a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt index 1e86df2aaf..44f8889dc8 100644 --- a/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt +++ b/engine/src/test/java/com/google/android/fhir/sync/upload/patch/PatchOrderingTest.kt @@ -25,7 +25,6 @@ import com.google.android.fhir.logicalId import com.google.android.fhir.sync.upload.patch.PatchOrdering.createAdjacencyListForCreateReferences import com.google.android.fhir.testing.jsonParser import com.google.android.fhir.versionId -import com.google.common.truth.Truth import com.google.common.truth.Truth.assertThat import java.time.Instant import java.util.LinkedList @@ -63,204 +62,31 @@ class PatchOrderingTest { @Test fun `createReferenceAdjacencyList with local changes for only new resources should only have edges to inserted resources`() = runTest { - val localChanges = LinkedList() - val localChangeResourceReferences = mutableListOf() - - var group = - Group() - .apply { - id = "group-1" - type = Group.GroupType.PERSON - } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-01 - Patient() - .apply { id = "patient-1" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - Encounter() - .apply { - id = "encounter-1" - subject = Reference("Patient/patient-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-1" - subject = Reference("Patient/patient-1") - encounter = Reference("Encounter/encounter-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-1", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Group.member", - ), - ) - } - - // pa-02 - Patient() - .apply { id = "patient-2" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - Encounter() - .apply { - id = "encounter-2" - subject = Reference("Patient/patient-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-2" - subject = Reference("Patient/patient-2") - encounter = Reference("Encounter/encounter-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-2", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Group.member", - ), - ) - } - - // pa-03 - Patient() - .apply { id = "patient-3" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - Encounter() - .apply { - id = "encounter-3" - subject = Reference("Patient/patient-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-3" - subject = Reference("Patient/patient-3") - encounter = Reference("Encounter/encounter-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-3", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Group.member", - ), - ) - } - + val helper = LocalChangeHelper() + + var group = helper.createGroup("group-1", 1) + helper.createPatient("patient-1", 2) + helper.createEncounter("encounter-1", 3, "Patient/patient-1") + helper.createObservation("observation-1", 4, "Patient/patient-1", "Encounter/encounter-1") + group = helper.updateGroup(group, 5, "Patient/patient-1") + + helper.createPatient("patient-2", 6) + helper.createEncounter("encounter-2", 7, "Patient/patient-2") + helper.createObservation("observation-2", 8, "Patient/patient-2", "Encounter/encounter-2") + group = helper.updateGroup(group, 9, "Patient/patient-2") + + helper.createPatient("patient-3", 10) + helper.createEncounter("encounter-3", 11, "Patient/patient-3") + helper.createObservation("observation-3", 12, "Patient/patient-3", "Encounter/encounter-3") + group = helper.updateGroup(group, 13, "Patient/patient-3") whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(localChangeResourceReferences) + .thenReturn(helper.localChangeResourceReferences) val result = patchGenerator - .generateSquashedChangesMapping(localChanges) + .generateSquashedChangesMapping(helper.localChanges) .createAdjacencyListForCreateReferences( - localChanges.map { "${it.resourceType}/${it.resourceId}" }.toSet(), - localChangeResourceReferences.groupBy { it.localChangeId }, + helper.localChangeResourceReferences.groupBy { it.localChangeId }, ) assertThat(result) @@ -284,232 +110,34 @@ class PatchOrderingTest { @Test fun `createReferenceAdjacencyList with local changes for new and old resources should only have edges to inserted resources`() = runTest { - val localChanges = mutableListOf() - val localChangeResourceReferences = mutableListOf() - - var group = - Group() - .apply { - id = "group-1" - type = Group.GroupType.PERSON - } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-01 - Patient() - .apply { id = "patient-1" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - Random.nextLong(), - ), - ) - } - - Encounter() - .apply { - id = "encounter-1" - subject = Reference("Patient/patient-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-1" - subject = Reference("Patient/patient-1") - encounter = Reference("Encounter/encounter-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-1", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Group.member", - ), - ) - } - - // pa-02 - Patient() - .apply { id = "patient-2" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - Random.nextLong(), - ), - ) - } - Encounter() - .apply { - id = "encounter-2" - subject = Reference("Patient/patient-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-2" - subject = Reference("Patient/patient-2") - encounter = Reference("Encounter/encounter-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-2", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Group.member", - ), - ) - } - - // pa-03 - Patient() - .apply { id = "patient-3" } - .also { - localChanges.add( - createUpdateLocalChange( - it, - it.copy().apply { active = true }, - Random.nextLong(), - ), - ) - } - - Encounter() - .apply { - id = "encounter-3" - subject = Reference("Patient/patient-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Encounter.subject", - ), - ) - } - - Observation() - .apply { - id = "observation-3" - subject = Reference("Patient/patient-3") - encounter = Reference("Encounter/encounter-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-3", - "Observation.encounter", - ), - ) - } - - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Group.member", - ), - ) - } - - whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(localChangeResourceReferences) + val helper = LocalChangeHelper() + var group = helper.createGroup("group-1", 1) + // The scenario is that Patient is already created on the server and device is just updating + // it. + helper.updatePatient(Patient().apply { id = "patient-1" }, 2) + helper.createEncounter("encounter-1", 3, "Patient/patient-1") + helper.createObservation("observation-1", 4, "Patient/patient-1", "Encounter/encounter-1") + + group = helper.updateGroup(group, 5, "Patient/patient-1") + // The scenario is that Patient is already created on the server and device is just updating + // it. + helper.updatePatient(Patient().apply { id = "patient-2" }, 6) + helper.createEncounter("encounter-2", 7, "Patient/patient-2") + helper.createObservation("observation-2", 8, "Patient/patient-2", "Encounter/encounter-2") + + group = helper.updateGroup(group, 9, "Patient/patient-2") + // The scenario is that Patient is already created on the server and device is just updating + // it. + helper.updatePatient(Patient().apply { id = "patient-3" }, 10) + helper.createEncounter("encounter-3", 11, "Patient/patient-3") + helper.createObservation("observation-3", 12, "Patient/patient-3", "Encounter/encounter-3") + group = helper.updateGroup(group, 13, "Patient/patient-3") val result = patchGenerator - .generateSquashedChangesMapping(localChanges) + .generateSquashedChangesMapping(helper.localChanges) .createAdjacencyListForCreateReferences( - localChanges - .filter { it.type == LocalChange.Type.INSERT } - .map { "${it.resourceType}/${it.resourceId}" } - .toSet(), - localChangeResourceReferences.groupBy { it.localChangeId }, + helper.localChangeResourceReferences.groupBy { it.localChangeId }, ) assertThat(result) @@ -531,202 +159,33 @@ class PatchOrderingTest { @Test fun `generate with acyclic references should return the list in topological order`() = runTest { - val localChanges = LinkedList() - val localChangeResourceReferences = mutableListOf() - - var group = - Group() - .apply { - id = "group-1" - type = Group.GroupType.PERSON - } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-01 - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-1"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Group.member", - ), - ) - } - - Observation() - .apply { - id = "observation-1" - subject = Reference("Patient/patient-1") - encounter = Reference("Encounter/encounter-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-1", - "Observation.encounter", - ), - ) - } - - Encounter() - .apply { - id = "encounter-1" - subject = Reference("Patient/patient-1") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Encounter.subject", - ), - ) - } - - Patient() - .apply { id = "patient-1" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } - - // pa-02 - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-2"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Group.member", - ), - ) - } - - Observation() - .apply { - id = "observation-2" - subject = Reference("Patient/patient-2") - encounter = Reference("Encounter/encounter-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-2", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-2", - "Observation.encounter", - ), - ) - } - - Encounter() - .apply { - id = "encounter-2" - subject = Reference("Patient/patient-2") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-1", - "Encounter.subject", - ), - ) - } + val helper = LocalChangeHelper() + var group = helper.createGroup("group-1", 1) - Patient() - .apply { id = "patient-2" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + group = helper.updateGroup(group, 2, "Patient/patient-1") + helper.createObservation("observation-1", 3, "Patient/patient-1", "Encounter/encounter-1") + helper.createEncounter("encounter-1", 4, "Patient/patient-1") + helper.createPatient("patient-1", 5) - // pa-03 - group = - group - .copy() - .apply { addMember(Group.GroupMemberComponent(Reference("Patient/patient-3"))) } - .also { - localChanges.add(createUpdateLocalChange(group, it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Group.member", - ), - ) - } + group = helper.updateGroup(group, 6, "Patient/patient-2") + helper.createObservation("observation-2", 7, "Patient/patient-2", "Encounter/encounter-2") + helper.createEncounter("encounter-2", 8, "Patient/patient-2") + helper.createPatient("patient-2", 9) - Observation() - .apply { - id = "observation-3" - subject = Reference("Patient/patient-3") - encounter = Reference("Encounter/encounter-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Observation.subject", - ), - ) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Encounter/encounter-3", - "Observation.encounter", - ), - ) - } - - Encounter() - .apply { - id = "encounter-3" - subject = Reference("Patient/patient-3") - } - .also { - localChanges.add(createInsertLocalChange(it, Random.nextLong())) - localChangeResourceReferences.add( - LocalChangeResourceReference( - localChanges.last().token.ids.first(), - "Patient/patient-3", - "Encounter.subject", - ), - ) - } - - Patient() - .apply { id = "patient-3" } - .also { localChanges.add(createInsertLocalChange(it, Random.nextLong())) } + group = helper.updateGroup(group, 10, "Patient/patient-3") + helper.createObservation("observation-3", 11, "Patient/patient-3", "Encounter/encounter-3") + helper.createEncounter("encounter-3", 12, "Patient/patient-3") + helper.createPatient("patient-3", 13) whenever(database.getLocalChangeResourceReferences(any())) - .thenReturn(localChangeResourceReferences) + .thenReturn(helper.localChangeResourceReferences) - val result = patchGenerator.generate(localChanges) - Truth.assertThat(result.map { it.generatedPatch.resourceId }) + val result = patchGenerator.generate(helper.localChanges) + + // This order is based on the current implementation of the topological sort in [PatchOrdering], + // it's entirely possible to generate different order here which is acceptable/correct, should + // we have a different implementation of the topological sort. + assertThat(result.map { it.generatedPatch.resourceId }) .containsExactly( "patient-1", "patient-2", @@ -805,7 +264,7 @@ class PatchOrderingTest { type = LocalChange.Type.UPDATE, payload = jsonDiff.toString(), versionId = oldEntity.versionId, - token = LocalChangeToken(listOf(currentChangeId + 1)), + token = LocalChangeToken(listOf(currentChangeId)), timestamp = Instant.now(), ) } @@ -822,4 +281,107 @@ class PatchOrderingTest { ) } } + + internal class LocalChangeHelper { + val localChanges = LinkedList() + val localChangeResourceReferences = mutableListOf() + + fun createGroup( + id: String, + changeId: Long, + ) = + Group() + .apply { + this.id = id + type = Group.GroupType.PERSON + } + .also { localChanges.add(createInsertLocalChange(it, changeId)) } + + fun updateGroup( + group: Group, + changeId: Long, + member: String, + ) = + group + .copy() + .apply { addMember(Group.GroupMemberComponent(Reference(member))) } + .also { + localChanges.add(createUpdateLocalChange(group, it, changeId)) + localChangeResourceReferences.add( + LocalChangeResourceReference( + changeId, + member, + "Group.member", + ), + ) + } + + fun createPatient( + id: String, + changeId: Long, + ) = + Patient() + .apply { this.id = id } + .also { localChanges.add(createInsertLocalChange(it, changeId)) } + + fun updatePatient( + patient: Patient, + changeId: Long, + ) = + patient + .copy() + .apply { active = true } + .also { localChanges.add(createUpdateLocalChange(patient, it, changeId)) } + + fun createEncounter( + id: String, + changeId: Long, + subject: String, + ) = + Encounter() + .apply { + this.id = id + this.subject = Reference(subject) + } + .also { + localChanges.add(createInsertLocalChange(it, changeId)) + localChangeResourceReferences.add( + LocalChangeResourceReference( + changeId, + subject, + "Encounter.subject", + ), + ) + } + + fun createObservation( + id: String, + changeId: Long, + subject: String, + encounter: String, + ) = + Observation() + .apply { + this.id = id + this.subject = Reference(subject) + this.encounter = Reference(encounter) + } + .also { + localChanges.add(createInsertLocalChange(it, changeId)) + localChangeResourceReferences.add( + LocalChangeResourceReference( + changeId, + subject, + "Observation.subject", + ), + ) + localChangeResourceReferences.add( + LocalChangeResourceReference( + changeId, + encounter, + "Observation.encounter", + ), + ) + } + } }