issue #2510 - make FHIRPathUtil threadsafe

FHIRPathUtil was using a shared static FHIRPathEvaluator but this is not thread-safe. Now we construct a new evaluator for each request. Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
LinuxForHealth · Jun 15, 2021 · ef95dac · ef95dac
1 parent e1270f8
commit ef95dac
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 7 deletions.
diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/evaluator/FHIRPathEvaluator.java b/fhir-path/src/main/java/com/ibm/fhir/path/evaluator/FHIRPathEvaluator.java
@@ -83,9 +83,12 @@
 import com.ibm.fhir.path.function.FHIRPathFunction;
 import com.ibm.fhir.path.util.FHIRPathUtil;
 
+import net.jcip.annotations.NotThreadSafe;
+
 /**
  * A FHIRPath evaluation engine that implements the FHIRPath 2.0.0 <a href="http://hl7.org/fhirpath/N1/">specification</a>
  */
+@NotThreadSafe
 public class FHIRPathEvaluator {
     private static final Logger log = Logger.getLogger(FHIRPathEvaluator.class.getName());
 

diff --git a/fhir-path/src/main/java/com/ibm/fhir/path/util/FHIRPathUtil.java b/fhir-path/src/main/java/com/ibm/fhir/path/util/FHIRPathUtil.java
@@ -88,8 +88,6 @@
 import com.ibm.fhir.path.exception.FHIRPathException;
 
 public final class FHIRPathUtil {
-    private static FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
-
     public static final Set<String> STRING_TRUE_VALUES = new HashSet<>(Arrays.asList("true", "t", "yes", "y", "1", "1.0"));
     public static final Set<String> STRING_FALSE_VALUES = new HashSet<>(Arrays.asList("false", "f", "no", "n", "0", "0.0"));
     public static final Integer INTEGER_TRUE = 1;
@@ -892,8 +890,10 @@ public static String unescape(String s) {
      * @throws FHIRPatchException
      * @throws NullPointerException if any of the passed arguments are null
      */
-    public static <T extends Visitable> T add(T elementOrResource, String fhirPath, String elementName, Visitable value) throws FHIRPathException, FHIRPatchException {
-        FHIRPathNode node = evaluateToSingle(elementOrResource, fhirPath);
+    public static <T extends Visitable> T add(T elementOrResource, String fhirPath, String elementName, Visitable value)
+            throws FHIRPathException, FHIRPatchException {
+        FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
+        FHIRPathNode node = evaluateToSingle(evaluator, elementOrResource, fhirPath);
         Visitable parent = node.isResourceNode() ?
                 node.asResourceNode().resource() : node.asElementNode().element();
 
@@ -913,7 +913,8 @@ public static <T extends Visitable> T add(T elementOrResource, String fhirPath,
      * @throws NullPointerException if any of the passed arguments are null
      */
     public static <T extends Visitable> T delete(T elementOrResource, String fhirPath) throws FHIRPathException, FHIRPatchException {
-        FHIRPathNode node = evaluateToSingle(elementOrResource, fhirPath);
+        FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
+        FHIRPathNode node = evaluateToSingle(evaluator, elementOrResource, fhirPath);
 
         try {
             DeletingVisitor<T> deletingVisitor = new DeletingVisitor<T>(node.path());
@@ -932,7 +933,8 @@ public static <T extends Visitable> T delete(T elementOrResource, String fhirPat
      * @throws NullPointerException if any of the passed arguments are null
      */
     public static <T extends Visitable> T replace(T elementOrResource, String fhirPath, Visitable value) throws FHIRPathException, FHIRPatchException {
-        FHIRPathNode node = evaluateToSingle(elementOrResource, fhirPath);
+        FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
+        FHIRPathNode node = evaluateToSingle(evaluator, elementOrResource, fhirPath);
         String elementName = node.name();
 
         FHIRPathTree tree = evaluator.getEvaluationContext().getTree();
@@ -955,13 +957,15 @@ public static <T extends Visitable> T replace(T elementOrResource, String fhirPa
     }
 
     /**
+     * @param evaluator
      * @param elementOrResource
      * @param fhirPath
      * @return
      * @throws FHIRPathException
      * @throws FHIRPatchException if the fhirPath does not evaluate to a single node
      */
-    private static FHIRPathNode evaluateToSingle(Visitable elementOrResource, String fhirPath) throws FHIRPathException, FHIRPatchException {
+    private static FHIRPathNode evaluateToSingle(FHIRPathEvaluator evaluator, Visitable elementOrResource, String fhirPath)
+            throws FHIRPathException, FHIRPatchException {
         /*
          * 1. The FHIRPath statement must return a single element.
          * 2. The FHIRPath statement SHALL NOT cross resources using the resolve() function
@@ -991,6 +995,7 @@ private static FHIRPathNode evaluateToSingle(Visitable elementOrResource, String
      */
     public static <T extends Visitable> T insert(T elementOrResource, String fhirPath, int index, Visitable value)
             throws FHIRPathException, FHIRPatchException {
+        FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
         Collection<FHIRPathNode> nodes = evaluator.evaluate(elementOrResource, fhirPath);
         if (index > nodes.size()) {
             throw new FHIRPatchException("Index must be equal or less than the number of elements in the list", fhirPath);
@@ -1021,6 +1026,7 @@ public static <T extends Visitable> T insert(T elementOrResource, String fhirPat
      */
     public static <T extends Visitable> T move(T elementOrResource, String fhirPath, int source, int target)
             throws FHIRPathException, FHIRPatchException {
+        FHIRPathEvaluator evaluator = FHIRPathEvaluator.evaluator();
         Collection<FHIRPathNode> nodes = evaluator.evaluate(elementOrResource, fhirPath);
         if (source > nodes.size() || target > nodes.size()) {
             throw new FHIRPatchException("Source and target indices must be less than or equal to"

diff --git a/fhir-validation/src/main/java/com/ibm/fhir/validation/FHIRValidator.java b/fhir-validation/src/main/java/com/ibm/fhir/validation/FHIRValidator.java
@@ -46,6 +46,13 @@
 import com.ibm.fhir.registry.FHIRRegistry;
 import com.ibm.fhir.validation.exception.FHIRValidationException;
 
+import net.jcip.annotations.NotThreadSafe;
+
+/**
+ * A validator that uses conformance resources from the {@link com.ibm.fhir.registry.FHIRRegistry}
+ * to validate resource instances against the base specification and, optionally, extended profiles.
+ */
+@NotThreadSafe
 public class FHIRValidator {
     private static final Logger log = Logger.getLogger(FHIRValidator.class.getName());