Use Calendar.Builder for MAP Client timestamp parsing
MAP Client was reporting different timestamps for a given
message-listing object each time getUnreadMessages() was called. The
milliseconds value was always different. This was a result of the
Calendar object pre-populating all fields with the current date-time. The
reported date-time would always have the current time's milliseconds
field.
Datetime parsing has been updated to use a Calendar.Builder which is
clearer and doesn't use the current time when initializing. The
milliseconds field is explicitly set to zero.
Tests were added for the ObexTime class to prevent this from happening
in the future.
Bug: b/135606822
Test: atest ObexTimeTest.java
Change-Id: Ie872b4bb30f68e0c00b15767eb0413a6e67ba630
(cherry picked from commit 85fb0fe5d730d93850c7db0bc7eb2dc26d2ced50)
Merged-In: Ie872b4bb30f68e0c00b15767eb0413a6e67ba630
Change-Id: Iab4e3cf5937958f500d58b9ba6fa64c439cd4dc9
diff --git a/src/com/android/bluetooth/mapclient/obex/ObexTime.java b/src/com/android/bluetooth/mapclient/obex/ObexTime.java
index 42a32c1..cc58a51 100644
--- a/src/com/android/bluetooth/mapclient/obex/ObexTime.java
+++ b/src/com/android/bluetooth/mapclient/obex/ObexTime.java
@@ -29,8 +29,17 @@
public ObexTime(String time) {
/*
- * match OBEX time string: YYYYMMDDTHHMMSS with optional UTF offset
- * +/-hhmm
+ * Match OBEX time string: YYYYMMDDTHHMMSS with optional UTF offset +/-hhmm
+ *
+ * Matched groups are numberes as follows:
+ *
+ * YYYY MM DD T HH MM SS + hh mm
+ * ^^^^ ^^ ^^ ^^ ^^ ^^ ^ ^^ ^^
+ * 1 2 3 4 5 6 8 9 10
+ * |---7---|
+ *
+ * All groups are guaranteed to be numeric so conversion will always succeed (except group 8
+ * which is either + or -)
*/
Pattern p = Pattern.compile(
"(\\d{4})(\\d{2})(\\d{2})T(\\d{2})(\\d{2})(\\d{2})(([+-])(\\d{2})(\\d{2})" + ")?");
@@ -39,20 +48,26 @@
if (m.matches()) {
/*
- * matched groups are numberes as follows: YYYY MM DD T HH MM SS +
- * hh mm ^^^^ ^^ ^^ ^^ ^^ ^^ ^ ^^ ^^ 1 2 3 4 5 6 8 9 10 all groups
- * are guaranteed to be numeric so conversion will always succeed
- * (except group 8 which is either + or -)
+ * MAP spec says to default to "Local Time basis" for a message listing timestamp. We'll
+ * use the system default timezone and assume it knows best what our local timezone is.
+ * The builder defaults to the default locale and timezone if none is provided.
*/
+ Calendar.Builder builder = new Calendar.Builder();
- Calendar cal = Calendar.getInstance();
- cal.set(Integer.parseInt(m.group(1)), Integer.parseInt(m.group(2)) - 1,
- Integer.parseInt(m.group(3)), Integer.parseInt(m.group(4)),
- Integer.parseInt(m.group(5)), Integer.parseInt(m.group(6)));
+ /* Note that Calendar months are zero-based */
+ builder.setDate(Integer.parseInt(m.group(1)), /* year */
+ Integer.parseInt(m.group(2)) - 1, /* month */
+ Integer.parseInt(m.group(3))); /* day of month */
+
+ /* Note the MAP timestamp doesn't have milliseconds and we're explicitly setting to 0 */
+ builder.setTimeOfDay(Integer.parseInt(m.group(4)), /* hours */
+ Integer.parseInt(m.group(5)), /* minutes */
+ Integer.parseInt(m.group(6)), /* seconds */
+ 0); /* milliseconds */
/*
- * if 7th group is matched then we have UTC offset information
- * included
+ * If 7th group is matched then we're no longer using "Local Time basis" and instead
+ * have a UTC based timestamp and offset information included
*/
if (m.group(7) != null) {
int ohh = Integer.parseInt(m.group(9));
@@ -68,10 +83,10 @@
TimeZone tz = TimeZone.getTimeZone("UTC");
tz.setRawOffset(offset);
- cal.setTimeZone(tz);
+ builder.setTimeZone(tz);
}
- mDate = cal.getTime();
+ mDate = builder.build().getTime();
}
}
diff --git a/tests/unit/src/com/android/bluetooth/mapclient/ObexTimeTest.java b/tests/unit/src/com/android/bluetooth/mapclient/ObexTimeTest.java
new file mode 100644
index 0000000..5ef2d45
--- /dev/null
+++ b/tests/unit/src/com/android/bluetooth/mapclient/ObexTimeTest.java
@@ -0,0 +1,114 @@
+/*
+ * Copyright (C) 2019 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 com.android.bluetooth.mapclient;
+
+import androidx.test.runner.AndroidJUnit4;
+
+import org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+import java.util.Date;
+import java.util.TimeZone;
+
+@RunWith(AndroidJUnit4.class)
+public class ObexTimeTest {
+ private static final String TAG = ObexTimeTest.class.getSimpleName();
+
+ private static final String VALID_TIME_STRING = "20190101T121314";
+ private static final String VALID_TIME_STRING_WITH_OFFSET_POS = "20190101T121314+0130";
+ private static final String VALID_TIME_STRING_WITH_OFFSET_NEG = "20190101T121314-0130";
+
+ private static final String INVALID_TIME_STRING_OFFSET_EXTRA_DIGITS = "20190101T121314-99999";
+ private static final String INVALID_TIME_STRING_BAD_DELIMITER = "20190101Q121314";
+
+ // MAP message listing times, per spec, use "local time basis" if UTC offset isn't given. The
+ // ObexTime class parses using the current default timezone (assumed to be the "local timezone")
+ // in the case that UTC isn't provided. However, the Date class assumes UTC ALWAYS when
+ // initializing off of a long value. We have to take that into account when determining our
+ // expected results for time strings that don't have an offset.
+ private static final long LOCAL_TIMEZONE_OFFSET = TimeZone.getDefault().getRawOffset();
+
+ // If you are a positive offset from GMT then GMT is in the "past" and you need to subtract that
+ // offset from the time. If you are negative then GMT is in the future and you need to add that
+ // offset to the time.
+ private static final long VALID_TS = 1546344794000L; // Jan 01, 2019 at 12:13:14 GMT
+ private static final long TS_OFFSET = 5400000L; // 1 Hour, 30 minutes -> milliseconds
+ private static final long VALID_TS_LOCAL_TZ = VALID_TS - LOCAL_TIMEZONE_OFFSET;
+ private static final long VALID_TS_OFFSET_POS = VALID_TS - TS_OFFSET;
+ private static final long VALID_TS_OFFSET_NEG = VALID_TS + TS_OFFSET;
+
+ private static final Date VALID_DATE_LOCAL_TZ = new Date(VALID_TS_LOCAL_TZ);
+ private static final Date VALID_DATE_WITH_OFFSET_POS = new Date(VALID_TS_OFFSET_POS);
+ private static final Date VALID_DATE_WITH_OFFSET_NEG = new Date(VALID_TS_OFFSET_NEG);
+
+ @Test
+ public void createWithValidDateTimeString_TimestampCorrect() {
+ ObexTime timestamp = new ObexTime(VALID_TIME_STRING);
+ Assert.assertEquals("Parsed timestamp must match expected", VALID_DATE_LOCAL_TZ,
+ timestamp.getTime());
+ }
+
+ @Test
+ public void createWithValidDateTimeStringWithPosOffset_TimestampCorrect() {
+ ObexTime timestamp = new ObexTime(VALID_TIME_STRING_WITH_OFFSET_POS);
+ Assert.assertEquals("Parsed timestamp must match expected", VALID_DATE_WITH_OFFSET_POS,
+ timestamp.getTime());
+ }
+
+ @Test
+ public void createWithValidDateTimeStringWithNegOffset_TimestampCorrect() {
+ ObexTime timestamp = new ObexTime(VALID_TIME_STRING_WITH_OFFSET_NEG);
+ Assert.assertEquals("Parsed timestamp must match expected", VALID_DATE_WITH_OFFSET_NEG,
+ timestamp.getTime());
+ }
+
+ @Test
+ public void createWithValidDate_TimestampCorrect() {
+ ObexTime timestamp = new ObexTime(VALID_DATE_LOCAL_TZ);
+ Assert.assertEquals("ObexTime created with a date must return the same date",
+ VALID_DATE_LOCAL_TZ, timestamp.getTime());
+ }
+
+ @Test
+ public void printValidTime_TimestampMatchesInput() {
+ ObexTime timestamp = new ObexTime(VALID_TIME_STRING);
+ Assert.assertEquals("Timestamp as a string must match the input string", VALID_TIME_STRING,
+ timestamp.toString());
+ }
+
+ @Test
+ public void createWithInvalidDelimiterString_TimestampIsNull() {
+ ObexTime timestamp = new ObexTime(INVALID_TIME_STRING_BAD_DELIMITER);
+ Assert.assertEquals("Parsed timestamp was invalid and must result in a null object", null,
+ timestamp.getTime());
+ }
+
+ @Test
+ public void createWithInvalidOffsetString_TimestampIsNull() {
+ ObexTime timestamp = new ObexTime(INVALID_TIME_STRING_OFFSET_EXTRA_DIGITS);
+ Assert.assertEquals("Parsed timestamp was invalid and must result in a null object", null,
+ timestamp.getTime());
+ }
+
+ @Test
+ public void printInvalidTime_ReturnsNull() {
+ ObexTime timestamp = new ObexTime(INVALID_TIME_STRING_BAD_DELIMITER);
+ Assert.assertEquals("Invalid timestamps must return null for toString()", null,
+ timestamp.toString());
+ }
+}
