Do not crash on RDATEs with PERIOD (#26)

Should close bitfireAT/davx5#74

Co-authored-by: Ricki Hirner <hirner@bitfire.at>

2 files changed, 155 insertions, 43 deletions

diff --git a/src/main/java/at/bitfire/ical4android/util/AndroidTimeUtils.kt b/src/main/java/at/bitfire/ical4android/util/AndroidTimeUtils.kt
index e0c8c11..9d22cd8 100644
--- a/src/main/java/at/bitfire/ical4android/util/AndroidTimeUtils.kt
+++ b/src/main/java/at/bitfire/ical4android/util/AndroidTimeUtils.kt
@@ -56,11 +56,7 @@ object AndroidTimeUtils {
     fun androidifyTimeZone(date: DateProperty?) {
         if (DateUtils.isDateTime(date) && date?.isUtc == false) {
             val tzID = date.timeZone?.id
-            val bestMatchingTzId = DateUtils.findAndroidTimezoneID(tzID)
-            if (tzID != bestMatchingTzId) {
-                Ical4Android.log.warning("Android doesn't know time zone ${tzID ?: "(floating)"}, setting default time zone $bestMatchingTzId")
-                date.timeZone = DateUtils.ical4jTimeZone(bestMatchingTzId)
-            }
+            date.timeZone = bestMatchingTzId(tzID)
         }
     }
 
@@ -73,18 +69,35 @@ object AndroidTimeUtils {
      * @param dateList [DateListProperty] to validate. Values which are not DATE-TIME will be ignored.
      */
     fun androidifyTimeZone(dateList: DateListProperty) {
+        // periods (RDate only)
+        val periods = (dateList as? RDate)?.periods
+        if (periods != null && periods.size > 0 && !periods.isUtc) {
+            val tzID = periods.timeZone?.id
+
+            // Won't work until resolved in ical4j (https://github.com/ical4j/ical4j/discussions/568)
+            // DateListProperty.setTimeZone() does not set the timeZone property when the DateList has PERIODs
+            dateList.timeZone = bestMatchingTzId(tzID)
+
+            return //  RDate can only contain periods OR dates - not both, bail out fast
+        }
+
+        // date-times (RDate and ExDate)
         val dates = dateList.dates
-        if (dates.type == Value.DATE_TIME && !dates.isUtc) {
-            val tzID = dateList.dates.timeZone?.id
-            val bestMatchingTzId = DateUtils.findAndroidTimezoneID(tzID)
-            if (tzID != bestMatchingTzId) {
-                Ical4Android.log.warning("Android doesn't know time zone ${tzID ?: "(floating)"}, setting default time zone $bestMatchingTzId")
-                dateList.timeZone = DateUtils.ical4jTimeZone(bestMatchingTzId)
+        if (dates != null && dates.size > 0) {
+            if (dates.type == Value.DATE_TIME && !dates.isUtc) {
+                val tzID = dates.timeZone?.id
+                dateList.timeZone = bestMatchingTzId(tzID)
             }
+        }
+    }
 
-            // keep the time zone of dateList in sync with the actual dates
-            if (dateList.timeZone != dates.timeZone)
-                dateList.timeZone = dates.timeZone
+    private fun bestMatchingTzId(tzID: String?): TimeZone? {
+        val bestMatchingTzId = DateUtils.findAndroidTimezoneID(tzID)
+        return if (tzID == bestMatchingTzId) {
+            DateUtils.ical4jTimeZone(tzID)
+        } else {
+            Ical4Android.log.warning("Android doesn't know time zone ${tzID ?: "\"null\" (floating)"}, setting default time zone $bestMatchingTzId")
+            DateUtils.ical4jTimeZone(bestMatchingTzId)
         }
     }
 
@@ -122,6 +135,7 @@ object AndroidTimeUtils {
     /**
      * Concatenates, if necessary, multiple RDATE/EXDATE lists and converts them to
      * a formatted string which Android calendar provider can process.
+     *
      * Android expects this format: "[TZID;]date1,date2,date3" where date is "yyyymmddThhmmss" (when
      * TZID is given) or "yyyymmddThhmmssZ". We don't use the TZID format here because then we're limited
      * to one time-zone, while an iCalendar may contain multiple EXDATE/RDATE lines with different time zones.
@@ -140,15 +154,15 @@ object AndroidTimeUtils {
         val strDates = LinkedList<String>()
 
         // use time zone of first entry for the whole set; null for UTC
-        val tz = dates.firstOrNull()?.dates?.timeZone
+        val tz =
+            (dates.firstOrNull() as? RDate)?.periods?.timeZone ?:   // VALUE=PERIOD (only RDate)
+            dates.firstOrNull()?.dates?.timeZone                    // VALUE=DATE/DATE-TIME
 
         for (dateListProp in dates) {
-            if (dateListProp is RDate)
-                if (dateListProp.periods.isNotEmpty())
-                    Ical4Android.log.warning("RDATE PERIOD not supported, ignoring")
-            else if (dateListProp is ExDate)
-                    if (dateListProp.periods.isNotEmpty())
-                        Ical4Android.log.warning("EXDATE PERIOD not supported, ignoring")
+            if (dateListProp is RDate && dateListProp.periods.isNotEmpty()) {
+                Ical4Android.log.warning("RDATE PERIOD not supported, ignoring")
+                break
+            }
 
             when (dateListProp.dates.type) {
                 Value.DATE_TIME -> {
@@ -172,9 +186,8 @@ object AndroidTimeUtils {
 
         // format: [tzid;]value1,value2,...
         val result = StringBuilder()
-        if (tz != null) {
+        if (tz != null)
             result.append(tz.id).append(RECURRENCE_LIST_TZID_SEPARATOR)
-        }
         result.append(strDates.joinToString(RECURRENCE_LIST_VALUE_SEPARATOR))
         return result.toString()
     }
@@ -277,7 +290,7 @@ object AndroidTimeUtils {
     }
 
 
-        // duration
+    // duration
 
     /**
      * Checks and fixes [Event.duration] values with incorrect format which can't be
diff --git a/src/test/java/at/bitfire/ical4android/AndroidTimeUtilsTest.kt b/src/test/java/at/bitfire/ical4android/AndroidTimeUtilsTest.kt
index fd8bca0..2c68b95 100644
--- a/src/test/java/at/bitfire/ical4android/AndroidTimeUtilsTest.kt
+++ b/src/test/java/at/bitfire/ical4android/AndroidTimeUtilsTest.kt
@@ -19,6 +19,7 @@ import net.fortuna.ical4j.model.property.RDate
 import net.fortuna.ical4j.util.TimeZones
 import org.junit.Assert.*
 import org.junit.Test
+import java.io.InputStreamReader
 import java.io.StringReader
 import java.time.Duration
 import java.time.Period
@@ -41,19 +42,23 @@ class AndroidTimeUtilsTest {
                 "END:STANDARD\n" +
                 "END:VTIMEZONE\n" +
                 "END:VCALENDAR"))
-        net.fortuna.ical4j.model.TimeZone(cal.getComponent(VTimeZone.VTIMEZONE) as VTimeZone)
+        TimeZone(cal.getComponent(VTimeZone.VTIMEZONE) as VTimeZone)
     }
 
     val tzIdDefault = java.util.TimeZone.getDefault().id
     val tzDefault = DateUtils.ical4jTimeZone(tzIdDefault)
 
+    // androidifyTimeZone
 
     @Test
-    fun testAndroidifyTimeZone_DateProperty_Null() {
+    fun testAndroidifyTimeZone_Null() {
         // must not throw an exception
         AndroidTimeUtils.androidifyTimeZone(null)
     }
 
+    // androidifyTimeZone
+    // DateProperty
+
     @Test
     fun testAndroidifyTimeZone_DateProperty_Date() {
         // dates (without time) should be ignored
@@ -104,6 +109,8 @@ class AndroidTimeUtilsTest {
         assertTrue(dtStart.isUtc)
     }
 
+    // androidifyTimeZone
+    // DateListProperty - date
 
     @Test
     fun testAndroidifyTimeZone_DateListProperty_Date() {
@@ -118,6 +125,9 @@ class AndroidTimeUtilsTest {
         assertFalse(rDate.dates.isUtc)
     }
 
+    // androidifyTimeZone
+    // DateListProperty - date-time
+
     @Test
     fun testAndroidifyTimeZone_DateListProperty_KnownTimeZone() {
         // times with known time zone should be unchanged
@@ -170,6 +180,81 @@ class AndroidTimeUtilsTest {
         assertTrue(rDate.dates.isUtc)
     }
 
+    // androidifyTimeZone
+    // DateListProperty - period-explicit
+
+    @Test
+    fun testAndroidifyTimeZone_DateListProperty_Period_FloatingTime() {
+        // times with floating time should be treated as system default time zone
+        val rDate = RDate(PeriodList("19970101T180000/19970102T070000,20220103T000000/20220108T000000"))
+        AndroidTimeUtils.androidifyTimeZone(rDate)
+        assertEquals(
+            setOf(Period(DateTime("19970101T18000000"), DateTime("19970102T07000000")),
+                Period(DateTime("20220103T000000"), DateTime("20220108T000000"))),
+            rDate.periods)
+        assertNull(rDate.timeZone)
+        assertNull(rDate.periods.timeZone)
+        assertTrue(rDate.periods.isUtc)
+    }
+
+    @Test
+    fun testAndroidifyTimeZone_DateListProperty_Period_KnownTimezone() {
+        // periods with known time zone should be unchanged
+        val rDate = RDate(PeriodList("19970101T180000/19970102T070000,19970102T180000/19970108T090000"))
+        rDate.periods.timeZone = tzToronto
+        AndroidTimeUtils.androidifyTimeZone(rDate)
+        assertEquals(
+            setOf(Period("19970101T180000/19970102T070000"), Period("19970102T180000/19970108T090000")),
+            mutableSetOf<net.fortuna.ical4j.model.Period>().also { it.addAll(rDate.periods) }
+        )
+        assertEquals(tzToronto, rDate.periods.timeZone)
+        assertNull(rDate.timeZone)
+        assertFalse(rDate.dates.isUtc)
+    }
+
+    @Test
+    fun testAndroidifyTimeZone_DateListProperty_Periods_UnknownTimeZone() {
+        // time zone that is not available on Android systems should be rewritten to system default
+        val rDate = RDate(PeriodList("19970101T180000/19970102T070000,19970102T180000/19970108T090000"))
+        rDate.periods.timeZone = tzCustom
+        AndroidTimeUtils.androidifyTimeZone(rDate)
+        assertEquals(
+            setOf(Period("19970101T180000/19970102T070000"), Period("19970102T180000/19970108T090000")),
+            mutableSetOf<net.fortuna.ical4j.model.Period>().also { it.addAll(rDate.periods) }
+        )
+        assertEquals(tzIdDefault, rDate.periods.timeZone.id)
+        assertNull(rDate.timeZone)
+        assertFalse(rDate.dates.isUtc)
+    }
+
+    @Test
+    fun testAndroidifyTimeZone_DateListProperty_Period_UTC() {
+        // times with UTC should be unchanged
+        val rDate = RDate(PeriodList("19970101T180000Z/19970102T070000Z,20220103T0000Z/20220108T0000Z"))
+        AndroidTimeUtils.androidifyTimeZone(rDate)
+        assertEquals(
+            setOf(Period(DateTime("19970101T180000Z"), DateTime("19970102T070000Z")),
+                Period(DateTime("20220103T0000Z"), DateTime("20220108T0000Z"))),
+            rDate.periods)
+        assertTrue(rDate.periods.isUtc)
+    }
+
+    // androidifyTimeZone
+    // DateListProperty - period-start
+
+    @Test
+    fun testAndroidifyTimeZone_DateListProperty_PeriodStart_UTC() {
+        // times with UTC should be unchanged
+        val rDate = RDate(PeriodList("19970101T180000Z/PT5H30M,20220103T0000Z/PT2H30M10S"))
+        AndroidTimeUtils.androidifyTimeZone(rDate)
+        assertEquals(
+            setOf(Period(DateTime("19970101T180000Z"), Duration.parse("PT5H30M")),
+                Period(DateTime("20220103T0000Z"), Duration.parse("PT2H30M10S"))),
+            rDate.periods)
+        assertTrue(rDate.periods.isUtc)
+    }
+
+    // storageTzId
 
     @Test
     fun testStorageTzId_Date() =
@@ -189,14 +274,14 @@ class AndroidTimeUtilsTest {
     }
 
 
-    // recurrence sets
+    // androidStringToRecurrenceSets
 
     @Test
     fun testAndroidStringToRecurrenceSets_UtcTimes() {
         // list of UTC times
-        var exDate = AndroidTimeUtils.androidStringToRecurrenceSet("20150101T103010Z,20150702T103020Z", false) { ExDate(it) }
+        val exDate = AndroidTimeUtils.androidStringToRecurrenceSet("20150101T103010Z,20150702T103020Z", false) { ExDate(it) }
         assertNull(exDate.timeZone)
-        var exDates = exDate.dates
+        val exDates = exDate.dates
         assertEquals(Value.DATE_TIME, exDates.type)
         assertTrue(exDates.isUtc)
         assertEquals(2, exDates.size)
@@ -235,11 +320,31 @@ class AndroidTimeUtilsTest {
         assertEquals(0, exDate.dates.size)
     }
 
+    // recurrenceSetsToAndroidString
+
     @Test
-    fun testRecurrenceSetsToAndroidString_UtcTime() {
+    fun testRecurrenceSetsToAndroidString_Date() {
+        // DATEs (without time) have to be converted to <date>T000000Z for Android
         val list = ArrayList<DateListProperty>(1)
-        list.add(RDate(DateList("20150101T103010Z,20150102T103020Z", Value.DATE_TIME)))
-        assertEquals("20150101T103010Z,20150102T103020Z", AndroidTimeUtils.recurrenceSetsToAndroidString(list, false))
+        list.add(RDate(DateList("20150101,20150702", Value.DATE)))
+        assertEquals("20150101T000000Z,20150702T000000Z", AndroidTimeUtils.recurrenceSetsToAndroidString(list, true))
+    }
+
+    @Test
+    fun testRecurrenceSetsToAndroidString_Period() {
+        // PERIODs are not supported yet — should be implemented later
+        val list = listOf(
+            RDate(PeriodList("19960403T020000Z/19960403T040000Z,19960404T010000Z/PT3H"))
+        )
+        assertEquals("", AndroidTimeUtils.recurrenceSetsToAndroidString(list, false))
+    }
+
+    @Test
+    fun testRecurrenceSetsToAndroidString_TimeAlthoughAllDay() {
+        // DATE-TIME (floating time or UTC) recurrences for all-day events have to converted to <date>T000000Z for Android
+        val list = ArrayList<DateListProperty>(1)
+        list.add(RDate(DateList("20150101T000000,20150702T000000Z", Value.DATE_TIME)))
+        assertEquals("20150101T000000Z,20150702T000000Z", AndroidTimeUtils.recurrenceSetsToAndroidString(list, true))
     }
 
     @Test
@@ -274,20 +379,14 @@ class AndroidTimeUtilsTest {
     }
 
     @Test
-    fun testRecurrenceSetsToAndroidString_Date() {
-        // DATEs (without time) have to be converted to <date>T000000Z for Android
+    fun testRecurrenceSetsToAndroidString_UtcTime() {
         val list = ArrayList<DateListProperty>(1)
-        list.add(RDate(DateList("20150101,20150702", Value.DATE)))
-        assertEquals("20150101T000000Z,20150702T000000Z", AndroidTimeUtils.recurrenceSetsToAndroidString(list, true))
+        list.add(RDate(DateList("20150101T103010Z,20150102T103020Z", Value.DATE_TIME)))
+        assertEquals("20150101T103010Z,20150102T103020Z", AndroidTimeUtils.recurrenceSetsToAndroidString(list, false))
     }
 
-    @Test
-    fun testRecurrenceSetsToAndroidString_TimeAlthoughAllDay() {
-        // DATE-TIME (floating time or UTC) recurrences for all-day events have to converted to <date>T000000Z for Android
-        val list = ArrayList<DateListProperty>(1)
-        list.add(RDate(DateList("20150101T000000,20150702T000000Z", Value.DATE_TIME)))
-        assertEquals("20150101T000000Z,20150702T000000Z", AndroidTimeUtils.recurrenceSetsToAndroidString(list, true))
-    }
+
+    // recurrenceSetsToOpenTasksString
 
     @Test
     fun testRecurrenceSetsToOpenTasksString_UtcTimes() {