Handle invalid events with DTEND < DTSTART / negative DURATION #148
Conversation
- Add validation to ensure DTEND is after DTSTART - Update tests to cover cases where DTEND is before DTSTART
- Add tests for events with DTEND before DTSTART and negative DURATION - Update DurationBuilder to ignore negative durations - Update EndTimeBuilder to ignore DTEND before DTSTART
rfc2822
force-pushed
the
147-handle-invalid-events-with-negative-duration
branch
from
c1e1c05 to
8061f2d
Compare
There was a problem hiding this comment.
Pull request overview
This PR handles invalid calendar events where DTEND is before DTSTART or DURATION is negative, ensuring RFC 5545 compliance. When such invalid values are encountered, the code now converts negative durations to positive values and ignores DTEND values that precede DTSTART, falling back to default durations.
- Added validation logic to reject DTEND < DTSTART and apply default durations
- Introduced
abs()extension function forTemporalAmountto convert negative durations to positive - Refactored duration calculation methods to work directly with
TemporalAmountinstead of wrapping inDurationproperty
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| EndTimeBuilder.kt | Added DTEND validation to ensure it's after DTSTART, applies abs() to negative DURATION values |
| DurationBuilder.kt | Simplified duration calculation to use TemporalAmount directly, applies abs() to negative durations |
| TimeApiExtensions.kt | Added abs() extension function for TemporalAmount (Duration and Period) |
| EndTimeBuilderTest.kt | Added tests for DTEND < DTSTART and negative DURATION scenarios |
| DurationBuilderTest.kt | Added tests for negative DURATION calculation, updated return type assertions |
| TimeApiExtensionsTest.kt | Added comprehensive tests for abs() function with positive and negative values |
| AndroidCalendarProviderBehaviorTest.kt | Reordered test for DTEND = DTSTART and RRULE UNTIL before DTSTART |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/EndTimeBuilder.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/ical4android/util/TimeApiExtensions.kt
Outdated
Show resolved
Hide resolved
…nvalid TemporalAmount - Fix typo in EndTimeBuilder.kt comment
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Always ignore explicit DTEND<DTSTART |
rfc2822
force-pushed
the
147-handle-invalid-events-with-negative-duration
branch
from
f15306f to
fc9e10f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/src/test/kotlin/at/bitfire/synctools/mapping/calendar/builder/EndTimeBuilderTest.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/DurationBuilder.kt
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/EndTimeBuilder.kt
Show resolved
Hide resolved
ArnyminerZ
left a comment
ArnyminerZ
left a comment
There was a problem hiding this comment.
Some small gimmicks. Otherwise looks good.
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/DurationBuilder.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/DurationBuilder.kt
Outdated
Show resolved
Hide resolved
lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/builder/DurationBuilder.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3 participants
Change builders from VEVENT:
DTEND <= DTSTARTwill now be ignored. For such events, the default duration will be applied (as ifDTENDwould be missing withoutDURATIONpresent).DURATIONvalues of events will be treated as positive, because negative durations are not allowed and don't make any sense (events can't go back in time). This only applies to the DTEND/DURATION of events (not reminders etc., where negative durations are a valid case.)