From 0318d9729098b8528b06a36de3c9a50cdbd80a47 Mon Sep 17 00:00:00 2001 From: maltaisn Date: Thu, 6 Aug 2020 13:49:27 -0400 Subject: [PATCH] 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 | 5 +- lib/build.gradle | 3 +- .../com/maltaisn/recurpicker/Recurrence.kt | 3 + .../recurpicker/format/RRuleFormatter.kt | 86 +++++++++++++++---- .../recurpicker/RecurrenceFinderTest.kt | 20 +++++ .../recurpicker/TestDateExtensions.kt | 32 ++++--- ...uleFormatTest.kt => RRuleFormatterTest.kt} | 62 ++++++++++--- 7 files changed, 168 insertions(+), 43 deletions(-) rename lib/src/test/kotlin/com/maltaisn/recurpicker/format/{RRuleFormatTest.kt => RRuleFormatterTest.kt} (73%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ccfaa5..004b252 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/lib/build.gradle b/lib/build.gradle index b6d711c..518ccdf 100644 --- a/lib/build.gradle +++ b/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 { diff --git a/lib/src/main/kotlin/com/maltaisn/recurpicker/Recurrence.kt b/lib/src/main/kotlin/com/maltaisn/recurpicker/Recurrence.kt index 0c0f320..578df27 100644 --- a/lib/src/main/kotlin/com/maltaisn/recurpicker/Recurrence.kt +++ b/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, diff --git a/lib/src/main/kotlin/com/maltaisn/recurpicker/format/RRuleFormatter.kt b/lib/src/main/kotlin/com/maltaisn/recurpicker/format/RRuleFormatter.kt index 853294e..aa2b219 100644 --- a/lib/src/main/kotlin/com/maltaisn/recurpicker/format/RRuleFormatter.kt +++ b/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,20 +52,20 @@ 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) @@ -63,21 +73,22 @@ class RRuleFormatter { 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): 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) { 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()) } } diff --git a/lib/src/test/kotlin/com/maltaisn/recurpicker/RecurrenceFinderTest.kt b/lib/src/test/kotlin/com/maltaisn/recurpicker/RecurrenceFinderTest.kt index 35735b1..a0ce526 100644 --- a/lib/src/test/kotlin/com/maltaisn/recurpicker/RecurrenceFinderTest.kt +++ b/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(), 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) diff --git a/lib/src/test/kotlin/com/maltaisn/recurpicker/TestDateExtensions.kt b/lib/src/test/kotlin/com/maltaisn/recurpicker/TestDateExtensions.kt index a4389d0..a709f68 100644 --- a/lib/src/test/kotlin/com/maltaisn/recurpicker/TestDateExtensions.kt +++ b/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") diff --git a/lib/src/test/kotlin/com/maltaisn/recurpicker/format/RRuleFormatTest.kt b/lib/src/test/kotlin/com/maltaisn/recurpicker/format/RRuleFormatterTest.kt similarity index 73% rename from lib/src/test/kotlin/com/maltaisn/recurpicker/format/RRuleFormatTest.kt rename to lib/src/test/kotlin/com/maltaisn/recurpicker/format/RRuleFormatterTest.kt index a36be30..fa73fe8 100644 --- a/lib/src/test/kotlin/com/maltaisn/recurpicker/format/RRuleFormatTest.kt +++ b/lib/src/test/kotlin/com/maltaisn/recurpicker/format/RRuleFormatterTest.kt @@ -20,10 +20,11 @@ import com.maltaisn.recurpicker.Recurrence import com.maltaisn.recurpicker.Recurrence.Period import com.maltaisn.recurpicker.dateFor import org.junit.Test +import java.util.TimeZone import kotlin.test.assertEquals import kotlin.test.assertFailsWith -internal class RRuleFormatTest { +internal class RRuleFormatterTest { private val formatter = RRuleFormatter() @@ -113,11 +114,21 @@ internal class RRuleFormatTest { } @Test - fun `should test daily recurrence with end date`() { + fun `should test daily recurrence with end date, default timezone`() { val r = Recurrence(Period.DAILY) { - endDate = dateFor("2020-01-01") + endDate = dateFor("2020-01-01T00:00:00.000") } - testRRule(r, "RRULE:FREQ=DAILY;UNTIL=20200101T000000") + formatter.timeZone = TimeZone.getDefault() + testRRule(r, "RRULE:FREQ=DAILY;UNTIL=20200101") + } + + @Test + fun `should test daily recurrence with end date, GMT timezone`() { + val r = Recurrence(Period.DAILY) { + endDate = dateFor("2020-01-01T00:00:00.000Z") + } + formatter.timeZone = TimeZone.getTimeZone("GMT") + testRRule(r, "RRULE:FREQ=DAILY;UNTIL=20200101") } @Test @@ -128,48 +139,79 @@ internal class RRuleFormatTest { testRRule(r, "RRULE:FREQ=DAILY;COUNT=42") } + @Test + fun `should parse end date (date only)`() { + val r = Recurrence(Period.DAILY) { + endDate = dateFor("2020-01-01") + } + assertEquals(r, formatter.parse("RRULE:FREQ=DAILY;UNTIL=20200101")) + } + + @Test + fun `should parse end date (date time)`() { + val r = Recurrence(Period.DAILY) { + endDate = dateFor("2020-01-01T10:11:12.000") + } + assertEquals(r, formatter.parse("RRULE:FREQ=DAILY;UNTIL=20200101T101112")) + } + + @Test + fun `should parse end date (UTC date time)`() { + val r = Recurrence(Period.DAILY) { + endDate = dateFor("2020-01-01T10:11:12.000Z") + } + assertEquals(r, formatter.parse("RRULE:FREQ=DAILY;UNTIL=20200101T101112Z")) + } + @Test fun `should fail to parse rrule with missing signature`() { - assertFailsWith { + assertFailsWith { formatter.parse("FREQ=DAILY") } } @Test fun `should fail to parse rrule with no FREQ attribute set`() { - assertFailsWith { + assertFailsWith { formatter.parse("RRULE:BYDAY=FR,SA") } } @Test fun `should fail to parse rrule with invalid end date format`() { - assertFailsWith { + assertFailsWith { formatter.parse("RRULE:UNTIL=2020-01-01") } } @Test fun `should fail to parse rrule with unsupported FREQ attribute value`() { - assertFailsWith { + assertFailsWith { formatter.parse("RRULE:FREQ=HOURLY") } } @Test fun `should fail to parse rrule with invalid day of the week literals`() { - assertFailsWith { + assertFailsWith { formatter.parse("RRULE:FREQ=WEEKLY;BYDAY=SUN,MON,TUE") } } @Test fun `should fail to parse rrule with invalid BYDAY value`() { - assertFailsWith { + assertFailsWith { formatter.parse("RRULE:FREQ=MONTHLY;BYDAY=-1FRI") } } + @Test + fun `should fail to parse rrule with invalid number format`() { + assertFailsWith { + formatter.parse("RRULE:FREQ=DAILY;INTERVAL=foo") + } + } + private fun testRRule(r: Recurrence, rrule: String) { assertEquals(rrule, formatter.format(r)) assertEquals(r, formatter.parse(rrule))