Fix setting a calendar to a time during transition into DST
The transition into Daylight Savings Time (DST), i.e. spring
forward, results in a period of time (equal to the DST offset)
during which the wall clock time is incorrect. Basically, the
GregorianCalendar does not handle that situation properly and
calculates the wrong UTC time (in millis since 1st January
1970).
The reason this exists in Enso is because in OpenJDK the
handling of that situation is not performed directly in
GregorianCalendar computeTime() but instead in a special method
in sun.util.calendar.ZoneInfo. That means OpenJDK has similar
behavior to enso when a different TimeZone implementation, such
as SimpleTimeZone is used. That special code was removed so it
fell back to the code that is used for SimpleTimeZone which is
broken.
In pre-enso GregorianCalendar has a special method getOffset()
that is called for all instances of TimeZone that handles the
case so unlike OpenJDK it does work for SimpleTimeZone.
The solution is to add some code into GregorianCalendar that
handles the transition into DST specially. That means it will
work the same as pre-enso, not OpenJDK for SimpleTimeZone. As
it is complex I have refactored the code a bit more than
necessary in order to improve the structure and add some extra
documentation. I also added extra tests, some of which do not
work in OpenJDK.
Bug: 25897679
Change-Id: I06d0c81e8da20ec00267ed8ba24b26a8a2c163ed
diff --git a/luni/src/test/java/libcore/java/util/GregorianCalendarTest.java b/luni/src/test/java/libcore/java/util/GregorianCalendarTest.java
index 4a89289..4cb15ac 100644
--- a/luni/src/test/java/libcore/java/util/GregorianCalendarTest.java
+++ b/luni/src/test/java/libcore/java/util/GregorianCalendarTest.java
@@ -16,11 +16,11 @@
package libcore.java.util;
-
import java.util.Calendar;
import java.util.Date;
import java.util.GregorianCalendar;
import java.util.Locale;
+import java.util.SimpleTimeZone;
import java.util.TimeZone;
import junit.framework.TestCase;
@@ -30,6 +30,14 @@
private static final TimeZone LONDON = TimeZone.getTimeZone("Europe/London");
+ private static final int HOUR_IN_MILLIS = 3600000;
+
+ private static final SimpleTimeZone CUSTOM_LOS_ANGELES_TIME_ZONE = new SimpleTimeZone(-28800000,
+ "Custom America/Los_Angeles",
+ Calendar.MARCH, 9, 0, hours(2),
+ Calendar.NOVEMBER, 2, 0, hours(2),
+ hours(1));
+
// Documented a previous difference in behavior between this and the RI, see
// https://code.google.com/p/android/issues/detail?id=61993 for more details.
// Switching to OpenJDK has fixed that issue and so this test has been changed to reflect
@@ -129,4 +137,224 @@
assertEquals(5, gc.getActualMaximum(Calendar.DAY_OF_WEEK_IN_MONTH));
assertEquals(4, gc.getLeastMaximum(Calendar.DAY_OF_WEEK_IN_MONTH));
}
+
+ public void test_computeTime_enteringDst_TimeZone_LosAngeles_2014() {
+ TimeZone timeZone = TimeZone.getTimeZone("America/Los_Angeles");
+ checkDstLosAngeles2014(timeZone);
+ }
+
+ /**
+ * This test will fail in the RI.
+ *
+ * <p>The AOSP behavior is different for backwards compatibility with previous versions of
+ * Android.
+ *
+ * <p>Search in this file for 'OpenJDK Failure' to see more details.
+ */
+ public void test_computeTime_enteringDst_DelegatingTimeZone_LosAngeles_2014() {
+ TimeZone timeZone = TimeZone.getTimeZone("America/Los_Angeles");
+ timeZone = new DelegatingTimeZone(timeZone);
+ checkDstLosAngeles2014(timeZone);
+ }
+
+ /**
+ * This test will fail in the RI.
+ *
+ * <p>The AOSP behavior is different for backwards compatibility with previous versions of
+ * Android.
+ *
+ * <p>Search in this file for 'OpenJDK Failure' to see more details.
+ */
+ public void test_computeTime_enteringDst_SimpleTimeZone_LosAngeles_2014() {
+ checkDstLosAngeles2014(CUSTOM_LOS_ANGELES_TIME_ZONE);
+ }
+
+ public void test_computeTime_enteringDst() {
+ // Get the DST entry time with a ZoneInfo implementation of TimeZone.
+ TimeZone zoneInfo = TimeZone.getTimeZone("America/Los_Angeles");
+ long zoneInfoTime = getDstLosAngeles2014(zoneInfo);
+
+ // Check that the time is correct.
+ assertTrue(zoneInfo.inDaylightTime(new Date(zoneInfoTime)));
+ assertFalse(zoneInfo.inDaylightTime(new Date(zoneInfoTime - 1)));
+
+ // Get the DST entry time with a SimpleTimeZone implementation of TimeZone.
+ SimpleTimeZone simpleTimeZone = new SimpleTimeZone(-28800000,
+ "Custom America/Los_Angeles",
+ Calendar.MARCH, 9, 0, 7200000,
+ Calendar.NOVEMBER, 2, 0, 7200000,
+ 3600000);
+ long simpleTimeZoneTime = getDstLosAngeles2014(simpleTimeZone);
+
+ }
+
+ private long getDstLosAngeles2014(TimeZone timeZone) {
+ GregorianCalendar cal = new GregorianCalendar(timeZone, Locale.ENGLISH);
+ cal.set(Calendar.MILLISECOND, 0);
+ cal.set(2014, Calendar.MARCH, 9, 2, 0, 0);
+
+ return cal.getTimeInMillis();
+ }
+
+ private void checkDstLosAngeles2014(TimeZone timeZone) {
+ Calendar cal = new GregorianCalendar(timeZone, Locale.ENGLISH);
+ // Clear the milliseconds field.
+ cal.set(Calendar.MILLISECOND, 0);
+
+ String description;
+
+ // Check milliseconds one second before the transition.
+ description = "01:59:59 - March 9th 2014";
+ cal.set(2014, Calendar.MARCH, 9, 1, 59, 59);
+ checkMillis(cal, description, 1394359199000L);
+
+ // Outside DST time.
+ checkOutsideDst(cal, description);
+
+ // Check milliseconds at the transition point but using an invalid wall clock
+ // (02:00 - 02:59:59.999) do not actually exist.
+ description = "02:00:00 - March 9th 2014";
+ cal.set(2014, Calendar.MARCH, 9, 2, 0, 0);
+
+ // OpenJDK Failure:
+ // This fails on OpenJDK when running with SimpleTimeZone (or any custom TimeZone
+ // implementation). It incorrectly calculates the time in millis to be 1394355600000.
+ // That is because GregorianCalendar treats the implementation that underpins
+ // TimeZone.getTimeZone(String) specially and the code that runs for other classes does
+ // not handle the invalid wall clock period on entry to DST properly.
+ checkMillis(cal, description, 1394359200000L);
+
+ // Invalid wall clock but treated as being inside DST time.
+ checkInsideDst(cal, description);
+
+ // Check milliseconds at the first valid wall clock time after transition, 03:00 - should
+ // be treated the same as 02:00.
+ description = "03:00:00 - March 9th 2014";
+ cal.set(2014, Calendar.MARCH, 9, 3, 0, 0);
+ checkMillis(cal, description, 1394359200000L);
+
+ // Valid wall clock treated as being inside DST time.
+ checkInsideDst(cal, description);
+
+ // Check milliseconds at the last invalid wall clock time, 02:59:59.999.
+ description = "02:59:59.999 - March 9th 2014";
+ cal.set(2014, Calendar.MARCH, 9, 2, 59, 59);
+ cal.set(Calendar.MILLISECOND, 999);
+ checkMillis(cal, description, 1394362799999L);
+
+ // Invalid wall clock but treated as being inside DST time.
+ checkInsideDst(cal, description);
+
+ // Check milliseconds at 03:59:59.999 - should be treated the same as 02:59:59.999
+ description = "03:59:59.999 - March 9th 2014";
+ cal.set(2014, Calendar.MARCH, 9, 3, 59, 59);
+ cal.set(Calendar.MILLISECOND, 999);
+ checkMillis(cal, description, 1394362799999L);
+
+ // Valid wall clock treated as being inside DST time.
+ checkInsideDst(cal, description);
+ }
+
+ private void checkMillis(Calendar cal, String description, long expectedMillis) {
+ assertEquals("Incorrect millis: " + description, expectedMillis, cal.getTimeInMillis());
+ }
+
+ private void checkOutsideDst(Calendar cal, String description) {
+ TimeZone timeZone = cal.getTimeZone();
+ checkOutsideDst(cal, description, timeZone.getRawOffset());
+ }
+
+ private void checkOutsideDst(Calendar cal, String description, int expectedZoneOffset) {
+ checkDstFields(cal, description, expectedZoneOffset, 0);
+ }
+
+ private void checkInsideDst(Calendar cal, String description) {
+ TimeZone timeZone = cal.getTimeZone();
+ checkDstFields(cal, description, timeZone.getRawOffset(), timeZone.getDSTSavings());
+ }
+
+ private void checkDstFields(Calendar cal, String description, int expectedZoneOffset, int expectedDstOffset) {
+ assertEquals("Incorrect ZONE_OFFSET: " + description, expectedZoneOffset, cal.get(Calendar.ZONE_OFFSET));
+ assertEquals("Incorrect DST_OFFSET: " + description, expectedDstOffset, cal.get(Calendar.DST_OFFSET));
+ }
+
+ /**
+ * A custom {@link TimeZone} implementation.
+ *
+ * <p>Used to show the behavior of {@link GregorianCalendar} when provided with a custom
+ * implementation of {@link TimeZone}, i.e. one that is unknown to the runtime,
+ */
+ private static class DelegatingTimeZone extends TimeZone {
+
+ private final TimeZone timeZone;
+
+ public DelegatingTimeZone(TimeZone timeZone) {
+ this.timeZone = timeZone;
+ }
+
+ @Override
+ public int getOffset(int era, int year, int month, int day, int dayOfWeek,
+ int milliseconds) {
+ return timeZone.getOffset(era, year, month, day, dayOfWeek, milliseconds);
+ }
+
+ @Override
+ public int getOffset(long date) {
+ return timeZone.getOffset(date);
+ }
+
+ @Override
+ public void setRawOffset(int offsetMillis) {
+ timeZone.setRawOffset(offsetMillis);
+ }
+
+ @Override
+ public int getRawOffset() {
+ return timeZone.getRawOffset();
+ }
+
+ @Override
+ public String getID() {
+ return timeZone.getID();
+ }
+
+ @Override
+ public void setID(String ID) {
+ timeZone.setID(ID);
+ }
+
+ @Override
+ public String getDisplayName(boolean daylightTime, int style, Locale locale) {
+ return timeZone.getDisplayName(daylightTime, style, locale);
+ }
+
+ @Override
+ public int getDSTSavings() {
+ return timeZone.getDSTSavings();
+ }
+
+ @Override
+ public boolean useDaylightTime() {
+ return timeZone.useDaylightTime();
+ }
+
+ @Override
+ public boolean observesDaylightTime() {
+ return timeZone.observesDaylightTime();
+ }
+
+ @Override
+ public boolean inDaylightTime(Date date) {
+ return timeZone.inDaylightTime(date);
+ }
+
+ @Override
+ public boolean hasSameRules(TimeZone other) {
+ return timeZone.hasSameRules(other);
+ }
+ }
+
+ private static int hours(int count) {
+ return HOUR_IN_MILLIS * count;
+ }
}
diff --git a/luni/src/test/java/libcore/java/util/SimpleTimeZoneTest.java b/luni/src/test/java/libcore/java/util/SimpleTimeZoneTest.java
index dc7773b..fdfcdcf 100644
--- a/luni/src/test/java/libcore/java/util/SimpleTimeZoneTest.java
+++ b/luni/src/test/java/libcore/java/util/SimpleTimeZoneTest.java
@@ -242,7 +242,7 @@
return transitions;
}
- private static String formatCalendar(Calendar cal) {
+ public static String formatCalendar(Calendar cal) {
SimpleDateFormat format = new SimpleDateFormat("yyyy-MM-dd'T'HH:mm:ss.SSSZ",
Locale.ENGLISH);
format.setTimeZone(cal.getTimeZone());
diff --git a/ojluni/src/main/java/java/util/GregorianCalendar.java b/ojluni/src/main/java/java/util/GregorianCalendar.java
index f74dbcb..bc0361fe 100755
--- a/ojluni/src/main/java/java/util/GregorianCalendar.java
+++ b/ojluni/src/main/java/java/util/GregorianCalendar.java
@@ -2723,26 +2723,10 @@
// We use the TimeZone object, unless the user has explicitly set the ZONE_OFFSET
// or DST_OFFSET fields; then we use those fields.
TimeZone zone = getZone();
- if (zoneOffsets == null) {
- zoneOffsets = new int[2];
- }
- int tzMask = fieldMask & (ZONE_OFFSET_MASK|DST_OFFSET_MASK);
- if (tzMask != (ZONE_OFFSET_MASK|DST_OFFSET_MASK)) {
- int gmtOffset = isFieldSet(fieldMask, ZONE_OFFSET) ?
- internalGet(ZONE_OFFSET) : zone.getRawOffset();
- zone.getOffsets(millis - gmtOffset, zoneOffsets);
- }
- if (tzMask != 0) {
- if (isFieldSet(tzMask, ZONE_OFFSET)) {
- zoneOffsets[0] = internalGet(ZONE_OFFSET);
- }
- if (isFieldSet(tzMask, DST_OFFSET)) {
- zoneOffsets[1] = internalGet(DST_OFFSET);
- }
- }
- // Adjust the time zone offset values to get the UTC time.
- millis -= zoneOffsets[0] + zoneOffsets[1];
+ int tzMask = fieldMask & (ZONE_OFFSET_MASK|DST_OFFSET_MASK);
+
+ millis = adjustForZoneAndDaylightSavingsTime(fieldMask, tzMask, millis, zone);
// Set this calendar's time in milliseconds
time = millis;
@@ -2766,6 +2750,171 @@
}
/**
+ * Calculates the time in milliseconds that this calendar represents using the UTC time,
+ * timezone information (specifically Daylight Savings Time (DST) rules, if any) and knowledge
+ * of what fields were explicitly set on the calendar.
+ *
+ * <p>A time is represented as the number of milliseconds since
+ * <i>1st January 1970 00:00:00.000 UTC</i>.
+ *
+ * <p>This uses the terms {@link SimpleTimeZone#STANDARD_TIME standard time},
+ * {@link SimpleTimeZone#WALL_TIME} wall time} and {@link SimpleTimeZone#UTC_TIME UTC time} as
+ * used in {@link SimpleTimeZone}. Specifically:
+ *
+ * <dl>
+ * <dt><b>UTC time</b></dt>
+ * <dd>This is the time within the UTC time zone. UTC does not support DST so the UTC time,
+ * standard time and wall time are all identical within the UTC time zone.</dd>
+ * <dt><b>standard time</b></dt>
+ * <dd>This is the local time within the time zone and is not affected by DST.</dd>
+ * <dt><b>wall time</b></dt>
+ * <dd>This is the local time within the time zone as shown on a wall clock. If the time zone
+ * supports DST then it will be the same as <b>standard time</b> when outside DST and it will
+ * differ (usually be an hour later) when inside DST. This is what the fields on the Calendar
+ * represent.</dd>
+ * </dl>
+ *
+ * <p>The {@code utcTimeInMillis} value supplied was calculated as if the fields represented
+ * a standard time in the {@code UTC} time zone. It is the value that would be returned by
+ * {@link #getTimeInMillis()} when called on this calendar if it was in UTC time zone. e.g. If
+ * the calendar was set to say <i>2014 March 19th 13:27.53 -08:00</i> then the value of
+ * {@code utcTimeInMillis} would be the value of {@link #getTimeInMillis()} when called on a
+ * calendar set to <i>2014 March 19th 13:27.53 -00:00</i>, note the time zone offset is set to
+ * 0.
+ *
+ * <p>To adjust from a UTC time in millis to the standard time in millis we must
+ * <em>subtract</em> the offset from UTC. e.g. given an offset of UTC-08:00, to convert
+ * "14:00 UTC" to "14:00 UTC-08:00" we must subtract -08:00 (i.e. add 8 hours). Another way to
+ * think about it is that 8 hours has to elapse after 14:00 UTC before it is 14:00 UTC-08:00.
+ *
+ * <p>As the zone offset can depend on the time and we cannot calculate the time properly until
+ * we know the time there is a bit of a catch-22. So, what this does is use the
+ * {@link TimeZone#getRawOffset() raw offset} to calculate a ballpark standard time and then
+ * uses that value to retrieve the appropriate zone and DST offsets from the time zone. They
+ * are then used to make the final wall time calculation.
+ *
+ * <p>The DST offset will need clearing if the standard time is not a valid wall clock. See
+ * {@link #adjustDstOffsetForInvalidWallClock(long, TimeZone, int)} for more information.
+ *
+ * @param fieldMask the set of fields that should be used to calculate the time.
+ * @param tzMask the set of time zone related fields, i.e. {@link #ZONE_OFFSET_MASK} and
+ * {@link #DST_OFFSET_MASK}
+ * @param utcTimeInMillis the time in millis, calculated assuming the time zone was GMT.
+ * @param zone the actual time zone.
+ * @return the UTC time in millis after adjusting for zone and DST offset.
+ */
+ private long adjustForZoneAndDaylightSavingsTime(
+ int fieldMask, int tzMask, long utcTimeInMillis, TimeZone zone) {
+
+ // The following don't actually need to be initialized because they are always set before
+ // they are used but the compiler cannot detect that.
+ int zoneOffset = 0;
+ int dstOffset = 0;
+
+ // If either of the ZONE_OFFSET or DST_OFFSET fields are not set then get the information
+ // from the TimeZone.
+ if (tzMask != (ZONE_OFFSET_MASK|DST_OFFSET_MASK)) {
+ if (zoneOffsets == null) {
+ zoneOffsets = new int[2];
+ }
+ int gmtOffset = isFieldSet(fieldMask, ZONE_OFFSET) ?
+ internalGet(ZONE_OFFSET) : zone.getRawOffset();
+
+ // Calculate the standard time (no DST) in the supplied zone. This is a ballpark figure
+ // and not used in the final calculation as the offset used here may not be the same as
+ // the actual offset the time zone requires be used for this time. This is to handle
+ // situations like Honolulu, where its raw offset changed from GMT-10:30 to GMT-10:00
+ // in 1947. The TimeZone always uses a raw offset of -10:00 but will return -10:30
+ // for dates before the change over.
+ long standardTimeInZone = utcTimeInMillis - gmtOffset;
+
+ // Retrieve the correct zone and DST offsets from the time zone.
+ zone.getOffsets(standardTimeInZone, zoneOffsets);
+ zoneOffset = zoneOffsets[0];
+ dstOffset = zoneOffsets[1];
+
+ // If necessary adjust the DST offset to handle an invalid wall clock sensibly.
+ dstOffset = adjustDstOffsetForInvalidWallClock(standardTimeInZone, zone, dstOffset);
+ }
+
+ // If either ZONE_OFFSET of DST_OFFSET fields are set then get the information from the
+ // fields, potentially overriding information from the TimeZone.
+ if (tzMask != 0) {
+ if (isFieldSet(tzMask, ZONE_OFFSET)) {
+ zoneOffset = internalGet(ZONE_OFFSET);
+ }
+ if (isFieldSet(tzMask, DST_OFFSET)) {
+ dstOffset = internalGet(DST_OFFSET);
+ }
+ }
+
+ // Adjust the time zone offset values to get the UTC time.
+ long standardTimeInZone = utcTimeInMillis - zoneOffset;
+ return standardTimeInZone - dstOffset;
+ }
+
+ /**
+ * If the supplied millis is in daylight savings time (DST) and is the result of an invalid
+ * wall clock then adjust the DST offset to ensure sensible behavior.
+ *
+ * <p>When transitioning into DST, i.e. when the clocks spring forward (usually by one hour)
+ * there is a wall clock period that is invalid, it literally doesn't exist. e.g. If clocks
+ * go forward one hour at 02:00 on 9th March 2014 (standard time) then the wall time of
+ * 02:00-02:59:59.999 is not a valid. The wall clock jumps straight from 01:59:59.999 to
+ * 03:00. The following table shows the relationship between the time in millis, the standard
+ * time and the wall time at the point of transitioning into DST. As can be seen there is no
+ * 02:00 in the wall time.
+ *
+ * <pre>
+ * Time In Millis - ...... x+1h ..... x+2h ..... x+3h
+ * Standard Time - ...... 01:00 ..... 02:00 ..... 03:00 .....
+ * Wall Time - ...... 01:00 ..... 03:00 ..... 04:00 .....
+ * ^
+ * 02:00 missing
+ * </pre>
+ *
+ * <p>The calendar fields represent wall time. If the user sets the fields on the calendar so
+ * that it is in that invalid period then this code attempts to do something sensible. It
+ * treats 02:MM:SS.SSS as if it is {@code 01:MM:SS.SSS + 1 hour}. That makes sense from both
+ * the input calendar fields perspective and from the time in millis perspective. Of course the
+ * result of that is that when the time is formatted in that time zone that the time is
+ * actually 03:MM:SS.SSS.
+ *
+ * <pre>
+ * Wall Time - ...... 01:00 ..... <b>02:00 .....</b> 03:00 ..... 04:00 .....
+ * Time In Millis - ...... x+1h ..... <b> x+2h .....</b> x+2h ..... x+3h .....
+ * </pre>
+ *
+ * <p>The way that works is as follows. First the standard time is calculated and the DST
+ * offset is determined. Then if the time is in DST (the DST offset is not 0) but it was not in
+ * DST an hour earlier (or however long the DST offset is) then it must be in that invalid
+ * period, in which case set the DST offset to 0. That is then subtracted from the time in
+ * millis to produce the correct result. The following diagram illustrates the process.
+ *
+ * <pre>
+ * Standard Time - ...... 01:00 ..... 02:00 ..... 03:00 ..... 04:00 .....
+ * Time In Millis - ...... x+1h ..... x+2h ..... x+3h ..... x+4h .....
+ * DST Offset - ...... 0h ..... 1h ..... 1h ..... 1h .....
+ * Adjusted DST - ...... 0h ..... <b>0h</b> ..... 1h ..... 1h .....
+ * Adjusted Time - ...... x+1h ..... x+2h ..... <b>x+2h</b> ..... <b>x+3h</b> .....
+ * </pre>
+ *
+ * @return the adjusted DST offset.
+ */
+ private int adjustDstOffsetForInvalidWallClock(
+ long standardTimeInZone, TimeZone zone, int dstOffset) {
+
+ if (dstOffset != 0) {
+ // If applying the DST offset produces a time that is outside DST then it must be
+ // an invalid wall clock so clear the DST offset to avoid that happening.
+ if (!zone.inDaylightTime(new Date(standardTimeInZone - dstOffset))) {
+ dstOffset = 0;
+ }
+ }
+ return dstOffset;
+ }
+
+ /**
* Computes the fixed date under either the Gregorian or the
* Julian calendar, using the given year and the specified calendar fields.
*
