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

[GitHub] stevedlawrence closed pull request #116: Fix comparison of calendars that do not have timezones

stevedlawrence closed pull request #116: Fix comparison of calendars that do not have timezones
URL: https://github.com/apache/incubator-daffodil/pull/116
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

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 6e630ef6c..b5b087a97 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 4e65aee61..0b488c790 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 2d2369d39..39fd52407 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 b237035d3..e13625293 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
@@ -157,9 +157,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") }


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services