issue #2111 - default useStoredCompartmentParam to true

and add it to our default fhir-server-config.json Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
LinuxForHealth · Mar 18, 2021 · daf2ca2 · daf2ca2
1 parent c2ea285
commit daf2ca2
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 42 deletions.
diff --git a/fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java b/fhir-search/src/main/java/com/ibm/fhir/search/util/SearchUtil.java
@@ -1432,7 +1432,7 @@ public static FHIRSearchContext parseCompartmentQueryParameters(String compartme
      * @return
      */
     public static boolean useStoredCompartmentParam() {
-        return FHIRConfigHelper.getBooleanProperty(FHIRConfiguration.PROPERTY_USE_STORED_COMPARTMENT_PARAM, false);
+        return FHIRConfigHelper.getBooleanProperty(FHIRConfiguration.PROPERTY_USE_STORED_COMPARTMENT_PARAM, true);
     }
 
     /**
@@ -1456,12 +1456,11 @@ public static FHIRSearchContext parseCompartmentQueryParameters(String compartme
                 // issue #1708. When enabled, use the ibm-internal-... compartment parameter. This
                 // results in faster queries because only a single parameter is used to represent the
                 // compartment membership.
+                CompartmentUtil.checkValidCompartmentAndResource(compartmentName, resourceType.getSimpleName());
                 inclusionCriteria = Collections.singletonList(CompartmentUtil.makeCompartmentParamName(compartmentName));
             } else {
-                // pre #1708 behavior, which is the default
-                inclusionCriteria =
-                        CompartmentUtil.getCompartmentResourceTypeInclusionCriteria(compartmentName,
-                                resourceType.getSimpleName());
+                // pre #1708 behavior
+                inclusionCriteria = CompartmentUtil.getCompartmentResourceTypeInclusionCriteria(compartmentName, resourceType.getSimpleName());
             }
 
             for (String criteria : inclusionCriteria) {

diff --git a/fhir-search/src/test/java/com/ibm/fhir/search/test/CompartmentParseQueryParmsTest.java b/fhir-search/src/test/java/com/ibm/fhir/search/test/CompartmentParseQueryParmsTest.java
@@ -20,8 +20,10 @@
 import java.util.List;
 import java.util.Map;
 
+import org.testng.annotations.DataProvider;
 import org.testng.annotations.Test;
 
+import com.ibm.fhir.config.FHIRRequestContext;
 import com.ibm.fhir.model.resource.CommunicationRequest;
 import com.ibm.fhir.model.resource.Condition;
 import com.ibm.fhir.model.resource.Device;
@@ -38,6 +40,7 @@
  * class.
  */
 public class CompartmentParseQueryParmsTest extends BaseSearchTest {
+    private static final String IBM_INTERNAL_PATIENT_COMPARTMENT_PARAM_NAME = "ibm-internal-Patient-Compartment";
 
     /**
      * This method tests parsing compartment related query parms, passing an invalid compartment.
@@ -58,11 +61,12 @@ public void testInvalidResource() throws Exception {
     }
 
     /**
-     * This method tests parsing compartment related query parms. Based on the compartment and resource type, a single
-     * inclusion criterion is expectedExceptions to be returned by SearchUtil.parseQueryParameters().
+     * This method tests parsing compartment related query parms.
+     * Based on the compartment and resource type, two inclusion criteria are expected.
      */
-    @Test
-    public void testSingleInclusionCriteria() throws Exception {
+    @Test(dataProvider = "config")
+    public void testTwoInclusionCriteria(boolean useStoredCompartmentParam) throws Exception {
+        FHIRRequestContext.get().setTenantId(useStoredCompartmentParam ? "default" : "tenant2");
         Map<String, List<String>> queryParameters = new HashMap<>();
         String compartmentName = "Patient";
         String compartmentLogicalId = "11";
@@ -73,34 +77,37 @@ public void testSingleInclusionCriteria() throws Exception {
         assertNotNull(context.getSearchParameters());
         assertEquals(1, context.getSearchParameters().size());
         QueryParameter parm1 = context.getSearchParameters().get(0);
-        assertEquals("patient", parm1.getCode());
-
-        /*
-         * The compartment > Resource is { "code" : "Condition", "param" : ["patient", "asserter"] },
-         */
-        QueryParameter p = parm1;
-        String out = "";
-        while (p != null) {
-            out += (" -> " + p.getCode());
-            p = p.getNextParameter();
+        assertEquals(useStoredCompartmentParam ? IBM_INTERNAL_PATIENT_COMPARTMENT_PARAM_NAME : "patient", parm1.getCode());
+
+        if (!useStoredCompartmentParam) {
+            /*
+             * The compartment > Resource is { "code" : "Condition", "param" : ["patient", "asserter"] },
+             */
+            QueryParameter p = parm1;
+            String out = "";
+            while (p != null) {
+                out += (" -> " + p.getCode());
+                p = p.getNextParameter();
+            }
+            if (DEBUG) {
+                System.out.println(out);
+            }
+            assertNotNull(parm1.getNextParameter());
+            assertEquals(" -> patient -> asserter", out);
         }
-        if (DEBUG) {
-            System.out.println(out);
-        }
-        assertNotNull(parm1.getNextParameter());
-        assertEquals(" -> patient -> asserter", out);
 
         assertEquals(Type.REFERENCE, parm1.getType());
         assertEquals(1, parm1.getValues().size());
         assertEquals(compartmentName + "/" + compartmentLogicalId, parm1.getValues().get(0).getValueString());
     }
 
     /**
-     * This method tests parsing compartment related query parms. Based on the compartment and resource type, multiple
-     * inclusion criteria is expectedExceptions to be returned by SearchUtil.parseQueryParameters().
+     * This method tests parsing compartment related query parms.
+     * Based on the compartment and resource type, three inclusion criteria are expected.
      */
-    @Test
-    public void testMultiInclusionCriteria() throws Exception {
+    @Test(dataProvider = "config")
+    public void testThreeInclusionCriteria(boolean useStoredCompartmentParam) throws Exception {
+        FHIRRequestContext.get().setTenantId(useStoredCompartmentParam ? "default" : "tenant2");
         Map<String, List<String>> queryParameters = new HashMap<>();
         String compartmentName = "RelatedPerson";
         String compartmentLogicalId = "22";
@@ -115,7 +122,8 @@ public void testMultiInclusionCriteria() throws Exception {
         int parmCount = 0;
         while (searchParm != null) {
             parmCount++;
-            assertTrue((searchParm.getCode().equals("recipient") || searchParm.getCode().equals("requester") || searchParm.getCode().equals("sender")));
+            assertTrue(useStoredCompartmentParam ? searchParm.getCode().equals(IBM_INTERNAL_PATIENT_COMPARTMENT_PARAM_NAME)
+                    : (searchParm.getCode().equals("recipient") || searchParm.getCode().equals("requester") || searchParm.getCode().equals("sender")));
             assertEquals(Type.REFERENCE, searchParm.getType());
             assertTrue(searchParm.isInclusionCriteria());
             assertFalse(searchParm.isChained());
@@ -131,8 +139,9 @@ public void testMultiInclusionCriteria() throws Exception {
      * Based on the compartment and resource type, multiple inclusion criteria is expectedExceptions to be returned by
      * SearchUtil.parseQueryParameters().
      */
-    @Test
-    public void testCompartmentWithQueryParms() throws Exception {
+    @Test(dataProvider = "config")
+    public void testCompartmentWithQueryParms(boolean useStoredCompartmentParam) throws Exception {
+        FHIRRequestContext.get().setTenantId(useStoredCompartmentParam ? "default" : "tenant2");
         Map<String, List<String>> queryParameters = new HashMap<>();
         String compartmentName = "Patient";
         String compartmentLogicalId = "33";
@@ -147,29 +156,30 @@ public void testCompartmentWithQueryParms() throws Exception {
 
         assertNotNull(context);
         assertNotNull(context.getSearchParameters());
-        assertEquals(3, context.getSearchParameters().size());
+        assertEquals(context.getSearchParameters().size(), 3);
 
         // Validate compartment related search parms.
         QueryParameter searchParm = context.getSearchParameters().get(0);
         int parmCount = 0;
         while (searchParm != null) {
             parmCount++;
-            assertTrue((searchParm.getCode().equals("performer") || searchParm.getCode().equals("subject")));
+            assertTrue(useStoredCompartmentParam ? searchParm.getCode().equals(IBM_INTERNAL_PATIENT_COMPARTMENT_PARAM_NAME)
+                    : (searchParm.getCode().equals("performer") || searchParm.getCode().equals("subject")));
             assertEquals(Type.REFERENCE, searchParm.getType());
             assertTrue(searchParm.isInclusionCriteria());
             assertFalse(searchParm.isChained());
             assertEquals(1, searchParm.getValues().size());
             assertEquals(compartmentName + "/" + compartmentLogicalId, searchParm.getValues().get(0).getValueString());
             searchParm = searchParm.getNextParameter();
         }
-        assertEquals(2, parmCount);
+        assertEquals(parmCount, useStoredCompartmentParam ? 1 : 2);
 
         // Validate non-compartment related search parms.
         for (int i = 1; i < 3; i++) {
             searchParm = context.getSearchParameters().get(i);
             assertTrue((searchParm.getCode().equals("category") || searchParm.getCode().equals("value-quantity")));
             assertNotNull(searchParm.getValues());
-            assertEquals(1, searchParm.getValues().size());
+            assertEquals(searchParm.getValues().size(), 1);
         }
 
         String selfUri = SearchUtil.buildSearchSelfUri("http://example.com/" + compartmentName + "/" + compartmentLogicalId + "/"
@@ -182,8 +192,9 @@ public void testCompartmentWithQueryParms() throws Exception {
      * This method tests parsing compartment related query parms which are not valid. In lenient mode, this is
      * expectedExceptions to ignore the query parameter. In strict mode (lenient=false) this should throw an exception.
      */
-    @Test
-    public void testCompartmentWithFakeQueryParm() throws Exception {
+    @Test(dataProvider = "config")
+    public void testCompartmentWithFakeQueryParm(boolean useStoredCompartmentParam) throws Exception {
+        FHIRRequestContext.get().setTenantId(useStoredCompartmentParam ? "default" : "tenant2");
         Map<String, List<String>> queryParameters = new HashMap<>();
         String compartmentName = "Patient";
         String compartmentLogicalId = "33";
@@ -202,15 +213,16 @@ public void testCompartmentWithFakeQueryParm() throws Exception {
         int parmCount = 0;
         while (searchParm != null) {
             parmCount++;
-            assertTrue((searchParm.getCode().equals("performer") || searchParm.getCode().equals("subject")));
+            assertTrue(useStoredCompartmentParam ? searchParm.getCode().equals(IBM_INTERNAL_PATIENT_COMPARTMENT_PARAM_NAME)
+                    : (searchParm.getCode().equals("performer") || searchParm.getCode().equals("subject")));
             assertEquals(Type.REFERENCE, searchParm.getType());
             assertTrue(searchParm.isInclusionCriteria());
             assertFalse(searchParm.isChained());
             assertEquals(1, searchParm.getValues().size());
             assertEquals(compartmentName + "/" + compartmentLogicalId, searchParm.getValues().get(0).getValueString());
             searchParm = searchParm.getNextParameter();
         }
-        assertEquals(2, parmCount);
+        assertEquals(useStoredCompartmentParam ? 1 : 2, parmCount);
 
         String selfUri = SearchUtil.buildSearchSelfUri("http://example.com/" + compartmentName + "/" + compartmentLogicalId + "/"
                 + resourceType.getSimpleName(), context);
@@ -229,8 +241,9 @@ public void testCompartmentWithFakeQueryParm() throws Exception {
      * SearchUtil.parseQueryParameters() should ignore the null compartment related parms and successfully process the
      * non-compartment parms.
      */
-    @Test
-    public void testNoComparmentWithQueryParms() throws Exception {
+    @Test(dataProvider = "config")
+    public void testNoComparmentWithQueryParms(boolean useStoredCompartmentParam) throws Exception {
+        FHIRRequestContext.get().setTenantId(useStoredCompartmentParam ? "default" : "tenant2");
         Map<String, List<String>> queryParameters = new HashMap<>();
         Class<? extends Resource> resourceType = Observation.class;
 
@@ -267,4 +280,8 @@ private String encodeQueryString(String queryString) {
         }
     }
 
+    @DataProvider(name = "config")
+    public static Object[][] configSwitcher() {
+       return new Object[][] {{true}, {false}};
+    }
 }
diff --git a/fhir-search/src/test/resources/config/tenant2/fhir-server-config.json b/fhir-search/src/test/resources/config/tenant2/fhir-server-config.json
@@ -1,5 +1,8 @@
 {
     "__comment": "FHIR Server configuration",
     "fhirServer": {
+        "search": {
+            "useStoredCompartmentParam": false
+        }
     }
 }
diff --git a/fhir-server/liberty-config/config/default/fhir-server-config.json b/fhir-server/liberty-config/config/default/fhir-server-config.json
@@ -11,7 +11,7 @@
             "disabledOperations": ""
         },
         "search": {
-            "useStoredCompartmentParam": false
+            "useStoredCompartmentParam": true
         },
         "security": {
             "cors": true,