Refactor default reminder builder to dedicated class + unit tests (#1815)

* [WIP] Move default reminder builder to dedicated class + unit tests

* Add tests

* Just turn off Conscrypt for now

* Fix library name

app/build.gradle.kts

@@ -227,4 +227,5 @@ dependencies {
     testImplementation(libs.junit)
     testImplementation(libs.mockk)
     testImplementation(libs.okhttp.mockwebserver)
+    testImplementation(libs.robolectric)
 }

app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncManager.kt

@@ -28,7 +28,6 @@ import at.bitfire.davdroid.resource.SyncState
 import at.bitfire.davdroid.settings.AccountSettings
 import at.bitfire.davdroid.util.DavUtils
 import at.bitfire.davdroid.util.DavUtils.lastSegment
-import at.bitfire.ical4android.util.DateUtils
 import at.bitfire.synctools.exception.InvalidICalendarException
 import at.bitfire.synctools.icalendar.CalendarUidSplitter
 import at.bitfire.synctools.icalendar.ICalendarGenerator
@@ -43,16 +42,13 @@ import dagger.assisted.AssistedInject
 import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.runInterruptible
 import net.fortuna.ical4j.model.Component
-import net.fortuna.ical4j.model.component.VAlarm
 import net.fortuna.ical4j.model.component.VEvent
-import net.fortuna.ical4j.model.property.Action
 import okhttp3.HttpUrl
 import okhttp3.OkHttpClient
 import okhttp3.RequestBody.Companion.toRequestBody
 import java.io.Reader
 import java.io.StringReader
 import java.io.StringWriter
-import java.time.Duration
 import java.time.ZonedDateTime
 import java.util.Optional
 import java.util.logging.Level
@@ -300,18 +296,6 @@ class CalendarSyncManager @AssistedInject constructor(
         // Event: main VEVENT and potentially attached exceptions (further VEVENTs with RECURRENCE-ID)
         val event = uidsAndEvents.values.first()
 
-        val defaultAlarmMinBefore = accountSettings.getDefaultAlarm()
-        val mainEvent = event.main
-        if (mainEvent != null && defaultAlarmMinBefore != null && DateUtils.isDateTime(mainEvent.startDate) && mainEvent.alarms.isEmpty()) {
-            val alarm = VAlarm(Duration.ofMinutes(-defaultAlarmMinBefore.toLong())).apply {
-                // Sets METHOD_ALERT instead of METHOD_DEFAULT in the calendar provider.
-                // Needed for calendars to actually show a notification.
-                properties += Action.DISPLAY
-            }
-            logger.log(Level.FINE, "${mainEvent.uid}: Adding default alarm", alarm)
-            mainEvent.components += alarm
-        }
-
         // map AssociatedEvents (VEVENTs) to EventAndExceptions (Android events)
         val androidEvent = AndroidEventBuilder(
             calendar = localCollection.androidCalendar,
@@ -321,7 +305,13 @@ class CalendarSyncManager @AssistedInject constructor(
             flags = LocalResource.FLAG_REMOTELY_PRESENT
         ).build(event)
 
-        // update local event, if it exists
+        // add default reminder (if desired)
+        accountSettings.getDefaultAlarm()?.let { minBefore ->
+            logger.log(Level.INFO, "Adding default alarm ($minBefore min before)", event)
+            DefaultReminderBuilder(minBefore = minBefore).add(to = androidEvent)
+        }
+
+        // create/update local event in calendar provider
         val local = localCollection.findByName(fileName)
         if (local != null) {
             SyncException.wrapWithLocalResource(local) {

app/src/main/kotlin/at/bitfire/davdroid/sync/DefaultReminderBuilder.kt

@@ -0,0 +1,60 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.sync
+
+import android.content.Entity
+import android.provider.CalendarContract.Events
+import android.provider.CalendarContract.Reminders
+import androidx.annotation.VisibleForTesting
+import androidx.core.content.contentValuesOf
+import at.bitfire.synctools.storage.calendar.EventAndExceptions
+
+/**
+ * Builder for default reminders / alarms that can be added to events
+ * if this is enabled in app settings.
+ *
+ * @param minBefore     how many minutes before the entry the alarm should be added (usually taken from app settings)
+ */
+class DefaultReminderBuilder(
+    private val minBefore: Int
+) {
+
+    /**
+     * Adds a default alarm ([minBefore] minutes before) to
+     *
+     * - the main event and
+     * - each exception event,
+     *
+     * except for those events which
+     *
+     * - are all-day, or
+     * - already have another reminder.
+     */
+    fun add(to: EventAndExceptions) {
+        // add default reminder to main event and exceptions
+        val events = mutableListOf(to.main)
+        events += to.exceptions
+
+        for (event in events)
+            addToEvent(to = event)
+    }
+
+    @VisibleForTesting
+    internal fun addToEvent(to: Entity) {
+        // don't add default reminder if there's already another reminder
+        if (to.subValues.any { it.uri == Reminders.CONTENT_URI })
+            return
+
+        // don't add default reminder to all-day events
+        if (to.entityValues.getAsInteger(Events.ALL_DAY) == 1)
+            return
+
+        to.addSubValue(Reminders.CONTENT_URI, contentValuesOf(
+            Reminders.MINUTES to minBefore,
+            Reminders.METHOD to Reminders.METHOD_ALERT      // will trigger an alarm on the Android device
+        ))
+    }
+
+}

app/src/test/kotlin/at/bitfire/davdroid/sync/DefaultReminderBuilderTest.kt

@@ -0,0 +1,95 @@
+/*
+ * Copyright © All Contributors. See LICENSE and AUTHORS in the root directory for details.
+ */
+
+package at.bitfire.davdroid.sync
+
+import android.content.ContentValues
+import android.content.Entity
+import android.provider.CalendarContract.Events
+import android.provider.CalendarContract.Reminders
+import androidx.core.content.contentValuesOf
+import at.bitfire.synctools.storage.calendar.EventAndExceptions
+import at.bitfire.synctools.test.assertEntitiesEqual
+import at.bitfire.synctools.test.assertEventAndExceptionsEqual
+import org.junit.Assert.assertFalse
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+import org.robolectric.annotation.ConscryptMode
+
+@RunWith(RobolectricTestRunner::class)
+@ConscryptMode(ConscryptMode.Mode.OFF)      // required because main project uses Conscrypt, but unit tests do not
+class DefaultReminderBuilderTest {
+
+    val builder = DefaultReminderBuilder(minBefore = 15)
+
+    @Test
+    fun `add() adds to main event and exceptions`() {
+        val event = EventAndExceptions(
+            main = Entity(ContentValues()),
+            exceptions = listOf(
+                Entity(ContentValues())
+            )
+        )
+        builder.add(to = event)
+        assertEventAndExceptionsEqual(
+            EventAndExceptions(
+                main = Entity(ContentValues()).apply {
+                    addSubValue(Reminders.CONTENT_URI, contentValuesOf(
+                        Reminders.MINUTES to 15,
+                        Reminders.METHOD to Reminders.METHOD_ALERT
+                    ))
+                },
+                exceptions = listOf(
+                    Entity(ContentValues()).apply {
+                        addSubValue(Reminders.CONTENT_URI, contentValuesOf(
+                            Reminders.MINUTES to 15,
+                            Reminders.METHOD to Reminders.METHOD_ALERT
+                        ))
+                    }
+                )
+            ),
+            event
+        )
+    }
+
+    @Test
+    fun `addToEvent() adds to non-all-day event without other reminder`() {
+        val entity = Entity(ContentValues())
+        builder.addToEvent(entity)
+        assertEntitiesEqual(Entity(ContentValues()).apply {
+            addSubValue(Reminders.CONTENT_URI, contentValuesOf(
+                Reminders.MINUTES to 15,
+                Reminders.METHOD to Reminders.METHOD_ALERT
+            ))
+        }, entity)
+    }
+
+    @Test
+    fun `addToEvent() doesn't add to all-day event`() {
+        val entity = Entity(contentValuesOf(
+            Events.ALL_DAY to 1
+        ))
+        builder.addToEvent(entity)
+        assertFalse(entity.subValues.any { it.uri == Reminders.CONTENT_URI })
+    }
+
+    @Test
+    fun `addToEvent() doesn't add to event with another reminder`() {
+        val entity = Entity(ContentValues()).apply {
+            addSubValue(Reminders.CONTENT_URI, contentValuesOf(
+                Reminders.MINUTES to 30,
+                Reminders.METHOD to Reminders.METHOD_ALERT
+            ))
+        }
+        builder.addToEvent(entity)
+        assertEntitiesEqual(Entity(ContentValues()).apply {
+            addSubValue(Reminders.CONTENT_URI, contentValuesOf(
+                Reminders.MINUTES to 30,
+                Reminders.METHOD to Reminders.METHOD_ALERT
+            ))
+        }, entity)
+    }
+
+}

gradle/libs.versions.toml

@@ -37,6 +37,7 @@ mikepenz-aboutLibraries = "13.1.0"
 mockk = "1.14.5"
 okhttp = "5.3.0"
 openid-appauth = "0.11.1"
+robolectric = "4.16"
 room = "2.8.3"
 unifiedpush = "3.1.2"
 unifiedpush-fcm = "3.0.0"
@@ -104,6 +105,7 @@ okhttp-brotli = { module = "com.squareup.okhttp3:okhttp-brotli", version.ref = "
 okhttp-logging = { module = "com.squareup.okhttp3:logging-interceptor", version.ref = "okhttp" }
 okhttp-mockwebserver = { module = "com.squareup.okhttp3:mockwebserver", version.ref = "okhttp" }
 openid-appauth = { module = "net.openid:appauth", version.ref = "openid-appauth" }
+robolectric = { module = "org.robolectric:robolectric", version.ref = "robolectric" }
 room-base = { module = "androidx.room:room-ktx", version.ref = "room" }
 room-compiler = { module = "androidx.room:room-compiler", version.ref = "room" }
 room-paging = { module = "androidx.room:room-paging", version.ref = "room" }