You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@daffodil.apache.org by sl...@apache.org on 2018/09/07 14:37:36 UTC

[incubator-daffodil] branch master updated: Fix comparison of calendars that do not have timezones

This is an automated email from the ASF dual-hosted git repository.

slawrence pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-daffodil.git


The following commit(s) were added to refs/heads/master by this push:
     new 1a0e654  Fix comparison of calendars that do not have timezones
1a0e654 is described below

commit 1a0e6544ad15ca6ea613759be7e98ce29252c5b9
Author: Steve Lawrence <sl...@apache.org>
AuthorDate: Mon Aug 27 11:37:23 2018 -0400

    Fix comparison of calendars that do not have timezones
    
    The assertions was just incorrect. It is perfectly valid to call the
    orderIgnoreTimeZone function where one calendar has a timezone and the
    other does not, as long as they have been normalized/modified according
    to the W3C spec before being called. Remove this invarient, and add a
    couple extra just to make sure we are modifying calendar appropriately.
    And move test out of scala debug.
    
    DAFFODIL-1986
---
 .../apache/daffodil/calendar/DFDLCalendar.scala    | 48 ++++++++++++----------
 .../section23/dfdl_expressions/expressions.tdml    |  8 ++--
 .../TestDFDLExpressionsDebug.scala                 |  3 --
 .../dfdl_expressions/TestDFDLExpressions.scala     |  4 +-
 4 files changed, 32 insertions(+), 31 deletions(-)

diff --git a/daffodil-runtime1/src/main/scala/org/apache/daffodil/calendar/DFDLCalendar.scala b/daffodil-runtime1/src/main/scala/org/apache/daffodil/calendar/DFDLCalendar.scala
index 6e630ef..b5b087a 100644
--- a/daffodil-runtime1/src/main/scala/org/apache/daffodil/calendar/DFDLCalendar.scala
+++ b/daffodil-runtime1/src/main/scala/org/apache/daffodil/calendar/DFDLCalendar.scala
@@ -24,19 +24,22 @@ import com.ibm.icu.util.TimeZone;
 import java.math.{ BigDecimal => JBigDecimal }
 
 import org.apache.daffodil.exceptions.Assert
-import scala.collection.mutable.ArraySeq
 import org.apache.daffodil.processors.parsers.TextCalendarConstants
 
 object DFDLCalendarOrder extends Enumeration {
   type DFDLCalendarOrder = Value
   val P_LESS_THAN_Q, P_GREATER_THAN_Q, P_EQUAL_Q, P_NOT_EQUAL_Q = Value
-}
 
-trait OrderedCalendar { self: DFDLCalendar =>
-  protected lazy val fields = ArraySeq(Calendar.EXTENDED_YEAR, Calendar.MONTH,
-    Calendar.DAY_OF_MONTH, Calendar.HOUR_OF_DAY, Calendar.MINUTE,
+  val fieldsForComparison = Array(
+    Calendar.EXTENDED_YEAR,
+    Calendar.MONTH,
+    Calendar.DAY_OF_MONTH,
+    Calendar.HOUR_OF_DAY,
+    Calendar.MINUTE,
     Calendar.SECOND)
+}
 
+trait OrderedCalendar { self: DFDLCalendar =>
   import DFDLCalendarOrder._
 
   /**
@@ -89,23 +92,23 @@ trait OrderedCalendar { self: DFDLCalendar =>
     val qPrime = q.getNormalizedCalendar
 
     val res: DFDLCalendarOrder = {
-      if ((pHasTZ && qHasTZ) || (!pHasTZ && !qHasTZ)) { orderIgnoreTimeZone(pPrime, qPrime) }
+      if ((pHasTZ && qHasTZ) || (!pHasTZ && !qHasTZ)) { orderCompareFields(pPrime, qPrime) }
       else if (pHasTZ && !qHasTZ) {
         val qPlus = qPrime.getDateTimePlusFourteenHours
 
-        if (orderIgnoreTimeZone(pPrime, qPlus) == DFDLCalendarOrder.P_LESS_THAN_Q) { DFDLCalendarOrder.P_LESS_THAN_Q }
+        if (orderCompareFields(pPrime, qPlus) == DFDLCalendarOrder.P_LESS_THAN_Q) { DFDLCalendarOrder.P_LESS_THAN_Q }
         else {
           val qMinus = q.getDateTimeMinusFourteenHours
-          if (orderIgnoreTimeZone(pPrime, qMinus) == DFDLCalendarOrder.P_GREATER_THAN_Q) { DFDLCalendarOrder.P_GREATER_THAN_Q }
+          if (orderCompareFields(pPrime, qMinus) == DFDLCalendarOrder.P_GREATER_THAN_Q) { DFDLCalendarOrder.P_GREATER_THAN_Q }
           else { DFDLCalendarOrder.P_NOT_EQUAL_Q }
         }
       } else if (!pHasTZ && qHasTZ) {
         val pMinus = pPrime.getDateTimeMinusFourteenHours
 
-        if (orderIgnoreTimeZone(pMinus, qPrime) == DFDLCalendarOrder.P_LESS_THAN_Q) { DFDLCalendarOrder.P_LESS_THAN_Q }
+        if (orderCompareFields(pMinus, qPrime) == DFDLCalendarOrder.P_LESS_THAN_Q) { DFDLCalendarOrder.P_LESS_THAN_Q }
         else {
           val pPlus = pPrime.getDateTimePlusFourteenHours
-          if (orderIgnoreTimeZone(pPlus, qPrime) == DFDLCalendarOrder.P_GREATER_THAN_Q) { DFDLCalendarOrder.P_GREATER_THAN_Q }
+          if (orderCompareFields(pPlus, qPrime) == DFDLCalendarOrder.P_GREATER_THAN_Q) { DFDLCalendarOrder.P_GREATER_THAN_Q }
           else { DFDLCalendarOrder.P_NOT_EQUAL_Q }
         }
       } else { Assert.impossibleCase }
@@ -115,19 +118,20 @@ trait OrderedCalendar { self: DFDLCalendar =>
   }
 
   /**
-   * If P and Q either both have a time zone or both do not have a
-   * time zone, compare P and Q field by field from the year field
-   * down to the second field, and return a result as soon as it
-   * can be determined.
+   * This method does the actual comparison of calendars that have been
+   * normalized/modified according to the W3C specification in the order()
+   * method above, by comparing the existence and values of various calendar
+   * fields. Note that by the point this method is called the calendars should
+   * either not have a timezone or have been normalized to a UTC timezone, so
+   * the timezone field does not play a role in this comparison.
    */
-  private def orderIgnoreTimeZone(p: DFDLDateTime, q: DFDLDateTime): DFDLCalendarOrder = {
-    Assert.invariant((p.hasTimeZone && q.hasTimeZone) || (!p.hasTimeZone && !q.hasTimeZone))
-
-    val length = fields.length
-    for (i <- 0 to length) {
-      if (i == length) return DFDLCalendarOrder.P_EQUAL_Q
+  private def orderCompareFields(p: DFDLDateTime, q: DFDLDateTime): DFDLCalendarOrder = {
+    Assert.invariant(!p.hasTimeZone || p.calendar.getTimeZone == TimeZone.GMT_ZONE)
+    Assert.invariant(!q.hasTimeZone || q.calendar.getTimeZone == TimeZone.GMT_ZONE)
 
-      val field = fields(i)
+    val length = fieldsForComparison.length
+    for (i <- 0 until length) {
+      val field = fieldsForComparison(i)
 
       val hasPSubI = p.isFieldSet(field)
       val hasQSubI = q.isFieldSet(field)
@@ -286,12 +290,14 @@ case class DFDLDateTime(calendar: Calendar, parsedTZ: Boolean)
   protected lazy val normalizedCalendar = normalizeCalendar(calendar)
 
   def getDateTimePlusFourteenHours: DFDLDateTime = {
+    Assert.invariant(!hasTimeZone)
     val adjustedCal = adjustTimeZone(normalizedCalendar, 14, 0)
     val dt = new DFDLDateTime(adjustedCal, parsedTZ)
     dt
   }
 
   def getDateTimeMinusFourteenHours: DFDLDateTime = {
+    Assert.invariant(!hasTimeZone)
     val adjustedCal = adjustTimeZone(normalizedCalendar, -14, 0)
     val dt = new DFDLDateTime(adjustedCal, parsedTZ)
     dt
diff --git a/daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions.tdml b/daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions.tdml
index 4e65aee..0b488c7 100644
--- a/daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions.tdml
+++ b/daffodil-test/src/test/resources/org/apache/daffodil/section23/dfdl_expressions/expressions.tdml
@@ -6029,11 +6029,11 @@ c]]></value>
     <tdml:document><![CDATA[2000-01-01T12:00:00,1999-12-31T23:00:00+00:00]]></tdml:document>
     <tdml:infoset>
       <tdml:dfdlInfoset>
-        <e35>
-          <dateTime1>2000-01-01T12:00:00.000000+00:00</dateTime1>
+        <e35a>
+          <dateTime1>2000-01-01T12:00:00.000000</dateTime1>
           <dateTime2>1999-12-31T23:00:00.000000+00:00</dateTime2>
-          <oneLTtwo>false</oneLTtwo>
-        </e35>
+          <oneLTtwo>true</oneLTtwo>
+        </e35a>
       </tdml:dfdlInfoset>
     </tdml:infoset>
   </tdml:parserTestCase>
diff --git a/daffodil-test/src/test/scala-debug/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressionsDebug.scala b/daffodil-test/src/test/scala-debug/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressionsDebug.scala
index 8d0605f..d9b71e4 100644
--- a/daffodil-test/src/test/scala-debug/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressionsDebug.scala
+++ b/daffodil-test/src/test/scala-debug/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressionsDebug.scala
@@ -58,9 +58,6 @@ class TestDFDLExpressionsDebug {
 
   import TestDFDLExpressionsDebug._
 
-  // DAFFODIL-1986
-  @Test def test_comparison_operators_46a() { runner.runOneTest("comparison_operators_46a") }
-
   //DFDL-1287
   @Test def test_internal_space_preserved4() { runner.runOneTest("internal_space_preserved4") }
   @Test def test_internal_space_not_preserved2() { runner.runOneTest("internal_space_not_preserved2") }
diff --git a/daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions.scala b/daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions.scala
index a4ab382..c568056 100644
--- a/daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions.scala
+++ b/daffodil-test/src/test/scala/org/apache/daffodil/section23/dfdl_expressions/TestDFDLExpressions.scala
@@ -162,9 +162,7 @@ class TestDFDLExpressions {
   @Test def test_comparison_operators_44() { runner.runOneTest("comparison_operators_44") }
   @Test def test_comparison_operators_45() { runner.runOneTest("comparison_operators_45") }
   @Test def test_comparison_operators_46() { runner.runOneTest("comparison_operators_46") }
-
-  // DAFFODIL-1986
-  // @Test def test_comparison_operators_46a() { runner.runOneTest("comparison_operators_46a") }
+  @Test def test_comparison_operators_46a() { runner.runOneTest("comparison_operators_46a") }
 
   // from XPath Spec Sec 10.4.6.1 Examples
   @Test def test_comparison_operators_47() { runner.runOneTest("comparison_operators_47") }