issue #528

- Refactor DateParmBehaviorUtil and DateParmBehaviorUtilTest to reflect the RANGE intersection - Tone down the logging Signed-off-by: Paul Bastide <pbastide@us.ibm.com>
LinuxForHealth · Dec 21, 2019 · c05880a · c05880a
1 parent d3539f9
commit c05880a
Show file tree

Hide file tree

Showing 7 changed files with 153 additions and 173 deletions.
diff --git a/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dto/DateParmVal.java b/fhir-persistence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/dto/DateParmVal.java
@@ -97,4 +97,10 @@ public String getBase() {
     public void setBase(String base) {
         this.base = base;
     }
-}
+
+    @Override
+    public String toString() {
+        return "DateParmVal [resourceType=" + resourceType + ", name=" + name + ", valueDate=" + valueDate
+                + ", valueDateStart=" + valueDateStart + ", valueDateEnd=" + valueDateEnd + ", base=" + base + "]";
+    }
+}
diff --git a/...e-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/JDBCParameterBuildingVisitor.java b/...e-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/JDBCParameterBuildingVisitor.java
@@ -651,6 +651,8 @@ private void setDateValues(DateParmVal p, Date date) {
             p.setValueDateStart(DateTimeHandler.generateTimestamp(inst));
             inst = DateTimeHandler.generateUpperBound(date.getValue());
             p.setValueDateEnd(DateTimeHandler.generateTimestamp(inst));
+
+            System.out.println(p);
         }
     }
 

diff --git a/...ence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/type/DateParmBehaviorUtil.java b/...ence-jdbc/src/main/java/com/ibm/fhir/persistence/jdbc/util/type/DateParmBehaviorUtil.java
@@ -10,8 +10,8 @@
 import static com.ibm.fhir.persistence.jdbc.JDBCConstants.BIND_VAR;
 import static com.ibm.fhir.persistence.jdbc.JDBCConstants.DATE_END;
 import static com.ibm.fhir.persistence.jdbc.JDBCConstants.DATE_START;
-import static com.ibm.fhir.persistence.jdbc.JDBCConstants.DATE_VALUE;
 import static com.ibm.fhir.persistence.jdbc.JDBCConstants.DOT;
+import static com.ibm.fhir.persistence.jdbc.JDBCConstants.EQ;
 import static com.ibm.fhir.persistence.jdbc.JDBCConstants.GT;
 import static com.ibm.fhir.persistence.jdbc.JDBCConstants.GTE;
 import static com.ibm.fhir.persistence.jdbc.JDBCConstants.LEFT_PAREN;
@@ -44,8 +44,7 @@ public DateParmBehaviorUtil() {
 
     public void executeBehavior(StringBuilder whereClauseSegment, QueryParameter queryParm,
             List<Timestamp> bindVariables,
-            String tableAlias)
-            throws Exception {
+            String tableAlias) {
         // Start the Clause 
         // Query: AND ((
         whereClauseSegment.append(AND).append(LEFT_PAREN).append(LEFT_PAREN);
@@ -69,10 +68,9 @@ public void executeBehavior(StringBuilder whereClauseSegment, QueryParameter que
                 prefix = Prefix.EQ;
             }
 
-            Instant v = value.getValueDate();
             Instant lowerBound = value.getValueDateLowerBound();
             Instant upperBound = value.getValueDateUpperBound();
-            buildPredicates(whereClauseSegment, bindVariables, tableAlias, prefix, v, lowerBound, upperBound);
+            buildPredicates(whereClauseSegment, bindVariables, tableAlias, prefix, lowerBound, upperBound);
         }
 
         // End the Clause started above, and closes the parameter expression. 
@@ -87,52 +85,45 @@ public void executeBehavior(StringBuilder whereClauseSegment, QueryParameter que
      * @param bindVariables
      * @param tableAlias
      * @param prefix
-     * @param value
      * @param lowerBound
      * @param upperBound
      */
     public void buildPredicates(StringBuilder whereClauseSegment, List<Timestamp> bindVariables, String tableAlias,
-            Prefix prefix, Instant value, Instant lowerBound, Instant upperBound) {
+            Prefix prefix, Instant lowerBound, Instant upperBound) {
         switch (prefix) {
         case EB:
             // EB - Ends Before
             // the value for the parameter in the resource is equal to the provided value
             // the range of the search value fully contains the range of the target value
-            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_VALUE, DATE_END,
-                    LT, lowerBound, lowerBound);
+            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_END, LT, lowerBound);
             break;
         case SA:
             // SA - Starts After
             // the range of the search value does not overlap with the range of the target value,
             // and the range below the search value contains the range of the target value
-            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_VALUE, DATE_START,
-                    GT, upperBound, upperBound);
+            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_START, GT, upperBound);
             break;
         case GE:
             // GE - Greater Than Equal
             // the range above the search value intersects (i.e. overlaps) with the range of the target value,
             // or the range of the search value fully contains the range of the target value
-            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_VALUE, DATE_END,
-                    GTE, lowerBound, lowerBound);
+            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_END, GTE, lowerBound);
             break;
         case GT:
             // GT - Greater Than
             // the range above the search value intersects (i.e. overlaps) with the range of the target value
-            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_VALUE, DATE_END,
-                    GT, lowerBound, lowerBound);
+            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_END, GT, lowerBound);
             break;
         case LE:
             // LE - Less Than Equal
             // the range below the search value intersects (i.e. overlaps) with the range of the target value
             // or the range of the search value fully contains the range of the target value
-            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_VALUE, DATE_START,
-                    LTE, upperBound, upperBound);
+            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_START, LTE, upperBound);
             break;
         case LT:
             // LT - Less Than
             // the range below the search value intersects (i.e. overlaps) with the range of the target value
-            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_VALUE, DATE_START,
-                    LT, upperBound, upperBound);
+            buildCommonClause(whereClauseSegment, bindVariables, tableAlias, DATE_START, LT, upperBound);
             break;
         case AP:
             // AP - Approximate - Relative
@@ -160,17 +151,13 @@ public void buildPredicates(StringBuilder whereClauseSegment, List<Timestamp> bi
      * @param whereClauseSegment
      * @param bindVariables
      * @param tableAlias
-     * @param columnName
      * @param columnNameLowOrHigh
      * @param operator
-     * @param value TODO: remove this
      * @param bound
      */
     public void buildCommonClause(StringBuilder whereClauseSegment, List<Timestamp> bindVariables, String tableAlias,
-            String columnName, String columnNameLowOrHigh, String operator, Instant value, Instant bound) {
-        whereClauseSegment
-                    .append(tableAlias).append(DOT).append(columnNameLowOrHigh).append(operator).append(BIND_VAR);
-
+            String columnNameLowOrHigh, String operator, Instant bound) {
+        whereClauseSegment.append(tableAlias).append(DOT).append(columnNameLowOrHigh).append(operator).append(BIND_VAR);
         bindVariables.add(generateTimestamp(bound));
     }
 
@@ -185,26 +172,25 @@ public void buildCommonClause(StringBuilder whereClauseSegment, List<Timestamp>
      */
     public void buildEqualsRangeClause(StringBuilder whereClauseSegment, List<Timestamp> bindVariables,
             String tableAlias, Instant lowerBound, Instant upperBound) {
-        //  buildEqualsRangeClause(whereClauseSegment, bindVariables, tableAlias, value, upperBound, value);
-        // @formatter:off
-        whereClauseSegment
-                .append(LEFT_PAREN).append(LEFT_PAREN)
-                        .append(tableAlias).append(DOT).append(DATE_VALUE).append(GTE).append(BIND_VAR)
-                        .append(AND)
-                        .append(tableAlias).append(DOT).append(DATE_VALUE).append(LTE).append(BIND_VAR)
-                    .append(RIGHT_PAREN)
-                    .append(OR)
+        bindVariables.add(generateTimestamp(lowerBound));
+
+        if (!lowerBound.equals(upperBound)) {
+            // @formatter:off
+            whereClauseSegment
                     .append(LEFT_PAREN)
                         .append(tableAlias).append(DOT).append(DATE_START).append(GTE).append(BIND_VAR)
-                        .append(AND)
+                    .append(AND)
                         .append(tableAlias).append(DOT).append(DATE_END).append(LTE).append(BIND_VAR)
-                .append(RIGHT_PAREN).append(RIGHT_PAREN);
-        // @formatter:on
-
-        bindVariables.add(generateTimestamp(lowerBound));
-        bindVariables.add(generateTimestamp(upperBound));
-        bindVariables.add(generateTimestamp(lowerBound));
-        bindVariables.add(generateTimestamp(upperBound));
+                    .append(RIGHT_PAREN);
+            // @formatter:on
+            bindVariables.add(generateTimestamp(upperBound));
+        } else {
+            // Exact match of an instant. 
+            whereClauseSegment
+                    .append(LEFT_PAREN)
+                    .append(tableAlias).append(DOT).append(DATE_START).append(EQ).append(BIND_VAR)
+                    .append(RIGHT_PAREN);
+        }
     }
 
     /**
@@ -221,24 +207,14 @@ public void buildApproxRangeClause(StringBuilder whereClauseSegment, List<Timest
         // @formatter:off
         whereClauseSegment
                 .append(LEFT_PAREN)
-                    .append(LEFT_PAREN)
-                        .append(tableAlias).append(DOT).append(DATE_VALUE).append(GTE).append(BIND_VAR)
-                    .append(AND)
-                        .append(tableAlias).append(DOT).append(DATE_VALUE).append(LTE).append(BIND_VAR)
-                    .append(RIGHT_PAREN)
-                    .append(OR)
-                    .append(LEFT_PAREN)
-                        .append(tableAlias).append(DOT).append(DATE_START).append(GTE).append(BIND_VAR)
-                    .append(AND)
-                        .append(tableAlias).append(DOT).append(DATE_END).append(LTE).append(BIND_VAR)
-                    .append(RIGHT_PAREN)
+                     .append(tableAlias).append(DOT).append(DATE_START).append(GTE).append(BIND_VAR)
+                .append(AND)
+                     .append(tableAlias).append(DOT).append(DATE_END).append(LTE).append(BIND_VAR)
                 .append(RIGHT_PAREN);
         // @formatter:on
 
         bindVariables.add(generateTimestamp(lowerBound));
         bindVariables.add(generateTimestamp(upperBound));
-        bindVariables.add(generateTimestamp(lowerBound));
-        bindVariables.add(generateTimestamp(upperBound));
     }
 
     /**
@@ -255,23 +231,13 @@ public void buildNotEqualsRangeClause(StringBuilder whereClauseSegment, List<Tim
         // @formatter:off
         whereClauseSegment
                 .append(LEFT_PAREN)
-                    .append(LEFT_PAREN)
-                        .append(tableAlias).append(DOT).append(DATE_VALUE).append(LT).append(BIND_VAR)
-                        .append(OR)
-                        .append(tableAlias).append(DOT).append(DATE_VALUE).append(GT).append(BIND_VAR)
-                    .append(RIGHT_PAREN)
-                    .append(OR)
-                    .append(LEFT_PAREN)
                         .append(tableAlias).append(DOT).append(DATE_START).append(LT).append(BIND_VAR)
                         .append(OR)
                         .append(tableAlias).append(DOT).append(DATE_END).append(GT).append(BIND_VAR)
-                    .append(RIGHT_PAREN)
                 .append(RIGHT_PAREN);
         // @formatter:on
 
         bindVariables.add(generateTimestamp(lowerBound));
         bindVariables.add(generateTimestamp(upperBound));
-        bindVariables.add(generateTimestamp(lowerBound));
-        bindVariables.add(generateTimestamp(upperBound));
     }
 }
diff --git a/...-jdbc/src/test/java/com/ibm/fhir/persistence/jdbc/test/util/DateParmBehaviorUtilTest.java b/...-jdbc/src/test/java/com/ibm/fhir/persistence/jdbc/test/util/DateParmBehaviorUtilTest.java
@@ -32,7 +32,7 @@
 
 public class DateParmBehaviorUtilTest {
     private static final Logger log = java.util.logging.Logger.getLogger(DateParmBehaviorUtilTest.class.getName());
-    private static final Level LOG_LEVEL = Level.INFO;
+    private static final Level LOG_LEVEL = Level.FINE;
 
     //---------------------------------------------------------------------------------------------------------
     // Supporting Methods: 
@@ -122,21 +122,19 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
         // gt - Greater Than
         QueryParameter queryParm = generateQueryParameter(SearchConstants.Prefix.GT, null, vTime);
         List<Timestamp> expectedBindVariables = new ArrayList<>();
-        expectedBindVariables.add(upper);
-        expectedBindVariables.add(upper);
+        expectedBindVariables.add(value);
         String expectedSql =
-                " AND (((Date.DATE_VALUE > ? OR Date.DATE_START > ?))))";
+                " AND ((Date.DATE_END > ?)))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
 
         // lt - Less Than
         queryParm             = generateQueryParameter(SearchConstants.Prefix.LT, null, vTime);
         expectedBindVariables = new ArrayList<>();
-        expectedBindVariables.add(value);
-        expectedBindVariables.add(value);
+        expectedBindVariables.add(upper);
         expectedSql =
-                " AND (((Date.DATE_VALUE < ? OR Date.DATE_END < ?))))";
+                " AND ((Date.DATE_START < ?)))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
@@ -145,9 +143,8 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
         queryParm             = generateQueryParameter(SearchConstants.Prefix.GE, null, vTime);
         expectedBindVariables = new ArrayList<>();
         expectedBindVariables.add(value);
-        expectedBindVariables.add(value);
         expectedSql =
-                " AND (((Date.DATE_VALUE >= ? OR Date.DATE_START >= ?))))";
+                " AND ((Date.DATE_END >= ?)))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
@@ -156,9 +153,8 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
         queryParm             = generateQueryParameter(SearchConstants.Prefix.LE, null, vTime);
         expectedBindVariables = new ArrayList<>();
         expectedBindVariables.add(upper);
-        expectedBindVariables.add(upper);
         expectedSql =
-                " AND (((Date.DATE_VALUE <= ? OR Date.DATE_END <= ?))))";
+                " AND ((Date.DATE_START <= ?)))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
@@ -167,9 +163,8 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
         queryParm             = generateQueryParameter(SearchConstants.Prefix.SA, null, vTime);
         expectedBindVariables = new ArrayList<>();
         expectedBindVariables.add(upper);
-        expectedBindVariables.add(upper);
         expectedSql =
-                " AND (((Date.DATE_VALUE > ? OR Date.DATE_START > ?))))";
+                " AND ((Date.DATE_START > ?)))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
@@ -178,9 +173,8 @@ public void testHandleDateRangeComparisonWithExact() throws Exception {
         queryParm             = generateQueryParameter(SearchConstants.Prefix.EB, null, vTime);
         expectedBindVariables = new ArrayList<>();
         expectedBindVariables.add(value);
-        expectedBindVariables.add(value);
         expectedSql =
-                " AND (((Date.DATE_VALUE < ? OR Date.DATE_END < ?))))";
+                " AND ((Date.DATE_END < ?)))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
@@ -200,7 +194,7 @@ public void testPrecisionWithEqual() throws Exception {
         expectedBindVariables.add(value);
         expectedBindVariables.add(value);
         String expectedSql =
-                " AND ((((Date.DATE_VALUE >= ? AND Date.DATE_VALUE <= ?) OR (Date.DATE_START >= ? AND Date.DATE_END <= ?)))))";
+                " AND (((Date.DATE_START >= ? AND Date.DATE_END <= ?))))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
@@ -225,7 +219,7 @@ public void testPrecisionWithMultipleValuesForEqual() throws Exception {
         expectedBindVariables.add(value);
 
         String expectedSql =
-                " AND ((((Date.DATE_VALUE >= ? AND Date.DATE_VALUE <= ?) OR (Date.DATE_START >= ? AND Date.DATE_END <= ?))) OR (((Date.DATE_VALUE >= ? AND Date.DATE_VALUE <= ?) OR (Date.DATE_START >= ? AND Date.DATE_END <= ?)))))";
+                " AND (((Date.DATE_START >= ? AND Date.DATE_END <= ?)) OR ((Date.DATE_START >= ? AND Date.DATE_END <= ?))))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
@@ -248,7 +242,7 @@ public void testPrecisionWithNotEqual() throws Exception {
         expectedBindVariables.add(value);
 
         String expectedSql =
-                " AND ((((Date.DATE_VALUE < ? OR Date.DATE_VALUE > ?) OR (Date.DATE_START < ? OR Date.DATE_END > ?)))))";
+                " AND (((Date.DATE_START < ? OR Date.DATE_END > ?))))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql);
@@ -262,7 +256,7 @@ public void testPrecisionWithApprox() throws Exception {
         List<Timestamp> expectedBindVariables = new ArrayList<>();
 
         String expectedSql =
-                " AND ((((Date.DATE_VALUE >= ? AND Date.DATE_VALUE <= ?) OR (Date.DATE_START >= ? AND Date.DATE_END <= ?)))))";
+                " AND (((Date.DATE_START >= ? AND Date.DATE_END <= ?))))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql, "Date", true);
@@ -277,7 +271,7 @@ public void testPrecisionWithMultipleDifferentApprox() throws Exception {
         List<Timestamp> expectedBindVariables = new ArrayList<>();
 
         String expectedSql =
-                " AND ((((Date.DATE_VALUE >= ? AND Date.DATE_VALUE <= ?) OR (Date.DATE_START >= ? AND Date.DATE_END <= ?))) OR (((Date.DATE_VALUE >= ? AND Date.DATE_VALUE <= ?) OR (Date.DATE_START >= ? AND Date.DATE_END <= ?)))))";
+                " AND (((Date.DATE_START >= ? AND Date.DATE_END <= ?)) OR ((Date.DATE_START >= ? AND Date.DATE_END <= ?))))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql, "Date", true);
@@ -292,7 +286,7 @@ public void testPrecisionWithMultipleDifferentApproxUTC() throws Exception {
         List<Timestamp> expectedBindVariables = new ArrayList<>();
 
         String expectedSql =
-                " AND ((((Date.DATE_VALUE >= ? AND Date.DATE_VALUE <= ?) OR (Date.DATE_START >= ? AND Date.DATE_END <= ?))) OR (((Date.DATE_VALUE >= ? AND Date.DATE_VALUE <= ?) OR (Date.DATE_START >= ? AND Date.DATE_END <= ?)))))";
+                " AND (((Date.DATE_START >= ? AND Date.DATE_END <= ?)) OR ((Date.DATE_START >= ? AND Date.DATE_END <= ?))))";
         runTest(queryParm,
                 expectedBindVariables,
                 expectedSql, "Date", true);

diff --git a/...c/src/test/java/com/ibm/fhir/persistence/jdbc/test/util/LocationParmBehaviorUtilTest.java b/...c/src/test/java/com/ibm/fhir/persistence/jdbc/test/util/LocationParmBehaviorUtilTest.java
@@ -27,7 +27,7 @@
 
 public class LocationParmBehaviorUtilTest {
     private static final Logger log = java.util.logging.Logger.getLogger(LocationParmBehaviorUtilTest.class.getName());
-    private static final Level LOG_LEVEL = Level.INFO;
+    private static final Level LOG_LEVEL = Level.FINE;
 
     private void runTestBoundingBox(List<Object> expectedBindVariables, String expectedSql,
             BoundingBox boundingBox)