From 8a7fc58e91073d5cefb6cb3bb051d7b2892fe240 Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Thu, 20 May 2021 15:48:00 -0400 Subject: [PATCH 1/6] Fixed primitive field merging --- .../java/ca/uhn/fhir/util/TerserUtil.java | 34 ++++++++++++++----- .../java/ca/uhn/fhir/util/TerserUtilTest.java | 34 +++++++++++++++++++ 2 files changed, 59 insertions(+), 9 deletions(-) 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 02cf84537f9..058ecfc5603 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 @@ -28,9 +28,12 @@ import ca.uhn.fhir.context.RuntimeResourceDefinition; import org.apache.commons.lang3.Validate; import org.apache.commons.lang3.tuple.Triple; import org.hl7.fhir.instance.model.api.IBase; +import org.hl7.fhir.instance.model.api.IBaseEnumeration; import org.hl7.fhir.instance.model.api.IBaseResource; +import org.hl7.fhir.instance.model.api.IPrimitiveType; import org.slf4j.Logger; +import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.Arrays; import java.util.Collection; @@ -283,9 +286,10 @@ public final class TerserUtil { */ public static void replaceFieldsByPredicate(FhirContext theFhirContext, IBaseResource theFrom, IBaseResource theTo, Predicate> thePredicate) { RuntimeResourceDefinition definition = theFhirContext.getResourceDefinition(theFrom); + FhirTerser terser = theFhirContext.newTerser(); for (BaseRuntimeChildDefinition childDefinition : definition.getChildrenAndExtension()) { if (thePredicate.test(Triple.of(childDefinition, theFrom, theTo))) { - replaceField(theFrom, theTo, childDefinition); + replaceField(terser, theFrom, theTo, childDefinition); } } } @@ -313,7 +317,7 @@ public final class TerserUtil { public static void replaceField(FhirContext theFhirContext, String theFieldName, IBaseResource theFrom, IBaseResource theTo) { RuntimeResourceDefinition definition = theFhirContext.getResourceDefinition(theFrom); Validate.notNull(definition); - replaceField(theFrom, theTo, theFhirContext.getResourceDefinition(theFrom).getChildByName(theFieldName)); + replaceField(theFhirContext.newTerser(), theFrom, theTo, theFhirContext.getResourceDefinition(theFrom).getChildByName(theFieldName)); } /** @@ -441,11 +445,12 @@ public final class TerserUtil { return values.get(0); } - private static void replaceField(IBaseResource theFrom, IBaseResource theTo, BaseRuntimeChildDefinition childDefinition) { - childDefinition.getAccessor().getFirstValueOrNull(theFrom).ifPresent(v -> { - childDefinition.getMutator().setValue(theTo, v); - } - ); + private static void replaceField(FhirTerser theTerser, IBaseResource theFrom, IBaseResource theTo, BaseRuntimeChildDefinition childDefinition) { + List fromValues = childDefinition.getAccessor().getValues(theFrom); + List toValues = childDefinition.getAccessor().getValues(theTo); + toValues.clear(); + + mergeFields(theTerser, theTo, childDefinition, fromValues, toValues); } /** @@ -532,13 +537,24 @@ public final class TerserUtil { } IBase newFieldValue = childDefinition.getChildByName(childDefinition.getElementName()).newInstance(); - theTerser.cloneInto(theFromFieldValue, newFieldValue, true); + if (theFromFieldValue instanceof IPrimitiveType) { + Method copyMethod = getMethod(theFromFieldValue, "copy"); + if (copyMethod != null) { + try { + newFieldValue = (IBase) copyMethod.invoke(theFromFieldValue, new Object[]{}); + } catch (Throwable t) { + ((IPrimitiveType) newFieldValue).setValueAsString(((IPrimitiveType) theFromFieldValue).getValueAsString()); + } + } + } else { + theTerser.cloneInto(theFromFieldValue, newFieldValue, true); + } try { theToFieldValues.add(newFieldValue); } catch (UnsupportedOperationException e) { childDefinition.getMutator().setValue(theTo, newFieldValue); - break; + theToFieldValues = childDefinition.getAccessor().getValues(theTo); } } } 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 a88fbcf5fac..a3f0dd14ca0 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 @@ -103,6 +103,38 @@ class TerserUtilTest { assertEquals(p1.getName().get(0).getNameAsSingleString(), p2.getName().get(0).getNameAsSingleString()); } + @Test + void testCloneIdentifiers() { + Patient p1 = new Patient(); + p1.addIdentifier(new Identifier().setSystem("uri:mi").setValue("123456")); + p1.addIdentifier(new Identifier().setSystem("uri:mdi").setValue("287351247K")); + p1.addIdentifier(new Identifier().setSystem("uri:cdns").setValue("654841918")); + p1.addIdentifier(new Identifier().setSystem("uri:ssn").setValue("855191882")); + p1.addName().setFamily("Sat").addGiven("Joe"); + + Patient p2 = new Patient(); + TerserUtil.mergeField(ourFhirContext, ourFhirContext.newTerser(), "identifier", p1, p2); + + assertEquals(4, p2.getIdentifier().size()); + assertTrue(p2.getName().isEmpty()); + } + + @Test + void testReplaceIdentifiers() { + Patient p1 = new Patient(); + p1.addIdentifier(new Identifier().setSystem("uri:mi").setValue("123456")); + p1.addIdentifier(new Identifier().setSystem("uri:mdi").setValue("287351247K")); + p1.addIdentifier(new Identifier().setSystem("uri:cdns").setValue("654841918")); + p1.addIdentifier(new Identifier().setSystem("uri:ssn").setValue("855191882")); + p1.addName().setFamily("Sat").addGiven("Joe"); + + Patient p2 = new Patient(); + TerserUtil.replaceField(ourFhirContext, "identifier", p1, p2); + + assertEquals(4, p2.getIdentifier().size()); + assertTrue(p2.getName().isEmpty()); + } + @Test void testCloneWithNonPrimitves() { Patient p1 = new Patient(); @@ -314,6 +346,8 @@ class TerserUtilTest { // expect p2 to have "Doe" and MALE after replace assertEquals(1, p2.getName().size()); assertEquals("Doe", p2.getName().get(0).getFamily()); + System.out.println(p2.getGender()); + assertEquals(Enumerations.AdministrativeGender.MALE, p2.getGender()); assertEquals(dob, p2.getBirthDate()); } From 2acb3501a51e501ecc7b25d92a26e38d906f6bfe Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Thu, 20 May 2021 16:15:15 -0400 Subject: [PATCH 2/6] Removed sout --- .../src/test/java/ca/uhn/fhir/util/TerserUtilTest.java | 1 - 1 file changed, 1 deletion(-) 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 a3f0dd14ca0..9dafe34dc63 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 @@ -346,7 +346,6 @@ class TerserUtilTest { // expect p2 to have "Doe" and MALE after replace assertEquals(1, p2.getName().size()); assertEquals("Doe", p2.getName().get(0).getFamily()); - System.out.println(p2.getGender()); assertEquals(Enumerations.AdministrativeGender.MALE, p2.getGender()); assertEquals(dob, p2.getBirthDate()); From 995ac96f739ffab540b3ab2b4db77fd94712899c Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Thu, 20 May 2021 17:21:04 -0400 Subject: [PATCH 3/6] Can't think today --- .../src/main/java/ca/uhn/fhir/util/TerserUtil.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) 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 058ecfc5603..ab0024bcee9 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 @@ -538,14 +538,14 @@ public final class TerserUtil { IBase newFieldValue = childDefinition.getChildByName(childDefinition.getElementName()).newInstance(); if (theFromFieldValue instanceof IPrimitiveType) { - Method copyMethod = getMethod(theFromFieldValue, "copy"); - if (copyMethod != null) { try { - newFieldValue = (IBase) copyMethod.invoke(theFromFieldValue, new Object[]{}); + Method copyMethod = getMethod(theFromFieldValue, "copy"); + if (copyMethod != null) { + newFieldValue = (IBase) copyMethod.invoke(theFromFieldValue, new Object[]{}); + } } catch (Throwable t) { ((IPrimitiveType) newFieldValue).setValueAsString(((IPrimitiveType) theFromFieldValue).getValueAsString()); } - } } else { theTerser.cloneInto(theFromFieldValue, newFieldValue, true); } From e243674f5da2bb8d02386864ee523668c48c26d6 Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Fri, 21 May 2021 11:54:04 -0400 Subject: [PATCH 4/6] Accounted for uninitialized fields --- .../src/main/java/ca/uhn/fhir/util/TerserUtil.java | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) 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 ab0024bcee9..157f6dd6ae2 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 @@ -329,7 +329,7 @@ public final class TerserUtil { */ public static void clearField(FhirContext theFhirContext, String theFieldName, IBaseResource theResource) { BaseRuntimeChildDefinition childDefinition = getBaseRuntimeChildDefinition(theFhirContext, theFieldName, theResource); - childDefinition.getAccessor().getValues(theResource).clear(); + clear(childDefinition.getAccessor().getValues(theResource)); } /** @@ -343,7 +343,7 @@ public final class TerserUtil { BaseRuntimeElementDefinition definition = theFhirContext.getElementDefinition(theBase.getClass()); BaseRuntimeChildDefinition childDefinition = definition.getChildByName(theFieldName); Validate.notNull(childDefinition); - childDefinition.getAccessor().getValues(theBase).clear(); + clear(childDefinition.getAccessor().getValues(theBase)); } /** @@ -448,7 +448,7 @@ public final class TerserUtil { private static void replaceField(FhirTerser theTerser, IBaseResource theFrom, IBaseResource theTo, BaseRuntimeChildDefinition childDefinition) { List fromValues = childDefinition.getAccessor().getValues(theFrom); List toValues = childDefinition.getAccessor().getValues(theTo); - toValues.clear(); + clear(toValues); mergeFields(theTerser, theTo, childDefinition, fromValues, toValues); } @@ -631,4 +631,12 @@ public final class TerserUtil { return (T) def.newInstance(theConstructorParam); } + private static void clear(List values) { + if (values == null) { + return; + } + + values.clear(); + } + } From d0d0ff9890bec8d8f0c32962a46d9d9ec63300ca Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Fri, 21 May 2021 12:00:01 -0400 Subject: [PATCH 5/6] Accounted for uninitialized fields --- .../java/ca/uhn/fhir/util/TerserUtil.java | 20 +++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) 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 157f6dd6ae2..5bd83d2bdc9 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 @@ -538,14 +538,14 @@ public final class TerserUtil { IBase newFieldValue = childDefinition.getChildByName(childDefinition.getElementName()).newInstance(); if (theFromFieldValue instanceof IPrimitiveType) { - try { - Method copyMethod = getMethod(theFromFieldValue, "copy"); - if (copyMethod != null) { - newFieldValue = (IBase) copyMethod.invoke(theFromFieldValue, new Object[]{}); - } - } catch (Throwable t) { - ((IPrimitiveType) newFieldValue).setValueAsString(((IPrimitiveType) theFromFieldValue).getValueAsString()); + try { + Method copyMethod = getMethod(theFromFieldValue, "copy"); + if (copyMethod != null) { + newFieldValue = (IBase) copyMethod.invoke(theFromFieldValue, new Object[]{}); } + } catch (Throwable t) { + ((IPrimitiveType) newFieldValue).setValueAsString(((IPrimitiveType) theFromFieldValue).getValueAsString()); + } } else { theTerser.cloneInto(theFromFieldValue, newFieldValue, true); } @@ -636,7 +636,11 @@ public final class TerserUtil { return; } - values.clear(); + try { + values.clear(); + } catch (Throwable t) { + ourLog.warn("Unable to clear values " + String.valueOf(values), t); + } } } From 80be95dd07870c0a80c22c2384913076776ddb77 Mon Sep 17 00:00:00 2001 From: Nick Goupinets Date: Fri, 21 May 2021 13:32:23 -0400 Subject: [PATCH 6/6] Accounted for uninitialized fields - part 3 --- hapi-fhir-base/src/main/java/ca/uhn/fhir/util/TerserUtil.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 5bd83d2bdc9..4f447ab25c3 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 @@ -639,7 +639,7 @@ public final class TerserUtil { try { values.clear(); } catch (Throwable t) { - ourLog.warn("Unable to clear values " + String.valueOf(values), t); + ourLog.debug("Unable to clear values " + String.valueOf(values), t); } }