From 28dcf907750d59dd2e453dce047ef73acbd1b229 Mon Sep 17 00:00:00 2001 From: Ricki Hirner Date: Mon, 17 Nov 2025 10:34:36 +0100 Subject: [PATCH] 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 | 1 + .../davdroid/sync/CalendarSyncManager.kt | 24 ++--- .../davdroid/sync/DefaultReminderBuilder.kt | 60 ++++++++++++ .../sync/DefaultReminderBuilderTest.kt | 95 +++++++++++++++++++ gradle/libs.versions.toml | 2 + 5 files changed, 165 insertions(+), 17 deletions(-) create mode 100644 app/src/main/kotlin/at/bitfire/davdroid/sync/DefaultReminderBuilder.kt create mode 100644 app/src/test/kotlin/at/bitfire/davdroid/sync/DefaultReminderBuilderTest.kt diff --git a/app/build.gradle.kts b/app/build.gradle.kts index 9ca90fa2c..42a8769c0 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -227,4 +227,5 @@ dependencies { testImplementation(libs.junit) testImplementation(libs.mockk) testImplementation(libs.okhttp.mockwebserver) + testImplementation(libs.robolectric) } diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncManager.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncManager.kt index c3c6537b1..f1a32edb6 100644 --- a/app/src/main/kotlin/at/bitfire/davdroid/sync/CalendarSyncManager.kt +++ b/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) { diff --git a/app/src/main/kotlin/at/bitfire/davdroid/sync/DefaultReminderBuilder.kt b/app/src/main/kotlin/at/bitfire/davdroid/sync/DefaultReminderBuilder.kt new file mode 100644 index 000000000..2453190d7 --- /dev/null +++ b/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 + )) + } + +} \ No newline at end of file diff --git a/app/src/test/kotlin/at/bitfire/davdroid/sync/DefaultReminderBuilderTest.kt b/app/src/test/kotlin/at/bitfire/davdroid/sync/DefaultReminderBuilderTest.kt new file mode 100644 index 000000000..8292d895d --- /dev/null +++ b/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) + } + +} \ No newline at end of file diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 04a5bfb9f..c2a16f529 100644 --- a/gradle/libs.versions.toml +++ b/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" }