Event builders: use VEvent instead of deprecated Event as input (#120)

* [WIP] Rewrite AndroidEventBuilder to use VEvent

* [WIP] Continue

* [WIP] Rewrite Builders

* [WIP] Rewrite tests

* [WIP] Fix remaining tests (UnknownPropertiesBuilder yet not working)

* Mark EventReader as deprecated, move util method

* Update UnknownPropertiesBuilder and Processor

- Add unknownProperties() method to filter known properties
- Update test for unknown properties handling
- Refactor KNOWN_PROPERTY_NAMES to include processed properties

* AndroidEventBuilder: document how main VEvent is generated when it's missing

Author: rfc2822

lib/src/main/kotlin/at/bitfire/ical4android/DmfsTask.kt

@@ -388,7 +388,13 @@ abstract class DmfsTask(
     protected open fun insertAlarms(batch: TasksBatchOperation, idxTask: Int?) {
         val task = requireNotNull(task)
         for (alarm in task.alarms) {
-            val (alarmRef, minutes) = ICalendar.vAlarmToMin(alarm, task, true) ?: continue
+            val (alarmRef, minutes) = ICalendar.vAlarmToMin(
+                alarm = alarm,
+                refStart = task.dtStart,
+                refEnd = task.due,
+                refDuration = task.duration,
+                allowRelEnd = true
+            ) ?: continue
             val ref = when (alarmRef) {
                 Related.END ->
                     Alarm.ALARM_REFERENCE_DUE_DATE

lib/src/main/kotlin/at/bitfire/ical4android/Event.kt

@@ -8,10 +8,8 @@ package at.bitfire.ical4android
 
 import at.bitfire.synctools.exception.InvalidICalendarException
 import at.bitfire.synctools.icalendar.Css3Color
-import net.fortuna.ical4j.model.Parameter
 import net.fortuna.ical4j.model.Property
 import net.fortuna.ical4j.model.component.VAlarm
-import net.fortuna.ical4j.model.parameter.Email
 import net.fortuna.ical4j.model.property.Attendee
 import net.fortuna.ical4j.model.property.Clazz
 import net.fortuna.ical4j.model.property.DtEnd
@@ -34,7 +32,11 @@ import java.util.LinkedList
  * - as it is extracted from an iCalendar or
  * - as it should be generated into an iCalendar.
  */
-@Deprecated("Use AssociatedEvents instead", replaceWith = ReplaceWith("AssociatedEvents", "at.bitfire.synctools.icalendar"))
+@Deprecated(
+    "Use AssociatedEvents instead",
+    replaceWith = ReplaceWith("AssociatedEvents", "at.bitfire.synctools.icalendar"),
+    level = DeprecationLevel.WARNING
+)
 data class Event(
     override var uid: String? = null,
     override var sequence: Int? = null,
@@ -79,19 +81,6 @@ data class Event(
     val unknownProperties: LinkedList<Property> = LinkedList()
 ) : ICalendar() {
 
-    val organizerEmail: String?
-        get() {
-            var email: String? = null
-            organizer?.let { organizer ->
-                val uri = organizer.calAddress
-                email = if (uri.scheme.equals("mailto", true))
-                    uri.schemeSpecificPart
-                else
-                    organizer.getParameter<Email>(Parameter.EMAIL)?.value
-            }
-            return email
-        }
-
     fun requireDtStart(): DtStart =
         dtStart ?: throw InvalidICalendarException("Missing DTSTART in VEVENT")
 

lib/src/main/kotlin/at/bitfire/ical4android/EventReader.kt

@@ -45,6 +45,7 @@ import java.util.logging.Logger
 /**
  * Generates an [Event] from an iCalendar in a [Reader] source.
  */
+@Deprecated("Use ICalendarParser and CalendarUidSplitter instead")
 class EventReader {
 
     private val logger

lib/src/main/kotlin/at/bitfire/ical4android/ICalendar.kt

@@ -23,11 +23,11 @@ import net.fortuna.ical4j.model.component.Daylight
 import net.fortuna.ical4j.model.component.Observance
 import net.fortuna.ical4j.model.component.Standard
 import net.fortuna.ical4j.model.component.VAlarm
-import net.fortuna.ical4j.model.component.VEvent
 import net.fortuna.ical4j.model.component.VTimeZone
-import net.fortuna.ical4j.model.component.VToDo
 import net.fortuna.ical4j.model.parameter.Related
 import net.fortuna.ical4j.model.property.Color
+import net.fortuna.ical4j.model.property.DateProperty
+import net.fortuna.ical4j.model.property.DtStart
 import net.fortuna.ical4j.model.property.ProdId
 import net.fortuna.ical4j.model.property.RDate
 import net.fortuna.ical4j.model.property.RRule
@@ -261,9 +261,10 @@ open class ICalendar {
         /**
          * Calculates the minutes before/after an event/task a given alarm occurs.
          *
-         * @param alarm the alarm to calculate the minutes from
-         * @param reference reference [VEvent] or [VToDo] to take start/end time from (required for calculations)
-         * @param allowRelEnd *true*: caller accepts minutes related to the end;
+         * @param alarm         the alarm to calculate the minutes from
+         * @param refStart      reference `DTSTART` from the calendar component
+         * @param refEnd        reference `DTEND` (`VEVENT`) or `DUE` (`VTODO`) from the calendar component
+         * @param allowRelEnd   *true*: caller accepts minutes related to the end;
          * *false*: caller only accepts minutes related to the start
          *
          * Android's alarm granularity is minutes. This methods calculates with milliseconds, but the result
@@ -276,44 +277,35 @@ open class ICalendar {
          *
          * May be *null* if there's not enough information to calculate the number of minutes.
          */
-        fun vAlarmToMin(alarm: VAlarm, reference: ICalendar, allowRelEnd: Boolean): Pair<Related, Int>? {
+        fun vAlarmToMin(
+            alarm: VAlarm,
+            refStart: DtStart?,
+            refEnd: DateProperty?,
+            refDuration: net.fortuna.ical4j.model.property.Duration?,
+            allowRelEnd: Boolean
+        ): Pair<Related, Int>? {
             val trigger = alarm.trigger ?: return null
 
             val minutes: Int    // minutes before/after the event
             var related = trigger.getParameter<Related>(Parameter.RELATED) ?: Related.START
 
             // event/task start time
-            val start: java.util.Date?
-            var end: java.util.Date?
-            when (reference) {
-                is Event -> {
-                    start = reference.dtStart?.date
-                    end = reference.dtEnd?.date
-                }
-                is Task -> {
-                    start = reference.dtStart?.date
-                    end = reference.due?.date
-                }
-                else -> throw IllegalArgumentException("reference must be Event or Task")
-            }
+            val start: java.util.Date? = refStart?.date
+            var end: java.util.Date? = refEnd?.date
 
             // event/task end time
             if (end == null && start != null) {
-                val duration = when (reference) {
-                    is Event -> reference.duration?.duration
-                    is Task -> reference.duration?.duration
-                    else -> throw IllegalArgumentException("reference must be Event or Task")
-                }
+                val duration = refDuration?.duration
                 if (duration != null)
                     end = java.util.Date.from(start.toInstant() + duration)
             }
 
             // event/task duration
             val duration: Duration? =
-                    if (start != null && end != null)
-                        Duration.between(start.toInstant(), end.toInstant())
-                    else
-                        null
+                if (start != null && end != null)
+                    Duration.between(start.toInstant(), end.toInstant())
+                else
+                    null
 
             val triggerDur = trigger.duration
             val triggerTime = trigger.dateTime

lib/src/main/kotlin/at/bitfire/synctools/icalendar/Ical4jHelpers.kt

@@ -7,10 +7,13 @@
 package at.bitfire.synctools.icalendar
 
 import at.bitfire.synctools.BuildConfig
+import at.bitfire.synctools.exception.InvalidICalendarException
 import net.fortuna.ical4j.model.ComponentList
 import net.fortuna.ical4j.model.Property
 import net.fortuna.ical4j.model.PropertyList
 import net.fortuna.ical4j.model.component.CalendarComponent
+import net.fortuna.ical4j.model.component.VEvent
+import net.fortuna.ical4j.model.property.DtStart
 import net.fortuna.ical4j.model.property.RecurrenceId
 import net.fortuna.ical4j.model.property.Sequence
 import net.fortuna.ical4j.model.property.Uid
@@ -42,3 +45,6 @@ val CalendarComponent.recurrenceId: RecurrenceId?
 
 val CalendarComponent.sequence: Sequence?
     get() = getProperty(Property.SEQUENCE)
+
+fun VEvent.requireDtStart(): DtStart =
+    startDate ?: throw InvalidICalendarException("Missing DTSTART in VEVENT")

…g/calendar/LegacyAndroidEventBuilder2.kt …/mapping/calendar/AndroidEventBuilder.ktlib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/LegacyAndroidEventBuilder2.kt renamed to lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/AndroidEventBuilder.kt lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/LegacyAndroidEventBuilder2.kt renamed to lib/src/main/kotlin/at/bitfire/synctools/mapping/calendar/AndroidEventBuilder.kt

@@ -8,7 +8,7 @@ package at.bitfire.synctools.mapping.calendar
 
 import android.content.ContentValues
 import android.content.Entity
-import at.bitfire.ical4android.Event
+import at.bitfire.synctools.icalendar.AssociatedEvents
 import at.bitfire.synctools.mapping.calendar.builder.AccessLevelBuilder
 import at.bitfire.synctools.mapping.calendar.builder.AllDayBuilder
 import at.bitfire.synctools.mapping.calendar.builder.AndroidEntityBuilder
@@ -38,20 +38,17 @@ import at.bitfire.synctools.mapping.calendar.builder.UnknownPropertiesBuilder
 import at.bitfire.synctools.mapping.calendar.builder.UrlBuilder
 import at.bitfire.synctools.storage.calendar.AndroidCalendar
 import at.bitfire.synctools.storage.calendar.EventAndExceptions
+import net.fortuna.ical4j.model.component.VEvent
 
 /**
- * Legacy mapper from an [Event] data object to Android content provider data rows
+ * Legacy mapper from an [AssociatedEvents] data object to Android content provider data rows
  * (former "build..." methods).
  *
  * Important: To use recurrence exceptions, you MUST set _SYNC_ID and ORIGINAL_SYNC_ID
  * in populateEvent() / buildEvent. Setting _ID and ORIGINAL_ID is not sufficient.
- *
- * Note: "Legacy" will be removed from the class name as soon as the [Event] dependency is
- * replaced by [at.bitfire.synctools.icalendar.AssociatedEvents].
  */
-class LegacyAndroidEventBuilder2(
+class AndroidEventBuilder(
     calendar: AndroidCalendar,
-    private val event: Event,
 
     // AndroidEvent-level fields
     syncId: String?,
@@ -92,19 +89,37 @@ class LegacyAndroidEventBuilder2(
         UrlBuilder()
     )
 
-    fun build() =
-        EventAndExceptions(
-            main = buildEvent(null),
-            exceptions = event.exceptions.map { exception ->
-                buildEvent(exception)
+    fun build(events: AssociatedEvents): EventAndExceptions {
+        val mainVEvent = events.main ?: createMainFromExceptions(events.exceptions)
+        return EventAndExceptions(
+            main = buildEvent(from = mainVEvent, main = mainVEvent),
+            exceptions = events.exceptions.map { exception ->
+                buildEvent(from = exception, main = mainVEvent)
             }
         )
+    }
 
-    fun buildEvent(recurrence: Event?): Entity {
+    fun buildEvent(from: VEvent, main: VEvent): Entity {
         val entity = Entity(ContentValues())
         for (builder in fieldBuilders)
-            builder.build(from = recurrence ?: event, main = event, to = entity)
+            builder.build(from = from, main = main, to = entity)
         return entity
     }
 
+    /**
+     * It is possible that a user receives only exceptions of an event, but not the main event itself.
+     * This happens when there's a recurring event that is not visible for the user, but the user is invited to
+     * a single recurrence. However, we always need a main event for Android, so we make up one from the
+     * exceptions.
+     */
+    private fun createMainFromExceptions(exceptions: List<VEvent>): VEvent {
+        // Should in the future be replaced by a real event that has a title like "(unknown event)".
+        // This main event should also have a special extended property that indicates that the event
+        // must not actually be generated as main VEvent when the event is locally edited and then uploaded.
+
+        // Currently, we just use the first exception as a main event, too. This is not correct and
+        // should be fixed.
+        return exceptions.firstOrNull() ?: VEvent()
+    }
+
 }

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

@@ -10,13 +10,13 @@ import android.content.Entity
 import android.provider.CalendarContract.Events
 import android.provider.CalendarContract.ExtendedProperties
 import androidx.core.content.contentValuesOf
-import at.bitfire.ical4android.Event
 import at.bitfire.ical4android.UnknownProperty
+import net.fortuna.ical4j.model.component.VEvent
 import net.fortuna.ical4j.model.property.Clazz
 
 class AccessLevelBuilder: AndroidEntityBuilder {
 
-    override fun build(from: Event, main: Event, to: Entity) {
+    override fun build(from: VEvent, main: VEvent, to: Entity) {
         val accessLevel: Int
         val retainValue: Boolean
 

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

@@ -8,13 +8,13 @@ package at.bitfire.synctools.mapping.calendar.builder
 
 import android.content.Entity
 import android.provider.CalendarContract.Events
-import at.bitfire.ical4android.Event
 import at.bitfire.ical4android.util.DateUtils
+import net.fortuna.ical4j.model.component.VEvent
 
 class AllDayBuilder: AndroidEntityBuilder {
 
-    override fun build(from: Event, main: Event, to: Entity) {
-        val allDay = DateUtils.isDate(from.dtStart)
+    override fun build(from: VEvent, main: VEvent, to: Entity) {
+        val allDay = DateUtils.isDate(from.startDate)
         to.entityValues.put(Events.ALL_DAY, if (allDay) 1 else 0)
     }
 

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

@@ -7,7 +7,7 @@
 package at.bitfire.synctools.mapping.calendar.builder
 
 import android.content.Entity
-import at.bitfire.ical4android.Event
+import net.fortuna.ical4j.model.component.VEvent
 
 interface AndroidEntityBuilder {
 
@@ -34,6 +34,6 @@ interface AndroidEntityBuilder {
      *
      * @throws at.bitfire.synctools.exception.InvalidICalendarException on missing or invalid required properties (like DTSTART)
      */
-    fun build(from: Event, main: Event, to: Entity)
+    fun build(from: VEvent, main: VEvent, to: Entity)
 
 }

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

@@ -9,10 +9,12 @@ package at.bitfire.synctools.mapping.calendar.builder
 import android.content.ContentValues
 import android.content.Entity
 import android.provider.CalendarContract.Attendees
-import at.bitfire.ical4android.Event
+import androidx.annotation.VisibleForTesting
 import at.bitfire.synctools.mapping.calendar.AttendeeMappings
 import at.bitfire.synctools.storage.calendar.AndroidCalendar
 import net.fortuna.ical4j.model.Parameter
+import net.fortuna.ical4j.model.Property
+import net.fortuna.ical4j.model.component.VEvent
 import net.fortuna.ical4j.model.parameter.Cn
 import net.fortuna.ical4j.model.parameter.Email
 import net.fortuna.ical4j.model.parameter.PartStat
@@ -22,14 +24,14 @@ class AttendeesBuilder(
     private val calendar: AndroidCalendar
 ): AndroidEntityBuilder {
 
-    override fun build(from: Event, main: Event, to: Entity) {
-        for (attendee in from.attendees)
+    override fun build(from: VEvent, main: VEvent, to: Entity) {
+        for (attendee in from.getProperties<Attendee>(Property.ATTENDEE))
             to.addSubValue(Attendees.CONTENT_URI, buildAttendee(attendee, from))
     }
 
-    private fun buildAttendee(attendee: Attendee, event: Event): ContentValues {
+    private fun buildAttendee(attendee: Attendee, event: VEvent): ContentValues {
         val values = ContentValues()
-        val organizer = event.organizerEmail ?:
+        val organizer = organizerEmail(event) ?:
             /* no ORGANIZER, use current account owner as ORGANIZER */
             calendar.ownerAccount ?: calendar.account.name
 
@@ -65,4 +67,16 @@ class AttendeesBuilder(
         return values
     }
 
+    @VisibleForTesting
+    internal fun organizerEmail(event: VEvent): String? {
+        event.organizer?.let { organizer ->
+            val uri = organizer.calAddress
+            return if (uri.scheme.equals("mailto", true))
+                uri.schemeSpecificPart
+            else
+                organizer.getParameter<Email>(Parameter.EMAIL)?.value
+        }
+        return null
+    }
+
 }