Always add DTEND to VEVENT for iCloud compatibility

lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/AndroidEventHandler.kt

@@ -17,7 +17,6 @@ import at.bitfire.synctools.mapping.calendar.handler.AvailabilityHandler
 import at.bitfire.synctools.mapping.calendar.handler.CategoriesHandler
 import at.bitfire.synctools.mapping.calendar.handler.ColorHandler
 import at.bitfire.synctools.mapping.calendar.handler.DescriptionHandler
-import at.bitfire.synctools.mapping.calendar.handler.DurationHandler
 import at.bitfire.synctools.mapping.calendar.handler.EndTimeHandler
 import at.bitfire.synctools.mapping.calendar.handler.LocationHandler
 import at.bitfire.synctools.mapping.calendar.handler.OrganizerHandler
@@ -67,7 +66,6 @@ class AndroidEventHandler(
         LocationHandler(),
         StartTimeHandler(tzRegistry),
         EndTimeHandler(tzRegistry),
-        DurationHandler(tzRegistry),
         RecurrenceFieldsHandler(tzRegistry),
         DescriptionHandler(),
         ColorHandler(),

lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/MappingUtil.kt

@@ -0,0 +1,96 @@
+/*
+ * This file is part of bitfireAT/synctools which is released under GPLv3.
+ * Copyright © All Contributors. See the LICENSE and AUTHOR files in the root directory for details.
+ * SPDX-License-Identifier: GPL-3.0-or-later
+ */
+
+package at.bitfire.synctools.mapping.calendar
+
+import androidx.annotation.VisibleForTesting
+import at.bitfire.ical4android.util.DateUtils
+import at.bitfire.ical4android.util.TimeApiExtensions.abs
+import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDate
+import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDateTime
+import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate
+import at.bitfire.ical4android.util.TimeApiExtensions.toZonedDateTime
+import net.fortuna.ical4j.model.DateTime
+import net.fortuna.ical4j.model.property.DtEnd
+import net.fortuna.ical4j.model.property.DtStart
+import java.time.Duration
+import java.time.LocalDate
+import java.time.Period
+import java.time.temporal.TemporalAmount
+
+/**
+ * Common methods mapping logic
+ */
+object MappingUtil {
+
+    /**
+     * Chooses a DTEND value for the content provider when the iCalendar doesn't have a DTEND.
+     *
+     * RFC 5545 says the following about empty DTEND values:
+     *
+     * > For cases where a "VEVENT" calendar component specifies a "DTSTART" property with a DATE value type but no
+     * > "DTEND" nor "DURATION" property, the event's duration is taken to be one day. For cases where a "VEVENT" calendar
+     * > component specifies a "DTSTART" property with a DATE-TIME value type but no "DTEND" property, the event
+     * > ends on the same calendar date and time of day specified by the "DTSTART" property.
+     *
+     * In iCalendar, `DTEND` is non-inclusive at must be at a later time than `DTEND`. However in Android we can use
+     * the same value for both the `DTSTART` and the `DTEND` field, and so we use this to indicate a missing DTEND in
+     * the original iCalendar.
+     *
+     * @param dtStart   start time to calculate end time from
+     * @return End time to use for content provider:
+     *
+     * - when [dtStart] is a `DATE`: [dtStart] + 1 day
+     * - when [dtStart] is a `DATE-TIME`: [dtStart]
+     */
+    fun dtEndFromDefault(dtStart: DtStart): DtEnd =
+        if (DateUtils.isDate(dtStart)) {
+            // DATE → one day duration
+            val endDate: LocalDate = dtStart.date.toLocalDate().plusDays(1)
+            DtEnd(endDate.toIcal4jDate())
+        } else {
+            // DATE-TIME → same as DTSTART to indicate there was no DTEND set
+            DtEnd(dtStart.value, dtStart.timeZone)
+        }
+
+    /**
+     * Calculates the DTEND from DTSTART + DURATION, if possible.
+     *
+     * @param dtStart   start date/date-time
+     * @param duration  (optional) duration
+     *
+     * @return end date/date-time (same value type as [dtStart]) or `null` if [duration] was not given
+     */
+    @VisibleForTesting
+    internal fun dtEndFromDuration(dtStart: DtStart, duration: TemporalAmount?): DtEnd? {
+        if (duration == null)
+            return null
+
+        val dur = duration.abs()   // always take positive temporal amount
+
+        return if (DateUtils.isDate(dtStart)) {
+            // DTSTART is DATE
+            if (dur is Period) {
+                // date-based amount of time ("4 days")
+                val result = dtStart.date.toLocalDate() + dur
+                DtEnd(result.toIcal4jDate())
+            } else if (dur is Duration) {
+                // time-based amount of time ("34 minutes")
+                val days = dur.toDays()
+                val result = dtStart.date.toLocalDate() + Period.ofDays(days.toInt())
+                DtEnd(result.toIcal4jDate())
+            } else
+                throw IllegalStateException()   // TemporalAmount neither Period nor Duration
+
+        } else {
+            // DTSTART is DATE-TIME
+            // We can add both date-based (Period) and time-based (Duration) amounts of time to an exact date/time.
+            val result = (dtStart.date as DateTime).toZonedDateTime() + dur
+            DtEnd(result.toIcal4jDateTime())
+        }
+    }
+
+}

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

@@ -10,12 +10,12 @@ import android.content.Entity
 import android.provider.CalendarContract.Events
 import androidx.annotation.VisibleForTesting
 import at.bitfire.ical4android.util.DateUtils
-import at.bitfire.ical4android.util.TimeApiExtensions.abs
 import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDate
 import at.bitfire.ical4android.util.TimeApiExtensions.toIcal4jDateTime
 import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate
 import at.bitfire.ical4android.util.TimeApiExtensions.toZonedDateTime
 import at.bitfire.synctools.icalendar.requireDtStart
+import at.bitfire.synctools.mapping.calendar.MappingUtil
 import at.bitfire.synctools.util.AndroidTimeUtils
 import net.fortuna.ical4j.model.DateTime
 import net.fortuna.ical4j.model.Property
@@ -24,12 +24,8 @@ import net.fortuna.ical4j.model.property.DtEnd
 import net.fortuna.ical4j.model.property.DtStart
 import net.fortuna.ical4j.model.property.RDate
 import net.fortuna.ical4j.model.property.RRule
-import java.time.Duration
-import java.time.LocalDate
-import java.time.Period
 import java.time.ZoneId
 import java.time.ZonedDateTime
-import java.time.temporal.TemporalAmount
 
 class EndTimeBuilder: AndroidEntityBuilder {
 
@@ -53,12 +49,12 @@ class EndTimeBuilder: AndroidEntityBuilder {
         // potentially calculate DTEND from DTSTART + DURATION, and always align with DTSTART value type
         val calculatedDtEnd = from.getEndDate(/* don't let ical4j calculate DTEND from DURATION */ false)
             ?.let { alignWithDtStart(it, dtStart = dtStart) }
-            ?: calculateFromDuration(dtStart, from.duration?.duration)
+            ?: MappingUtil.dtEndFromDuration(dtStart, from.duration?.duration)
 
         // ignore DTEND when not after DTSTART and use default duration, if necessary
         val dtEnd = calculatedDtEnd
             ?.takeIf { it.date.toInstant() > dtStart.date.toInstant() }     // only use DTEND if it's after DTSTART [1]
-            ?: calculateFromDefault(dtStart)
+            ?: MappingUtil.dtEndFromDefault(dtStart)
 
         /**
          * [1] RFC 5545 3.8.2.2 Date-Time End:
@@ -136,72 +132,4 @@ class EndTimeBuilder: AndroidEntityBuilder {
         }
     }
 
-    /**
-     * Calculates the DTEND from DTSTART + DURATION, if possible.
-     *
-     * @param dtStart   start date/date-time
-     * @param duration  (optional) duration
-     *
-     * @return end date/date-time (same value type as [dtStart]) or `null` if [duration] was not given
-     */
-    @VisibleForTesting
-    internal fun calculateFromDuration(dtStart: DtStart, duration: TemporalAmount?): DtEnd? {
-        if (duration == null)
-            return null
-
-        val dur = duration.abs()   // always take positive temporal amount
-
-        return if (DateUtils.isDate(dtStart)) {
-            // DTSTART is DATE
-            if (dur is Period) {
-                // date-based amount of time ("4 days")
-                val result = dtStart.date.toLocalDate() + dur
-                DtEnd(result.toIcal4jDate())
-            } else if (dur is Duration) {
-                // time-based amount of time ("34 minutes")
-                val days = dur.toDays()
-                val result = dtStart.date.toLocalDate() + Period.ofDays(days.toInt())
-                DtEnd(result.toIcal4jDate())
-            } else
-                throw IllegalStateException()   // TemporalAmount neither Period nor Duration
-
-        } else {
-            // DTSTART is DATE-TIME
-            // We can add both date-based (Period) and time-based (Duration) amounts of time to an exact date/time.
-            val result = (dtStart.date as DateTime).toZonedDateTime() + dur
-            DtEnd(result.toIcal4jDateTime())
-        }
-    }
-
-    /**
-     * Chooses a DTEND value for the content provider when the iCalendar doesn't have a DTEND.
-     *
-     * RFC 5545 says the following about empty DTEND values:
-     *
-     * > For cases where a "VEVENT" calendar component specifies a "DTSTART" property with a DATE value type but no
-     * > "DTEND" nor "DURATION" property, the event's duration is taken to be one day. For cases where a "VEVENT" calendar
-     * > component specifies a "DTSTART" property with a DATE-TIME value type but no "DTEND" property, the event
-     * > ends on the same calendar date and time of day specified by the "DTSTART" property.
-     *
-     * In iCalendar, `DTEND` is non-inclusive at must be at a later time than `DTEND`. However in Android we can use
-     * the same value for both the `DTSTART` and the `DTEND` field, and so we use this to indicate a missing DTEND in
-     * the original iCalendar.
-     *
-     * @param dtStart   start time to calculate end time from
-     * @return End time to use for content provider:
-     *
-     * - when [dtStart] is a `DATE`: [dtStart] + 1 day
-     * - when [dtStart] is a `DATE-TIME`: [dtStart]
-     */
-    @VisibleForTesting
-    internal fun calculateFromDefault(dtStart: DtStart): DtEnd =
-        if (DateUtils.isDate(dtStart)) {
-            // DATE → one day duration
-            val endDate: LocalDate = dtStart.date.toLocalDate().plusDays(1)
-            DtEnd(endDate.toIcal4jDate())
-        } else {
-            // DATE-TIME → same as DTSTART to indicate there was no DTEND set
-            DtEnd(dtStart.value, dtStart.timeZone)
-        }
-
 }

lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/handler/EndTimeHandler.kt

@@ -8,42 +8,57 @@ package at.bitfire.synctools.mapping.calendar.handler
 
 import android.content.Entity
 import android.provider.CalendarContract.Events
+import at.bitfire.synctools.mapping.calendar.MappingUtil
+import at.bitfire.synctools.util.AndroidTimeUtils
 import net.fortuna.ical4j.model.TimeZoneRegistry
 import net.fortuna.ical4j.model.component.VEvent
 import net.fortuna.ical4j.model.property.DtEnd
-import java.util.logging.Logger
+import net.fortuna.ical4j.model.property.DtStart
 
 class EndTimeHandler(
     private val tzRegistry: TimeZoneRegistry
-): AndroidEventFieldHandler {
-
-    private val logger
-        get() = Logger.getLogger(javaClass.name)
+) : AndroidEventFieldHandler {
 
     override fun process(from: Entity, main: Entity, to: VEvent) {
         val values = from.entityValues
+
+        // Skip if start is not set (end can not exist without start)
+        val tsStart = values.getAsLong(Events.DTSTART) ?: return
+
+        // Get other values
+        val tsEnd: Long? = values.getAsLong(Events.DTEND)
+        val startTz = values.getAsString(Events.EVENT_TIMEZONE)
+        val endTz = values.getAsString(Events.EVENT_END_TIMEZONE)
         val allDay = (values.getAsInteger(Events.ALL_DAY) ?: 0) != 0
 
-        // Skip if DTEND is not set – then usually DURATION is set; however it's also OK to have neither DTEND nor DURATION in a VEVENT.
-        val tsEnd = values.getAsLong(Events.DTEND) ?: return
+        // DTSTART from DATE or DATE-TIME according to allDay
+        val dtStart = DtStart(AndroidTimeField(tsStart, startTz, allDay, tzRegistry).asIcal4jDate())
 
-        // Also skip if DTEND is not after DTSTART (not allowed in iCalendar)
-        val tsStart = values.getAsLong(Events.DTSTART) ?: return
-        if (tsEnd <= tsStart) {
-            logger.warning("Ignoring DTEND=$tsEnd that is not after DTSTART=$tsStart")
-            return
+        // Create duration
+        val duration = values.getAsString(Events.DURATION)?.let { durStr ->
+            AndroidTimeUtils.parseDuration(durStr)
         }
 
-        // DATE or DATE-TIME according to allDay
-        val end = AndroidTimeField(
-            timestamp = tsEnd,
-            timeZone = values.getAsString(Events.EVENT_END_TIMEZONE)
-                ?: values.getAsString(Events.EVENT_TIMEZONE),   // if end timezone is not present, assume same as for start
-            allDay = allDay,
-            tzRegistry = tzRegistry
-        ).asIcal4jDate()
+        // Create end if it is set and after start
+        val end = tsEnd
+            ?.takeIf { it >= tsStart } // End is only useful to us, if after start
+            ?.let {
+                // DATE or DATE-TIME according to allDay
+                AndroidTimeField(
+                    timestamp = tsEnd,
+                    timeZone = endTz ?: startTz, // if end timezone is not present, assume same as for start
+                    allDay = allDay,
+                    tzRegistry = tzRegistry
+                ).asIcal4jDate()
+            }
+
+        // Use the set end if possible, otherwise generate from start + duration if set. Last resort
+        // is to use a default value.
+        val dtEnd = end?.let { DtEnd(it) }
+            ?: MappingUtil.dtEndFromDuration(dtStart, duration)
+            ?: MappingUtil.dtEndFromDefault(dtStart) // for compatibility with iCloud. See davx5-ose#1859
 
-        to.properties += DtEnd(end)
+        to.properties += dtEnd
     }
 
 }