Minimal fix for Calendar.setTimeZone.
The interpretation of Calendar's competing member variables when the time,
time zone, and fields[] don't agree is confusing, and led to
Calendar.setTimeZone having no effect. I've improved the documentation
based on my reverse-engineering and fixed this bug. I noticed a lot of
other code that looks incorrect though, and I've raised http://b/2419810
suggesting that we switch to icu4j's calendars in the longer term.
Bug: http://code.google.com/p/android/issues/detail?id=6184
diff --git a/libcore/luni/src/main/java/java/util/Calendar.java b/libcore/luni/src/main/java/java/util/Calendar.java
index 7f2e92d..4d7ede9 100644
--- a/libcore/luni/src/main/java/java/util/Calendar.java
+++ b/libcore/luni/src/main/java/java/util/Calendar.java
@@ -285,36 +285,48 @@
* @see GregorianCalendar
* @see TimeZone
*/
-public abstract class Calendar implements Serializable, Cloneable,
- Comparable<Calendar> {
+public abstract class Calendar implements Serializable, Cloneable, Comparable<Calendar> {
private static final long serialVersionUID = -1807547505821590642L;
/**
- * Set to {@code true} when the calendar fields have been set from the time, set to
- * {@code false} when a field is changed and the fields must be recomputed.
+ * True iff the values in {@code fields[]} correspond to {@code time}. Despite the name, this
+ * is effectively "are the values in fields[] up-to-date?" --- {@code fields[]} may contain
+ * non-zero values and {@code isSet[]} may contain {@code true} values even when
+ * {@code areFieldsSet} is false.
+ * Accessing the fields via {@code get} will ensure the fields are up-to-date.
*/
protected boolean areFieldsSet;
/**
- * An integer array of calendar fields. The length is {@code FIELD_COUNT}.
+ * Contains broken-down field values for the current value of {@code time} if
+ * {@code areFieldsSet} is true, or stale data corresponding to some previous value otherwise.
+ * Accessing the fields via {@code get} will ensure the fields are up-to-date.
+ * The array length is always {@code FIELD_COUNT}.
*/
protected int[] fields;
/**
- * A boolean array. Each element indicates if the corresponding field has
- * been set. The length is {@code FIELD_COUNT}.
+ * Whether the corresponding element in {@code field[]} has been set. Initially, these are all
+ * false. The first time the fields are computed, these are set to true and remain set even if
+ * the data becomes stale: you <i>must</i> check {@code areFieldsSet} if you want to know
+ * whether the value is up-to-date.
+ * Note that {@code isSet} is <i>not</i> a safe alternative to accessing this array directly,
+ * and will likewise return stale data!
+ * The array length is always {@code FIELD_COUNT}.
*/
protected boolean[] isSet;
/**
- * Set to {@code true} when the time has been set, set to {@code false} when a field is
- * changed and the time must be recomputed.
+ * Whether {@code time} corresponds to the values in {@code fields[]}. If false, {@code time}
+ * is out-of-date with respect to changes {@code fields[]}.
+ * Accessing the time via {@code getTimeInMillis} will always return the correct value.
*/
protected boolean isTimeSet;
/**
- * The time in milliseconds since January 1, 1970.
+ * A time in milliseconds since January 1, 1970. See {@code isTimeSet}.
+ * Accessing the time via {@code getTimeInMillis} will always return the correct value.
*/
protected long time;
@@ -1116,7 +1128,16 @@
}
/**
- * Returns whether the specified field is set.
+ * Returns whether the specified field is set. Note that the interpretation of "is set" is
+ * somewhat technical. In particular, it does <i>not</i> mean that the field's value is up
+ * to date. If you want to know whether a field contains an up-to-date value, you must also
+ * check {@code areFieldsSet}, making this method somewhat useless unless you're a subclass,
+ * in which case you can access the {@code isSet} array directly.
+ * <p>
+ * A field remains "set" from the first time its value is computed until it's cleared by one
+ * of the {@code clear} methods. Thus "set" does not mean "valid". You probably want to call
+ * {@code get} -- which will update fields as necessary -- rather than try to make use of
+ * this method.
*
* @param field
* a {@code Calendar} field number.
diff --git a/libcore/luni/src/main/java/java/util/GregorianCalendar.java b/libcore/luni/src/main/java/java/util/GregorianCalendar.java
index 7339151..846ac3e 100644
--- a/libcore/luni/src/main/java/java/util/GregorianCalendar.java
+++ b/libcore/luni/src/main/java/java/util/GregorianCalendar.java
@@ -591,15 +591,14 @@
@Override
protected void computeFields() {
- int zoneOffset = getTimeZone().getRawOffset();
-
- if(!isSet[ZONE_OFFSET]) {
- fields[ZONE_OFFSET] = zoneOffset;
- }
+ TimeZone timeZone = getTimeZone();
+ int dstOffset = timeZone.inDaylightTime(new Date(time)) ? timeZone.getDSTSavings() : 0;
+ int zoneOffset = timeZone.getRawOffset();
+ fields[DST_OFFSET] = dstOffset;
+ fields[ZONE_OFFSET] = zoneOffset;
int millis = (int) (time % 86400000);
int savedMillis = millis;
- int dstOffset = fields[DST_OFFSET];
// compute without a change in daylight saving time
int offset = zoneOffset + dstOffset;
long newTime = time + offset;
@@ -610,6 +609,8 @@
newTime = 0x8000000000000000L;
}
+ // FIXME: I don't think this caching ever really gets used, because it requires that the
+ // time zone doesn't use daylight savings (ever). So unless you're somewhere like Taiwan...
if (isCached) {
if (millis < 0) {
millis += 86400000;
@@ -636,11 +637,11 @@
fields[AM_PM] = fields[HOUR_OF_DAY] > 11 ? 1 : 0;
fields[HOUR] = fields[HOUR_OF_DAY] % 12;
+ // FIXME: this has to be wrong; useDaylightTime doesn't mean what they think it means!
long newTimeAdjusted = newTime;
- if (getTimeZone().useDaylightTime()) {
+ if (timeZone.useDaylightTime()) {
// BEGIN android-changed: removed unnecessary cast
- int dstSavings = (/* (SimpleTimeZone) */ getTimeZone())
- .getDSTSavings();
+ int dstSavings = timeZone.getDSTSavings();
// END android-changed
newTimeAdjusted += (dstOffset == 0) ? dstSavings : -dstSavings;
}
@@ -665,7 +666,7 @@
if (!isCached
&& newTime != 0x7fffffffffffffffL
&& newTime != 0x8000000000000000L
- && (!getTimeZone().useDaylightTime() || getTimeZone() instanceof SimpleTimeZone)) {
+ && (!timeZone.useDaylightTime() || timeZone instanceof SimpleTimeZone)) {
int cacheMillis = 0;
cachedFields[0] = fields[YEAR];
diff --git a/libcore/luni/src/test/java/java/util/AllTests.java b/libcore/luni/src/test/java/java/util/AllTests.java
index 774d48b..07fee27 100644
--- a/libcore/luni/src/test/java/java/util/AllTests.java
+++ b/libcore/luni/src/test/java/java/util/AllTests.java
@@ -22,6 +22,7 @@
public class AllTests {
public static final Test suite() {
TestSuite suite = tests.TestSuiteFactory.createTestSuite();
+ suite.addTestSuite(java.util.CalendarTest.class);
suite.addTestSuite(java.util.CurrencyTest.class);
suite.addTestSuite(java.util.DateTest.class);
suite.addTestSuite(java.util.FormatterTest.class);
diff --git a/libcore/luni/src/test/java/java/util/CalendarTest.java b/libcore/luni/src/test/java/java/util/CalendarTest.java
new file mode 100644
index 0000000..f9f6fbe
--- /dev/null
+++ b/libcore/luni/src/test/java/java/util/CalendarTest.java
@@ -0,0 +1,33 @@
+/*
+ * Copyright (C) 2010 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package java.util;
+
+import junit.framework.Test;
+import junit.framework.TestSuite;
+
+public class CalendarTest extends junit.framework.TestCase {
+ // http://code.google.com/p/android/issues/detail?id=6184
+ public void test_setTimeZone() {
+ // The specific time zones don't matter; they just have to be different so we can see that
+ // get(Calendar.ZONE_OFFSET) returns the zone offset of the time zone passed to setTimeZone.
+ Calendar cal = Calendar.getInstance(TimeZone.getTimeZone("UTC"), Locale.US);
+ assertEquals(0, cal.get(Calendar.ZONE_OFFSET));
+ TimeZone tz = java.util.TimeZone.getTimeZone("GMT+7");
+ cal.setTimeZone(tz);
+ assertEquals(25200000, cal.get(Calendar.ZONE_OFFSET));
+ }
+}
