From 535bf88efe19d5461d0104552dbf31f4edc2f4a2 Mon Sep 17 00:00:00 2001 From: James Agnew Date: Mon, 25 May 2020 13:16:58 -0400 Subject: [PATCH] Fixes to diff operation (#1866) --- .../java/ca/uhn/fhir/jpa/patch/FhirPatch.java | 39 +++++++++++++++---- .../fhir/jpa/patch/FhirPatchApplyR4Test.java | 7 +++- .../fhir/jpa/patch/FhirPatchDiffR4Test.java | 26 +++++++++++++ 3 files changed, 63 insertions(+), 9 deletions(-) diff --git a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java index 3e66965339c..15de32b78f4 100644 --- a/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java +++ b/hapi-fhir-jpaserver-base/src/main/java/ca/uhn/fhir/jpa/patch/FhirPatch.java @@ -369,14 +369,7 @@ public class FhirPatch { // Find newly inserted items while (targetIndex < targetValues.size()) { String path = theTargetPath + "." + elementName; - - - IBase operation = ParametersUtil.addParameterToParameters(myContext, theDiff, "operation"); - ParametersUtil.addPartCode(myContext, operation, "type", "insert"); - ParametersUtil.addPartString(myContext, operation, "path", path); - ParametersUtil.addPartInteger(myContext, operation, "index", targetIndex); - ParametersUtil.addPart(myContext, operation, "value", targetValues.get(targetIndex)); - + addInsertItems(theDiff, targetValues, targetIndex, path, theChildDef); targetIndex++; } @@ -393,6 +386,36 @@ public class FhirPatch { theSourceEncodePath.popPath(); } + private void addInsertItems(IBaseParameters theDiff, List theTargetValues, int theTargetIndex, String thePath, BaseRuntimeChildDefinition theChildDefinition) { + IBase operation = ParametersUtil.addParameterToParameters(myContext, theDiff, "operation"); + ParametersUtil.addPartCode(myContext, operation, "type", "insert"); + ParametersUtil.addPartString(myContext, operation, "path", thePath); + ParametersUtil.addPartInteger(myContext, operation, "index", theTargetIndex); + + IBase value = theTargetValues.get(theTargetIndex); + BaseRuntimeElementDefinition valueDef = myContext.getElementDefinition(value.getClass()); + + /* + * If the value is a Resource or a datatype, we can put it into the part.value and that will cover + * all of its children. If it's an infrastructure element though, such as Patient.contact we can't + * just put it into part.value because it isn't an actual type. So we have to put all of its + * childen in instead. + */ + if (valueDef.isStandardType()) { + ParametersUtil.addPart(myContext, operation, "value", value); + } else { + for (BaseRuntimeChildDefinition nextChild : valueDef.getChildren()) { + List childValues = nextChild.getAccessor().getValues(value); + for (int index = 0; index < childValues.size(); index++) { + boolean childRepeatable = theChildDefinition.getMax() != 1; + String elementName = nextChild.getChildNameByDatatype(childValues.get(index).getClass()); + String targetPath = thePath + (childRepeatable ? "[" + index + "]" : "") + "." + elementName; + addInsertItems(theDiff, childValues, index, targetPath, nextChild); + } + } + } + } + private void addValueToDiff(IBase theOperationPart, IBase theOldValue, IBase theNewValue) { if (myIncludePreviousValueInDiff) { diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchApplyR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchApplyR4Test.java index 35b9e95f977..d48e1ee9f84 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchApplyR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchApplyR4Test.java @@ -12,6 +12,7 @@ import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.StringType; import org.junit.Test; +import javax.annotation.Nullable; import java.util.stream.Collectors; import static org.junit.Assert.assertEquals; @@ -170,9 +171,13 @@ public class FhirPatchApplyR4Test { return ((IPrimitiveType) part.getValue()).getValueAsString(); } + @Nullable public static T extractPartValue(Parameters theDiff, int theIndex, String theParameterName, String thePartName, Class theExpectedType) { Parameters.ParametersParameterComponent component = theDiff.getParameter().stream().filter(t -> t.getName().equals(theParameterName)).collect(Collectors.toList()).get(theIndex); - Parameters.ParametersParameterComponent part = component.getPart().stream().filter(t -> t.getName().equals(thePartName)).findFirst().orElseThrow(() -> new IllegalArgumentException()); + Parameters.ParametersParameterComponent part = component.getPart().stream().filter(t -> t.getName().equals(thePartName)).findFirst().orElse(null); + if (part == null) { + return null; + } if (IBaseResource.class.isAssignableFrom(theExpectedType)) { return (T) part.getResource(); diff --git a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchDiffR4Test.java b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchDiffR4Test.java index 11b643942d5..6d3f80de0d5 100644 --- a/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchDiffR4Test.java +++ b/hapi-fhir-jpaserver-base/src/test/java/ca/uhn/fhir/jpa/patch/FhirPatchDiffR4Test.java @@ -1,9 +1,11 @@ package ca.uhn.fhir.jpa.patch; import ca.uhn.fhir.context.FhirContext; +import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.r4.model.BooleanType; import org.hl7.fhir.r4.model.DateTimeType; 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.Parameters; import org.hl7.fhir.r4.model.Patient; @@ -319,6 +321,30 @@ public class FhirPatchDiffR4Test { validateDiffProducesSameResults(oldValue, newValue, svc, diff); } + @Test + public void testInsertContact() { + Patient oldValue = new Patient(); + + Patient newValue = new Patient(); + newValue.addContact().getName().setFamily("My Family"); + + FhirPatch svc = new FhirPatch(ourCtx); + Parameters diff = (Parameters) svc.diff(oldValue, newValue); + + ourLog.info(ourCtx.newJsonParser().setPrettyPrint(true).encodeResourceToString(diff)); + + assertEquals(2, diff.getParameter().size()); + assertEquals("insert", extractPartValuePrimitive(diff, 0, "operation", "type")); + assertEquals("Patient.contact", extractPartValuePrimitive(diff, 0, "operation", "path")); + assertEquals(null, extractPartValue(diff, 0, "operation", "value", IBase.class)); + assertEquals("insert", extractPartValuePrimitive(diff, 1, "operation", "type")); + assertEquals("Patient.contact[0].name", extractPartValuePrimitive(diff, 1, "operation", "path")); + assertEquals("My Family", extractPartValue(diff, 1, "operation", "value", HumanName.class).getFamily()); + + validateDiffProducesSameResults(oldValue, newValue, svc, diff); + } + + @Test public void testIgnoreElementComposite_Resource() { Patient oldValue = new Patient();