Issue #2155 - Update after review comments

Signed-off-by: Troy Biesterfeld <tbieste@us.ibm.com>
LinuxForHealth · Jun 30, 2021 · 3840354 · 3840354
1 parent 5fc0efe
commit 3840354
Show file tree

Hide file tree

Showing 3 changed files with 52 additions and 3 deletions.
diff --git a/...ence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/ExtractedSearchParameters.java b/...ence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/ExtractedSearchParameters.java
@@ -7,7 +7,6 @@
 package com.ibm.fhir.persistence.jdbc.util;
 
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
@@ -18,8 +17,8 @@
  */
 public class ExtractedSearchParameters {
 
-    private List<ExtractedParameterValue> parameters = new ArrayList<>();
-    private String parameterHashB64 = null;
+    private final List<ExtractedParameterValue> parameters;
+    private final String parameterHashB64;
 
     public ExtractedSearchParameters(List<ExtractedParameterValue> parameters, String parameterHashB64) {
         this.parameters = Collections.unmodifiableList(parameters);

diff --git a/...rsistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/ParameterHashVisitor.java b/...rsistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/ParameterHashVisitor.java
@@ -35,6 +35,8 @@
  */
 public class ParameterHashVisitor implements ExtractedParameterValueVisitor {
     private static final String SHA_256 = "SHA-256";
+    // Since the digest is not updated when values are null, use a delimiter to ensure that
+    // different combinations subfields being null do not end up generating the same hash
     private static final byte[] DELIMITER = "|".getBytes(StandardCharsets.UTF_8);
 
     // The digest being accumulated as the parameters are visited

diff --git a/...istence-jdbc/src/test/java/com/ibm/fhir/persistence/jdbc/test/util/ParameterHashTest.java b/...istence-jdbc/src/test/java/com/ibm/fhir/persistence/jdbc/test/util/ParameterHashTest.java
@@ -68,6 +68,54 @@ public void testNoExtractedParameters() throws Exception {
         assertEquals(hash1, hash2);
     }
 
+    @Test
+    public void testNullValues() throws Exception {
+
+        // Create two number param values, one with null low, and one with null high
+        BigDecimal value = new BigDecimal(5);
+
+        NumberParmVal num1 = new NumberParmVal();
+        num1.setResourceType("Patient");
+        num1.setName("code5");
+        num1.setUrl("url5");
+        num1.setVersion("version5");
+        num1.setValueNumber(value);
+        num1.setValueNumberLow(NumberParmBehaviorUtil.generateLowerBound(value));
+
+        NumberParmVal num2 = new NumberParmVal();
+        num2.setResourceType("Patient");
+        num2.setName("code5");
+        num2.setUrl("url5");
+        num2.setVersion("version5");
+        num2.setValueNumber(value);
+        num2.setValueNumberHigh(NumberParmBehaviorUtil.generateUpperBound(value));
+
+        List<ExtractedParameterValue> parameters1 = Arrays.asList(num1);
+        List<ExtractedParameterValue> parameters2 = Arrays.asList(num2);
+
+        // Sort parameters to ensure hash is deterministic
+        sortExtractedParameterValues(parameters1);
+        sortExtractedParameterValues(parameters2);
+
+        // Visit parameters
+        ParameterHashVisitor phv1 = new ParameterHashVisitor();
+        for (ExtractedParameterValue p: parameters1) {
+            p.accept(phv1);
+        }
+        String hash1 = phv1.getBase64Hash();
+        assertNotNull(hash1);
+
+        ParameterHashVisitor phv2 = new ParameterHashVisitor();
+        for (ExtractedParameterValue p: parameters2) {
+            p.accept(phv2);
+        }
+        String hash2 = phv2.getBase64Hash();
+        assertNotNull(hash2);
+
+        // Check hashes
+        assertNotEquals(hash1, hash2);
+    }
+
     @Test
     public void testExtractedParametersDifferentOrders() throws Exception {
         Instant instant = Instant.now();