Fix duration mapping for all-day events

- Align DURATION with DTSTART type
- Add tests for alignWithDtStart function

Author: rfc2822

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

@@ -154,7 +154,7 @@ object TimeApiExtensions {
             if (secs == 0L)
                 return "PT0S"
 
-            var weeks = secs / SECONDS_PER_WEEK
+            val weeks = secs / SECONDS_PER_WEEK
             secs -= weeks * SECONDS_PER_WEEK
 
             var days = secs / SECONDS_PER_DAY
@@ -170,7 +170,6 @@ object TimeApiExtensions {
                 return "P${weeks}W"
 
             days += weeks * DAYS_PER_WEEK
-            weeks = 0
 
             if (days != 0L)
                 builder.append("${days}D")

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

@@ -10,6 +10,7 @@ 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.toDuration
 import at.bitfire.ical4android.util.TimeApiExtensions.toLocalDate
 import at.bitfire.ical4android.util.TimeApiExtensions.toRfc5545Duration
 import at.bitfire.synctools.icalendar.requireDtStart
@@ -21,6 +22,7 @@ import net.fortuna.ical4j.model.property.Duration
 import net.fortuna.ical4j.model.property.RDate
 import net.fortuna.ical4j.model.property.RRule
 import java.time.Period
+import java.time.temporal.TemporalAmount
 
 class DurationBuilder: AndroidEntityBuilder {
 
@@ -52,15 +54,52 @@ class DurationBuilder: AndroidEntityBuilder {
         so we wouldn't have to take care of that. However it expects seconds to be in "P<n>S" format,
         whereas we provide an RFC 5545-compliant "PT<n>S", which causes the provider to crash:
         https://github.com/bitfireAT/synctools/issues/144. So we must convert it ourselves to be on the safe side. */
+        val alignedDuration = alignWithDtStart(duration.duration, dtStart)
 
-        // TODO: adapt DURATION according to DATE/DATE-TIME
+        /* TemporalAmount can have months and years, but the RFC 5545 value must only contain weeks, days and time.
+        So we have to recalculate the months/years to days according to their position in the calendar.
 
-        // TemporalAmount can have months and years, but the RFC 5545 must only contain weeks, days and time.
-        // So we have to recalculate the months/years to days according to their position in the calendar.
-        val durationStr = duration.duration.toRfc5545Duration(dtStart.date.toInstant())
+        The calendar provider accepts every DURATION that `com.android.calendarcommon2.Duration` can parse,
+        which is weeks, days, hours, minutes and seconds. */
+        val durationStr = alignedDuration.toRfc5545Duration(dtStart.date.toInstant())
         values.put(Events.DURATION, durationStr)
     }
 
+    /**
+     * Aligns the given temporal amount (taken from DURATION) to the VALUE-type (DATE-TIME/DATE) of DTSTART.
+     *
+     * @param amount    temporal amount that shall be aligned
+     * @param dtStart   DTSTART to compare with
+     *
+     * @return Temporal amount that is
+     *
+     * - a [Period] (days/months/years that can't be represented by an exact number of seconds) when [dtStart] is a DATE, and
+     * - a [java.time.Duration] (exact time that can be represented by an exact number of seconds) when [dtStart] is a DATE-TIME.
+     */
+    @VisibleForTesting
+    internal fun alignWithDtStart(amount: TemporalAmount, dtStart: DtStart): TemporalAmount {
+        if (DateUtils.isDate(dtStart)) {
+            // DTSTART is DATE
+            return if (amount is java.time.Duration) {
+                // amount is Duration, change to Period of seconds instead
+                Period.ofDays(amount.toDays().toInt())
+            } else {
+                // amount is already Period
+                amount
+            }
+
+        } else {
+            // DTSTART is DATE-TIME
+            return if (amount is java.time.Period) {
+                // amount is Period, change to Duration instead
+                amount.toDuration(dtStart.date.toInstant())
+            } else {
+                // amount is already Duration
+                amount
+            }
+        }
+    }
+
     @VisibleForTesting
     internal fun calculateFromDtEnd(dtStart: DtStart, dtEnd: DtEnd?): Duration? {
         if (dtEnd == null)

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

@@ -142,6 +142,41 @@ class DurationBuilderTest {
     }
 
 
+    // alignWithDtStart
+
+    @Test
+    fun `alignWithDtStart (DTSTART all-day, DURATION all-day)`() {
+        assertEquals(
+            Period.ofDays(1),       // may not be 24 hours (for instance on DST switch)
+            builder.alignWithDtStart(Period.ofDays(1), DtStart(Date()))
+        )
+    }
+
+    @Test
+    fun `alignWithDtStart (DTSTART non-all-day, DURATION all-day)`() {
+        assertEquals(
+            java.time.Duration.ofDays(1),   // exactly 24 hours
+            builder.alignWithDtStart(Period.ofDays(1), DtStart(DateTime()))
+        )
+    }
+
+    @Test
+    fun `alignWithDtStart (DTSTART all-day, DURATION non-all-day)`() {
+        assertEquals(
+            Period.ofDays(1),       // may not be 24 hours (for instance on DST switch)
+            builder.alignWithDtStart(java.time.Duration.ofDays(1), DtStart(Date()))
+        )
+    }
+
+    @Test
+    fun `alignWithDtStart (DTSTART non-all-day, DURATION non-all-day)`() {
+        assertEquals(
+            java.time.Duration.ofDays(1),   // exactly 24 hours
+            builder.alignWithDtStart(Period.ofDays(1), DtStart(DateTime()))
+        )
+    }
+
+
     // calculateFromDtEnd
 
     @Test