From a7a446903fd949fc08b584b7efbf4d986b8a5e86 Mon Sep 17 00:00:00 2001 From: Ken Stevens Date: Wed, 25 Oct 2023 23:35:20 -0400 Subject: [PATCH] 5401 add terserutil clear by fhirpath (#5402) * begin with failing test * test passes * test passes * changelog * moar testor --- .../context/BaseRuntimeChildDefinition.java | 5 + .../java/ca/uhn/fhir/parser/JsonParser.java | 21 +- .../java/ca/uhn/fhir/util/TerserUtil.java | 34 ++- .../7_0_0/5401-terserutil-clear.yaml | 5 + .../java/ca/uhn/fhir/util/TerserUtilTest.java | 267 +++++++++++------- 5 files changed, 210 insertions(+), 122 deletions(-) create mode 100644 hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5401-terserutil-clear.yaml diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeChildDefinition.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeChildDefinition.java index bafc0cce4ce7..e55d2fa2b25b 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeChildDefinition.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/context/BaseRuntimeChildDefinition.java @@ -20,6 +20,7 @@ package ca.uhn.fhir.context; import ca.uhn.fhir.i18n.Msg; +import ca.uhn.fhir.model.api.annotation.Child; import org.hl7.fhir.instance.model.api.IBase; import org.hl7.fhir.instance.model.api.IBaseReference; @@ -78,6 +79,10 @@ public void setReplacedParentDefinition(BaseRuntimeChildDefinition myReplacedPar this.myReplacedParentDefinition = myReplacedParentDefinition; } + public boolean isMultipleCardinality() { + return this.getMax() > 1 || this.getMax() == Child.MAX_UNLIMITED; + } + public interface IAccessor { List getValues(IBase theTarget); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java index 846d5dab271d..f82222b4ddbc 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/parser/JsonParser.java @@ -39,7 +39,6 @@ import ca.uhn.fhir.model.api.ResourceMetadataKeyEnum; import ca.uhn.fhir.model.api.Tag; import ca.uhn.fhir.model.api.TagList; -import ca.uhn.fhir.model.api.annotation.Child; import ca.uhn.fhir.model.base.composite.BaseCodingDt; import ca.uhn.fhir.model.base.composite.BaseContainedDt; import ca.uhn.fhir.model.primitive.IdDt; @@ -659,9 +658,8 @@ private void encodeCompositeElementChildrenToStreamWriter( theEventWriter.endArray(); } BaseRuntimeChildDefinition replacedParentDefinition = nextChild.getReplacedParentDefinition(); - if (isMultipleCardinality(nextChild.getMax()) - || (replacedParentDefinition != null - && isMultipleCardinality(replacedParentDefinition.getMax()))) { + if (nextChild.isMultipleCardinality() + || (replacedParentDefinition != null && replacedParentDefinition.isMultipleCardinality())) { beginArray(theEventWriter, nextChildSpecificName); inArray = true; encodeChildElementToStreamWriter( @@ -728,14 +726,14 @@ && isMultipleCardinality(replacedParentDefinition.getMax()))) { List heldModExts = Collections.emptyList(); if (extensions.size() > i && extensions.get(i) != null - && extensions.get(i).isEmpty() == false) { + && !extensions.get(i).isEmpty()) { haveContent = true; heldExts = extensions.get(i); } if (modifierExtensions.size() > i && modifierExtensions.get(i) != null - && modifierExtensions.get(i).isEmpty() == false) { + && !modifierExtensions.get(i).isEmpty()) { haveContent = true; heldModExts = modifierExtensions.get(i); } @@ -746,7 +744,7 @@ && isMultipleCardinality(replacedParentDefinition.getMax()))) { } else { nextComments = null; } - if (nextComments != null && nextComments.isEmpty() == false) { + if (nextComments != null && !nextComments.isEmpty()) { haveContent = true; } @@ -804,10 +802,6 @@ private boolean isSupportsFhirComment() { return myIsSupportsFhirComment; } - private boolean isMultipleCardinality(int maxCardinality) { - return maxCardinality > 1 || maxCardinality == Child.MAX_UNLIMITED; - } - private void encodeCompositeElementToStreamWriter( RuntimeResourceDefinition theResDef, IBaseResource theResource, @@ -917,10 +911,7 @@ private void encodeResourceToJsonStreamWriter( // Undeclared extensions extractUndeclaredExtensions( theResourceId, extensions, modifierExtensions, null, null, theEncodeContext, theContainedResource); - boolean haveExtension = false; - if (!extensions.isEmpty()) { - haveExtension = true; - } + boolean haveExtension = !extensions.isEmpty(); if (theResourceId.hasFormatComment() || haveExtension) { beginObject(theEventWriter, "_id"); diff --git a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java index 11b416fdbbf7..d20fe991c160 100644 --- a/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java +++ b/hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java @@ -351,6 +351,29 @@ public static void clearField(FhirContext theFhirContext, IBaseResource theResou clear(childDefinition.getAccessor().getValues(theResource)); } + /** + * Clears the specified field on the resource provided by the FHIRPath. If more than one value matches + * the FHIRPath, all values will be cleared. + * + * @param theFhirContext + * @param theResource + * @param theFhirPath + */ + public static void clearFieldByFhirPath(FhirContext theFhirContext, IBaseResource theResource, String theFhirPath) { + + if (theFhirPath.contains(".")) { + String parentPath = theFhirPath.substring(0, theFhirPath.lastIndexOf(".")); + String fieldName = theFhirPath.substring(theFhirPath.lastIndexOf(".") + 1); + FhirTerser terser = theFhirContext.newTerser(); + List parents = terser.getValues(theResource, parentPath); + for (IBase parent : parents) { + clearField(theFhirContext, fieldName, parent); + } + } else { + clearField(theFhirContext, theResource, theFhirPath); + } + } + /** * Clears the specified field on the element provided * @@ -362,7 +385,16 @@ public static void clearField(FhirContext theFhirContext, String theFieldName, I BaseRuntimeElementDefinition definition = theFhirContext.getElementDefinition(theBase.getClass()); BaseRuntimeChildDefinition childDefinition = definition.getChildByName(theFieldName); Validate.notNull(childDefinition); - clear(childDefinition.getAccessor().getValues(theBase)); + BaseRuntimeChildDefinition.IAccessor accessor = childDefinition.getAccessor(); + clear(accessor.getValues(theBase)); + List newValue = accessor.getValues(theBase); + + if (newValue != null && !newValue.isEmpty()) { + // Our clear failed, probably because it was an immutable SingletonList returned by a FieldPlainAccessor + // that cannot be cleared. + // Let's just null it out instead. + childDefinition.getMutator().setValue(theBase, null); + } } /** diff --git a/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5401-terserutil-clear.yaml b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5401-terserutil-clear.yaml new file mode 100644 index 000000000000..733f62c5bc74 --- /dev/null +++ b/hapi-fhir-docs/src/main/resources/ca/uhn/hapi/fhir/changelog/7_0_0/5401-terserutil-clear.yaml @@ -0,0 +1,5 @@ +--- +type: add +issue: 5401 +title: "Previously, it was only possible to clear a top-level field on a resource using TerserUtil. A new method has been added +to TerserUtil to support clearing a value by FhirPath." diff --git a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java index 493f2eb05b94..ddf94c26105f 100644 --- a/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java +++ b/hapi-fhir-structures-r4/src/test/java/ca/uhn/fhir/util/TerserUtilTest.java @@ -13,6 +13,7 @@ import org.hl7.fhir.r4.model.Organization; import org.hl7.fhir.r4.model.Patient; import org.hl7.fhir.r4.model.PrimitiveType; +import org.hl7.fhir.r4.model.StringType; import org.junit.jupiter.api.Test; import java.util.Date; @@ -30,88 +31,88 @@ class TerserUtilTest { private FhirContext ourFhirContext = FhirContext.forR4(); private static final String SAMPLE_PERSON = - """ - { - "resourceType": "Patient", - "extension": [ - { - "url": "http://hl7.org/fhir/us/core/StructureDefinition/us-core-race", - "valueCoding": { - "system": "MyInternalRace", - "code": "X", - "display": "Eks" + """ + { + "resourceType": "Patient", + "extension": [ + { + "url": "http://hl7.org/fhir/us/core/StructureDefinition/us-core-race", + "valueCoding": { + "system": "MyInternalRace", + "code": "X", + "display": "Eks" + } + }, + { + "url": "http://hl7.org/fhir/us/core/StructureDefinition/us-core-ethnicity'", + "valueCoding": { + "system": "MyInternalEthnicity", + "display": "NNN" + } } - }, - { - "url": "http://hl7.org/fhir/us/core/StructureDefinition/us-core-ethnicity'", - "valueCoding": { - "system": "MyInternalEthnicity", - "display": "NNN" + ], + "identifier": [ + { + "system": "http://example.org/member_id", + "value": "123123" + }, + { + "system": "http://example.org/medicaid_id", + "value": "12312323123Z" + }, + { + "system": "http://example.org/CDNS_id", + "value": "123123123E" + }, + { + "system": "http://example.org/SSN" } - } - ], - "identifier": [ - { - "system": "http://example.org/member_id", - "value": "123123" - }, - { - "system": "http://example.org/medicaid_id", - "value": "12312323123Z" - }, - { - "system": "http://example.org/CDNS_id", - "value": "123123123E" - }, - { - "system": "http://example.org/SSN" - } - ], - "active": true, - "name": [ - { - "family": "TestFamily", - "given": [ - "Given" - ] - } - ], - "telecom": [ - { - "system": "email", - "value": "email@email.io" - }, - { - "system": "phone", - "value": "123123231" - }, - { - "system": "phone", - "value": "1231232312" - }, - { - "system": "phone", - "value": "1231232314" - } - ], - "gender": "male", - "birthDate": "1900-01-01", - "deceasedBoolean": true, - "contained": [ - { - "id": "1", - "identifier": [ - { - "system": "urn:hssc:srhs:contact:organizationId", - "value": "1000" - } - ], - "name": "BUILDERS FIRST SOURCE", - "resourceType": "Organization" - } - ] - } - """; + ], + "active": true, + "name": [ + { + "family": "TestFamily", + "given": [ + "Given" + ] + } + ], + "telecom": [ + { + "system": "email", + "value": "email@email.io" + }, + { + "system": "phone", + "value": "123123231" + }, + { + "system": "phone", + "value": "1231232312" + }, + { + "system": "phone", + "value": "1231232314" + } + ], + "gender": "male", + "birthDate": "1900-01-01", + "deceasedBoolean": true, + "contained": [ + { + "id": "1", + "identifier": [ + { + "system": "urn:hssc:srhs:contact:organizationId", + "value": "1000" + } + ], + "name": "BUILDERS FIRST SOURCE", + "resourceType": "Organization" + } + ] + } + """; @Test void testCloneEidIntoResource() { @@ -180,7 +181,7 @@ void testCloneEidIntoResourceViaHelper() { RuntimeResourceDefinition definition = p1Helper.getResourceDefinition(); TerserUtil.cloneEidIntoResource(ourFhirContext, definition.getChildByName("identifier"), - p1.getIdentifier().get(0), p2Helper.getResource()); + p1.getIdentifier().get(0), p2Helper.getResource()); assertEquals(1, p2Helper.getFieldValues("identifier").size()); @@ -291,12 +292,12 @@ void testMergeForAddressWithExtensions() { Patient p1 = new Patient(); p1.addAddress() - .addLine("10 Main Street") - .setCity("Hamilton") - .setState("ON") - .setPostalCode("Z0Z0Z0") - .setCountry("Canada") - .addExtension(ext); + .addLine("10 Main Street") + .setCity("Hamilton") + .setState("ON") + .setPostalCode("Z0Z0Z0") + .setCountry("Canada") + .addExtension(ext); Patient p2 = new Patient(); p2.addAddress().addLine("10 Lenin Street").setCity("Severodvinsk").setCountry("Russia"); @@ -328,12 +329,12 @@ void testReplaceForAddressWithExtensions() { Patient p1 = new Patient(); p1.addAddress() - .addLine("10 Main Street") - .setCity("Hamilton") - .setState("ON") - .setPostalCode("Z0Z0Z0") - .setCountry("Canada") - .addExtension(ext); + .addLine("10 Main Street") + .setCity("Hamilton") + .setState("ON") + .setPostalCode("Z0Z0Z0") + .setCountry("Canada") + .addExtension(ext); Patient p2 = new Patient(); p2.addAddress().addLine("10 Lenin Street").setCity("Severodvinsk").setCountry("Russia"); @@ -353,21 +354,21 @@ void testMergeForSimilarAddresses() { Patient p1 = new Patient(); p1.addAddress() - .addLine("10 Main Street") - .setCity("Hamilton") - .setState("ON") - .setPostalCode("Z0Z0Z0") - .setCountry("Canada") - .addExtension(ext); + .addLine("10 Main Street") + .setCity("Hamilton") + .setState("ON") + .setPostalCode("Z0Z0Z0") + .setCountry("Canada") + .addExtension(ext); Patient p2 = new Patient(); p2.addAddress() - .addLine("10 Main Street") - .setCity("Hamilton") - .setState("ON") - .setPostalCode("Z0Z0Z1") - .setCountry("Canada") - .addExtension(ext); + .addLine("10 Main Street") + .setCity("Hamilton") + .setState("ON") + .setPostalCode("Z0Z0Z1") + .setCountry("Canada") + .addExtension(ext); TerserUtil.mergeField(ourFhirContext, "address", p1, p2); @@ -469,7 +470,8 @@ public void testReplaceFields_SameValues() { assertEquals(1, p2.getName().size()); assertEquals("Doe", p2.getName().get(0).getFamily()); } - @Test + + @Test public void testReplaceFieldByEmptyValue() { Patient p1 = new Patient(); Patient p2 = new Patient(); @@ -507,6 +509,7 @@ public void testClearFields() { { Patient p1 = new Patient(); p1.addName().setFamily("Doe"); + assertEquals(1, p1.getName().size()); TerserUtil.clearField(ourFhirContext, p1, "name"); @@ -517,6 +520,7 @@ public void testClearFields() { Address a1 = new Address(); a1.addLine("Line 1"); a1.addLine("Line 2"); + assertEquals(2, a1.getLine().size()); a1.setCity("Test"); TerserUtil.clearField(ourFhirContext, "line", a1); @@ -525,6 +529,47 @@ public void testClearFields() { } } + @Test + public void testClearFieldByFhirPath() { + Patient p1 = new Patient(); + p1.addName().setFamily("Doe"); + assertEquals(1, p1.getName().size()); + + TerserUtil.clearFieldByFhirPath(ourFhirContext, p1, "name"); + + assertEquals(0, p1.getName().size()); + + Address a1 = new Address(); + a1.addLine("Line 1"); + a1.addLine("Line 2"); + assertEquals(2, a1.getLine().size()); + a1.setCity("Test"); + a1.getPeriod().setStartElement(new DateTimeType("2021-01-01")); + p1.addAddress(a1); + + assertEquals("2021-01-01", p1.getAddress().get(0).getPeriod().getStartElement().toHumanDisplay()); + assertNotNull(p1.getAddress().get(0).getPeriod().getStart()); + + Address a2 = new Address(); + a2.addLine("Line 1"); + a2.addLine("Line 2"); + a2.setCity("Test"); + a2.getPeriod().setStartElement(new DateTimeType("2021-01-01")); + p1.addAddress(a2); + + + TerserUtil.clearFieldByFhirPath(ourFhirContext, p1, "address.line"); + TerserUtil.clearFieldByFhirPath(ourFhirContext, p1, "address.period.start"); + + assertNull(p1.getAddress().get(0).getPeriod().getStart()); + + assertEquals(2, p1.getAddress().size()); + assertEquals(0, p1.getAddress().get(0).getLine().size()); + assertEquals(0, p1.getAddress().get(1).getLine().size()); + assertEquals("Test", p1.getAddress().get(0).getCity()); + assertEquals("Test", p1.getAddress().get(1).getCity()); + } + @Test public void testSetField() { Patient p1 = new Patient(); @@ -551,6 +596,16 @@ public void testSetFieldByFhirPath() { assertEquals("CITY", p1.getAddress().get(0).getCity()); } + @Test + public void testSetFieldByCompositeFhirPath() { + Patient p1 = new Patient(); + + TerserUtil.setFieldByFhirPath(ourFhirContext, "address.city", p1, new StringType("CITY")); + + assertEquals(1, p1.getAddress().size()); + assertEquals("CITY", p1.getAddress().get(0).getCity()); + } + @Test public void testClone() { Patient p1 = new Patient();