diff --git a/app/src/androidTest/java/com/todoroo/astrid/alarms/AlarmJobServiceTest.kt b/app/src/androidTest/java/com/todoroo/astrid/alarms/AlarmJobServiceTest.kt index 0e7e3b6a9..7bd2dbfb5 100644 --- a/app/src/androidTest/java/com/todoroo/astrid/alarms/AlarmJobServiceTest.kt +++ b/app/src/androidTest/java/com/todoroo/astrid/alarms/AlarmJobServiceTest.kt @@ -1,108 +1,258 @@ package com.todoroo.astrid.alarms -import com.natpryce.makeiteasy.MakeItEasy.with -import com.todoroo.andlib.utility.DateUtilities import dagger.hilt.android.testing.HiltAndroidTest import dagger.hilt.android.testing.UninstallModules import kotlinx.coroutines.runBlocking import org.junit.Assert.assertEquals import org.junit.Test -import org.tasks.data.entity.Alarm -import org.tasks.data.entity.Alarm.Companion.TYPE_DATE_TIME -import org.tasks.data.entity.Alarm.Companion.TYPE_RANDOM -import org.tasks.data.entity.Alarm.Companion.TYPE_SNOOZE -import org.tasks.data.entity.Alarm.Companion.whenDue -import org.tasks.data.entity.Alarm.Companion.whenOverdue -import org.tasks.data.dao.AlarmDao +import org.tasks.SuspendFreeze.Companion.freezeAt +import org.tasks.data.createDueDate import org.tasks.data.dao.TaskDao -import org.tasks.date.DateTimeUtils.newDateTime +import org.tasks.data.entity.Alarm +import org.tasks.data.entity.Notification +import org.tasks.data.entity.Task import org.tasks.injection.InjectingTestCase import org.tasks.injection.ProductionModule -import org.tasks.jobs.AlarmEntry -import org.tasks.makers.TaskMaker.COMPLETION_TIME -import org.tasks.makers.TaskMaker.DELETION_TIME -import org.tasks.makers.TaskMaker.DUE_DATE -import org.tasks.makers.TaskMaker.DUE_TIME -import org.tasks.makers.TaskMaker.REMINDER_LAST -import org.tasks.makers.TaskMaker.newTask import org.tasks.time.DateTime +import org.tasks.time.DateTimeUtils2 +import java.util.concurrent.TimeUnit import javax.inject.Inject @UninstallModules(ProductionModule::class) @HiltAndroidTest class AlarmJobServiceTest : InjectingTestCase() { - @Inject lateinit var alarmDao: AlarmDao @Inject lateinit var taskDao: TaskDao @Inject lateinit var alarmService: AlarmService @Test - fun scheduleAlarm() = runBlocking { - val task = taskDao.createNew(newTask()) - val alarm = insertAlarm(Alarm(task, DateTime(2017, 9, 24, 19, 57).millis, TYPE_DATE_TIME)) + fun testNoAlarms() = runBlocking { + testResults(emptyList(), 0) + } - verify(overdue = listOf(AlarmEntry(alarm, task, DateTime(2017, 9, 24, 19, 57).millis, TYPE_DATE_TIME))) + @Test + fun futureAlarmWithNoPastAlarm() = runBlocking { + freezeAt(DateTime(2024, 5, 17, 23, 20)) { + taskDao.insert( + Task( + dueDate = createDueDate( + Task.URGENCY_SPECIFIC_DAY, + DateTime(2024, 5, 18).millis + ) + ) + ) + alarmService.synchronizeAlarms(1, mutableSetOf(Alarm(0, 0, Alarm.TYPE_REL_END))) + + testResults(emptyList(), DateTime(2024, 5, 18, 18, 0).millis) + } } @Test - fun ignoreStaleAlarm() = runBlocking { - val alarmTime = DateTime(2017, 9, 24, 19, 57) - val task = taskDao.createNew(newTask(with(REMINDER_LAST, alarmTime.endOfMinute()))) - alarmDao.insert(Alarm(task, alarmTime.millis, TYPE_DATE_TIME)) + fun pastAlarmWithNoFutureAlarm() = runBlocking { + freezeAt(DateTime(2024, 5, 17, 23, 20)) { + taskDao.insert( + Task( + dueDate = createDueDate( + Task.URGENCY_SPECIFIC_DAY, + DateTime(2024, 5, 17).millis + ) + ) + ) + alarmService.synchronizeAlarms(1, mutableSetOf(Alarm(0, 0, Alarm.TYPE_REL_END))) - verify() + testResults( + listOf( + Notification( + taskId = 1L, + timestamp = DateTimeUtils2.currentTimeMillis(), + type = Alarm.TYPE_REL_END + ) + ), + 0 + ) + } } @Test - fun dontScheduleReminderForCompletedTask() = runBlocking { - val task = taskDao.insert( - newTask( - with(DUE_DATE, newDateTime()), - with(COMPLETION_TIME, newDateTime()) + fun pastRecurringAlarmWithFutureRecurrence() = runBlocking { + freezeAt(DateTime(2024, 5, 17, 23, 20)) { + taskDao.insert( + Task( + dueDate = createDueDate( + Task.URGENCY_SPECIFIC_DAY, + DateTime(2024, 5, 17).millis + ) + ) + ) + alarmService.synchronizeAlarms( + 1, + mutableSetOf( + Alarm( + 0, + 0, + Alarm.TYPE_REL_END, + repeat = 1, + interval = TimeUnit.HOURS.toMillis(6) + ) + ) ) - ) - alarmDao.insert(whenDue(task)) - verify() + testResults( + listOf( + Notification( + taskId = 1L, + timestamp = DateTimeUtils2.currentTimeMillis(), + type = Alarm.TYPE_REL_END + ) + ), + DateTime(2024, 5, 18, 0, 0).millis + ) + } } @Test - fun dontScheduleReminderForDeletedTask() = runBlocking { - val task = taskDao.insert( - newTask( - with(DUE_DATE, newDateTime()), - with(DELETION_TIME, newDateTime()) + fun pastAlarmsRemoveSnoozed() = runBlocking { + freezeAt(DateTime(2024, 5, 17, 23, 20)) { + taskDao.insert( + Task( + dueDate = createDueDate( + Task.URGENCY_SPECIFIC_DAY, + DateTime(2024, 5, 17).millis + ) + ) + ) + alarmService.synchronizeAlarms( + 1, + mutableSetOf( + Alarm(0, 0, Alarm.TYPE_REL_END), + Alarm(0, DateTimeUtils2.currentTimeMillis(), Alarm.TYPE_SNOOZE) + ) + ) + + testResults( + listOf( + Notification( + taskId = 1L, + timestamp = DateTimeUtils2.currentTimeMillis(), + type = Alarm.TYPE_REL_END + ) + ), + 0 ) - ) - alarmDao.insert(whenDue(task)) - verify() + assertEquals( + listOf(Alarm(id = 1, task = 1, time = 0, type = Alarm.TYPE_REL_END)), + alarmService.getAlarms(1) + ) + } } @Test - fun snoozeOverridesAll() = runBlocking { - val now = newDateTime() - val task = taskDao.insert(newTask(with(DUE_TIME, now))) + fun futureSnoozeOverrideOverdue() = runBlocking { + freezeAt(DateTime(2024, 5, 17, 23, 20)) { + taskDao.insert( + Task( + dueDate = createDueDate( + Task.URGENCY_SPECIFIC_DAY, + DateTime(2024, 5, 17).millis + ) + ) + ) + alarmService.synchronizeAlarms( + 1, + mutableSetOf( + Alarm(0, 0, Alarm.TYPE_REL_END), + Alarm( + 0, + DateTimeUtils2.currentTimeMillis() + TimeUnit.MINUTES.toMillis(5), + Alarm.TYPE_SNOOZE + ) + ) + ) + + testResults( + emptyList(), + DateTimeUtils2.currentTimeMillis() + TimeUnit.MINUTES.toMillis(5) + ) + } + } - alarmDao.insert(whenDue(task)) - alarmDao.insert(whenOverdue(task)) - alarmDao.insert(Alarm(task, DateUtilities.ONE_HOUR, TYPE_RANDOM)) - val alarm = alarmDao.insert(Alarm(task, now.plusMonths(12).millis, TYPE_SNOOZE)) + @Test + fun ignoreStaleAlarm() = runBlocking { + freezeAt(DateTime(2024, 5, 17, 23, 20)) { + taskDao.insert( + Task( + dueDate = createDueDate( + Task.URGENCY_SPECIFIC_DAY, + DateTime(2024, 5, 17).millis + ), + reminderLast = DateTime(2024, 5, 17, 18, 0).millis, + ) + ) + alarmService.synchronizeAlarms( + 1, + mutableSetOf(Alarm(0, 0, Alarm.TYPE_REL_END)) + ) - verify(future = listOf(AlarmEntry(alarm, task, now.plusMonths(12).millis, TYPE_SNOOZE))) + testResults( + emptyList(), + 0 + ) + } } - private suspend fun insertAlarm(alarm: Alarm): Long { - alarm.id = alarmDao.insert(alarm) - return alarm.id + @Test + fun dontScheduleForCompletedTask() = runBlocking { + freezeAt(DateTime(2024, 5, 17, 23, 20)) { + taskDao.insert( + Task( + dueDate = createDueDate( + Task.URGENCY_SPECIFIC_DAY, + DateTime(2024, 5, 17).millis + ), + completionDate = DateTime(2024, 5, 17, 14, 0).millis, + ) + ) + alarmService.synchronizeAlarms( + 1, + mutableSetOf(Alarm(0, 0, Alarm.TYPE_REL_END)) + ) + + testResults( + emptyList(), + 0 + ) + } } - private suspend fun verify( - overdue: List = emptyList(), - future: List = emptyList(), - ) { - val (actualOverdue, actualFuture) = alarmService.getAlarms() + @Test + fun dontScheduleForDeletedTask() = runBlocking { + freezeAt(DateTime(2024, 5, 17, 23, 20)) { + taskDao.insert( + Task( + dueDate = createDueDate( + Task.URGENCY_SPECIFIC_DAY, + DateTime(2024, 5, 17).millis + ), + deletionDate = DateTime(2024, 5, 17, 14, 0).millis, + ) + ) + alarmService.synchronizeAlarms( + 1, + mutableSetOf(Alarm(0, 0, Alarm.TYPE_REL_END)) + ) + + testResults( + emptyList(), + 0 + ) + } + } - assertEquals(overdue, actualOverdue) - assertEquals(future, actualFuture) + private suspend fun testResults(notifications: List, nextAlarm: Long) { + val actualNextAlarm = alarmService.triggerAlarms { + assertEquals(notifications, it) + it.forEach { taskDao.setLastNotified(it.taskId, DateTimeUtils2.currentTimeMillis()) } + } + assertEquals(nextAlarm, actualNextAlarm) } } \ No newline at end of file diff --git a/app/src/main/java/com/todoroo/astrid/alarms/AlarmService.kt b/app/src/main/java/com/todoroo/astrid/alarms/AlarmService.kt index 355367a8e..9fb77bb01 100644 --- a/app/src/main/java/com/todoroo/astrid/alarms/AlarmService.kt +++ b/app/src/main/java/com/todoroo/astrid/alarms/AlarmService.kt @@ -8,11 +8,14 @@ package com.todoroo.astrid.alarms import org.tasks.LocalBroadcastManager import org.tasks.data.dao.AlarmDao import org.tasks.data.dao.TaskDao +import org.tasks.data.db.DbUtils import org.tasks.data.entity.Alarm import org.tasks.data.entity.Alarm.Companion.TYPE_SNOOZE +import org.tasks.data.entity.Notification import org.tasks.jobs.AlarmEntry import org.tasks.jobs.WorkManager import org.tasks.notifications.NotificationManager +import org.tasks.preferences.Preferences import org.tasks.time.DateTime import org.tasks.time.DateTimeUtils2.currentTimeMillis import timber.log.Timber @@ -30,6 +33,7 @@ class AlarmService @Inject constructor( private val notificationManager: NotificationManager, private val workManager: WorkManager, private val alarmCalculator: AlarmCalculator, + private val preferences: Preferences, ) { suspend fun getAlarms(taskId: Long): List = alarmDao.getAlarms(taskId) @@ -70,7 +74,33 @@ class AlarmService @Inject constructor( workManager.triggerNotifications() } - suspend fun getAlarms(): Pair, List> { + suspend fun triggerAlarms( + trigger: suspend (List) -> Unit + ): Long { + if (preferences.isCurrentlyQuietHours) { + return preferences.adjustForQuietHours(currentTimeMillis()) + } + val (overdue, _) = getAlarms() + overdue + .sortedBy { it.time } + .also { alarms -> + alarms + .map { it.taskId } + .chunked(DbUtils.MAX_SQLITE_ARGS) + .onEach { alarmDao.deleteSnoozed(it) } + } + .map { it.toNotification() } + .let { trigger(it) } + val alreadyTriggered = overdue.map { it.taskId }.toSet() + val (moreOverdue, future) = getAlarms() + return moreOverdue + .filterNot { it.type == Alarm.TYPE_RANDOM || alreadyTriggered.contains(it.taskId) } + .plus(future) + .minOfOrNull { it.time } + ?: 0 + } + + internal suspend fun getAlarms(): Pair, List> { val start = currentTimeMillis() val overdue = ArrayList() val future = ArrayList() @@ -83,7 +113,8 @@ class AlarmService @Inject constructor( } val (now, later) = alarmEntries.partition { it.time <= DateTime().startOfMinute().plusMinutes(1).millis } later - .find { it.type == TYPE_SNOOZE } + .filter { it.type == TYPE_SNOOZE } + .maxByOrNull { it.time } ?.let { future.add(it) } ?: run { now.firstOrNull()?.let { overdue.add(it) } diff --git a/app/src/main/java/org/tasks/jobs/NotificationWork.kt b/app/src/main/java/org/tasks/jobs/NotificationWork.kt index 542510984..9c61e0705 100644 --- a/app/src/main/java/org/tasks/jobs/NotificationWork.kt +++ b/app/src/main/java/org/tasks/jobs/NotificationWork.kt @@ -11,13 +11,8 @@ import dagger.assisted.AssistedInject import org.tasks.Notifier import org.tasks.R import org.tasks.analytics.Firebase -import org.tasks.data.entity.Alarm.Companion.TYPE_RANDOM -import org.tasks.data.entity.Alarm.Companion.TYPE_SNOOZE -import org.tasks.data.dao.AlarmDao import org.tasks.date.DateTimeUtils.toDateTime import org.tasks.notifications.NotificationManager -import org.tasks.preferences.Preferences -import org.tasks.time.DateTimeUtils2.currentTimeMillis import timber.log.Timber @HiltWorker @@ -27,44 +22,19 @@ class NotificationWork @AssistedInject constructor( firebase: Firebase, private val workManager: WorkManager, private val alarmService: AlarmService, - private val alarmDao: AlarmDao, - private val preferences: Preferences, private val notifier: Notifier, ) : RepeatingWorker(context, workerParams, firebase) { private var nextAlarm: Long = 0 override suspend fun run(): Result { - if (preferences.isCurrentlyQuietHours) { - nextAlarm = preferences.adjustForQuietHours(currentTimeMillis()) - return Result.success() - } - val (overdue, _) = alarmService.getAlarms() - if (overdue.isNotEmpty()) { - overdue - .sortedBy { it.time } - .also { alarms -> - alarms - .filter { it.type == TYPE_SNOOZE } - .map { it.id } - .let { alarmDao.deleteByIds(it) } - } - .map { it.toNotification() } - .let { notifier.triggerNotifications(it) } - } - val alreadyTriggered = overdue.map { it.taskId }.toSet() - val (moreOverdue, future) = alarmService.getAlarms() - nextAlarm = moreOverdue - .filterNot { it.type == TYPE_RANDOM || alreadyTriggered.contains(it.taskId)} - .plus(future) - .minOfOrNull { it.time } - ?: 0 + nextAlarm = alarmService.triggerAlarms { notifier.triggerNotifications(it) } return Result.success() } override suspend fun scheduleNext() { if (nextAlarm > 0) { Timber.d("nextAlarm=${nextAlarm.toDateTime()}") - workManager.scheduleNotification(preferences.adjustForQuietHours(nextAlarm)) + workManager.scheduleNotification(nextAlarm) } } diff --git a/data/src/main/kotlin/org/tasks/data/dao/AlarmDao.kt b/data/src/main/kotlin/org/tasks/data/dao/AlarmDao.kt index 9e2ce6307..b6df1966b 100644 --- a/data/src/main/kotlin/org/tasks/data/dao/AlarmDao.kt +++ b/data/src/main/kotlin/org/tasks/data/dao/AlarmDao.kt @@ -4,9 +4,9 @@ import androidx.room.Dao import androidx.room.Delete import androidx.room.Insert import androidx.room.Query -import org.tasks.data.entity.Task import org.tasks.data.entity.Alarm import org.tasks.data.entity.Alarm.Companion.TYPE_SNOOZE +import org.tasks.data.entity.Task @Dao interface AlarmDao { @@ -35,8 +35,8 @@ WHERE tasks._id = :taskId @Query("SELECT * FROM alarms WHERE task = :taskId") suspend fun getAlarms(taskId: Long): List - @Query("DELETE FROM alarms WHERE _id IN(:alarmIds)") - suspend fun deleteByIds(alarmIds: List) + @Query("DELETE FROM alarms WHERE type = $TYPE_SNOOZE AND task IN(:taskIds)") + suspend fun deleteSnoozed(taskIds: List) @Delete suspend fun delete(alarm: Alarm) diff --git a/data/src/main/kotlin/org/tasks/data/entity/Alarm.kt b/data/src/main/kotlin/org/tasks/data/entity/Alarm.kt index 003e6a869..ea23980d9 100644 --- a/data/src/main/kotlin/org/tasks/data/entity/Alarm.kt +++ b/data/src/main/kotlin/org/tasks/data/entity/Alarm.kt @@ -1,18 +1,19 @@ package org.tasks.data.entity -import android.os.Parcel import android.os.Parcelable import androidx.room.ColumnInfo import androidx.room.Entity import androidx.room.ForeignKey import androidx.room.Ignore import androidx.room.PrimaryKey +import kotlinx.parcelize.Parcelize import kotlinx.serialization.Serializable import kotlinx.serialization.Transient import org.tasks.data.db.Table import org.tasks.time.printTimestamp import java.util.concurrent.TimeUnit +@Parcelize @Serializable @Entity( tableName = Alarm.TABLE_NAME, @@ -25,64 +26,33 @@ import java.util.concurrent.TimeUnit ) ] ) -class Alarm : Parcelable { +data class Alarm( @PrimaryKey(autoGenerate = true) @ColumnInfo(name = "_id") @Transient - var id: Long = 0 - + var id: Long = 0, @ColumnInfo(name = "task", index = true) @Transient - var task: Long = 0 - + var task: Long = 0, @ColumnInfo(name = "time") - var time: Long = 0 - + var time: Long = 0, @ColumnInfo(name = "type", defaultValue = "0") - var type: Int = 0 - + var type: Int = 0, @ColumnInfo(name = "repeat", defaultValue = "0") - var repeat: Int = 0 - + var repeat: Int = 0, @ColumnInfo(name = "interval", defaultValue = "0") - var interval: Long = 0 - - constructor() - + var interval: Long = 0, +) : Parcelable { @Ignore - constructor(parcel: Parcel) { - id = parcel.readLong() - task = parcel.readLong() - time = parcel.readLong() - type = parcel.readInt() - repeat = parcel.readInt() - interval = parcel.readLong() - } - - @Ignore - constructor(task: Long, time: Long, type: Int, repeat: Int = 0, interval: Long = 0) { - this.task = task - this.time = time - this.type = type - this.repeat = repeat - this.interval = interval - } - - override fun writeToParcel(parcel: Parcel, flags: Int) { - parcel.writeLong(id) - parcel.writeLong(task) - parcel.writeLong(time) - parcel.writeInt(type) - parcel.writeInt(repeat) - parcel.writeLong(interval) - } - - override fun describeContents() = 0 + constructor(task: Long, time: Long, type: Int, repeat: Int = 0, interval: Long = 0): this( + id = 0, + task = task, + time = time, + type = type, + repeat = repeat, + interval = interval + ) - override fun toString(): String { - val timestamp = if (type == TYPE_DATE_TIME) printTimestamp(time) else time - return "Alarm(id=$id, task=$task, time=$timestamp, type=$type, repeat=$repeat, interval=$interval)" - } fun same(other: Alarm) = type == other.type && @@ -90,30 +60,9 @@ class Alarm : Parcelable { repeat == other.repeat && interval == other.interval - override fun equals(other: Any?): Boolean { - if (this === other) return true - if (javaClass != other?.javaClass) return false - - other as Alarm - - if (id != other.id) return false - if (task != other.task) return false - if (time != other.time) return false - if (type != other.type) return false - if (repeat != other.repeat) return false - if (interval != other.interval) return false - - return true - } - - override fun hashCode(): Int { - var result = id.hashCode() - result = 31 * result + task.hashCode() - result = 31 * result + time.hashCode() - result = 31 * result + type - result = 31 * result + repeat - result = 31 * result + interval.hashCode() - return result + override fun toString(): String { + val timestamp = if (type == TYPE_DATE_TIME) printTimestamp(time) else time + return "Alarm(id=$id, task=$task, time=$timestamp, type=$type, repeat=$repeat, interval=$interval)" } companion object { @@ -137,12 +86,5 @@ class Alarm : Parcelable { fun whenOverdue(task: Long) = Alarm(task, TimeUnit.DAYS.toMillis(1), TYPE_REL_END, 6, TimeUnit.DAYS.toMillis(1)) - - @JvmField - val CREATOR = object : Parcelable.Creator { - override fun createFromParcel(parcel: Parcel) = Alarm(parcel) - - override fun newArray(size: Int): Array = arrayOfNulls(size) - } } } \ No newline at end of file diff --git a/data/src/main/kotlin/org/tasks/data/entity/Notification.kt b/data/src/main/kotlin/org/tasks/data/entity/Notification.kt index 7876cc4c2..b237c266b 100644 --- a/data/src/main/kotlin/org/tasks/data/entity/Notification.kt +++ b/data/src/main/kotlin/org/tasks/data/entity/Notification.kt @@ -21,26 +21,19 @@ import org.tasks.data.db.Table ) ] ) -class Notification { +data class Notification( @PrimaryKey(autoGenerate = true) @ColumnInfo(name = "uid") - var uid = 0 - + var uid: Long = 0, @ColumnInfo(name = "task") - var taskId: Long = 0 - + var taskId: Long = 0, @ColumnInfo(name = "timestamp") - var timestamp: Long = 0 - + var timestamp: Long = 0, @ColumnInfo(name = "type") - var type = 0 - + var type: Int = 0, @ColumnInfo(name = "location") - var location: Long? = null - - override fun toString(): String = - "Notification(uid=$uid, taskId=$taskId, timestamp=$timestamp, type=$type, location=$location)" - + var location: Long? = null, +) { companion object { const val TABLE_NAME = "notification" val TABLE = Table(TABLE_NAME)