From 1bd11f26df1d2f8e2355e1f9f8735f1292a82509 Mon Sep 17 00:00:00 2001
From: Lee Surprenant <lmsurpre@us.ibm.com>
Date: Thu, 5 May 2022 10:09:26 -0400
Subject: [PATCH 1/3] issue #3619 - populate 'path' field for SystemValueNodes
 in context tree

I also updated the javadoc (relates to #2153) because the user was
confused about the FHIRPathNode API and I thought it could use further
clarification.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
---
 .../path/FHIRPathAbstractSystemValue.java     | 24 +------
 .../ibm/fhir/path/FHIRPathBooleanValue.java   |  8 ++-
 .../ibm/fhir/path/FHIRPathDateTimeValue.java  |  8 ++-
 .../com/ibm/fhir/path/FHIRPathDateValue.java  |  8 ++-
 .../ibm/fhir/path/FHIRPathDecimalValue.java   |  8 ++-
 .../ibm/fhir/path/FHIRPathIntegerValue.java   |  8 ++-
 .../ibm/fhir/path/FHIRPathStringValue.java    |  8 ++-
 .../ibm/fhir/path/FHIRPathSystemValue.java    | 72 +++++++++++++------
 .../com/ibm/fhir/path/FHIRPathTimeValue.java  |  8 ++-
 .../java/com/ibm/fhir/path/FHIRPathTree.java  | 22 +++---
 .../ibm/fhir/path/FHIRPathTypeInfoNode.java   | 42 +++++------
 11 files changed, 118 insertions(+), 98 deletions(-)

diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathAbstractSystemValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathAbstractSystemValue.java
index 68510632abc..0a3f992b0f1 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathAbstractSystemValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathAbstractSystemValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2021
+ * (C) Copyright IBM Corp. 2021, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -20,28 +20,6 @@ protected FHIRPathAbstractSystemValue(Builder builder) {
         super(builder);
     }
 
-    /**
-     * This method always returns false
-     *
-     * @return
-     *     false
-     */
-    @Override
-    public final boolean hasValue() {
-        return false;
-    }
-
-    /**
-     * This method always returns null
-     *
-     * @return
-     *     null
-     */
-    @Override
-    public final FHIRPathSystemValue getValue() {
-        return null;
-    }
-
     /**
      * This method always returns an empty {@link Collection}
      *
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathBooleanValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathBooleanValue.java
index da85904a425..f2b544b1626 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathBooleanValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathBooleanValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -74,6 +74,8 @@ public static FHIRPathBooleanValue booleanValue(Boolean _boolean) {
     /**
      * Static factory method for creating named FHIRPathBooleanValue instances from a {@link Boolean} value
      *
+     * @param path
+     *     the path of the FHIRPathNode
      * @param name
      *    the name
      * @param _boolean
@@ -81,8 +83,8 @@ public static FHIRPathBooleanValue booleanValue(Boolean _boolean) {
      * @return
      *    a new named FHIRPathBooleanValue instance
      */
-    public static FHIRPathBooleanValue booleanValue(String name, Boolean _boolean) {
-        return FHIRPathBooleanValue.builder(_boolean).name(name).build();
+    public static FHIRPathBooleanValue booleanValue(String path, String name, Boolean _boolean) {
+        return FHIRPathBooleanValue.builder(_boolean).name(name).path(path).build();
     }
 
     @Override
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDateTimeValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDateTimeValue.java
index 9b42544f6aa..65c5214f894 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDateTimeValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDateTimeValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -119,6 +119,8 @@ public static FHIRPathDateTimeValue dateTimeValue(TemporalAccessor dateTime) {
     /**
      * Static factory method for creating named FHIRPathDateTimeValue instances from a {@link TemporalAccessor} date/time value
      *
+     * @param path
+     *     the path of the FHIRPathNode
      * @param name
      *     the name
      * @param dateTime
@@ -126,8 +128,8 @@ public static FHIRPathDateTimeValue dateTimeValue(TemporalAccessor dateTime) {
      * @return
      *     a new named FHIRPathDateTimeValue instance
      */
-    public static FHIRPathDateTimeValue dateTimeValue(String name, TemporalAccessor dateTime) {
-        return FHIRPathDateTimeValue.builder(dateTime, getPrecision(dateTime)).name(name).build();
+    public static FHIRPathDateTimeValue dateTimeValue(String path, String name, TemporalAccessor dateTime) {
+        return FHIRPathDateTimeValue.builder(dateTime, getPrecision(dateTime)).name(name).path(path).build();
     }
 
     @Override
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDateValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDateValue.java
index 5e7a7aa8ca6..9c96ac8a94e 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDateValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDateValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -92,6 +92,8 @@ public static FHIRPathDateValue dateValue(TemporalAccessor date) {
     /**
      * Static factory method for creating named FHIRPathDateValue instances from a {@link TemporalAccessor} date value
      *
+     * @param path
+     *     the path of the FHIRPathNode
      * @param name
      *     the name
      * @param dateTime
@@ -99,8 +101,8 @@ public static FHIRPathDateValue dateValue(TemporalAccessor date) {
      * @return
      *     a new named FHIRPathDateValue instance
      */
-    public static FHIRPathDateValue dateValue(String name, TemporalAccessor date) {
-        return FHIRPathDateValue.builder(date, getPrecision(date)).name(name).build();
+    public static FHIRPathDateValue dateValue(String path, String name, TemporalAccessor date) {
+        return FHIRPathDateValue.builder(date, getPrecision(date)).name(name).path(path).build();
     }
 
     @Override
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDecimalValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDecimalValue.java
index 021c52cf11a..d94bd7ed763 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDecimalValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathDecimalValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -53,6 +53,8 @@ public static FHIRPathDecimalValue decimalValue(BigDecimal decimal) {
     /**
      * Static factory method for creating named FHIRPathDecimalValue instances from a {@link BigDecimal} value
      *
+     * @param path
+     *     the path of the FHIRPathNode
      * @param name
      *     the name
      * @param decimal
@@ -60,8 +62,8 @@ public static FHIRPathDecimalValue decimalValue(BigDecimal decimal) {
      * @return
      *     a new named FHIRPathDecimalValue instance
      */
-    public static FHIRPathDecimalValue decimalValue(String name, BigDecimal decimal) {
-        return FHIRPathDecimalValue.builder(decimal).name(name).build();
+    public static FHIRPathDecimalValue decimalValue(String path, String name, BigDecimal decimal) {
+        return FHIRPathDecimalValue.builder(decimal).name(name).path(path).build();
     }
 
     @Override
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathIntegerValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathIntegerValue.java
index 31774bb4b1e..bbac4b4d638 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathIntegerValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathIntegerValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -58,6 +58,8 @@ public static FHIRPathIntegerValue integerValue(Integer integer) {
     /**
      * Static factory method for creating named FHIRPathIntegerValue instances from an {@link Integer} value
      *
+     * @param path
+     *     the path of the FHIRPathNode
      * @param name
      *     the name
      * @param integer
@@ -65,8 +67,8 @@ public static FHIRPathIntegerValue integerValue(Integer integer) {
      * @return
      *     a new named FHIRPathIntegerValue instance
      */
-    public static FHIRPathIntegerValue integerValue(String name, Integer integer) {
-        return FHIRPathIntegerValue.builder(integer).name(name).build();
+    public static FHIRPathIntegerValue integerValue(String path, String name, Integer integer) {
+        return FHIRPathIntegerValue.builder(integer).name(name).path(path).build();
     }
 
     @Override
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathStringValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathStringValue.java
index 2bd393f2a0d..f8910f32583 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathStringValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathStringValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -53,6 +53,8 @@ public static FHIRPathStringValue stringValue(String string) {
     /**
      * Static factory method for creating named FHIRPathStringValue instances from a {@link String} value
      *
+     * @param path
+     *     the path of the FHIRPathNode
      * @param name
      *     the name
      * @param string
@@ -60,8 +62,8 @@ public static FHIRPathStringValue stringValue(String string) {
      * @return
      *     a new named FHIRPathStringValue instance
      */
-    public static FHIRPathStringValue stringValue(String name, String string) {
-        return FHIRPathStringValue.builder(string).name(name).build();
+    public static FHIRPathStringValue stringValue(String path, String name, String string) {
+        return FHIRPathStringValue.builder(string).name(name).path(path).build();
     }
 
     @Override
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathSystemValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathSystemValue.java
index 88caf93216a..49c4a687a93 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathSystemValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathSystemValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2020
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -10,69 +10,97 @@
  * An interface that represents FHIRPath system data types
  */
 public interface FHIRPathSystemValue extends FHIRPathNode {
+    /**
+     * @return false
+     * @implSpec Always false because FHIRPathSystemValues are always leaf nodes.
+     */
     @Override
     default boolean hasValue() {
         return false;
     }
-    
+
+    /**
+     * @return null
+     * @implSpec FHIRPathSystemValues are always leaf nodes and so this method is not applicable
+     *          to instances of this interface. To get the primitive value, cast to a specific SystemValue
+     *          type and use the appropriate getter.
+     * @see #isBooleanValue()
+     * @see #isStringValue()
+     * @see #isQuantityValue()
+     * @see #isNumberValue()
+     * @see #isTemporalValue()
+     * @see #asBooleanValue()
+     * @see #asStringValue()
+     * @see #asQuantityValue()
+     * @see #asNumberValue()
+     * @see #asTemporalValue()
+     */
+    @Override
+    default FHIRPathSystemValue getValue() {
+        return null;
+    }
+
+    /**
+     * Always true
+     */
     @Override
     default boolean isSystemValue() {
         return true;
     }
-    
+
     /**
      * Indicates whether this FHIRPathSystemValue is type compatible with {@link FHIRPathBooleanValue}
-     * 
+     *
      * @return
      *     true if this FHIRPathSystemValue is type compatible with {@link FHIRPathBooleanValue}, otherwise false
      */
     default boolean isBooleanValue() {
         return false;
     }
-    
+
     /**
      * Indicates whether this FHIRPathSystemValue is type compatible with {@link FHIRPathStringValue}
-     * 
+     *
      * @return
      *     true if this FHIRPathSystemValue is type compatible with {@link FHIRPathStringValue}, otherwise false
      */
     default boolean isStringValue() {
         return false;
     }
-    
+
     /**
      * Indicates whether this FHIRPathSystemValue is type compatible with {@link FHIRPathQuantityValue}
-     * 
+     *
      * @return
      *     true if this FHIRPathSystemValue is type compatible with {@link FHIRPathQuantityValue}, otherwise false
      */
     default boolean isQuantityValue() {
         return false;
     }
-    
+
     /**
      * Indicates whether this FHIRPathSystemValue is type compatible with {@link FHIRPathNumberValue}
-     * 
+     *
      * @return
      *     true if this FHIRPathSystemValue is type compatible with {@link FHIRPathNumberValue}, otherwise false
      */
     default boolean isNumberValue() {
         return false;
     }
-    
+
     /**
      * Indicates whether this FHIRPathSystemValue is type compatible with {@link FHIRPathTemporalValue}
-     * 
+     *
      * @return
      *     true if this FHIRPathSystemValue is type compatible with {@link FHIRPathTemporalValue}, otherwise false
      */
     default boolean isTemporalValue() {
         return false;
     }
-    
+
     /**
      * Cast this FHIRPathSystemValue to a {@link FHIRPathBooleanValue}
-     * 
+     *
      * @return
      *     this FHIRPathSystemValue as a {@link FHIRPathBooleanValue}
      * @throws
@@ -81,10 +109,10 @@ default boolean isTemporalValue() {
     default FHIRPathBooleanValue asBooleanValue() {
         return as(FHIRPathBooleanValue.class);
     }
-    
+
     /**
      * Cast this FHIRPathSystemValue to a {@link FHIRPathStringValue}
-     * 
+     *
      * @return
      *     this FHIRPathSystemValue as a {@link FHIRPathStringValue}
      * @throws
@@ -93,10 +121,10 @@ default FHIRPathBooleanValue asBooleanValue() {
     default FHIRPathStringValue asStringValue() {
         return as(FHIRPathStringValue.class);
     }
-    
+
     /**
      * Cast this FHIRPathSystemValue to a {@link FHIRPathQuantityValue}
-     * 
+     *
      * @return
      *     this FHIRPathSystemValue as a {@link FHIRPathQuantityValue}
      * @throws
@@ -105,10 +133,10 @@ default FHIRPathStringValue asStringValue() {
     default FHIRPathQuantityValue asQuantityValue() {
         return as(FHIRPathQuantityValue.class);
     }
-    
+
    /**
     * Cast this FHIRPathSystemValue to a {@link FHIRPathNumberValue}
-    * 
+    *
     * @return
     *     this FHIRPathSystemValue as a {@link FHIRPathNumberValue}
     * @throws
@@ -117,10 +145,10 @@ default FHIRPathQuantityValue asQuantityValue() {
     default FHIRPathNumberValue asNumberValue() {
         return as(FHIRPathNumberValue.class);
     }
-    
+
     /**
      * Cast this FHIRPathSystemValue to a {@link FHIRPathTemporalValue}
-     * 
+     *
      * @return
      *     this FHIRPathSystemValue as a {@link FHIRPathTemporalValue}
      * @throws
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTimeValue.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTimeValue.java
index bd3f06005e8..8fcee053ac1 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTimeValue.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTimeValue.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -95,6 +95,8 @@ public static FHIRPathTimeValue timeValue(LocalTime time) {
     /**
      * Static factory method for creating named FHIRPathTimeValue instances from a {@link LocalTime} value
      *
+     * @param path
+     *     the path of the FHIRPathNode
      * @param name
      *     the name
      * @param time
@@ -102,8 +104,8 @@ public static FHIRPathTimeValue timeValue(LocalTime time) {
      * @return
      *     a new named FHIRPathTimeValue instance
      */
-    public static FHIRPathTimeValue timeValue(String name, LocalTime time) {
-        return FHIRPathTimeValue.builder(time, getPrecision(time)).name(name).build();
+    public static FHIRPathTimeValue timeValue(String path, String name, LocalTime time) {
+        return FHIRPathTimeValue.builder(time, getPrecision(time)).name(name).path(path).build();
     }
 
     @Override
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTree.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTree.java
index cfddd68cd24..3be7abf35ce 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTree.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTree.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -247,52 +247,52 @@ protected void doVisitStart(String elementName, int elementIndex, Resource resou
 
         @Override
         public void visit(java.lang.String elementName, BigDecimal value) {
-            builderStack.peek().value(FHIRPathDecimalValue.decimalValue(elementName, value));
+            builderStack.peek().value(FHIRPathDecimalValue.decimalValue(getPath(), elementName, value));
         }
 
         @Override
         public void visit(java.lang.String elementName, byte[] value) {
-            builderStack.peek().value(FHIRPathStringValue.stringValue(elementName, Base64.getEncoder().encodeToString(value)));
+            builderStack.peek().value(FHIRPathStringValue.stringValue(getPath(), elementName, Base64.getEncoder().encodeToString(value)));
         }
 
         @Override
         public void visit(java.lang.String elementName, java.lang.Boolean value) {
-            builderStack.peek().value(FHIRPathBooleanValue.booleanValue(elementName, value));
+            builderStack.peek().value(FHIRPathBooleanValue.booleanValue(getPath(), elementName, value));
         }
 
         @Override
         public void visit(java.lang.String elementName, java.lang.Integer value) {
-            builderStack.peek().value(FHIRPathIntegerValue.integerValue(elementName, value));
+            builderStack.peek().value(FHIRPathIntegerValue.integerValue(getPath(), elementName, value));
         }
 
         @Override
         public void doVisit(java.lang.String elementName, java.lang.String value) {
-            builderStack.peek().value(FHIRPathStringValue.stringValue(elementName, value));
+            builderStack.peek().value(FHIRPathStringValue.stringValue(getPath(), elementName, value));
         }
 
         @Override
         public void visit(java.lang.String elementName, LocalDate value) {
-            builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(elementName, value));
+            builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(getPath(), elementName, value));
         }
 
         @Override
         public void visit(java.lang.String elementName, LocalTime value) {
-            builderStack.peek().value(FHIRPathTimeValue.timeValue(elementName, value));
+            builderStack.peek().value(FHIRPathTimeValue.timeValue(getPath(), elementName, value));
         }
 
         @Override
         public void visit(java.lang.String elementName, Year value) {
-            builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(elementName, value));
+            builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(getPath(), elementName, value));
         }
 
         @Override
         public void visit(java.lang.String elementName, YearMonth value) {
-            builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(elementName, value));
+            builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(getPath(), elementName, value));
         }
 
         @Override
         public void visit(java.lang.String elementName, ZonedDateTime value) {
-            builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(elementName, value));
+            builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(getPath(), elementName, value));
         }
     }
 }
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTypeInfoNode.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTypeInfoNode.java
index edad6ce7448..c7109f3f966 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTypeInfoNode.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTypeInfoNode.java
@@ -1,6 +1,6 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2020
- * 
+ * (C) Copyright IBM Corp. 2019, 2022
+ *
  * SPDX-License-Identifier: Apache-2.0
  */
 
@@ -15,30 +15,30 @@
  */
 public class FHIRPathTypeInfoNode extends FHIRPathAbstractNode {
     private final TypeInfo typeInfo;
-    
+
     protected FHIRPathTypeInfoNode(Builder builder) {
         super(builder);
         this.typeInfo = Objects.requireNonNull(builder.typeInfo);
     }
-    
+
     @Override
     public boolean isTypeInfoNode() {
         return true;
     }
-    
+
     /**
      * The {@link TypeInfo} wrapped by this FHIRPathTypeInfoNode
-     * 
+     *
      * @return
      *     the {@link TypeInfo} wrapped by this FHIRPathTypeInfoNode
      */
     public TypeInfo typeInfo() {
         return typeInfo;
     }
-    
+
     /**
      * Static factory method for creating {@link TypeInfo} instances from a {@link TypeInfo}
-     * 
+     *
      * @param typeInfo
      *     the typeInfo
      * @return
@@ -47,7 +47,7 @@ public TypeInfo typeInfo() {
     public static FHIRPathTypeInfoNode typeInfoNode(TypeInfo typeInfo) {
         return new Builder(FHIRPathType.from(typeInfo.getClass()), typeInfo).build();
     }
-    
+
     /**
      * This toBuilder is not supported for this FHIRPathTypeInfoNode
      */
@@ -55,24 +55,24 @@ public static FHIRPathTypeInfoNode typeInfoNode(TypeInfo typeInfo) {
     public Builder toBuilder() {
         throw new UnsupportedOperationException();
     }
-    
+
     private static class Builder extends FHIRPathAbstractNode.Builder {
         private final TypeInfo typeInfo;
-        
+
         private Builder(FHIRPathType type, TypeInfo typeInfo) {
             super(type);
             this.typeInfo = typeInfo;
             if (typeInfo.getNamespace() != null) {
-                children.add(FHIRPathStringValue.stringValue("namespace", typeInfo.getNamespace()));
+                children.add(FHIRPathStringValue.stringValue(null, "namespace", typeInfo.getNamespace()));
             }
             if (typeInfo.getName() != null) {
-                children.add(FHIRPathStringValue.stringValue("name", typeInfo.getName()));
+                children.add(FHIRPathStringValue.stringValue(null, "name", typeInfo.getName()));
             }
         }
-        
+
         /**
          * Build a FHIRPathTypeInfoNode instance using this builder
-         * 
+         *
          * @return
          *     a new FHIRPathTypeInfoNode instance
          */
@@ -81,7 +81,7 @@ public FHIRPathTypeInfoNode build() {
             return new FHIRPathTypeInfoNode(this);
         }
     }
-    
+
     /**
      * The compareTo operation is not supported for this FHIRPathTypeInfoNode
      */
@@ -89,10 +89,10 @@ public FHIRPathTypeInfoNode build() {
     public int compareTo(FHIRPathNode o) {
         throw new UnsupportedOperationException();
     }
-    
+
     /**
      * Indicates whether the {@link TypeInfo} wrapped by this FHIRPathTypeInfoNode is equal the parameter
-     * 
+     *
      * @param obj
      *     the other {@link Object}
      * @return
@@ -112,17 +112,17 @@ public boolean equals(Object obj) {
         FHIRPathTypeInfoNode other = (FHIRPathTypeInfoNode) obj;
         return Objects.equals(typeInfo, other.typeInfo);
     }
-    
+
     @Override
     public int hashCode() {
         return Objects.hashCode(typeInfo);
     }
-    
+
     @Override
     public String toString() {
         return typeInfo.toString();
     }
-    
+
     @Override
     public void accept(FHIRPathNodeVisitor visitor) {
         visitor.visit(this);

From 438bae3294ee4498df44ed39680dc89c3905f2ea Mon Sep 17 00:00:00 2001
From: Lee Surprenant <lmsurpre@us.ibm.com>
Date: Thu, 5 May 2022 13:25:56 -0400
Subject: [PATCH 2/3] issue #3619 - set the path for all value nodes in the
 context tree

The previous commit set the path for Resource.id, Element.id, and
Extension.url primitive values. This commit extends that to include
*all* SystemValue nodes.

Additionally Resource.id, Element.id, and Extension.url value nodes are
now added to the context tree. Primitive value nodes named "value" are
still kept out of the tree and are still only accessible through their
parent's `getValue()` accessor.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
---
 .../fhir/model/visitor/PathAwareVisitor.java  | 200 ++++++++++++++++--
 .../visitor/ResourceFingerprintVisitor.java   |  20 +-
 .../util/test/ResourceComparatorVisitor.java  |  50 +++--
 .../java/com/ibm/fhir/path/FHIRPathTree.java  |  32 ++-
 4 files changed, 243 insertions(+), 59 deletions(-)

diff --git a/fhir-model/src/main/java/com/ibm/fhir/model/visitor/PathAwareVisitor.java b/fhir-model/src/main/java/com/ibm/fhir/model/visitor/PathAwareVisitor.java
index 40db09c4fd1..19c200c466a 100644
--- a/fhir-model/src/main/java/com/ibm/fhir/model/visitor/PathAwareVisitor.java
+++ b/fhir-model/src/main/java/com/ibm/fhir/model/visitor/PathAwareVisitor.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -9,6 +9,12 @@
 import static com.ibm.fhir.model.util.ModelSupport.delimit;
 import static com.ibm.fhir.model.util.ModelSupport.isKeyword;
 
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.Year;
+import java.time.YearMonth;
+import java.time.ZonedDateTime;
 import java.util.Stack;
 import java.util.logging.Level;
 import java.util.logging.Logger;
@@ -128,26 +134,194 @@ public final void visitStart(java.lang.String elementName, int elementIndex, Res
     }
 
     /**
-     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for non-visitable elements
-     *           of type {@code http://hl7.org/fhirpath/System.String}.
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code byte[]}.
+     *           Subclasses can override {@link #doVisit(String, byte[])} to provide specific visit behavior.
+     * @implNote Needed for FHIR.base64binary element values.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, byte[] value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code BigDecimal}.
+     *           Subclasses can override {@link #doVisit(String, BigDecimal)} to provide specific visit behavior.
+     * @implNote Needed for FHIR.decimal element values.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, BigDecimal value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code Boolean}.
+     *           Subclasses can override {@link #doVisit(String, Boolean)} to provide specific visit behavior.
+     * @implNote Needed for FHIR.decimal element values.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, java.lang.Boolean value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code Integer}.
+     *           Subclasses can override {@link #doVisit(String, Integer)} to provide specific visit behavior.
+     * @implNote Needed for FHIR.integer, FHIR.unsignedInt, and FHIR.positiveInt element values.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, java.lang.Integer value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code LocalDate}.
+     *           Subclasses can override {@link #doVisit(String, LocalDate)} to provide specific visit behavior.
+     * @implNote Needed for FHIR.date element values.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, LocalDate value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code LocalTime}.
+     *           Subclasses can override {@link #doVisit(String, LocalTime)} to provide specific visit behavior.
+     * @implNote Needed for FHIR.time element values.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, LocalTime value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values and non-visitable elements of type {@code String}.
      *           Subclasses can override {@link #doVisit(String, String)} to provide specific visit behavior.
-     * @implNote Needed for FHIR elements like Resource.id, Element.id, and Extension.url which are system strings
-     *           but also valid FHIRPath nodes.
+     * @implNote Needed for both
+     *           1. FHIR elements like Resource.id, Element.id, and Extension.url which are system strings; and
+     *           2. FHIR.string, FHIR.uri, FHIR.code, FHIR.oid, FHIR.id, FHIR.uuid, FHIR.sid, and FHIR.markdown element values.
      */
     @Override
-    public final void visit(String elementName, String value) {
-        if (!"value".equals(elementName)) {
-            pathStackPush(elementName, -1);
-        }
+    public final void visit(java.lang.String elementName, java.lang.String value) {
+        pathStackPush(elementName, -1);
         doVisit(elementName, value);
-        if (!"value".equals(elementName)) {
-            pathStackPop();
-        }
+        pathStackPop();
     }
 
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code Year}.
+     *           Subclasses can override {@link #doVisit(String, Year)} to provide specific visit behavior.
+     * @implNote Needed for FHIR.date and FHIR.dateTime element values with resolution to the year.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, Year value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code YearMonth}.
+     *           Subclasses can override {@link #doVisit(String, YearMonth)} to provide specific visit behavior.
+     * @implNote Needed for FHIR.date and FHIR.dateTime element values with resolution to the month.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, YearMonth value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec PathAwareVisitor makes this method final to ensure that we always set the path for primitive
+     *           values of type {@code ZonedDateTime}.
+     *           Subclasses can override {@link #doVisit(String, ZonedDateTime)} to provide specific visit behavior.
+     * @implNote Needed for FHIR.dateTime and FHIR.instant element values.
+     */
+    @Override
+    public final void visit(java.lang.String elementName, ZonedDateTime value) {
+        pathStackPush(elementName, -1);
+        doVisit(elementName, value);
+        pathStackPop();
+    }
+
+    /**
+     * @implSpec {@link #visit(String, byte[])} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for byte[] values.
+     */
+    protected void doVisit(java.lang.String elementName, byte[] value) { }
+
+    /**
+     * @implSpec {@link #visit(String, BigDecimal)} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for BigDecimal values.
+     */
+    protected void doVisit(java.lang.String elementName, BigDecimal value) { }
+
+    /**
+     * @implSpec {@link #visit(String, Boolean)} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for java.lang.Boolean values.
+     */
+    protected void doVisit(java.lang.String elementName, Boolean value) { }
+
+    /**
+     * @implSpec {@link #visit(String, Integer)} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for java.lang.Integer values.
+     */
+    protected void doVisit(java.lang.String elementName, Integer value) { }
+
+    /**
+     * @implSpec {@link #visit(String, LocalDate)} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for LocalDate values.
+     */
+    protected void doVisit(java.lang.String elementName, LocalDate value) { }
+
+    /**
+     * @implSpec {@link #visit(String, LocalTime)} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for LocalTime values.
+     */
+    protected void doVisit(java.lang.String elementName, LocalTime value) { }
+
     /**
      * @implSpec {@link #visit(String, String)} was made final (to avoid potential issues with the pathStack)
      * and so this method was introduced to allow subclasses to implement visit behavior for java.lang.String values.
      */
-    protected void doVisit(String elementName, String value) { }
+    protected void doVisit(java.lang.String elementName, String value) { }
+
+    /**
+     * @implSpec {@link #visit(String, Year)} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for Year values.
+     */
+    protected void doVisit(java.lang.String elementName, Year value) { }
+
+    /**
+     * @implSpec {@link #visit(String, YearMonth)} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for YearMonth values.
+     */
+    protected void doVisit(java.lang.String elementName, YearMonth value) { }
+
+    /**
+     * @implSpec {@link #visit(String, ZonedDateTime)} was made final (to avoid potential issues with the pathStack)
+     * and so this method was introduced to allow subclasses to implement visit behavior for ZonedDateTime values.
+     */
+    protected void doVisit(java.lang.String elementName, ZonedDateTime value) { }
 }
diff --git a/fhir-model/src/main/java/com/ibm/fhir/model/visitor/ResourceFingerprintVisitor.java b/fhir-model/src/main/java/com/ibm/fhir/model/visitor/ResourceFingerprintVisitor.java
index 5455c39d26b..80925797cd4 100644
--- a/fhir-model/src/main/java/com/ibm/fhir/model/visitor/ResourceFingerprintVisitor.java
+++ b/fhir-model/src/main/java/com/ibm/fhir/model/visitor/ResourceFingerprintVisitor.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019, 2021
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -116,35 +116,35 @@ public boolean visit(java.lang.String elementName, int index, com.ibm.fhir.model
     }
 
     @Override
-    public void visit(java.lang.String elementName, byte[] value) {
+    public void doVisit(java.lang.String elementName, byte[] value) {
         digest.update(getPath().getBytes(StandardCharsets.UTF_8));
         digest.update(value);
     }
 
     @Override
-    public void visit(java.lang.String elementName, BigDecimal value) {
+    public void doVisit(java.lang.String elementName, BigDecimal value) {
         updateDigest(getPath(), value.toString());
     }
 
     @Override
-    public void visit(java.lang.String elementName, java.lang.Boolean value) {
+    public void doVisit(java.lang.String elementName, java.lang.Boolean value) {
         updateDigest(getPath(), value.toString());
     }
 
     @Override
-    public void visit(java.lang.String elementName, java.lang.Integer value) {
+    public void doVisit(java.lang.String elementName, java.lang.Integer value) {
         ByteBuffer bb = ByteBuffer.allocate(4);
         bb.putInt(value);
         digest.update(bb);
     }
 
     @Override
-    public void visit(java.lang.String elementName, LocalDate value) {
+    public void doVisit(java.lang.String elementName, LocalDate value) {
         updateDigest(getPath(), value.toString());
     }
 
     @Override
-    public void visit(java.lang.String elementName, LocalTime value) {
+    public void doVisit(java.lang.String elementName, LocalTime value) {
         updateDigest(getPath(), value.toString());
     }
 
@@ -157,17 +157,17 @@ public void doVisit(java.lang.String elementName, java.lang.String value) {
     }
 
     @Override
-    public void visit(java.lang.String elementName, Year value) {
+    public void doVisit(java.lang.String elementName, Year value) {
         updateDigest(getPath(), value.toString());
     }
 
     @Override
-    public void visit(java.lang.String elementName, YearMonth value) {
+    public void doVisit(java.lang.String elementName, YearMonth value) {
         updateDigest(getPath(), value.toString());
     }
 
     @Override
-    public void visit(java.lang.String elementName, ZonedDateTime value) {
+    public void doVisit(java.lang.String elementName, ZonedDateTime value) {
         updateDigest(getPath(), value.toString());
     }
 
diff --git a/fhir-model/src/test/java/com/ibm/fhir/model/util/test/ResourceComparatorVisitor.java b/fhir-model/src/test/java/com/ibm/fhir/model/util/test/ResourceComparatorVisitor.java
index 85c0cac92a0..4068472aa8b 100644
--- a/fhir-model/src/test/java/com/ibm/fhir/model/util/test/ResourceComparatorVisitor.java
+++ b/fhir-model/src/test/java/com/ibm/fhir/model/util/test/ResourceComparatorVisitor.java
@@ -1,5 +1,5 @@
 /*
- * (C) Copyright IBM Corp. 2019
+ * (C) Copyright IBM Corp. 2019, 2022
  *
  * SPDX-License-Identifier: Apache-2.0
  */
@@ -23,19 +23,17 @@
 /**
  * Simple visitor to flatten the structure of a resource and compare
  * it with another instance
-
- *
  */
 public class ResourceComparatorVisitor extends PathAwareVisitor {
     public boolean compare = false;
 
     // First set of values we collect
     public Map<String, Comparer> values = new HashMap<>();
-    
+
     public static interface Comparer {
         public boolean compare(Comparer other);
     }
-    
+
     public static class ByteArrayComparer implements Comparer {
         final byte[] value;
         public ByteArrayComparer(byte[] value) {
@@ -77,7 +75,7 @@ public String toString() {
             return value == null ? "null" : value.toString();
         }
     }
-    
+
     public static class StringComparer implements Comparer {
         final String value;
         public StringComparer(String value) {
@@ -254,7 +252,7 @@ public static boolean compare(Map<String, Comparer> originals, Map<String, Compa
         boolean result = true;
 
         for (Map.Entry<String, Comparer> entry: originals.entrySet()) {
-            
+
             Comparer other = others.get(entry.getKey());
             if (other != null) {
                 if (!entry.getValue().compare(other)) {
@@ -278,62 +276,62 @@ public static boolean compare(Map<String, Comparer> originals, Map<String, Compa
                 result = false;
             }
         }
-        
-        
+
+
         return result;
     }
-    
+
     public Map<String, Comparer> getValues() {
         return this.values;
     }
 
     @Override
-    public void visit(java.lang.String elementName, byte[] value) {
+    public void doVisit(java.lang.String elementName, byte[] value) {
         this.values.put(getPath(), new ByteArrayComparer(value));
     }
-    
+
     @Override
-    public void visit(java.lang.String elementName, BigDecimal value) {
+    public void doVisit(java.lang.String elementName, BigDecimal value) {
         this.values.put(getPath(), new BigDecimalComparer(value));
     }
 
     @Override
-    public void visit(java.lang.String elementName, java.lang.Boolean value) {
+    public void doVisit(java.lang.String elementName, java.lang.Boolean value) {
         this.values.put(getPath(), new BooleanComparer(value));
     }
-    
+
     @Override
-    public void visit(java.lang.String elementName, java.lang.Integer value) {
+    public void doVisit(java.lang.String elementName, java.lang.Integer value) {
         this.values.put(getPath(), new IntComparer(value));
     }
 
     @Override
-    public void visit(java.lang.String elementName, LocalDate value) {
+    public void doVisit(java.lang.String elementName, LocalDate value) {
         this.values.put(getPath(), new LocalDateComparer(value));
     }
-    
+
     @Override
-    public void visit(java.lang.String elementName, LocalTime value) {
+    public void doVisit(java.lang.String elementName, LocalTime value) {
         this.values.put(getPath(), new LocalTimeComparer(value));
     }
-    
+
     @Override
     public void doVisit(java.lang.String elementName, java.lang.String value) {
         this.values.put(getPath(), new StringComparer(value));
     }
-    
+
     @Override
-    public void visit(java.lang.String elementName, Year value) {
+    public void doVisit(java.lang.String elementName, Year value) {
         this.values.put(getPath(), new YearComparer(value));
     }
-    
+
     @Override
-    public void visit(java.lang.String elementName, YearMonth value) {
+    public void doVisit(java.lang.String elementName, YearMonth value) {
         this.values.put(getPath(), new YearMonthComparer(value));
     }
-    
+
     @Override
-    public void visit(java.lang.String elementName, ZonedDateTime value) {
+    public void doVisit(java.lang.String elementName, ZonedDateTime value) {
         this.values.put(getPath(), new ZonedDateTimeComparer(value));
     }
 }
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTree.java b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTree.java
index 3be7abf35ce..ac56dd783b6 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTree.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/FHIRPathTree.java
@@ -246,52 +246,64 @@ protected void doVisitStart(String elementName, int elementIndex, Resource resou
         }
 
         @Override
-        public void visit(java.lang.String elementName, BigDecimal value) {
+        public void doVisit(java.lang.String elementName, BigDecimal value) {
             builderStack.peek().value(FHIRPathDecimalValue.decimalValue(getPath(), elementName, value));
         }
 
         @Override
-        public void visit(java.lang.String elementName, byte[] value) {
+        public void doVisit(java.lang.String elementName, byte[] value) {
             builderStack.peek().value(FHIRPathStringValue.stringValue(getPath(), elementName, Base64.getEncoder().encodeToString(value)));
         }
 
         @Override
-        public void visit(java.lang.String elementName, java.lang.Boolean value) {
+        public void doVisit(java.lang.String elementName, java.lang.Boolean value) {
             builderStack.peek().value(FHIRPathBooleanValue.booleanValue(getPath(), elementName, value));
         }
 
         @Override
-        public void visit(java.lang.String elementName, java.lang.Integer value) {
+        public void doVisit(java.lang.String elementName, java.lang.Integer value) {
             builderStack.peek().value(FHIRPathIntegerValue.integerValue(getPath(), elementName, value));
         }
 
         @Override
         public void doVisit(java.lang.String elementName, java.lang.String value) {
-            builderStack.peek().value(FHIRPathStringValue.stringValue(getPath(), elementName, value));
+            if ("value".equals(elementName)) {
+                builderStack.peek().value(FHIRPathStringValue.stringValue(getPath(), elementName, value));
+            } else {
+                String path = getPath();
+                FHIRPathNode node = FHIRPathStringValue.builder(value).name(elementName).path(path).build();
+                pathNodeMap.put(path, node);
+
+                if (builderStack.isEmpty()) {
+                    throw new IllegalStateException("builderStack was unexpectedly empty "
+                            + "while visiting string primitive with name: " + elementName);
+                }
+                builderStack.peek().children(node);
+            }
         }
 
         @Override
-        public void visit(java.lang.String elementName, LocalDate value) {
+        public void doVisit(java.lang.String elementName, LocalDate value) {
             builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(getPath(), elementName, value));
         }
 
         @Override
-        public void visit(java.lang.String elementName, LocalTime value) {
+        public void doVisit(java.lang.String elementName, LocalTime value) {
             builderStack.peek().value(FHIRPathTimeValue.timeValue(getPath(), elementName, value));
         }
 
         @Override
-        public void visit(java.lang.String elementName, Year value) {
+        public void doVisit(java.lang.String elementName, Year value) {
             builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(getPath(), elementName, value));
         }
 
         @Override
-        public void visit(java.lang.String elementName, YearMonth value) {
+        public void doVisit(java.lang.String elementName, YearMonth value) {
             builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(getPath(), elementName, value));
         }
 
         @Override
-        public void visit(java.lang.String elementName, ZonedDateTime value) {
+        public void doVisit(java.lang.String elementName, ZonedDateTime value) {
             builderStack.peek().value(FHIRPathDateTimeValue.dateTimeValue(getPath(), elementName, value));
         }
     }

From 7663d0f303b8855fb81b8614022ae2c3cdb9c2b6 Mon Sep 17 00:00:00 2001
From: Lee Surprenant <lmsurpre@us.ibm.com>
Date: Thu, 5 May 2022 13:44:24 -0400
Subject: [PATCH 3/3] issue #3619 - support removal of primitive values via
 FHIRPath

The FHIRPathPatch API is implemented via utilities in FHIRPathUtil.
Both the FHIRPatchPatch API and the unerlying utility methods were
designed to operate on FHIR nodes (FHIRPathResourceNode and
FHIRPathElementNode).

However, the delete API in particular is not strictly bound to paths
that select FHIR nodes. Previously, calling delete on a path that
selects one of these nodes would result in a NullPointerException.
With the previous commit, it would instead become a no-op.
With this commit, we will now attempt to remove the primitive value node
and rebuild the parent accordingly.

This allows users to remove fields like Resource.id and Element.id via
FHIRPath and it even allows you to remove the value of primitive FHIR
elements (e.g. Patient.active.value) in cases where those primitives
already have an extension (otherwise they need to have a value).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
---
 .../ibm/fhir/path/util/DeletingVisitor.java   | 100 ++++++++++++++++++
 ...ilTest.java => FHIRPathUtilPatchTest.java} |  75 ++++++++++++-
 2 files changed, 172 insertions(+), 3 deletions(-)
 rename fhir-path/src/test/java/com/ibm/fhir/path/patch/test/{FHIRPathUtilTest.java => FHIRPathUtilPatchTest.java} (65%)

diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/util/DeletingVisitor.java b/fhir-path/src/main/java/com/ibm/fhir/path/util/DeletingVisitor.java
index 425711efd9b..d4ab3b18ade 100644
--- a/fhir-path/src/main/java/com/ibm/fhir/path/util/DeletingVisitor.java
+++ b/fhir-path/src/main/java/com/ibm/fhir/path/util/DeletingVisitor.java
@@ -5,13 +5,24 @@
  */
 package com.ibm.fhir.path.util;
 
+import java.lang.reflect.Method;
+import java.math.BigDecimal;
+import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.Year;
+import java.time.YearMonth;
+import java.time.ZonedDateTime;
 import java.util.Objects;
+import java.util.Set;
 
+import com.ibm.fhir.model.builder.Builder;
 import com.ibm.fhir.model.visitor.CopyingVisitor;
 import com.ibm.fhir.model.visitor.Visitable;
 
 class DeletingVisitor<T extends Visitable> extends CopyingVisitor<T> {
     private String pathToDelete;
+    private static final String VALUE = "value";
+    private static final Set<String> ALLOWED_STRING_ID = Set.of("id", "url", "value");
 
     public DeletingVisitor(String pathToDelete) {
         this.pathToDelete = Objects.requireNonNull(pathToDelete, "pathToDelete");
@@ -26,4 +37,93 @@ public boolean visit(String elementName, int index, Visitable value) {
         }
         return true;
     }
+
+    @Override
+    public void visit(java.lang.String elementName, byte[] value) {
+        removeValueIfTargeted(elementName, byte[].class);
+    }
+
+    @Override
+    public void visit(java.lang.String elementName, BigDecimal value) {
+        removeValueIfTargeted(elementName, BigDecimal.class);
+    }
+
+    @Override
+    public void visit(java.lang.String elementName, java.lang.Boolean value) {
+        removeValueIfTargeted(elementName, Boolean.class);
+    }
+
+    @Override
+    public void visit(java.lang.String elementName, java.lang.Integer value) {
+        removeValueIfTargeted(elementName, Integer.class);
+    }
+
+    @Override
+    public void visit(java.lang.String elementName, LocalDate value) {
+        removeValueIfTargeted(elementName, LocalDate.class);
+    }
+
+    @Override
+    public void visit(java.lang.String elementName, LocalTime value) {
+        removeValueIfTargeted(elementName, LocalTime.class);
+    }
+
+    /**
+     * @implNote String has special logic because Resource.id, Element.id, and Extension.url are of type System.String
+     */
+    @Override
+    public void visit(String elementName, String value) {
+        if (isTargeted(elementName)) {
+            if (!ALLOWED_STRING_ID.contains(elementName)) {
+                throw new IllegalArgumentException("Primitive value with name '" + elementName + "' cannot be modified");
+            }
+
+            removeValue(elementName, String.class);
+        }
+    }
+
+    @Override
+    public void visit(java.lang.String elementName, Year value) {
+        removeValueIfTargeted(elementName, Year.class);
+    }
+
+    @Override
+    public void visit(java.lang.String elementName, YearMonth value) {
+        removeValueIfTargeted(elementName, YearMonth.class);
+    }
+
+    @Override
+    public void visit(java.lang.String elementName, ZonedDateTime value) {
+        removeValueIfTargeted(elementName, ZonedDateTime.class);
+    }
+
+    private void removeValueIfTargeted(String elementName, Class<?> valueType) {
+        if (isTargeted(elementName)) {
+            validateElementNameIsValue(elementName);
+            removeValue(elementName, valueType);
+        }
+    }
+
+    private boolean isTargeted(String elementName) {
+        String pathToElement = getPath() + ".";
+        return pathToDelete.startsWith(pathToElement) && pathToDelete.substring(pathToElement.length()).equals(elementName);
+    }
+
+    private void validateElementNameIsValue(java.lang.String elementName) {
+        if (!VALUE.equals(elementName)) {
+            throw new IllegalArgumentException("Primitive node with name '" + elementName + "' cannot be modified");
+        }
+    }
+
+    private void removeValue(String elementName, Class<?> valueType) {
+        Builder<?> builder = getBuilder();
+        try {
+            // reflection can be slow
+            Method method = builder.getClass().getDeclaredMethod(elementName, valueType);
+            method.invoke(builder, (Object)null);
+        } catch (Exception e) {
+            throw new RuntimeException("Error while setting '" + elementName + "' on builder of type " + builder.getClass().getCanonicalName(), e);
+        }
+        markDirty();
+    }
 }
\ No newline at end of file
diff --git a/fhir-path/src/test/java/com/ibm/fhir/path/patch/test/FHIRPathUtilTest.java b/fhir-path/src/test/java/com/ibm/fhir/path/patch/test/FHIRPathUtilPatchTest.java
similarity index 65%
rename from fhir-path/src/test/java/com/ibm/fhir/path/patch/test/FHIRPathUtilTest.java
rename to fhir-path/src/test/java/com/ibm/fhir/path/patch/test/FHIRPathUtilPatchTest.java
index a8e24bd9fe9..700a338b078 100644
--- a/fhir-path/src/test/java/com/ibm/fhir/path/patch/test/FHIRPathUtilTest.java
+++ b/fhir-path/src/test/java/com/ibm/fhir/path/patch/test/FHIRPathUtilPatchTest.java
@@ -6,6 +6,8 @@
 package com.ibm.fhir.path.patch.test;
 
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertNotNull;
+import static org.testng.Assert.assertNull;
 
 import java.util.ArrayList;
 import java.util.List;
@@ -24,16 +26,17 @@
 import com.ibm.fhir.model.resource.Practitioner;
 import com.ibm.fhir.model.type.Uri;
 import com.ibm.fhir.model.type.code.BundleType;
+import com.ibm.fhir.model.util.FHIRUtil;
 import com.ibm.fhir.path.exception.FHIRPathException;
 import com.ibm.fhir.path.util.FHIRPathUtil;
 
 /**
  * Tests against the FHIRPathPatch helper methods in FHIRPathUtil
  */
-public class FHIRPathUtilTest {
+public class FHIRPathUtilPatchTest {
     Bundle bundle = Bundle.builder().type(BundleType.COLLECTION).build();
-    Patient patient = Patient.builder().id("test").build();
-    Practitioner practitioner = Practitioner.builder().id("test").build();
+    Patient patient = Patient.builder().id("patientId").build();
+    Practitioner practitioner = Practitioner.builder().id("practitionerId").build();
 
     @Test
     private void testAddListResource() throws FHIRPathException, FHIRPatchException {
@@ -54,6 +57,72 @@ private void testAddSingleResource() throws FHIRPathException, FHIRPatchExceptio
         assertEquals(modifiedBundle.getEntry().get(1).getResource(), practitioner);
     }
 
+    @Test
+    private void testRemoveResourceId() throws FHIRPathException, FHIRPatchException {
+        assertNotNull(patient.getId());
+        Patient modifiedPatient = FHIRPathUtil.delete(patient, "Patient.id");
+        assertNull(modifiedPatient.getId());
+    }
+
+    @Test
+    private void testRemoveElementId() throws FHIRPathException, FHIRPatchException {
+        Patient modifiedPatient = FHIRPathUtil.add(patient, "Patient", "active", com.ibm.fhir.model.type.Boolean.builder()
+                .id("elementId")
+                .value(true)
+                .build());
+        assertNotNull(modifiedPatient.getActive().getId());
+        modifiedPatient = FHIRPathUtil.delete(modifiedPatient, "Patient.active.id");
+        assertNull(modifiedPatient.getActive().getId());
+    }
+
+    /**
+     * Add a Patient.extension element and then try removing its url. This should fail because
+     * Extension.url is a required field.
+     */
+    @Test(expectedExceptions = FHIRPatchException.class)
+    private void testRemoveExtensionUrl() throws FHIRPathException, FHIRPatchException {
+        Patient modifiedPatient = FHIRPathUtil.add(patient, "Patient", "extension", FHIRUtil.DATA_ABSENT_REASON_UNKNOWN);
+        modifiedPatient = FHIRPathUtil.delete(modifiedPatient, "Patient.extension[0].url");
+    }
+
+    /**
+     * Add a Patient.active element and then try removing its value. This should fail because
+     * an element must have either a value or and extension and the removal results in neither.
+     */
+    @Test(expectedExceptions = FHIRPatchException.class)
+    private void testRemoveValue_invalid() throws FHIRPathException, FHIRPatchException {
+        Patient modifiedPatient = FHIRPathUtil.add(patient, "Patient", "active", com.ibm.fhir.model.type.Boolean.TRUE);
+        modifiedPatient = FHIRPathUtil.delete(modifiedPatient, "Patient.active.value");
+    }
+
+    /**
+     * Add a Patient.active element with an extension and then try removing its value. This should work
+     * because a primitive element can have a null value as long as it has an extension.
+     */
+    @Test
+    private void testRemoveValue_valid() throws FHIRPathException, FHIRPatchException {
+        Patient modifiedPatient = FHIRPathUtil.add(patient, "Patient", "active", com.ibm.fhir.model.type.Boolean.builder()
+                .value(true)
+                .extension(FHIRUtil.DATA_ABSENT_REASON_UNKNOWN)
+                .build());
+        modifiedPatient = FHIRPathUtil.delete(modifiedPatient, "Patient.active.value");
+        assertNull(modifiedPatient.getActive().getValue());
+    }
+
+    /**
+     * Add a Patient.active element with an extension and then try removing its value. This should work
+     * because a primitive element can have a null value as long as it has an extension.
+     */
+    @Test
+    private void testRemoveValue_allTypes() throws FHIRPathException, FHIRPatchException {
+        Patient modifiedPatient = FHIRPathUtil.add(patient, "Patient", "active", com.ibm.fhir.model.type.Boolean.builder()
+                .value(true)
+                .extension(FHIRUtil.DATA_ABSENT_REASON_UNKNOWN)
+                .build());
+        modifiedPatient = FHIRPathUtil.delete(modifiedPatient, "Patient.active.value");
+        assertNull(modifiedPatient.getActive().getValue());
+    }
+
     @Test
     private void testConcurrentPatches() throws FHIRPathException, FHIRPatchException, InterruptedException, ExecutionException {
         List<Callable<Bundle>> concurrentUpdates = new ArrayList<>();
