Implicit base not picked up in FHIR Reference search #300

Signed-off-by: Paul Bastide <pbastide@us.ibm.com>

docs/src/pages/Conformance.md

@@ -2,7 +2,7 @@
 layout: post
 title:  Conformance
 description: Notes on the Conformance of the IBM FHIR Server
-date:   2020-05-15 09:59:05 -0400
+date:   2020-09-24 09:59:05 -0400
 permalink: /conformance/
 ---
 
@@ -142,7 +142,7 @@ FHIR search modifiers are described at https://www.hl7.org/fhir/R4/search.html#m
 |FHIR Search Parameter Type|Supported Modifiers|"Default" search behavior when no Modifier or Prefix is present|
 |--------------------------|-------------------|---------------------------------------------------------------|
 |String                    |`:exact`,`:contains`,`:missing` |"starts with" search that is case-insensitive and accent-insensitive|
-|Reference                 |`:[type]`,`:missing`            |exact match search|
+|Reference                 |`:[type]`,`:missing`            |exact match search and targets are implicitly added|
 |URI                       |`:below`,`:above`,`:missing`    |exact match search|
 |Token                     |`:missing`                      |exact match search|
 |Number                    |`:missing`                      |implicit range search (see http://hl7.org/fhir/R4/search.html#number)|

fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/JDBCQueryBuilder.java

@@ -64,6 +64,7 @@
 import com.ibm.fhir.persistence.jdbc.util.type.NumberParmBehaviorUtil;
 import com.ibm.fhir.persistence.jdbc.util.type.QuantityParmBehaviorUtil;
 import com.ibm.fhir.persistence.util.AbstractQueryBuilder;
+import com.ibm.fhir.search.SearchConstants;
 import com.ibm.fhir.search.SearchConstants.Modifier;
 import com.ibm.fhir.search.SearchConstants.Type;
 import com.ibm.fhir.search.context.FHIRSearchContext;
@@ -528,9 +529,16 @@ private SqlQueryData processReferenceParm(Class<?> resourceType, QueryParameter
             // Handle query parm representing this name/value pair construct:
             // <code>{name}:{Resource Type} = {resource-id}</code>
             if (queryParm.getModifier() != null && queryParm.getModifier().equals(Modifier.TYPE)) {
-                searchValue =
-                        queryParm.getModifierResourceTypeName() + "/"
-                                + SqlParameterEncoder.encode(value.getValueString());
+                if (!SearchConstants.Type.REFERENCE.equals(queryParm.getType())) {
+                    // Not a Reference
+                    searchValue =
+                            queryParm.getModifierResourceTypeName() + "/"
+                                    + SqlParameterEncoder.encode(value.getValueString());
+                } else {
+                    // This is a Reference type.
+                    // As of versions greater than 4.4.0, we defer to the Search Layer to append the value.
+                    searchValue = SqlParameterEncoder.encode(value.getValueString());
+                }
             } else if (!isAbsoluteURL(searchValue)) {
                 SearchParameter definition = SearchUtil.getSearchParameter(resourceType, queryParm.getCode());
                 if (definition != null) {

fhir-persistence-jdbc/src/test/java/com/ibm/fhir/persistence/jdbc/search/test/JDBCSearchReferenceTest.java

@@ -19,7 +19,6 @@
 import com.ibm.fhir.persistence.jdbc.test.util.DerbyInitializer;
 import com.ibm.fhir.persistence.search.test.AbstractSearchReferenceTest;
 
-
 public class JDBCSearchReferenceTest extends AbstractSearchReferenceTest {
 
     private Properties testProps;
@@ -58,7 +57,6 @@ protected void shutdownPools() throws Exception {
         }
     }
 
-
     /*
      * Currently, documented in our conformance statement. We do not support
      * modifiers on chained parameters.
@@ -70,9 +68,10 @@ protected void shutdownPools() throws Exception {
     public void testSearchReference_Reference_chained_missing() throws Exception {
         super.testSearchReference_Reference_chained_missing();
     }
+
     @Override
     @Test(expectedExceptions = FHIRPersistenceNotSupportedException.class)
     public void testSearchReference_uri_chained_missing() throws Exception {
         super.testSearchReference_uri_chained_missing();
     }
-}
+}

fhir-persistence/src/test/java/com/ibm/fhir/persistence/search/test/AbstractSearchReferenceTest.java

@@ -13,9 +13,11 @@
 import java.util.List;
 import java.util.Map;
 
+import org.testng.annotations.BeforeClass;
 import org.testng.annotations.Test;
 
 import com.ibm.fhir.config.FHIRRequestContext;
+import com.ibm.fhir.exception.FHIRException;
 import com.ibm.fhir.model.resource.Basic;
 import com.ibm.fhir.model.test.TestUtil;
 
@@ -34,22 +36,27 @@ protected void setTenant() throws Exception {
         FHIRRequestContext.get().setTenantId("reference");
     }
 
+    @BeforeClass
+    public void createReference() throws FHIRException {
+        String originalRequestUri = "https://example.com/Patient/123";
+        FHIRRequestContext context = FHIRRequestContext.get();
+        context.setOriginalRequestUri(originalRequestUri);
+    }
+
     @Test
     public void testSearchReference_Reference_relative() throws Exception {
         // Reference by id really only works when the system knows which resource type(s)
         // can be referenced from a given element.
-        // TODO does this work if you define the extension in a StructureDefinition
-        // and declare the allowed types?
-//        assertSearchReturnsSavedResource("Reference-relative", "123");
-
         assertSearchReturnsSavedResource("Reference-relative", "Patient/123");
+        assertSearchReturnsSavedResource("Reference-relative", "123");
 
-        // TODO if this matched the hostname that the test was running on, would it work?
-//        assertSearchReturnsSavedResource("Reference-relative", "https://example.com/Patient/123");
+        assertSearchReturnsSavedResource("Reference-relative", "https://example.com/Patient/123");
 
         assertSearchReturnsSavedResource("Reference-relative:Patient", "123");
-        assertSearchDoesntReturnSavedResource("Reference-relative:Basic", "123");
 
+        // Originally the following test was set to not return a value, as we are now using 123 as a valid value, it
+        // does return the expected resource
+        assertSearchReturnsSavedResource("Reference-relative:Basic", "123");
     }
 
     @Test
@@ -60,13 +67,13 @@ public void testSearchReference_Reference_relative_chained() throws Exception {
          * See https://ibm.github.io/FHIR/Conformance#search-modifiers and
          * refer to https://github.com/IBM/FHIR/issues/473 to track the issue.
          */
-//        assertSearchReturnsComposition("subject:Basic.Reference-relative:Patient", "123");
-//        assertSearchReturnsComposition("subject:Basic.Reference-relative:Basic", "123");
+        // assertSearchReturnsComposition("subject:Basic.Reference-relative:Patient", "123");
+        // assertSearchReturnsComposition("subject:Basic.Reference-relative:Basic", "123");
     }
 
     @Test
     public void testSearchReference_Reference_relative_revinclude() throws Exception {
-        Map<String, List<String>> queryParms = new HashMap<String, List<String>>(1);
+        Map<String, List<String>> queryParms = new HashMap<>(1);
         queryParms.put("_revinclude", Collections.singletonList("Composition:subject"));
         queryParms.put("Reference-relative", Collections.singletonList("Patient/123"));
         assertTrue(searchReturnsResource(Basic.class, queryParms, savedResource));
@@ -75,13 +82,12 @@ public void testSearchReference_Reference_relative_revinclude() throws Exception
 
     @Test
     public void testSearchReference_Reference_absolute() throws Exception {
-        // TODO if the resource contained an absolute URI which matches the hostname
-        // where the current test was running, would these work?
-//        assertSearchReturnsSavedResource("Reference-absolute", "123");
-//        assertSearchReturnsSavedResource("Reference-absolute", "Patient/123");
-//        assertSearchReturnsSavedResource("Reference-absolute:Patient", "123");
-
         assertSearchReturnsSavedResource("Reference-absolute", "https://example.com/Patient/123");
+        assertSearchReturnsSavedResource("Reference-absolute", "Patient/123");
+        assertSearchReturnsSavedResource("Reference-absolute", "123");
+
+        //
+        assertSearchReturnsSavedResource("Reference-absolute:Patient", "123");
     }
 
     @Test

fhir-persistence/src/test/java/com/ibm/fhir/persistence/test/common/AbstractPersistenceTest.java

@@ -68,7 +68,7 @@ protected void shutdownDatabase() throws Exception {}
     protected void shutdownPools() throws Exception {}
 
     // A hook for subclasses to debug locks in the case of a lock timeout in the current transaction
-    protected void debugLocks() {};
+    protected void debugLocks() {}
 
     // The following persistence context-related methods can be overridden in subclasses to
     // provide a more specific instance of the FHIRPersistenceContext if necessary.
@@ -123,15 +123,15 @@ public void tearDown() throws Exception {
     }
 
     protected List<Resource> runQueryTest(Class<? extends Resource> resourceType, String parmName, String parmValue) throws Exception {
-        Map<String, List<String>> queryParms = new HashMap<String, List<String>>(1);
+        Map<String, List<String>> queryParms = new HashMap<>(1);
         if (parmName != null && parmValue != null) {
             queryParms.put(parmName, Collections.singletonList(parmValue));
         }
         return runQueryTest(resourceType, queryParms);
     }
 
     protected List<Resource> runQueryTest(Class<? extends Resource> resourceType, String parmName, String parmValue, Integer maxPageSize) throws Exception {
-        Map<String, List<String>> queryParms = new HashMap<String, List<String>>(1);
+        Map<String, List<String>> queryParms = new HashMap<>(1);
         if (parmName != null && parmValue != null) {
             queryParms.put(parmName, Collections.singletonList(parmValue));
         }
@@ -186,7 +186,7 @@ protected List<Resource> runQueryTest(String compartmentName, String compartment
     }
 
     protected List<Resource> runQueryTest(String compartmentName, String compartmentLogicalId, Class<? extends Resource> resourceType, String parmName, String parmValue, Integer maxPageSize) throws Exception {
-        Map<String, List<String>> queryParms = new HashMap<String, List<String>>(1);
+        Map<String, List<String>> queryParms = new HashMap<>(1);
         if (parmName != null && parmValue != null) {
             queryParms.put(parmName, Collections.singletonList(parmValue));
         }

fhir-persistence/src/test/resources/config/reference/extension-search-parameters.json

@@ -219,7 +219,8 @@
             "type": "reference",
             "expression": "Basic.extension.where(url='http://example.org/Reference').value",
             "xpath": "f:Basic/f:extension[@url='http://example.org/Reference']/f:valueReference",
-            "xpathUsage": "normal"
+            "xpathUsage": "normal",
+            "target": ["Patient"]
         }
     },
     {
@@ -305,7 +306,8 @@
             "type": "reference",
             "expression": "Basic.extension.where(url='http://example.org/Reference-id').value",
             "xpath": "f:Basic/f:extension[@url='http://example.org/Reference-id']/f:valueReference",
-            "xpathUsage": "normal"
+            "xpathUsage": "normal",
+            "target": ["Patient"]
         }
     },
     {
@@ -322,7 +324,8 @@
             "type": "reference",
             "expression": "Basic.extension.where(url='http://example.org/Reference-relative').value",
             "xpath": "f:Basic/f:extension[@url='http://example.org/Reference-relative']/f:valueReference",
-            "xpathUsage": "normal"
+            "xpathUsage": "normal",
+            "target": ["Patient"]
         }
     },
     {
@@ -339,7 +342,8 @@
             "type": "reference",
             "expression": "Basic.extension.where(url='http://example.org/Reference-absolute').value",
             "xpath": "f:Basic/f:extension[@url='http://example.org/Reference-absolute']/f:valueReference",
-            "xpathUsage": "normal"
+            "xpathUsage": "normal",
+            "target": ["Patient"]
         }
     },
     {
@@ -356,7 +360,8 @@
             "type": "reference",
             "expression": "Basic.extension.where(url='http://example.org/Reference-display').value",
             "xpath": "f:Basic/f:extension[@url='http://example.org/Reference-display']/f:valueReference",
-            "xpathUsage": "normal"
+            "xpathUsage": "normal",
+            "target": ["Patient"]
         }
     },
 
@@ -374,7 +379,8 @@
             "type": "reference",
             "expression": "Basic.extension.where(url='http://example.org/missing-Reference').value",
             "xpath": "f:Basic/f:extension[@url='http://example.org/missing-Reference']/f:valueReference",
-            "xpathUsage": "normal"
+            "xpathUsage": "normal",
+            "target": ["Patient"]
         }
     },
     {

fhir-persistence/src/test/resources/logging.unitTest.properties

@@ -27,7 +27,7 @@ handlers= java.util.logging.ConsoleHandler
 # can be overriden by a facility specific level
 # Note that the ConsoleHandler also has a separate level
 # setting to limit messages printed to the console.
-.level= INFO
+.level= FINE
 
 ############################################################
 # Handler specific properties.
@@ -49,5 +49,5 @@ java.util.logging.ConsoleHandler.formatter = java.util.logging.SimpleFormatter
 ############################################################
 
 # Set this to FINE or higher to output the SQL statements and related debug info
-com.ibm.fhir.persistence.level = INFO
-com.ibm.fhir.search.level = INFO
+com.ibm.fhir.persistence.level = FINE
+com.ibm.fhir.search.level = FINE

fhir-search/src/main/java/com/ibm/fhir/search/parameters/QueryParameterValue.java

@@ -20,6 +20,8 @@
  */
 public class QueryParameterValue {
 
+    private boolean hidden = false;
+
     private Prefix prefix = null;
 
     private String valueString = null;
@@ -36,7 +38,7 @@ public class QueryParameterValue {
 
     // Used for composite search parameters
     private List<QueryParameter> component = new ArrayList<>();
-    
+
     // The delimiter starts off as EMPTY and goes to SearchConstants.PARAMETER_DELIMETER
     private String delim = "";
 
@@ -127,6 +129,21 @@ public void addComponent(QueryParameter... component) {
     public void setComponent(Collection<QueryParameter> component) {
         this.component = new ArrayList<>(component);
     }
+
+    /**
+     * @return the hidden
+     */
+    public boolean isHidden() {
+        return hidden;
+    }
+
+    /**
+     * @param b
+     */
+    public void setHidden(boolean hidden) {
+        this.hidden = hidden;
+    }
+
     /**
      * Serialize the ParameterValue to a query parameter string
      */
@@ -168,7 +185,7 @@ public String toString() {
 
     /**
      * simple build method to apply consistent usage of StringBuilder.
-     * 
+     *
      * @param outputBuilder
      * @param value
      */
@@ -179,4 +196,4 @@ private void outputBuilder(StringBuilder outputBuilder, Object value) {
             delim = SearchConstants.PARAMETER_DELIMITER;
         }
     }
-}
+}