From ae1e7400dd4a8d618a27b1a015370ad0636d9ed5 Mon Sep 17 00:00:00 2001 From: JasonRoberts-smile <85363818+JasonRoberts-smile@users.noreply.github.com> Date: Thu, 27 Apr 2023 20:01:28 -0400 Subject: [PATCH] delete operation removes element from list (#4782) * delete operation removes element from list * changelog --- .../6_6_0/4782-fhir-patch-delete.yaml | 5 +++ .../fhir/jpa/patch/FhirPatchApplyR4Test.java | 42 +++++++++++++++++++ .../java/ca/uhn/fhir/jpa/patch/FhirPatch.java | 31 ++++++++++++-- 3 files changed, 75 insertions(+), 3 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4782-fhir-patch-delete.yaml diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4782-fhir-patch-delete.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4782-fhir-patch-delete.yaml new file mode 100644 index 00000000000..83a86daa776 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/6_6_0/4782-fhir-patch-delete.yaml @@ -0,0 +1,5 @@ +--- +type: fix +issue: 4782 +title: "Previously, a FHIRPath patch delete operation where the path resolved to an element in a collection, the element +was not always being removed from the collection. This has been fixed." diff --git a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchApplyR4Test.java b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchApplyR4Test.java index b83beebeb9b..09d3fd981f4 100644 --- a/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchApplyR4Test.java +++ b/hapi-fhir-jpaserver-test-r4/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchApplyR4Test.java @@ -8,7 +8,10 @@ import org.hl7.fhir.instance.model.api.IBaseResource; import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.hl7.fhir.r4.model.BooleanType; import org.hl7.fhir.r4.model.CodeType; +import org.hl7.fhir.r4.model.CodeableConcept; import org.hl7.fhir.r4.model.Coding; +import org.hl7.fhir.r4.model.Extension; +import org.hl7.fhir.r4.model.HumanName; import org.hl7.fhir.r4.model.Identifier; import org.hl7.fhir.r4.model.IntegerType; import org.hl7.fhir.r4.model.Parameters; @@ -175,10 +178,49 @@ public class FhirPatchApplyR4Test { svc.apply(patient, patch); + assertEquals(1, patient.getIdentifier().size()); + assertEquals("{\"resourceType\":\"Patient\",\"identifier\":[{\"system\":\"sys\",\"value\":\"val\"}],\"active\":true}", ourCtx.newJsonParser().encodeResourceToString(patient)); } + @Test + public void testDeleteExtensionFromListByFilter() { + FhirPatch svc = new FhirPatch(ourCtx); + + Patient patient = new Patient(); + patient.setActive(true); + patient.addExtension().setUrl("url1") + .addExtension(new Extension().setUrl("text").setValue(new StringType("first text"))) + .addExtension(new Extension().setUrl("code").setValue(new CodeableConcept().addCoding(new Coding("sys", "123", "Abc")))); + patient.addExtension().setUrl("url2") + .addExtension(new Extension().setUrl("code").setValue(new CodeableConcept().addCoding(new Coding("sys", "234", "Def")))) + .addExtension(new Extension().setUrl("detail").setValue(new IntegerType(5))); + patient.addExtension().setUrl("url3") + .addExtension(new Extension().setUrl("text").setValue(new StringType("third text"))) + .addExtension(new Extension().setUrl("code").setValue(new CodeableConcept().addCoding(new Coding("sys", "345", "Ghi")))) + .addExtension(new Extension().setUrl("detail").setValue(new IntegerType(12))); + + Parameters patch = new Parameters(); + Parameters.ParametersParameterComponent operation = patch.addParameter(); + operation.setName("operation"); + operation + .addPart() + .setName("type") + .setValue(new CodeType("delete")); + operation + .addPart() + .setName("path") + .setValue(new StringType("Patient.extension.where(url = 'url2')")); + + svc.apply(patient, patch); + + assertEquals(2, patient.getExtension().size()); + + assertEquals("{\"resourceType\":\"Patient\",\"extension\":[{\"url\":\"url1\",\"extension\":[{\"url\":\"text\",\"valueString\":\"first text\"},{\"url\":\"code\",\"valueCodeableConcept\":{\"coding\":[{\"system\":\"sys\",\"code\":\"123\",\"display\":\"Abc\"}]}}]},{\"url\":\"url3\",\"extension\":[{\"url\":\"text\",\"valueString\":\"third text\"},{\"url\":\"code\",\"valueCodeableConcept\":{\"coding\":[{\"system\":\"sys\",\"code\":\"345\",\"display\":\"Ghi\"}]}},{\"url\":\"detail\",\"valueInteger\":12}]}],\"active\":true}", ourCtx.newJsonParser().encodeResourceToString(patient)); + + } + /** * See #1999 */ diff --git a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java index a4e077b3ad9..6bb1494bf3c 100644 --- a/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java +++ b/hapi-fhir-storage/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java @@ -96,9 +96,23 @@ public class FhirPatch { Integer removeIndex = null; Integer insertIndex = null; if ("delete".equals(type)) { - - doDelete(theResource, path); - return; + if (path.endsWith(")")) { + // This is probably a filter, so we're probably dealing with a list + int filterArgsIndex = path.lastIndexOf('('); // Let's hope there aren't nested parentheses + int lastDotIndex = path.lastIndexOf('.', filterArgsIndex); // There might be a dot inside the parentheses, so look to the left of that + int secondLastDotIndex = path.lastIndexOf('.', lastDotIndex-1); + containingPath = path.substring(0, secondLastDotIndex); + elementName = path.substring(secondLastDotIndex + 1, lastDotIndex); + } else if (path.endsWith("]")) { + // This is almost definitely a list + int openBracketIndex = path.lastIndexOf('['); + int lastDotIndex = path.lastIndexOf('.', openBracketIndex); + containingPath = path.substring(0, lastDotIndex); + elementName = path.substring(lastDotIndex + 1, openBracketIndex); + } else { + doDelete(theResource, path); + continue; + } } else if ("add".equals(type)) { @@ -180,6 +194,17 @@ public class FhirPatch { childDef.getMutator().addValue(next, nextNewValue); } + continue; + } else if ("delete".equals(type)) { + List existingValues = new ArrayList<>(childDef.getAccessor().getValues(next)); + List elementsToRemove = myContext.newFhirPath().evaluate(theResource, path, IBase.class); + existingValues.removeAll(elementsToRemove); + + childDef.getMutator().setValue(next, null); + for (IBase nextNewValue : existingValues) { + childDef.getMutator().addValue(next, nextNewValue); + } + continue; }