delete operation removes element from list (#4782)

* delete operation removes element from list

* changelog

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."

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
 	 */

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)) {
-
+				if (path.endsWith(")")) {
-				doDelete(theResource, path);
+					// This is probably a filter, so we're probably dealing with a list
-				return;
+					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<IBase> existingValues = new ArrayList<>(childDef.getAccessor().getValues(next));
+					List<IBase> 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;
 				}