Skip to content

Commit 017187c

Browse files
rfc2822rfc2822
authored
[Events] Adapt DURATION according to all-day flag before inserting (#145)
* Add tests to verify problem * Fix duration mapping for all-day events - Align DURATION with DTSTART type - Add tests for alignWithDtStart function * Comments * Fix test * Remove unnecessary lines in test
1 parent 730c3c5 commit 017187c

File tree

4 files changed

+147
-12
lines changed

4 files changed

+147
-12
lines changed

‎lib/src/androidTest/kotlin/at/bitfire/synctools/storage/calendar/AndroidCalendarProviderBehaviorTest.kt‎

Lines changed: 49 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import androidx.test.platform.app.InstrumentationRegistry
1818
import androidx.test.rule.GrantPermissionRule
1919
import at.bitfire.ical4android.impl.TestCalendar
2020
import at.bitfire.ical4android.util.MiscUtils.closeCompat
21+
import at.bitfire.synctools.storage.LocalStorageException
2122
import at.bitfire.synctools.test.assertContentValuesEqual
2223
import org.junit.After
2324
import org.junit.Before
@@ -33,7 +34,7 @@ class AndroidCalendarProviderBehaviorTest {
3334
val permissonRule = GrantPermissionRule.grant(
3435
Manifest.permission.READ_CALENDAR,
3536
Manifest.permission.WRITE_CALENDAR
36-
)
37+
)!!
3738

3839
private val testAccount = Account(javaClass.name, ACCOUNT_TYPE_LOCAL)
3940

@@ -56,14 +57,55 @@ class AndroidCalendarProviderBehaviorTest {
5657
}
5758

5859

60+
/**
61+
* To verify that it's a problem to insert a recurring all-day event with a duration of zero seconds.
62+
* See:
63+
*
64+
* - https://github.com/bitfireAT/davx5-ose/issues/1823
65+
* - https://github.com/bitfireAT/synctools/issues/144
66+
*/
67+
@Test(expected = LocalStorageException::class)
68+
fun testInsertRecurringAllDayEventWithDurationZeroSeconds() {
69+
val values = contentValuesOf(
70+
Events.CALENDAR_ID to calendar.id,
71+
Events.ALL_DAY to 1,
72+
Events.DTSTART to 1763510400000, // Wed Nov 19 2025 00:00:00 GMT+0000
73+
Events.DURATION to "PT0S",
74+
Events.TITLE to "Recurring all-day event with zero seconds duration",
75+
Events.RRULE to "FREQ=DAILY;UNTIL=20251122"
76+
)
77+
calendar.addEvent(Entity(values))
78+
}
79+
80+
/**
81+
* To make sure that it's not a problem to insert a recurring all-day event with a duration of zero days.
82+
*/
5983
@Test
60-
fun testInsertEventWithDurationZeroSeconds() {
61-
// To make sure that it's not a problem to insert a recurring event with a duration of zero seconds.
84+
fun testInsertRecurringAllDayEventWithDurationZeroDays() {
85+
val values = contentValuesOf(
86+
Events.CALENDAR_ID to calendar.id,
87+
Events.ALL_DAY to 1,
88+
Events.DTSTART to 1763510400000, // Wed Nov 19 2025 00:00:00 GMT+0000
89+
Events.DURATION to "P0D",
90+
Events.TITLE to "Recurring all-day event with zero seconds duration",
91+
Events.RRULE to "FREQ=DAILY;UNTIL=20251122"
92+
)
93+
val id = calendar.addEvent(Entity(values))
94+
95+
val event2 = calendar.getEventRow(id)
96+
assertContentValuesEqual(values, event2!!, onlyFieldsInExpected = true)
97+
}
98+
99+
/**
100+
* To make sure that it's not a problem to insert a recurring event with a duration of zero seconds.
101+
*/
102+
@Test
103+
fun testInsertRecurringNonAllDayEventWithDurationZeroSeconds() {
62104
val values = contentValuesOf(
63105
Events.CALENDAR_ID to calendar.id,
64106
Events.DTSTART to 1759403653000, // Thu Oct 02 2025 11:14:13 GMT+0000
65107
Events.DURATION to "PT0S",
66-
Events.TITLE to "Event with useless RRULE",
108+
Events.TITLE to "Recurring non-all-day event with zero seconds duration",
67109
Events.RRULE to "FREQ=DAILY;UNTIL=20251002T000000Z"
68110
)
69111
val id = calendar.addEvent(Entity(values))
@@ -72,9 +114,11 @@ class AndroidCalendarProviderBehaviorTest {
72114
assertContentValuesEqual(values, event2!!, onlyFieldsInExpected = true)
73115
}
74116

117+
/**
118+
* To make sure that's not a problem to insert an (invalid/useless) RRULE with UNTIL before the event's DTSTART.
119+
*/
75120
@Test
76121
fun testInsertEventWithRRuleUntilBeforeDtStart() {
77-
// To make sure that's not a problem to insert an (invalid/useless) RRULE with UNTIL before the event's DTSTART.
78122
val values = contentValuesOf(
79123
Events.CALENDAR_ID to calendar.id,
80124
Events.DTSTART to 1759403653000, // Thu Oct 02 2025 11:14:13 GMT+0000

‎lib/src/main/kotlin/at/bitfire/ical4android/util/TimeApiExtensions.kt‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ object TimeApiExtensions {
154154
if (secs == 0L)
155155
return "PT0S"
156156

157-
var weeks = secs / SECONDS_PER_WEEK
157+
val weeks = secs / SECONDS_PER_WEEK
158158
secs -= weeks * SECONDS_PER_WEEK
159159

160160
var days = secs / SECONDS_PER_DAY
@@ -170,7 +170,6 @@ object TimeApiExtensions {
170170
return "P${weeks}W"
171171

172172
days += weeks * DAYS_PER_WEEK
173-
weeks = 0
174173

175174
if (days != 0L)
176175
builder.append("${days}D")

‎lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/DurationBuilder.kt‎

Lines changed: 48 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import android.content.Entity
1010
import android.provider.CalendarContract.Events
1111
import androidx.annotation.VisibleForTesting
1212
import at.bitfire.ical4android.util.DateUtils
13+
import at.bitfire.ical4android.util.TimeApiExtensions.toDuration
1314
import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate
1415
import at.bitfire.ical4android.util.TimeApiExtensions.toRfc5545Duration
1516
import at.bitfire.synctools.icalendar.requireDtStart
@@ -21,6 +22,7 @@ import net.fortuna.ical4j.model.property.Duration
2122
import net.fortuna.ical4j.model.property.RDate
2223
import net.fortuna.ical4j.model.property.RRule
2324
import java.time.Period
25+
import java.time.temporal.TemporalAmount
2426

2527
class DurationBuilder: AndroidEntityBuilder {
2628

@@ -48,15 +50,56 @@ class DurationBuilder: AndroidEntityBuilder {
4850
> When the "DURATION" property relates to a "DTSTART" property that is specified as a DATE value, then the
4951
>"DURATION" property MUST be specified as a "dur-day" or "dur-week" value.
5052
51-
The calendar provider automatically converts the DURATION of an all-day event to unit: days,
52-
so we don't have to take care of that. */
53+
The calendar provider theoretically converts the DURATION of an all-day event to unit "days",
54+
so we wouldn't have to take care of that. However it expects seconds to be in "P<n>S" format,
55+
whereas we provide an RFC 5545-compliant "PT<n>S", which causes the provider to crash:
56+
https://github.com/bitfireAT/synctools/issues/144. So we must convert it ourselves to be on the safe side. */
57+
val alignedDuration = alignWithDtStart(duration.duration, dtStart)
5358

54-
// TemporalAmount can have months and years, but the RFC 5545 must only contain weeks, days and time.
55-
// So we have to recalculate the months/years to days according to their position in the calendar.
56-
val durationStr = duration.duration.toRfc5545Duration(dtStart.date.toInstant())
59+
/* TemporalAmount can have months and years, but the RFC 5545 value must only contain weeks, days and time.
60+
So we have to recalculate the months/years to days according to their position in the calendar.
61+
62+
The calendar provider accepts every DURATION that `com.android.calendarcommon2.Duration` can parse,
63+
which is weeks, days, hours, minutes and seconds, like for the RFC 5545 duration. */
64+
val durationStr = alignedDuration.toRfc5545Duration(dtStart.date.toInstant())
5765
values.put(Events.DURATION, durationStr)
5866
}
5967

68+
/**
69+
* Aligns the given temporal amount (taken from DURATION) to the VALUE-type (DATE-TIME/DATE) of DTSTART.
70+
*
71+
* @param amount temporal amount that shall be aligned
72+
* @param dtStart DTSTART to compare with
73+
*
74+
* @return Temporal amount that is
75+
*
76+
* - a [Period] (days/months/years that can't be represented by an exact number of seconds) when [dtStart] is a DATE, and
77+
* - a [java.time.Duration] (exact time that can be represented by an exact number of seconds) when [dtStart] is a DATE-TIME.
78+
*/
79+
@VisibleForTesting
80+
internal fun alignWithDtStart(amount: TemporalAmount, dtStart: DtStart): TemporalAmount {
81+
if (DateUtils.isDate(dtStart)) {
82+
// DTSTART is DATE
83+
return if (amount is java.time.Duration) {
84+
// amount is Duration, change to Period of days instead
85+
Period.ofDays(amount.toDays().toInt())
86+
} else {
87+
// amount is already Period
88+
amount
89+
}
90+
91+
} else {
92+
// DTSTART is DATE-TIME
93+
return if (amount is java.time.Period) {
94+
// amount is Period, change to Duration instead
95+
amount.toDuration(dtStart.date.toInstant())
96+
} else {
97+
// amount is already Duration
98+
amount
99+
}
100+
}
101+
}
102+
60103
@VisibleForTesting
61104
internal fun calculateFromDtEnd(dtStart: DtStart, dtEnd: DtEnd?): Duration? {
62105
if (dtEnd == null)

‎lib/src/test/kotlin/at/bitfire/synctools/mapping/calendar/builder/DurationBuilderTest.kt‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,18 @@ class DurationBuilderTest {
7171
assertEquals("P3D", result.entityValues.get(Events.DURATION))
7272
}
7373

74+
@Test
75+
fun `Recurring all-day event (with zero seconds DURATION)`() {
76+
val result = Entity(ContentValues())
77+
val event = VEvent(propertyListOf(
78+
DtStart(Date("20251010")),
79+
Duration(java.time.Duration.ofSeconds(0)),
80+
RRule("FREQ=DAILY;COUNT=5")
81+
))
82+
builder.build(event, event, result)
83+
assertEquals("P0D", result.entityValues.get(Events.DURATION))
84+
}
85+
7486
@Test
7587
fun `Recurring non-all-day event (with DURATION)`() {
7688
val result = Entity(ContentValues())
@@ -130,6 +142,43 @@ class DurationBuilderTest {
130142
}
131143

132144

145+
// alignWithDtStart
146+
147+
@Test
148+
fun `alignWithDtStart (DTSTART all-day, DURATION all-day)`() {
149+
assertEquals(
150+
Period.ofDays(1), // may not be 24 hours (for instance on DST switch)
151+
builder.alignWithDtStart(Period.ofDays(1), DtStart(Date()))
152+
)
153+
}
154+
155+
@Test
156+
fun `alignWithDtStart (DTSTART non-all-day, DURATION all-day)`() {
157+
assertEquals(
158+
java.time.Duration.ofDays(1), // exactly 24 hours
159+
builder.alignWithDtStart(Period.ofDays(1), DtStart(DateTime()))
160+
)
161+
}
162+
163+
@Test
164+
fun `alignWithDtStart (DTSTART all-day, DURATION non-all-day)`() {
165+
assertEquals(
166+
Period.ofDays(1), // may not be 24 hours (for instance on DST switch)
167+
builder.alignWithDtStart(java.time.Duration.ofHours(25), DtStart(Date()))
168+
)
169+
}
170+
171+
@Test
172+
fun `alignWithDtStart (DTSTART non-all-day, DURATION non-all-day)`() {
173+
assertEquals(
174+
java.time.Duration.ofDays(1), // exactly 24 hours
175+
builder.alignWithDtStart(java.time.Duration.ofHours(24), DtStart(DateTime()))
176+
)
177+
}
178+
179+
180+
// calculateFromDtEnd
181+
133182
@Test
134183
fun `calculateFromDtEnd (dtStart=DATE, DtEnd=DATE)`() {
135184
val result = builder.calculateFromDtEnd(

0 commit comments

Comments
 (0)