Skip to content

Commit c1e1c05

Browse files
rfc2822rfc2822
committed
Take absolute amount of negative durations
1 parent 858c706 commit c1e1c05

File tree

6 files changed

+93
-43
lines changed

6 files changed

+93
-43
lines changed

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,22 @@ object TimeApiExtensions {
112112

113113
/***** Durations *****/
114114

115+
/**
116+
* Returns the absolute (positive) temporal amount.
117+
*/
118+
fun TemporalAmount.abs(): TemporalAmount =
119+
when (this) {
120+
is Duration ->
121+
this.abs()
122+
is Period ->
123+
if (this.isNegative)
124+
this.negated()
125+
else
126+
this
127+
else ->
128+
this // never called
129+
}
130+
115131
fun TemporalAmount.toDuration(position: Instant): Duration =
116132
when (this) {
117133
is Duration -> this

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

Lines changed: 14 additions & 26 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.abs
1314
import at.bitfire.ical4android.util.TimeApiExtensions.toDuration
1415
import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate
1516
import at.bitfire.ical4android.util.TimeApiExtensions.toRfc5545Duration
@@ -18,7 +19,6 @@ import net.fortuna.ical4j.model.Property
1819
import net.fortuna.ical4j.model.component.VEvent
1920
import net.fortuna.ical4j.model.property.DtEnd
2021
import net.fortuna.ical4j.model.property.DtStart
21-
import net.fortuna.ical4j.model.property.Duration
2222
import net.fortuna.ical4j.model.property.RDate
2323
import net.fortuna.ical4j.model.property.RRule
2424
import java.time.Period
@@ -44,21 +44,11 @@ class DurationBuilder: AndroidEntityBuilder {
4444
val dtStart = from.requireDtStart()
4545

4646
// calculate DURATION from DTEND - DTSTART, if necessary
47-
val calculatedDuration = from.duration
47+
val calculatedDuration = from.duration?.duration
4848
?: calculateFromDtEnd(dtStart, from.endDate)
4949

50-
// ignore potentially negative duration and use default duration, if necessary
51-
val duration = calculatedDuration
52-
.takeUnless { // ignore negative duration
53-
when (val d = it?.duration) {
54-
is Period ->
55-
d.isNegative
56-
is java.time.Duration ->
57-
d.isNegative
58-
else ->
59-
/* never used */ false
60-
}
61-
}
50+
// always use positive duration; use default duration, if necessary
51+
val duration = calculatedDuration?.abs()
6252
?: defaultDuration(DateUtils.isDate(dtStart))
6353

6454
/* [RFC 5545 3.8.2.5]
@@ -69,7 +59,7 @@ class DurationBuilder: AndroidEntityBuilder {
6959
so we wouldn't have to take care of that. However it expects seconds to be in "P<n>S" format,
7060
whereas we provide an RFC 5545-compliant "PT<n>S", which causes the provider to crash:
7161
https://github.com/bitfireAT/synctools/issues/144. So we must convert it ourselves to be on the safe side. */
72-
val alignedDuration = alignWithDtStart(duration.duration, dtStart)
62+
val alignedDuration = alignWithDtStart(duration, dtStart)
7363

7464
/* TemporalAmount can have months and years, but the RFC 5545 value must only contain weeks, days and time.
7565
So we have to recalculate the months/years to days according to their position in the calendar.
@@ -105,7 +95,7 @@ class DurationBuilder: AndroidEntityBuilder {
10595

10696
} else {
10797
// DTSTART is DATE-TIME
108-
return if (amount is java.time.Period) {
98+
return if (amount is Period) {
10999
// amount is Period, change to Duration instead
110100
amount.toDuration(dtStart.date.toInstant())
111101
} else {
@@ -116,31 +106,29 @@ class DurationBuilder: AndroidEntityBuilder {
116106
}
117107

118108
@VisibleForTesting
119-
internal fun calculateFromDtEnd(dtStart: DtStart, dtEnd: DtEnd?): Duration? {
109+
internal fun calculateFromDtEnd(dtStart: DtStart, dtEnd: DtEnd?): TemporalAmount? {
120110
if (dtEnd == null)
121111
return null
122112

123113
return if (DateUtils.isDateTime(dtStart) && DateUtils.isDateTime(dtEnd)) {
124114
// DTSTART and DTEND are DATE-TIME → calculate difference between timestamps
125115
val seconds = (dtEnd.date.time - dtStart.date.time) / 1000
126-
Duration(java.time.Duration.ofSeconds(seconds))
116+
java.time.Duration.ofSeconds(seconds)
127117
} else {
128118
// Either DTSTART or DTEND or both are DATE:
129119
// - DTSTART and DTEND are DATE → DURATION is exact number of days (no time part)
130120
// - DTSTART is DATE, DTEND is DATE-TIME → only use date part of DTEND → DURATION is exact number of days (no time part)
131121
// - DTSTART is DATE-TIME, DTEND is DATE → amend DTEND with time of DTSTART → DURATION is exact number of days (no time part)
132122
val startDate = dtStart.date.toLocalDate()
133123
val endDate = dtEnd.date.toLocalDate()
134-
Duration(Period.between(startDate, endDate))
124+
Period.between(startDate, endDate)
135125
}
136126
}
137127

138-
private fun defaultDuration(allDay: Boolean): Duration =
139-
Duration(
140-
if (allDay)
141-
Period.ofDays(1)
142-
else
143-
java.time.Duration.ZERO
144-
)
128+
private fun defaultDuration(allDay: Boolean): TemporalAmount =
129+
if (allDay)
130+
Period.ofDays(1)
131+
else
132+
java.time.Duration.ZERO
145133

146134
}

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

Lines changed: 5 additions & 2 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.abs
1314
import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDate
1415
import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDateTime
1516
import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate
@@ -49,7 +50,8 @@ class EndTimeBuilder: AndroidEntityBuilder {
4950
val dtStart = from.requireDtStart()
5051

5152
// potentially calculate DTEND from DTSTART + DURATION, and always align with DTSTART value type
52-
val calculatedDtEnd = from.endDate?.let { alignWithDtStart(it, dtStart = dtStart) }
53+
val calculatedDtEnd = from.getEndDate(/* deriveFromDuration = */ false)
54+
?.let { alignWithDtStart(it, dtStart = dtStart) }
5355
?: calculateFromDuration(dtStart, from.duration)
5456

5557
// ignore DTEND when not after DTSTART and use default duration, if necessary
@@ -138,7 +140,8 @@ class EndTimeBuilder: AndroidEntityBuilder {
138140
if (duration == null)
139141
return null
140142

141-
val dur = duration.duration
143+
val dur = duration.duration.abs() // always take positive temporal amount
144+
142145
return if (DateUtils.isDate(dtStart)) {
143146
// DTSTART is DATE
144147
if (dur is Period) {

‎lib/src/test/kotlin/at/bitfire/ical4android/util/TimeApiExtensionsTest.kt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
package at.bitfire.ical4android.util
88

9+
import at.bitfire.ical4android.util.TimeApiExtensions.abs
910
import at.bitfire.ical4android.util.TimeApiExtensions.requireTimeZone
1011
import at.bitfire.ical4android.util.TimeApiExtensions.toDuration
1112
import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDate
@@ -172,6 +173,39 @@ class TimeApiExtensionsTest {
172173
}
173174

174175

176+
@Test
177+
fun testTemporalAmount_abs_Duration_negative() {
178+
assertEquals(
179+
Duration.ofMinutes(1),
180+
Duration.ofMinutes(-1).abs()
181+
)
182+
}
183+
184+
@Test
185+
fun testTemporalAmount_abs_Duration_positive() {
186+
assertEquals(
187+
Duration.ofDays(1),
188+
Duration.ofDays(1).abs()
189+
)
190+
}
191+
192+
@Test
193+
fun testTemporalAmount_abs_Period_negative() {
194+
assertEquals(
195+
Period.ofWeeks(1),
196+
Period.ofWeeks(-1).abs()
197+
)
198+
}
199+
200+
@Test
201+
fun testTemporalAmount_abs_Period_positive() {
202+
assertEquals(
203+
Period.ofDays(1),
204+
Period.ofDays(1).abs()
205+
)
206+
}
207+
208+
175209
@Test
176210
fun testTemporalAmount_toDuration() {
177211
assertEquals(Duration.ofHours(1), Duration.ofHours(1).toDuration(Instant.EPOCH))

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

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -112,12 +112,11 @@ class DurationBuilderTest {
112112
val result = Entity(ContentValues())
113113
val event = VEvent(propertyListOf(
114114
DtStart(Date("20251017")),
115-
DtEnd(Date("20251010")), // before DTSTART, should be ignored
115+
DtEnd(Date("20251010")), // calculates to P-1W, which will be changed to P1W
116116
RRule("FREQ=DAILY;COUNT=5")
117117
))
118118
builder.build(event, event, result)
119-
// default duration for all-day events: one day
120-
assertEquals("P1D", result.entityValues.get(Events.DURATION))
119+
assertEquals("P1W", result.entityValues.get(Events.DURATION))
121120
}
122121

123122
@Test
@@ -137,12 +136,12 @@ class DurationBuilderTest {
137136
val result = Entity(ContentValues())
138137
val event = VEvent(propertyListOf(
139138
DtStart(DateTime("20251010T010203", tzVienna)),
140-
DtEnd(DateTime("20251009T010203", tzVienna)), // before DTSTART, should be ignored
139+
DtEnd(DateTime("20251010T000203", tzVienna)), // calculates to PT-1H, will be rewritten to PT1H
141140
RRule("FREQ=DAILY;COUNT=5")
142141
))
143142
builder.build(event, event, result)
144143
// default duration for non-all-day events: PT0S
145-
assertEquals("PT0S", result.entityValues.get(Events.DURATION))
144+
assertEquals("PT1H", result.entityValues.get(Events.DURATION))
146145
}
147146

148147
@Test
@@ -212,7 +211,7 @@ class DurationBuilderTest {
212211
DtEnd(Date("20240330"))
213212
)
214213
assertEquals(
215-
Duration(Period.ofDays(2)),
214+
Period.ofDays(2),
216215
result
217216
)
218217
}
@@ -224,7 +223,7 @@ class DurationBuilderTest {
224223
DtEnd(DateTime("20240330T123412", tzVienna))
225224
)
226225
assertEquals(
227-
Duration(Period.ofDays(2)),
226+
Period.ofDays(2),
228227
result
229228
)
230229
}
@@ -236,7 +235,7 @@ class DurationBuilderTest {
236235
DtEnd(Date("20240330"))
237236
)
238237
assertEquals(
239-
Duration(Period.ofDays(2)),
238+
Period.ofDays(2),
240239
result
241240
)
242241
}
@@ -248,7 +247,7 @@ class DurationBuilderTest {
248247
DtEnd(DateTime("20240728T010203Z")) // GMT+1 with DST → 2 hours difference
249248
)
250249
assertEquals(
251-
Duration(java.time.Duration.ofHours(2)),
250+
java.time.Duration.ofHours(2),
252251
result
253252
)
254253
}

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

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -133,11 +133,10 @@ class EndTimeBuilderTest {
133133
fun `Non-recurring all-day event (with negative DURATION)`() {
134134
val result = Entity(ContentValues())
135135
val event = VEvent(propertyListOf(
136-
DtStart(Date("20251012")),
137-
Duration(Period.ofDays(-3)) // invalid negative duration, should be ignored
136+
DtStart(Date("20251010")),
137+
Duration(Period.ofDays(-3)) // invalid negative DURATION will be treated as positive
138138
))
139139
builder.build(event, event, result)
140-
// default duration for all-day events: one day
141140
assertEquals(1760313600000, result.entityValues.get(Events.DTEND))
142141
}
143142

@@ -156,11 +155,10 @@ class EndTimeBuilderTest {
156155
fun `Non-recurring non-all-day event (with negative DURATION)`() {
157156
val result = Entity(ContentValues())
158157
val event = VEvent(propertyListOf(
159-
DtStart(DateTime("20251010T023203", tzVienna)),
160-
Duration(java.time.Duration.ofMinutes(-90)) // invalid negative duration, should be ignored
158+
DtStart(DateTime("20251010T010203", tzVienna)),
159+
Duration(java.time.Duration.ofMinutes(-90)) // invalid negative DURATION will be treated as positive
161160
))
162161
builder.build(event, event, result)
163-
// default duration for non-all-day events: PT0S
164162
assertEquals(1760056323000, result.entityValues.get(Events.DTEND))
165163
}
166164

@@ -290,4 +288,16 @@ class EndTimeBuilderTest {
290288
)
291289
}
292290

291+
@Test
292+
fun `calculateFromDuration (dtStart=DATE-TIME, duration is time-based and negative)`() {
293+
val result = builder.calculateFromDuration(
294+
DtStart(DateTime("20250101T045623", tzVienna)),
295+
Duration(null, "PT-25H")
296+
)
297+
assertEquals(
298+
DtEnd(DateTime("20250102T055623", tzVienna)),
299+
result
300+
)
301+
}
302+
293303
}

0 commit comments

Comments
 (0)