Add RRule formatter timezone setting, RRuleParseException

- RRuleFormatter use date only format (time not added anymore)
- Fix RRuleFormatter not thread-safe
- Fix test date extension allowing invalid formats
- Add more RecurrenceFinder tests

CHANGELOG.md

@@ -1,9 +1,12 @@
 ### v2.1.2
-- Added support for changing time zone in `RecurrenceFinder`.
+- Added support for changing time zone in `RecurrenceFinder` and `RRuleFormatter`.
 - `RecurrenceFinder` now returns an empty list instead of an exception when trying to find 0 events.
 - Better `Recurrence.Builder` syntax when used from Java.
+- Changed date pattern for RRule to date only.
+
 - Fixed recurrence builder allowing creation of non-equal recurrences of period `NONE`, leading to equality issues.
 All recurrences built with `NONE` period now return the same `Recurrence.DOES_NOT_REPEAT` instance.
+- Fixed `RRuleFormatter` not thread-safe due to use of static date format for formatting and parsing.
 
 ### v2.1.1
 - `RecurrencePickerFragment` now handles back press by itself.

lib/build.gradle

@@ -34,7 +34,8 @@ android {
     kotlinOptions {
         jvmTarget = "1.8"
         freeCompilerArgs += [
-                "-Xopt-in=kotlin.ExperimentalStdlibApi"
+                "-Xopt-in=kotlin.ExperimentalStdlibApi",
+                "-Xuse-experimental=kotlin.contracts.ExperimentalContracts"
         ]
     }
     defaultConfig {

lib/src/main/kotlin/com/maltaisn/recurpicker/Recurrence.kt

@@ -29,6 +29,7 @@ import com.maltaisn.recurpicker.Recurrence.Period.MONTHLY
 import com.maltaisn.recurpicker.Recurrence.Period.NONE
 import com.maltaisn.recurpicker.Recurrence.Period.WEEKLY
 import com.maltaisn.recurpicker.Recurrence.Period.YEARLY
+import com.maltaisn.recurpicker.format.RRuleFormatter
 import com.maltaisn.recurpicker.format.RecurrenceFormatter
 import java.text.DateFormatSymbols
 import java.text.SimpleDateFormat
@@ -70,6 +71,8 @@ import kotlin.math.absoluteValue
  *
  * @property endDate The end date if end type is [EndType.BY_DATE]. If not, end date is
  * always [DATE_NONE]. The end date is inclusive so the last event might be on the end date.
+ * The date is given in UTC milliseconds since epoch time. The time of the day has no importance.
+ * To account for time zones, use [RecurrenceFinder.timeZone] and [RRuleFormatter.timeZone].
  *
  * @property endCount The number of events before the end of the recurrence if end type is
  * [EndType.BY_COUNT]. If not, end count is always `0`. Since the start date is exclusive,

lib/src/main/kotlin/com/maltaisn/recurpicker/format/RRuleFormatter.kt

@@ -22,6 +22,8 @@ import com.maltaisn.recurpicker.Recurrence.Period
 import java.text.SimpleDateFormat
 import java.util.Calendar
 import java.util.Locale
+import java.util.TimeZone
+import kotlin.contracts.contract
 
 /**
  * Utility class to write a [Recurrence] as a RRule and read it back.
@@ -33,6 +35,14 @@ import java.util.Locale
  */
 class RRuleFormatter {
 
+    /**
+     * The timezone used for formatting and parsing end dates (`UNTIL` attribute),
+     * unless the end date uses UTC time (time form #2, section 3.3.5).
+     */
+    var timeZone: TimeZone = TimeZone.getDefault()
+
+    private val dateFormat = SimpleDateFormat("", Locale.ROOT)
+
     /**
      * Parse a RFC 5545 string recurrence rule and return a recurrence.
      * Note that this method is designed to parse only the output of [format].
@@ -42,42 +52,43 @@ class RRuleFormatter {
      * yearly but not on the same day as start date, this information is lost when parsing,
      * since yearly recurrence can only happen on the same day as start date.
      *
-     * @throws IllegalArgumentException If recurrence rule cannot be parsed.
+     * @throws RRuleParseException If recurrence rule cannot be parsed.
      */
     fun parse(rrule: String): Recurrence {
-        require(rrule.startsWith(RRULE_SIGNATURE)) { "Recurrence rule string is invalid." }
+        parseError(rrule.startsWith(RRULE_SIGNATURE)) { "Recurrence rule string is invalid." }
 
         val attrs = rrule.substring(RRULE_SIGNATURE.length).split(';').associate {
             val pos = it.indexOf('=')
-            it.substring(0, pos) to it.substring(pos + 1)
+            it.substring(0, pos).toUpperCase(Locale.ROOT) to it.substring(pos + 1)
         }
 
         val period = parsePeriod(attrs)
 
-        return try {
-            Recurrence(period) {
+        return Recurrence(period) {
+            try {
                 frequency = attrs["INTERVAL"]?.toInt() ?: 1
                 if (period == Period.WEEKLY) {
                     parseWeeklyDetails(attrs)
                 } else if (period == Period.MONTHLY) {
                     parseMonthlyDetails(attrs)
                 }
                 parseEndTypeDetails(attrs)
+            } catch (e: NumberFormatException) {
+                parseError("Bad number format in recurrence rule.", e)
             }
-        } catch (e: NumberFormatException) {
-            throw IllegalArgumentException("Bad number format in recurrence rule.", e)
         }
     }
 
     private fun parsePeriod(attrs: Map<String, String>): Period {
-        val periodStr = requireNotNull(attrs["FREQ"]) { "Recurrence rule must specify period." }
+        val periodStr = attrs["FREQ"]
+        parseError(periodStr != null) { "Recurrence rule must specify period." }
         return when (periodStr) {
             "NONE" -> Period.NONE
             "DAILY" -> Period.DAILY
             "WEEKLY" -> Period.WEEKLY
             "MONTHLY" -> Period.MONTHLY
             "YEARLY" -> Period.YEARLY
-            else -> throw IllegalArgumentException("Unsupported recurrence period.") // Secondly, minutely, hourly
+            else -> parseError("Unsupported recurrence period.") // Secondly, minutely, hourly
         }
     }
 
@@ -88,7 +99,7 @@ class RRuleFormatter {
         if (daysAttr != null) {
             for (dayStr in daysAttr.split(',')) {
                 val index = BYDAY_VALUES.indexOf(dayStr)
-                require(index >= 0) { "Invalid day of week literal." }
+                parseError(index >= 0) { "Invalid day of week literal." }
                 days = days or (1 shl (index + 1))
             }
             setDaysOfWeek(days)
@@ -100,7 +111,7 @@ class RRuleFormatter {
         val byDay = attrs["BYDAY"]
         if (byDay != null) {
             val day = BYDAY_VALUES.indexOf(byDay.takeLast(2))
-            require(day >= 0) { "Invalid day of week literal." }
+            parseError(day >= 0) { "Invalid day of week literal." }
             val week = attrs["BYSETPOS"]?.toInt() ?: byDay.dropLast(2).toInt()
             setDayOfWeekInMonth(1 shl (day + 1), week)
         } else {
@@ -111,9 +122,7 @@ class RRuleFormatter {
     private fun Recurrence.Builder.parseEndTypeDetails(attrs: Map<String, String>) {
         val endDateStr = attrs["UNTIL"]
         if (endDateStr != null) {
-            endDate = requireNotNull(DATE_FORMAT.parse(endDateStr)) {
-                "Invalid end date format '$endDateStr'."
-            }.time
+            endDate = parseDate(endDateStr)
         } else {
             val endCountStr = attrs["COUNT"]
             if (endCountStr != null) {
@@ -122,6 +131,19 @@ class RRuleFormatter {
         }
     }
 
+    private fun parseDate(dateStr: String): Long {
+        for (pattern in DATE_PATTERNS) {
+            if (dateStr.length == pattern.length) {
+                if (dateFormat.toPattern() != pattern.pattern) {
+                    dateFormat.applyPattern(pattern.pattern)
+                }
+                dateFormat.timeZone = pattern.timeZone ?: timeZone
+                return dateFormat.parse(dateStr)?.time ?: continue
+            }
+        }
+        parseError("Invalid date format '$dateStr'.")
+    }
+
     /**
      * Format a [recurrence][r] to a string recurrence rule and return it.
      * Note that a valid RRule should technically include the DTSTART attribute, but since this attribute is not
@@ -197,7 +219,11 @@ class RRuleFormatter {
             EndType.NEVER -> Unit
             EndType.BY_DATE -> {
                 sb.append("UNTIL=")
-                sb.append(DATE_FORMAT.format(r.endDate))
+                if (dateFormat.toPattern() != FORMAT_DATE_PATTERN) {
+                    dateFormat.applyPattern(FORMAT_DATE_PATTERN)
+                }
+                dateFormat.timeZone = timeZone
+                sb.append(dateFormat.format(r.endDate))
                 sb.append(';')
             }
             EndType.BY_COUNT -> {
@@ -208,10 +234,36 @@ class RRuleFormatter {
         }
     }
 
+    private data class DatePattern(val pattern: String, val timeZone: TimeZone?, val length: Int)
+
     companion object {
+        private const val RRULE_SIGNATURE = "RRULE:"
+
+        private val DATE_PATTERNS = listOf(
+            DatePattern("yyyyMMdd", null, 8),
+            DatePattern("yyyyMMdd'T'HHmmss", null, 15),
+            DatePattern("yyyyMMdd'T'HHmmss'Z'", TimeZone.getTimeZone("GMT"), 16))
+
+        private const val FORMAT_DATE_PATTERN = "yyyyMMdd"
+
         private val BYDAY_VALUES = arrayOf("SU", "MO", "TU", "WE", "TH", "FR", "SA")
-        private val DATE_FORMAT = SimpleDateFormat("yyyyMMdd'T'HHmmss", Locale.ENGLISH)
+    }
+}
 
-        private const val RRULE_SIGNATURE = "RRULE:"
+/**
+ * Exception thrown by [RRuleFormatter.parse] when RRule couldn't be parsed.
+ */
+class RRuleParseException internal constructor(message: String, cause: Throwable? = null) :
+    IllegalArgumentException(message, cause)
+
+private fun parseError(message: String, cause: Throwable? = null): Nothing =
+    throw RRuleParseException(message, cause)
+
+private inline fun parseError(condition: Boolean, message: () -> String) {
+    contract {
+        returns() implies condition
+    }
+    if (!condition) {
+        throw RRuleParseException(message())
     }
 }

lib/src/test/kotlin/com/maltaisn/recurpicker/RecurrenceFinderTest.kt

@@ -625,6 +625,13 @@ internal class RecurrenceFinderTest {
             dateFor("2019-01-04"), dateFor("2019-01-10")))
     }
 
+    @Test
+    fun `should find events between two dates (no events)`() {
+        val r = Recurrence(Period.MONTHLY)
+        assertEquals(emptyList<Long>(), finder.findBetween(r, dateFor("2019-01-01"),
+            dateFor("2019-01-04"), dateFor("2019-01-10")))
+    }
+
     @Test
     fun `should find events for recurrence using non-default timezone`() {
         // This test should fail if not setting the timezone.
@@ -638,6 +645,19 @@ internal class RecurrenceFinderTest {
         ), finder.find(r, dateFor("2020-07-29T00:00:00.000+07:00"), 2))
     }
 
+    @Test
+    fun `should find events for recurrence until date with timezone`() {
+        finder.timeZone = TimeZone.getTimeZone("GMT-04:00")
+        val r = Recurrence(Period.DAILY) {
+            endDate = dateFor("2020-08-08T00:00:00.000+04:00")
+        }
+        assertEquals(listOf(
+            dateFor("2020-08-06T00:00:00.000+04:00"),
+            dateFor("2020-08-07T00:00:00.000+04:00"),
+            dateFor("2020-08-08T00:00:00.000+04:00")
+        ), finder.find(r, dateFor("2020-08-06T00:00:00.000+04:00"), 1000))
+    }
+
     @Test
     fun `should find events for recurrence, keeping original time of the day`() {
         val r = Recurrence(Period.DAILY)

lib/src/test/kotlin/com/maltaisn/recurpicker/TestDateExtensions.kt

@@ -20,13 +20,15 @@ import java.text.ParseException
 import java.text.SimpleDateFormat
 import java.util.TimeZone
 
-val patterns = listOf(
-    "yyyy-MM-dd'T'HH:mm:ss.SSSXXX" to TimeZone.getTimeZone("GMT"),
-    "yyyy-MM-dd'T'HH:mm:ss.SSS'Z'" to TimeZone.getTimeZone("GMT"),
-    "yyyy-MM-dd'T'HH:mm:ss.SSS" to TimeZone.getDefault(),
-    "yyyy-MM-dd" to TimeZone.getDefault()
+val datePatterns = listOf(
+    DatePattern("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'", TimeZone.getTimeZone("GMT"), 24),
+    DatePattern("yyyy-MM-dd'T'HH:mm:ss.SSSXXX", null, null),
+    DatePattern("yyyy-MM-dd'T'HH:mm:ss.SSS", TimeZone.getDefault(), 23),
+    DatePattern("yyyy-MM-dd", TimeZone.getDefault(), 10)
 )
 
+data class DatePattern(val pattern: String, val timeZone: TimeZone?, val length: Int?)
+
 /**
  * Get UTC millis since epoch time for date patterns:
  * - `2020-01-05`: in UTC time zone, time is set to 00:00:00.000.
@@ -37,15 +39,17 @@ val patterns = listOf(
  */
 internal fun dateFor(date: String): Long {
     val dateFormat = SimpleDateFormat()
-    for ((pattern, timeZone) in patterns) {
-        if (timeZone != null) {
-            dateFormat.timeZone = timeZone
-        }
-        dateFormat.applyPattern(pattern)
-        return try {
-            dateFormat.parse(date)?.time ?: continue
-        } catch (e: ParseException) {
-            continue
+    for (pattern in datePatterns) {
+        if (pattern.length == null || date.length == pattern.length) {
+            if (pattern.timeZone != null) {
+                dateFormat.timeZone = pattern.timeZone
+            }
+            dateFormat.applyPattern(pattern.pattern)
+            return try {
+                dateFormat.parse(date)?.time
+            } catch (e: ParseException) {
+                null
+            } ?: continue
         }
     }
     throw IllegalArgumentException("Invalid date literal")