diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt index 90a655016d64..c86f5e853ff0 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/DeckPicker.kt @@ -158,6 +158,7 @@ import com.ichi2.anki.preferences.AdvancedSettingsFragment import com.ichi2.anki.preferences.PreferencesActivity import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.anki.receiver.SdCardReceiver +import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase import com.ichi2.anki.servicelayer.PreferenceUpgradeService import com.ichi2.anki.servicelayer.ScopedStorageService import com.ichi2.anki.settings.Prefs @@ -600,6 +601,10 @@ open class DeckPicker : checkWebviewVersion(this) + // If a review reminder deserialization error has recently occurred + // (ex. on app boot, when the app opened, etc.), inform the user via a dialog + ReviewRemindersDatabase.checkDeserializationErrors(this) + setFragmentResultListener(REQUEST_KEY) { _, bundle -> when (CustomStudyAction.fromBundle(bundle)) { CustomStudyAction.CUSTOM_STUDY_SESSION -> { diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt index 3f1b017ae0c6..0d533a73ca20 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminder.kt @@ -22,6 +22,7 @@ import android.text.format.DateFormat import com.ichi2.anki.CollectionManager.withCol import com.ichi2.anki.common.time.TimeManager import com.ichi2.anki.libanki.DeckId +import com.ichi2.anki.libanki.EpochMilliseconds import com.ichi2.anki.settings.Prefs import kotlinx.parcelize.IgnoredOnParcel import kotlinx.parcelize.Parcelize @@ -185,6 +186,9 @@ sealed class ReviewReminderScope : Parcelable { * @param enabled Whether the review reminder's notifications are active or disabled. * @param profileID ID representing the profile which created this review reminder, as review reminders for * multiple profiles might be active simultaneously. + * @param latestNotifTime The time at which this review reminder last attempted to fire a routine daily (non-snooze) + * notification, in epoch milliseconds, or the time at which it was created if no notification has ever been fired. + * See [shouldImmediatelyFire]. * @param onlyNotifyIfNoReviews If true, only notify the user if this scope has not been reviewed today yet. */ @Serializable @@ -196,8 +200,9 @@ data class ReviewReminder private constructor( val cardTriggerThreshold: ReviewReminderCardTriggerThreshold, val scope: ReviewReminderScope, var enabled: Boolean, + var latestNotifTime: EpochMilliseconds, val profileID: String, - val onlyNotifyIfNoReviews: Boolean = false, + val onlyNotifyIfNoReviews: Boolean, ) : Parcelable, ReviewReminderSchema { companion object { @@ -219,11 +224,42 @@ data class ReviewReminder private constructor( cardTriggerThreshold, scope, enabled, + latestNotifTime = TimeManager.time.calendar().timeInMillis, profileID, onlyNotifyIfNoReviews, ) } + /** + * Updates [latestNotifTime] to the current time. + * This should be called whenever this review reminder attempts to fire a routine daily (non-snooze) notification. + */ + fun updateLatestNotifTime() { + latestNotifTime = TimeManager.time.calendar().timeInMillis + } + + /** + * Checks if this review reminder has tried to fire a routine daily (non-snooze) notification in the time between + * its latest scheduled firing time and now. If not, this method returns true, indicating that a notification + * should be immediately fired for this review reminder. + */ + fun shouldImmediatelyFire(): Boolean { + val (hour, minute) = this.time + + val currentTimestamp = TimeManager.time.calendar() + val latestScheduledTimestamp = currentTimestamp.clone() as Calendar + latestScheduledTimestamp.apply { + set(Calendar.HOUR_OF_DAY, hour) + set(Calendar.MINUTE, minute) + set(Calendar.SECOND, 0) + if (after(currentTimestamp)) { + add(Calendar.DAY_OF_YEAR, -1) + } + } + + return latestNotifTime < latestScheduledTimestamp.timeInMillis + } + /** * This is the up-to-date schema, we cannot migrate to a newer version. */ diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderGroup.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderGroup.kt new file mode 100644 index 000000000000..de66f6f1f9aa --- /dev/null +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderGroup.kt @@ -0,0 +1,119 @@ +/* + * Copyright (c) 2026 Eric Li + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation; either version 3 of the License, or (at your option) any later + * version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A + * PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +package com.ichi2.anki.reviewreminders + +import kotlinx.serialization.SerializationException +import kotlinx.serialization.json.Json + +/** + * A group of review reminders, all for the same [ReviewReminderScope], + * persisted as a single JSON string in SharedPreferences. + * + * Essentially a wrapper around a HashMap of [ReviewReminderId] to [ReviewReminder], + * explicitly defined to restrict what can be done with the interface and to eliminate the + * need to verbosely type out "HashMap" everywhere. + * + * A HashMap is used to allow for O(1) access to individual reminders by [ReviewReminderId]. + */ +class ReviewReminderGroup( + private val underlyingMap: HashMap, +) { + constructor() : this(HashMap()) + + constructor(map: Map) : this(HashMap(map)) + + /** + * Manually construct a [ReviewReminderGroup] from key-value pairs. + */ + constructor(vararg pairs: Pair) : this( + buildMap { pairs.forEach { put(it.first, it.second) } }, + ) + + /** + * Merge multiple [ReviewReminderGroup]s into one. + * [ReviewReminderId] should be unique, so there should be no collisions. + */ + constructor(vararg groups: ReviewReminderGroup) : this( + buildMap { groups.forEach { putAll(it.underlyingMap) } }, + ) + + /** + * Constructs a [ReviewReminderGroup] from a serialized JSON string. + * + * @throws SerializationException If the stored string is not a valid JSON string. + * @throws IllegalArgumentException If the decoded reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>. + */ + constructor(serializedString: String) : this( + Json.decodeFromString>(serializedString), + ) + + /** + * Serializes this [ReviewReminderGroup] to a JSON string for storage. + */ + fun serializeToString(): String = Json.encodeToString(underlyingMap) + + /** + * Merge two [ReviewReminderGroup]s into one. + * [ReviewReminderId] should be unique, so there should be no collisions. + */ + operator fun plus(other: ReviewReminderGroup): ReviewReminderGroup = ReviewReminderGroup(underlyingMap + other.underlyingMap) + + operator fun get(id: ReviewReminderId) = underlyingMap[id] + + operator fun set( + id: ReviewReminderId, + reminder: ReviewReminder, + ) { + underlyingMap[id] = reminder + } + + fun isEmpty(): Boolean = underlyingMap.isEmpty() + + fun remove(id: ReviewReminderId) { + underlyingMap.remove(id) + } + + fun forEach(action: (ReviewReminderId, ReviewReminder) -> Unit) { + underlyingMap.forEach { (id, reminder) -> action(id, reminder) } + } + + /** + * Get the values of the underlying map, removing the [ReviewReminderId] keys. + */ + fun getRemindersList(): List = underlyingMap.values.toList() + + /** + * Toggles whether a [ReviewReminder] is enabled. + */ + fun toggleEnabled(id: ReviewReminderId) { + underlyingMap[id]?.apply { enabled = !enabled } + } + + override fun equals(other: Any?): Boolean = other is ReviewReminderGroup && other.underlyingMap == this.underlyingMap + + override fun hashCode(): Int = underlyingMap.hashCode() +} + +/** + * Convenience extension constructor for merging a list of [ReviewReminderGroup]s into one. + */ +fun List.mergeAll() = ReviewReminderGroup(*this.toTypedArray()) + +/** + * Convenience typealias for the mutation functions passed to editors of [ReviewReminderGroup]. + */ +typealias ReviewReminderGroupEditor = (ReviewReminderGroup) -> ReviewReminderGroup diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderSchema.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderSchema.kt new file mode 100644 index 000000000000..c4787a96b74f --- /dev/null +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewReminderSchema.kt @@ -0,0 +1,184 @@ +/* + * Copyright (c) 2026 Eric Li + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation; either version 3 of the License, or (at your option) any later + * version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A + * PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +package com.ichi2.anki.reviewreminders + +import com.ichi2.anki.libanki.DeckId +import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase.StoredReviewReminderGroup +import kotlinx.serialization.Serializable + +/** + * Inline value class for review reminder schema versions. + * @see [StoredReviewReminderGroup] + * @see [ReviewReminder] + */ +@JvmInline +@Serializable +value class ReviewReminderSchemaVersion( + val value: Int, +) { + init { + require(value >= 1) { "Review reminder schema version must be >= 1" } + // We do not check that it is <= SCHEMA_VERSION here because then declaring SCHEMA_VERSION would be circular + } +} + +/** + * When [ReviewReminder] is updated by a developer, implement this interface in a new data class which + * has the same fields as the old version of [ReviewReminder], then implement the [migrate] method which + * transforms old [ReviewReminder]s to new [ReviewReminder]s. Also ensure that the previous [ReviewReminderSchema] + * in the migration version chain ([ReviewRemindersDatabase.oldReviewReminderSchemasForMigration]) has its [migrate] method + * edited to return instances of the newly-created [ReviewReminderSchema]. Then, increment [ReviewRemindersDatabase.schemaVersion]. + * + * Data classes implementing this interface should be marked as @Serializable. Any new types defined for ReviewReminderSchemas + * should also be marked as @Serializable. + * + * @see [ReviewRemindersDatabase.performSchemaMigration]. + * @see [ReviewReminder] + */ +@Serializable +sealed interface ReviewReminderSchema { + /** + * All review reminders must have an identifying ID. + * This is necessary to facilitate migrations. See the implementation of [ReviewRemindersDatabase.performSchemaMigration] for details. + */ + val id: ReviewReminderId + + /** + * Transforms this [ReviewReminderSchema] to the next version of the [ReviewReminderSchema]. + */ + fun migrate(): ReviewReminderSchema +} + +/** + * Version 1 of [ReviewReminderSchema]. Updated to Version 2 by adding [ReviewReminder.onlyNotifyIfNoReviews]. + */ +@Serializable +data class ReviewReminderSchemaV1( + override val id: ReviewReminderId, + val time: ReviewReminderTime, + val cardTriggerThreshold: ReviewReminderCardTriggerThreshold, + val scope: ReviewReminderScope, + var enabled: Boolean, + val profileID: String, + val onlyNotifyIfNoReviews: Boolean = false, +) : ReviewReminderSchema { + override fun migrate(): ReviewReminderSchema = + ReviewReminderSchemaV2( + id = id, + time = time, + cardTriggerThreshold = cardTriggerThreshold, + scope = scope, + enabled = enabled, + profileID = profileID, + onlyNotifyIfNoReviews = onlyNotifyIfNoReviews, + ) +} + +/** + * Version 2 of [ReviewReminderSchema]. Updated to Version 3 by adding [ReviewReminder.latestNotifTime]. + */ +@Serializable +data class ReviewReminderSchemaV2( + override val id: ReviewReminderId, + val time: ReviewReminderTime, + val cardTriggerThreshold: ReviewReminderCardTriggerThreshold, + val scope: ReviewReminderScope, + var enabled: Boolean, + val profileID: String, + val onlyNotifyIfNoReviews: Boolean, +) : ReviewReminderSchema { + override fun migrate(): ReviewReminderSchema = + ReviewReminder.createReviewReminder( + time = time, + cardTriggerThreshold = cardTriggerThreshold, + scope = scope, + enabled = enabled, + profileID = profileID, + onlyNotifyIfNoReviews = onlyNotifyIfNoReviews, + ) +} + +/** + * Schema migration settings for testing purposes. + * Consult this as an example of how to save old schemas and define their [ReviewReminderSchema.migrate] methods. + */ +object TestingReviewReminderMigrationSettings { + /** + * A sample old review reminder schema. Perhaps this was how the [ReviewReminder] data class was originally implemented. + * We would like to test the code that checks if review reminders stored on the device adhere to an old, outdated schema. + * In particular, does the code correctly migrate the serialized data class strings to the updated, current version of [ReviewReminder]? + */ + @Serializable + data class ReviewReminderTestSchemaVersionOne( + override val id: ReviewReminderId, + val hour: Int, + val minute: Int, + val cardTriggerThreshold: Int, + val did: DeckId, + val enabled: Boolean = true, + ) : ReviewReminderSchema { + override fun migrate(): ReviewReminderSchema = + ReviewReminderTestSchemaVersionTwo( + id = this.id, + time = VersionTwoDataClasses.ReviewReminderTime(hour, minute), + snoozeAmount = 1, + cardTriggerThreshold = this.cardTriggerThreshold, + did = this.did, + enabled = enabled, + ) + } + + /** + * Here's an example of how you can handle renamed fields in a data class stored as part of a [ReviewReminder]. + * Otherwise, there's a namespace collision with [ReviewReminderTime]. + * + * This class will be serialized into "ReviewReminderTime(timeHour=#, timeMinute=#)", which otherwise might conflict + * with the updated definition of [ReviewReminderTime], which is serialized as "ReviewReminderTime(hour=#, minute=#)". + * When we read the outdated schema from the disk, we need to tell the deserializer that it is reading a + * [VersionTwoDataClasses.ReviewReminderTime] rather than a [ReviewReminderTime], even though the names are the same. + * + * @see ReviewReminderTestSchemaVersionTwo + */ + object VersionTwoDataClasses { + @Serializable + data class ReviewReminderTime( + val timeHour: Int, + val timeMinute: Int, + ) + } + + /** + * Another example of an old review reminder schema. See [ReviewReminderTestSchemaVersionOne] for more details. + */ + @Serializable + data class ReviewReminderTestSchemaVersionTwo( + override val id: ReviewReminderId, + val time: VersionTwoDataClasses.ReviewReminderTime, + val snoozeAmount: Int, + val cardTriggerThreshold: Int, + val did: DeckId, + val enabled: Boolean = true, + ) : ReviewReminderSchema { + override fun migrate(): ReviewReminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(this.time.timeHour, this.time.timeMinute), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(this.cardTriggerThreshold), + scope = if (this.did == -1L) ReviewReminderScope.Global else ReviewReminderScope.DeckSpecific(this.did), + enabled = enabled, + ) + } +} diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabase.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabase.kt index 3a6ed9bbfd14..869238328686 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabase.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabase.kt @@ -21,8 +21,10 @@ import android.content.SharedPreferences import androidx.annotation.VisibleForTesting import androidx.core.content.edit import com.ichi2.anki.AnkiDroidApp +import com.ichi2.anki.CrashReportService import com.ichi2.anki.libanki.DeckId -import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase.StoredReviewRemindersMap +import com.ichi2.anki.settings.Prefs +import com.ichi2.anki.showError import kotlinx.serialization.InternalSerializationApi import kotlinx.serialization.Serializable import kotlinx.serialization.SerializationException @@ -68,20 +70,20 @@ object ReviewRemindersDatabase { /** * Key in SharedPreferences for retrieving deck-specific reminders. * Should have deck ID appended to its end, ex. "deck_12345". - * Its value is a HashMap<[ReviewReminderId], [ReviewReminder]> serialized as a JSON String. + * Its value is a [ReviewReminderGroup] serialized as a JSON String. */ @VisibleForTesting const val DECK_SPECIFIC_KEY = "deck_" /** * Key in SharedPreferences for retrieving app-wide reminders. - * Its value is a HashMap<[ReviewReminderId], [ReviewReminder]> serialized as a JSON String. + * Its value is a [ReviewReminderGroup] serialized as a JSON String. */ @VisibleForTesting const val APP_WIDE_KEY = "app_wide" /** - * The form in which HashMap<[ReviewReminderId], [ReviewReminder]> are actually written to SharedPreferences. + * The form in which [ReviewReminderGroup] are actually written to SharedPreferences. * This allows us to check the version of [ReviewReminder] stored before trying to deserialize the JSON string, * allowing us to carefully handle schema migration. Otherwise, if an older version of [ReviewReminder] is encoded * and we try to decode it into a newer form of [ReviewReminder], an error will be thrown. @@ -89,10 +91,13 @@ object ReviewRemindersDatabase { * We assume that the version is accurate; e.x. if the version is 3, then the [ReviewReminder] stored is indeed * of schema version 3. This should be safe to assume since writing this data class to SharedPreferences is an * atomic operation. + * + * @param version The version of the [remindersMapJson] which is stored. + * @param remindersMapJson The stored [ReviewReminderGroup]'s underlying hashmap as a serialized JSON string. */ @Serializable @VisibleForTesting - data class StoredReviewRemindersMap( + data class StoredReviewReminderGroup( val version: ReviewReminderSchemaVersion, val remindersMapJson: String, ) @@ -100,13 +105,15 @@ object ReviewRemindersDatabase { /** * Current [ReviewReminder] schema version. Whenever [ReviewReminder] is modified, this integer MUST be incremented. * - * Version 1: 3 August 2025 + * Version 1: 3 August 2025 - Initial version + * Version 2: 25 January 2026 - Added [ReviewReminder.onlyNotifyIfNoReviews] + * Version 3: 8 February 2026 - Added [ReviewReminder.latestNotifTime] * * @see [oldReviewReminderSchemasForMigration] * @see [ReviewReminder] */ @VisibleForTesting - var schemaVersion = ReviewReminderSchemaVersion(1) + var schemaVersion = ReviewReminderSchemaVersion(3) /** * A map of all old [ReviewReminderSchema]s that [ReviewRemindersDatabase.performSchemaMigration] will attempt to migrate old @@ -123,7 +130,9 @@ object ReviewRemindersDatabase { @VisibleForTesting var oldReviewReminderSchemasForMigration: Map> = mapOf( - ReviewReminderSchemaVersion(1) to ReviewReminder::class, // Most up to date version + ReviewReminderSchemaVersion(1) to ReviewReminderSchemaV1::class, + ReviewReminderSchemaVersion(2) to ReviewReminderSchemaV2::class, + ReviewReminderSchemaVersion(3) to ReviewReminder::class, // Most up to date version ) /** @@ -136,26 +145,26 @@ object ReviewRemindersDatabase { * dynamically via [oldReviewReminderSchemasForMigration] rather than at compile-time. * The possible schemas to deserialize from are inputted dynamically so that unit tests are possible. * - * @param encodedReviewRemindersKey The key with which the [encodedReviewRemindersMap] is stored in SharedPreferences, + * @param encodedReviewReminderKey The key with which the [encodedReviewReminderGroup] is stored in SharedPreferences, * used for writing the migrated map back into SharedPreferences. - * @param encodedReviewRemindersMap The encoded review reminders map to migrate. - * @param fromVersion The schema version of [encodedReviewRemindersMap]. + * @param encodedReviewReminderGroup The encoded review reminders map to migrate. + * @param fromVersion The schema version of [encodedReviewReminderGroup]. * @param toVersion The schema version of the new review reminders map. * * @throws SerializationException If the [fromVersion] is less than 1 or greater than [schemaVersion], or if the - * [encodedReviewRemindersMap] is not a valid JSON string, or if the final result of migration is somehow not a [ReviewReminder]. - * @throws IllegalArgumentException If the [encodedReviewRemindersMap] is not actually of version [fromVersion], + * [encodedReviewReminderGroup] is not a valid JSON string, or if the final result of migration is somehow not a [ReviewReminder]. + * @throws IllegalArgumentException If the [encodedReviewReminderGroup] is not actually of version [fromVersion], * or if the [fromVersion] is not in [oldReviewReminderSchemasForMigration]. * * @see [ReviewReminder] */ @OptIn(InternalSerializationApi::class) private fun performSchemaMigration( - encodedReviewRemindersKey: String, - encodedReviewRemindersMap: String, + encodedReviewReminderKey: String, + encodedReviewReminderGroup: String, fromVersion: ReviewReminderSchemaVersion, toVersion: ReviewReminderSchemaVersion = schemaVersion, - ): HashMap { + ): ReviewReminderGroup { Timber.i("Beginning migration from $fromVersion to $toVersion") if (fromVersion.value < 1 || fromVersion.value > toVersion.value @@ -168,7 +177,7 @@ object ReviewRemindersDatabase { oldReviewReminderSchemasForMigration[fromVersion] ?: throw IllegalArgumentException("Review reminder schema version not found: $fromVersion") val mapDeserializer = MapSerializer(ReviewReminderId.serializer(), oldSchema.serializer()) - val mapDecoded = Json.decodeFromString(mapDeserializer, encodedReviewRemindersMap) + val mapDecoded = Json.decodeFromString(mapDeserializer, encodedReviewReminderGroup) // Migrate step by step var currentMap = mapDecoded @@ -185,99 +194,134 @@ object ReviewRemindersDatabase { } // Write to SharedPreferences, then return deserialized map - val finalMap = - currentMap.mapValues { (_, value) -> - value as? ReviewReminder ?: throw SerializationException("Expected ReviewReminder, got ${value::class.qualifiedName}") - } - val jsonString = encodeJson(finalMap) + val finalGroup = + ReviewReminderGroup( + currentMap.mapValues { (_, value) -> + value as? ReviewReminder ?: throw SerializationException("Expected ReviewReminder, got ${value::class.qualifiedName}") + }, + ) remindersSharedPrefs.edit { - putString(encodedReviewRemindersKey, jsonString) + putString(encodedReviewReminderKey, encodeJson(finalGroup)) } - return HashMap(finalMap) + return finalGroup } /** - * Decode an encoded HashMap<[ReviewReminderId], [ReviewReminder]> JSON string which has been stored as a [StoredReviewRemindersMap]. - * @see Json.decodeFromString - * @throws SerializationException If the stored string is not a valid JSON string. - * @throws IllegalArgumentException If the decoded reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>, + * Decode an encoded [ReviewReminderGroup] JSON string which has been stored as a [StoredReviewReminderGroup]. + * Deletes the stored review reminders and returns an empty [ReviewReminderGroup] if deserialization fails * and no valid schema migrations exist. + * + * @see Json.decodeFromString */ private fun decodeJson( jsonString: String, - deckKeyForMigrationPurposes: String, - ): HashMap { - val storedReviewRemindersMap = Json.decodeFromString(jsonString) - return if (storedReviewRemindersMap.version.value != schemaVersion.value) { - performSchemaMigration( - deckKeyForMigrationPurposes, - storedReviewRemindersMap.remindersMapJson, - storedReviewRemindersMap.version, - schemaVersion, - ) - } else { - Json.decodeFromString>(storedReviewRemindersMap.remindersMapJson) + jsonStringKey: String, + ): ReviewReminderGroup { + Timber.v("Decoding review reminders JSON string: $jsonString") + try { + val storedReviewReminderGroup = Json.decodeFromString(jsonString) + return if (storedReviewReminderGroup.version.value != schemaVersion.value) { + performSchemaMigration( + jsonStringKey, + storedReviewReminderGroup.remindersMapJson, + storedReviewReminderGroup.version, + schemaVersion, + ) + } else { + ReviewReminderGroup(storedReviewReminderGroup.remindersMapJson) + } + } catch (e: Exception) { + when (e) { + is SerializationException, + is IllegalArgumentException, + -> { + // Log error, it will be displayed to the user either immediately if the app is open or when they next open the app if not + val errorString = "Encountered (${e.message}) while parsing $jsonString" + Prefs.reviewReminderDeserializationErrors = Prefs.reviewReminderDeserializationErrors.orEmpty() + "[$errorString]" + Timber.e(e, errorString) + CrashReportService.sendExceptionReport( + e, + origin = "ReviewRemindersDatabase:decodeJson", + additionalInfo = jsonString, + ) + + // Delete corrupted value, then return an empty group + remindersSharedPrefs.edit { remove(jsonStringKey) } + return ReviewReminderGroup() + } + else -> throw e + } } } /** - * Encode a Map<[ReviewReminderId], [ReviewReminder]> as a [StoredReviewRemindersMap] JSON string. + * Encode a [ReviewReminderGroup] as a [StoredReviewReminderGroup] JSON string. + * * @see Json.encodeToString - * @throws SerializationException If the stored string is somehow not a valid JSON string, even though the input parameter is type-checked. */ - private fun encodeJson(reminders: Map): String = - Json.encodeToString(StoredReviewRemindersMap.serializer(), StoredReviewRemindersMap(schemaVersion, Json.encodeToString(reminders))) + private fun encodeJson(reminders: ReviewReminderGroup): String = + Json.encodeToString(StoredReviewReminderGroup.serializer(), StoredReviewReminderGroup(schemaVersion, reminders.serializeToString())) /** * Get the [ReviewReminder]s for a specific key. - * @throws SerializationException If the value associated with this key is not valid JSON string. - * @throws IllegalArgumentException If the decoded reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>. + * Deletes the stored review reminders and returns an empty [ReviewReminderGroup] if deserialization fails + * and no valid schema migrations exist. */ - private fun getRemindersForKey(key: String): HashMap { - val jsonString = remindersSharedPrefs.getString(key, null) ?: return hashMapOf() - return decodeJson(jsonString, deckKeyForMigrationPurposes = key) + private fun getRemindersForKey(key: String): ReviewReminderGroup { + val jsonString = remindersSharedPrefs.getString(key, null) ?: return ReviewReminderGroup() + return decodeJson(jsonString, jsonStringKey = key) } /** * Get the [ReviewReminder]s for a specific deck. - * @throws SerializationException If the reminders map has not been stored in SharedPreferences as a valid JSON string. - * @throws IllegalArgumentException If the decoded reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>. + * Deletes the stored review reminders and returns an empty [ReviewReminderGroup] if deserialization fails + * and no valid schema migrations exist. */ - fun getRemindersForDeck(did: DeckId): HashMap = getRemindersForKey(DECK_SPECIFIC_KEY + did) + fun getRemindersForDeck(did: DeckId): ReviewReminderGroup = getRemindersForKey(DECK_SPECIFIC_KEY + did) /** * Get the app-wide [ReviewReminder]s. - * @throws SerializationException If the reminders map has not been stored in SharedPreferences as a valid JSON string. - * @throws IllegalArgumentException If the decoded reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>. + * Deletes the stored review reminders and returns an empty [ReviewReminderGroup] if deserialization fails + * and no valid schema migrations exist. */ - fun getAllAppWideReminders(): HashMap = getRemindersForKey(APP_WIDE_KEY) + fun getAllAppWideReminders(): ReviewReminderGroup = getRemindersForKey(APP_WIDE_KEY) /** * Get all [ReviewReminder]s that are associated with a specific deck, all in a single flattened map. - * @throws SerializationException If the reminders maps have not been stored in SharedPreferences as valid JSON strings. - * @throws IllegalArgumentException If the decoded reminders maps are not instances of HashMap<[ReviewReminderId], [ReviewReminder]>. + * If the stored review reminders for any specific deck fail to deserialize and no valid schema migrations exist, + * those reminders are deleted and not included in the resulting [ReviewReminderGroup]. */ - fun getAllDeckSpecificReminders(): HashMap = + fun getAllDeckSpecificReminders(): ReviewReminderGroup = remindersSharedPrefs .all .filterKeys { it.startsWith(DECK_SPECIFIC_KEY) } - .flatMap { (key, value) -> decodeJson(value.toString(), deckKeyForMigrationPurposes = key).entries } - .associateTo(hashMapOf()) { it.toPair() } + .map { (key, value) -> decodeJson(value.toString(), jsonStringKey = key) } + .mergeAll() /** - * Edit the [ReviewReminder]s for a specific key. Deletes the review reminders map for this key if, after the editing operation, - * no review reminders remain. + * Edit the [ReviewReminder]s for a specific key. + * Deletes the review reminders map for this key if, after the editing operation, no review reminders remain. + * If the stored review reminders for any specific deck fail to deserialize and no valid schema migrations exist, + * those reminders are deleted and not included in the resulting [ReviewReminderGroup]. + * * @param key - * @param reminderEditor A lambda that takes the current map and returns the updated map. - * @throws SerializationException If the current reminders map has not been stored in SharedPreferences as a valid JSON string. - * @throws IllegalArgumentException If the decoded current reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>. + * @param editor A lambda that takes the current map and returns the updated map. + * @param expectedScope The expected scope of all review reminders in the resulting map. + * + * @throws IllegalArgumentException If the result of applying the [editor] contains review reminders of a scope other than [expectedScope]. */ private fun editRemindersForKey( key: String, - reminderEditor: (HashMap) -> Map, + editor: ReviewReminderGroupEditor, + expectedScope: ReviewReminderScope, ) { val existingReminders = getRemindersForKey(key) - val updatedReminders = reminderEditor(existingReminders) + val updatedReminders = editor(existingReminders) + + require(updatedReminders.getRemindersList().all { it.scope == expectedScope }) { + "Tried to write review reminders of an unexpected, incompatible scope to scope: $expectedScope" + } + remindersSharedPrefs.edit { if (updatedReminders.isEmpty()) { remove(key) @@ -289,66 +333,81 @@ object ReviewRemindersDatabase { /** * Edit the [ReviewReminder]s for a specific deck. - * This assumes the resulting map contains only reminders of scope [ReviewReminderScope.DeckSpecific]. + * * @param did - * @param reminderEditor A lambda that takes the current map and returns the updated map. - * @throws SerializationException If the current reminders map has not been stored in SharedPreferences as a valid JSON string. - * @throws IllegalArgumentException If the decoded current reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>. + * @param editor A lambda that takes the current map and returns the updated map. + * + * @throws IllegalArgumentException If the result of applying the [editor] contains review reminders + * of a scope other than [ReviewReminderScope.DeckSpecific]. */ fun editRemindersForDeck( did: DeckId, - reminderEditor: (HashMap) -> Map, - ) = editRemindersForKey(DECK_SPECIFIC_KEY + did, reminderEditor) + editor: ReviewReminderGroupEditor, + ) = editRemindersForKey(DECK_SPECIFIC_KEY + did, editor, ReviewReminderScope.DeckSpecific(did)) /** * Edit the app-wide [ReviewReminder]s. - * This assumes the resulting map contains only reminders of scope [ReviewReminderScope.Global]. - * @param reminderEditor A lambda that takes the current map and returns the updated map. - * @throws SerializationException If the current reminders map has not been stored in SharedPreferences as a valid JSON string. - * @throws IllegalArgumentException If the decoded current reminders map is not a HashMap<[ReviewReminderId], [ReviewReminder]>. + * + * @param editor A lambda that takes the current map and returns the updated map. + * + * @throws IllegalArgumentException If the result of applying the [editor] contains review reminders + * of a scope other than [ReviewReminderScope.Global]. + */ + fun editAllAppWideReminders(editor: ReviewReminderGroupEditor) = editRemindersForKey(APP_WIDE_KEY, editor, ReviewReminderScope.Global) + + /** + * Shows an error message dialog if a review reminder deserialization error has recently happened. + * Checks if a deserialization error has recently occurred by checking if anything is present in + * [Prefs.reviewReminderDeserializationErrors], emptying the preference after reading it. + * + * @param context A valid themed context (ie. not applicationContext) to display the error dialog in. */ - fun editAllAppWideReminders(reminderEditor: (HashMap) -> Map) = - editRemindersForKey(APP_WIDE_KEY, reminderEditor) + fun checkDeserializationErrors(context: Context) { + Prefs.reviewReminderDeserializationErrors?.let { errorString -> + if (errorString.isNotEmpty()) { + context.showError( + message = + "An error occurred while loading your review reminders, corrupted reminders have been deleted. " + + "Details:\n\n$errorString", + crashReportData = null, // Crash reports are sent when the error is first encountered + ) + Prefs.reviewReminderDeserializationErrors = "" + } + } + } } /** - * Inline value class for review reminder schema versions. - * @see [StoredReviewRemindersMap] - * @see [ReviewReminder] + * Lambda that can be fed into [ReviewRemindersDatabase.editRemindersForDeck] or + * [ReviewRemindersDatabase.editAllAppWideReminders] which deletes the given review reminder. */ -@JvmInline -@Serializable -value class ReviewReminderSchemaVersion( - val value: Int, -) { - init { - require(value >= 1) { "Review reminder schema version must be >= 1" } - // We do not check that it is <= SCHEMA_VERSION here because then declaring SCHEMA_VERSION would be circular +fun deleteReminder(reminder: ReviewReminder) = + { reminders: ReviewReminderGroup -> + reminders.apply { + remove(reminder.id) + } } -} /** - * When [ReviewReminder] is updated by a developer, implement this interface in a new data class which - * has the same fields as the old version of [ReviewReminder], then implement the [migrate] method which - * transforms old [ReviewReminder]s to new [ReviewReminder]s. Also ensure that the previous [ReviewReminderSchema] - * in the migration version chain ([ReviewRemindersDatabase.oldReviewReminderSchemasForMigration]) has its [migrate] method - * edited to return instances of the newly-created [ReviewReminderSchema]. Then, increment [ReviewRemindersDatabase.schemaVersion]. - * - * Data classes implementing this interface should be marked as @Serializable. Any new types defined for ReviewReminderSchemas - * should also be marked as @Serializable. - * - * @see [ReviewRemindersDatabase.performSchemaMigration]. - * @see [ReviewReminder] + * Lambda that can be fed into [ReviewRemindersDatabase.editRemindersForDeck] or + * [ReviewRemindersDatabase.editAllAppWideReminders] which updates the given review reminder if it + * exists or inserts it if it doesn't (an "upsert" operation) */ -interface ReviewReminderSchema { - /** - * All review reminders must have an identifying ID. - * This is necessary to facilitate migrations. See the implementation of [ReviewRemindersDatabase.performSchemaMigration] for details. - */ - val id: ReviewReminderId +fun upsertReminder(reminder: ReviewReminder) = + { reminders: ReviewReminderGroup -> + reminders.apply { + this[reminder.id] = reminder + } + } - /** - * Transforms this [ReviewReminderSchema] to the next version of the [ReviewReminderSchema]. - */ - fun migrate(): ReviewReminderSchema -} +/** + * Lambda that can be fed into [ReviewRemindersDatabase.editRemindersForDeck] or + * [ReviewRemindersDatabase.editAllAppWideReminders] which toggles whether the given review reminder + * is enabled. + */ +fun toggleReminder(reminder: ReviewReminder) = + { reminders: ReviewReminderGroup -> + reminders.apply { + toggleEnabled(reminder.id) + } + } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ScheduleReminders.kt b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ScheduleReminders.kt index 04e69dfdcd0e..880ac8db3bcb 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ScheduleReminders.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/reviewreminders/ScheduleReminders.kt @@ -34,7 +34,6 @@ import androidx.recyclerview.widget.DividerItemDecoration import androidx.recyclerview.widget.LinearLayoutManager import com.google.android.material.snackbar.Snackbar import com.ichi2.anki.CollectionManager.withCol -import com.ichi2.anki.CrashReportData.Companion.toCrashReportData import com.ichi2.anki.R import com.ichi2.anki.SingleFragmentActivity import com.ichi2.anki.canUserAccessDeck @@ -45,7 +44,6 @@ import com.ichi2.anki.libanki.DeckId import com.ichi2.anki.model.SelectableDeck import com.ichi2.anki.services.AlarmManagerService import com.ichi2.anki.settings.Prefs -import com.ichi2.anki.showError import com.ichi2.anki.snackbar.BaseSnackbarBuilderProvider import com.ichi2.anki.snackbar.SnackbarBuilder import com.ichi2.anki.snackbar.showSnackbar @@ -54,7 +52,6 @@ import com.ichi2.anki.withProgress import com.ichi2.utils.Permissions import com.ichi2.utils.Permissions.requestPermissionThroughDialogOrSettings import dev.androidbroadcast.vbpd.viewBinding -import kotlinx.serialization.SerializationException import timber.log.Timber /** @@ -101,7 +98,7 @@ class ScheduleReminders : * use [triggerUIUpdate]. Note that editing this map does not also automatically write to the database. * Writing to the database must be done separately. */ - private lateinit var reminders: HashMap + private lateinit var reminders: ReviewReminderGroup /** * Retrieving deck names for a given deck ID in [retrieveDeckNameFromID] requires a call to the collection. @@ -188,11 +185,11 @@ class ScheduleReminders : catchDatabaseExceptions { when (val scope = scheduleRemindersScope) { is ReviewReminderScope.Global -> { - HashMap(ReviewRemindersDatabase.getAllAppWideReminders() + ReviewRemindersDatabase.getAllDeckSpecificReminders()) + ReviewRemindersDatabase.getAllAppWideReminders() + ReviewRemindersDatabase.getAllDeckSpecificReminders() } is ReviewReminderScope.DeckSpecific -> ReviewRemindersDatabase.getRemindersForDeck(scope.did) } - } ?: hashMapOf() + } ?: ReviewReminderGroup() triggerUIUpdate() Timber.d("Database review reminders successfully loaded") } @@ -262,27 +259,6 @@ class ScheduleReminders : } } - /** - * Lambda that can be fed into [ReviewRemindersDatabase.editRemindersForDeck] or - * [ReviewRemindersDatabase.editAllAppWideReminders] which deletes the given review reminder. - */ - private fun deleteReminder(reminder: ReviewReminder) = - { reminders: HashMap -> - reminders.remove(reminder.id) - reminders - } - - /** - * Lambda that can be fed into [ReviewRemindersDatabase.editRemindersForDeck] or - * [ReviewRemindersDatabase.editAllAppWideReminders] which updates the given review reminder if it - * exists or inserts it if it doesn't (an "upsert" operation) - */ - private fun upsertReminder(reminder: ReviewReminder) = - { reminders: HashMap -> - reminders[reminder.id] = reminder - reminders - } - /** * Update the RecyclerView with the new or modified reminder. * @see handleAddEditDialogResult @@ -368,19 +344,12 @@ class ScheduleReminders : val reminder = reminders[id] ?: return val newState = !reminder.enabled - val performToggle: - (HashMap) -> Map = - { reminders -> - reminders[id]?.enabled = newState - reminders - } - // Update database launchCatchingTask { catchDatabaseExceptions { when (scope) { - is ReviewReminderScope.Global -> ReviewRemindersDatabase.editAllAppWideReminders(performToggle) - is ReviewReminderScope.DeckSpecific -> ReviewRemindersDatabase.editRemindersForDeck(scope.did, performToggle) + is ReviewReminderScope.Global -> ReviewRemindersDatabase.editAllAppWideReminders(toggleReminder(reminder)) + is ReviewReminderScope.DeckSpecific -> ReviewRemindersDatabase.editRemindersForDeck(scope.did, toggleReminder(reminder)) } } } @@ -424,7 +393,7 @@ class ScheduleReminders : /** * [AddEditReminderDialog] requires a [DeckSelectionDialog.DeckSelectionListener] to catch changes to - * the [com.ichi2.anki.DeckSpinnerSelection]. However, [AddEditReminderDialog] is removed from the + * the [DeckSelectionDialog]. However, [AddEditReminderDialog] is removed from the * fragment stack when the [DeckSelectionDialog] appears, so we set [ScheduleReminders] as the listener * and forward data to [AddEditReminderDialog] when a deck is selected. */ @@ -445,7 +414,7 @@ class ScheduleReminders : private fun triggerUIUpdate() { val listToDisplay = reminders - .values + .getRemindersList() .sortedBy { it.time.toSecondsFromMidnight() } .toList() adapter.submitList(listToDisplay) @@ -511,28 +480,14 @@ class ScheduleReminders : */ const val DECK_SELECTION_RESULT_REQUEST_KEY = "reminder_deck_selection_result_request_key" - private const val SERIALIZATION_ERROR_MESSAGE = - "Something went wrong. A serialization error was encountered while working with review reminders." - private const val DATA_TYPE_ERROR_MESSAGE = - "Something went wrong. An unexpected data type was found while working with review reminders." - /** - * Wrapper for database access. - * Shows an error dialog if [SerializationException]s or [IllegalArgumentException]s are thrown. + * Wrapper for database access in this fragment. + * Shows an error dialog via [ReviewRemindersDatabase.checkDeserializationErrors] if there are deserialization errors. * Shows a progress dialog if database access takes a long time. */ - private suspend fun Fragment.catchDatabaseExceptions(block: () -> T): T? = - try { - Timber.d("Attempting ReviewRemindersDatabase operation") - withProgress { block() } - } catch (e: SerializationException) { - Timber.e("JSON Serialization error occurred") - requireContext().showError("$SERIALIZATION_ERROR_MESSAGE: $e", e.toCrashReportData(requireContext())) - null - } catch (e: IllegalArgumentException) { - Timber.e("JSON Illegal argument exception occurred") - requireContext().showError("$DATA_TYPE_ERROR_MESSAGE: $e", e.toCrashReportData(requireContext())) - null + private suspend fun Fragment.catchDatabaseExceptions(block: suspend () -> T): T? = + withProgress { block() }.also { + ReviewRemindersDatabase.checkDeserializationErrors(requireContext()) } /** diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/services/AlarmManagerService.kt b/AnkiDroid/src/main/java/com/ichi2/anki/services/AlarmManagerService.kt index d8244de8060d..a4c07b8269de 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/services/AlarmManagerService.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/services/AlarmManagerService.kt @@ -22,13 +22,16 @@ import android.app.PendingIntent import android.content.BroadcastReceiver import android.content.Context import android.content.Intent +import androidx.annotation.VisibleForTesting import androidx.core.app.PendingIntentCompat import androidx.core.content.getSystemService import androidx.core.os.BundleCompat import com.ichi2.anki.R import com.ichi2.anki.common.time.TimeManager import com.ichi2.anki.reviewreminders.ReviewReminder +import com.ichi2.anki.reviewreminders.ReviewReminderScope import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase +import com.ichi2.anki.reviewreminders.upsertReminder import com.ichi2.anki.showThemedToast import timber.log.Timber import java.util.Calendar @@ -65,7 +68,8 @@ class AlarmManagerService : BroadcastReceiver() { * by at much this amount of time. We set it to 10 minutes, which is the minimum allowable duration * according to [the docs](https://developer.android.com/reference/android/app/AlarmManager). */ - private val WINDOW_LENGTH_MS: Long = 10.minutes.inWholeMilliseconds + @VisibleForTesting + val WINDOW_LENGTH_MS: Long = 10.minutes.inWholeMilliseconds /** * Shows error messages if an error occurs when scheduling review reminders via AlarmManager. @@ -132,6 +136,11 @@ class AlarmManagerService : BroadcastReceiver() { * Queues a review reminder to have its notification fired at its specified time. Does not check * if the review reminder is enabled or not, the caller must handle this. * + * If the review reminder has failed to fire a notification at its most recent specified time for some + * reason (ex. if the device was off, or if the OS delayed the notification for some reason), + * a notification will be fired immediately and no alarm will be immediately scheduled, as the + * notification should automatically trigger the scheduling of the next alarm. + * * Note that this only schedules the next upcoming notification, using [AlarmManager.setWindow] * rather than [AlarmManager.setRepeating]. This is because [AlarmManager.setRepeating] sometimes * postpones alarm firings for long periods of time, with intervals as long as one hour observed @@ -164,6 +173,13 @@ class AlarmManagerService : BroadcastReceiver() { ) ?: return Timber.v("Pending intent for ${reviewReminder.id} is $pendingIntent") + if (reviewReminder.shouldImmediatelyFire()) { + immediatelyFireNotification(context, reviewReminder) + // Once the notification has fired, it will automatically trigger the setting of the next alarm + // so we can return immediately + return + } + val currentTimestamp = TimeManager.time.calendar() val alarmTimestamp = currentTimestamp.clone() as Calendar alarmTimestamp.apply { @@ -186,6 +202,37 @@ class AlarmManagerService : BroadcastReceiver() { } } + /** + * Immediately fires a review reminder notification for a review reminder, which in turn then schedules the next notification. + * Used when a review reminder's notification has been delayed and failed to fire for some reason. + */ + private fun immediatelyFireNotification( + context: Context, + reviewReminder: ReviewReminder, + ) { + Timber.d("Review reminder ${reviewReminder.id} should have fired already, sending notification immediately") + + // Immediately (redundantly) record this latest routine notification-firing attempt's timestamp + // to prevent this from being triggered multiple times in rapid succession + reviewReminder.updateLatestNotifTime() + when (val scope = reviewReminder.scope) { + is ReviewReminderScope.DeckSpecific -> + ReviewRemindersDatabase.editRemindersForDeck( + scope.did, + upsertReminder(reviewReminder), + ) + is ReviewReminderScope.Global -> ReviewRemindersDatabase.editAllAppWideReminders(upsertReminder(reviewReminder)) + } + + val immediateNotificationIntent = + NotificationService.getIntent( + context, + reviewReminder, + NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, + ) + context.sendBroadcast(immediateNotificationIntent) + } + /** * Deletes any scheduled notifications for this review reminder. Does not actually delete the * review reminder itself from anywhere, only deletes any queued alarms for the review reminder. @@ -224,7 +271,7 @@ class AlarmManagerService : BroadcastReceiver() { Timber.d("scheduleAllEnabledReviewReminderNotifications") val allReviewRemindersAsMap = ReviewRemindersDatabase.getAllAppWideReminders() + ReviewRemindersDatabase.getAllDeckSpecificReminders() - val enabledReviewReminders = allReviewRemindersAsMap.values.filter { it.enabled } + val enabledReviewReminders = allReviewRemindersAsMap.getRemindersList().filter { it.enabled } for (reviewReminder in enabledReviewReminders) { scheduleReviewReminderNotification(context, reviewReminder) } @@ -277,6 +324,7 @@ class AlarmManagerService : BroadcastReceiver() { * To extend the notifications created by AnkiDroid, add more functionality to the body of this method. */ fun scheduleAllNotifications(context: Context) { + // currently, the only scheduled notifications supported by AnkiDroid are review reminder notifications scheduleAllEnabledReviewReminderNotifications(context) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/services/NotificationService.kt b/AnkiDroid/src/main/java/com/ichi2/anki/services/NotificationService.kt index ae094e1a43f8..35df9eeb05c3 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/services/NotificationService.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/services/NotificationService.kt @@ -20,6 +20,7 @@ import android.content.BroadcastReceiver import android.content.Context import android.content.Intent import android.graphics.Color +import androidx.annotation.VisibleForTesting import androidx.core.app.NotificationCompat import androidx.core.app.PendingIntentCompat import androidx.core.content.getSystemService @@ -37,6 +38,8 @@ import com.ichi2.anki.preferences.PENDING_NOTIFICATIONS_ONLY import com.ichi2.anki.preferences.sharedPrefs import com.ichi2.anki.reviewreminders.ReviewReminder import com.ichi2.anki.reviewreminders.ReviewReminderScope +import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase +import com.ichi2.anki.reviewreminders.upsertReminder import com.ichi2.anki.runGloballyWithTimeout import com.ichi2.anki.settings.Prefs import com.ichi2.anki.utils.ext.allDecksCounts @@ -71,7 +74,8 @@ class NotificationService : BroadcastReceiver() { /** * Extra key for sending a review reminder as an extra to this broadcast receiver. */ - private const val EXTRA_REVIEW_REMINDER = "notification_service_review_reminder" + @VisibleForTesting + const val EXTRA_REVIEW_REMINDER = "notification_service_review_reminder" /** * Timeout for the process of sending a review reminder notification. @@ -417,8 +421,20 @@ class NotificationService : BroadcastReceiver() { ) ?: return Timber.d("onReceive: ${reviewReminder.id}") - // Schedule the next instance of this review reminder notification if this is a recurring notification + // If this is a recurring notification... if (action == NotificationServiceAction.ScheduleRecurringNotifications.actionString) { + // Record this latest routine notification-firing attempt's timestamp + reviewReminder.updateLatestNotifTime() + when (val scope = reviewReminder.scope) { + is ReviewReminderScope.DeckSpecific -> + ReviewRemindersDatabase.editRemindersForDeck( + scope.did, + upsertReminder(reviewReminder), + ) + is ReviewReminderScope.Global -> ReviewRemindersDatabase.editAllAppWideReminders(upsertReminder(reviewReminder)) + } + + // Schedule the next routine notification-firing Timber.d("Scheduling next review reminder notification") AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } diff --git a/AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt b/AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt index 016e4c14f08d..2e7273d4a7d7 100644 --- a/AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt +++ b/AnkiDroid/src/main/java/com/ichi2/anki/settings/Prefs.kt @@ -296,6 +296,14 @@ open class PrefsRepository( */ var reminderNotifsRequestShown by booleanPref(R.string.reminder_notifs_request_shown_key, defaultValue = false) + /** + * A list of all recent deserialization errors that have occurred when trying to load review reminders from storage. + * For example, review reminders are deserialized and have their alarms scheduled when the device starts, but + * if the deserialization process fails and no valid migrations are available, the error can be put into this string + * so that the next time the user opens the app, an error dialog can be shown to inform them of the issue. + */ + var reviewReminderDeserializationErrors by stringPref(R.string.review_reminder_deserialization_errors_key) + // *************************************** Permissions ************************************** // // Flags for whether the system UI dialog for requesting certain permissions has been shown before. diff --git a/AnkiDroid/src/main/res/values/preferences.xml b/AnkiDroid/src/main/res/values/preferences.xml index b3696cade493..fc202b5bf026 100644 --- a/AnkiDroid/src/main/res/values/preferences.xml +++ b/AnkiDroid/src/main/res/values/preferences.xml @@ -213,6 +213,7 @@ reviewRemindersScreen reviewRemindersNextFreeId reminderNotifsRequestShown + reviewReminderDeserializationErrors notificationsPermissionRequested recordAudioPermissionRequested diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewReminderTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewReminderTest.kt index cfa8551c65e6..2b6fe02bdffc 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewReminderTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewReminderTest.kt @@ -19,24 +19,52 @@ package com.ichi2.anki.reviewreminders import androidx.core.content.edit import androidx.test.ext.junit.runners.AndroidJUnit4 import com.ichi2.anki.RobolectricTest +import com.ichi2.anki.common.time.MockTime +import com.ichi2.anki.common.time.TimeManager +import com.ichi2.anki.settings.Prefs import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith +import java.util.Calendar +import kotlin.time.Duration.Companion.days +import kotlin.time.Duration.Companion.hours +import kotlin.time.Duration.Companion.minutes @RunWith(AndroidJUnit4::class) class ReviewReminderTest : RobolectricTest() { + companion object { + /** + * A mock time for a consistent date and a specific time: noon in the local time zone (not UTC! this is required because + * the subject-under-test calculates times based on the local time zone, hence the convoluted construction of this mock). + */ + private val mockTime = + MockTime( + initTime = + MockTime(2024, 0, 1, 0, 0, 0, 0, 0) + .calendar() + .apply { + set(Calendar.HOUR_OF_DAY, 12) + set(Calendar.MINUTE, 0) + set(Calendar.SECOND, 0) + }.timeInMillis, + step = 1000, + ) + } + @Before override fun setUp() { super.setUp() + Prefs.reviewReminderNextFreeId = 0 ReviewRemindersDatabase.remindersSharedPrefs.edit { clear() } } @After override fun tearDown() { super.tearDown() + Prefs.reviewReminderNextFreeId = 0 ReviewRemindersDatabase.remindersSharedPrefs.edit { clear() } } @@ -47,4 +75,135 @@ class ReviewReminderTest : RobolectricTest() { assertThat(reminder.id, equalTo(ReviewReminderId(i))) } } + + @Test + fun `latestNotifTime is set at creation`() { + val timeBeforeCreation = TimeManager.time.calendar().timeInMillis + val reviewReminder = ReviewReminder.createReviewReminder(time = ReviewReminderTime(hour = 1, minute = 0)) + val timeAfterCreation = TimeManager.time.calendar().timeInMillis + + assertThat(reviewReminder.latestNotifTime in timeBeforeCreation..timeAfterCreation, equalTo(true)) + } + + @Test + fun `updateLatestNotifTime works correctly`() { + val reviewReminder = ReviewReminder.createReviewReminder(time = ReviewReminderTime(hour = 1, minute = 0)) + + val timeBeforeUpdate = TimeManager.time.calendar().timeInMillis + reviewReminder.updateLatestNotifTime() + val timeAfterUpdate = TimeManager.time.calendar().timeInMillis + + assertThat(reviewReminder.latestNotifTime in timeBeforeUpdate..timeAfterUpdate, equalTo(true)) + } + + @Test + fun `notification should immediately fire if there was no scheduled firing`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = 6.hours.inWholeMilliseconds.toInt(), + lastFiringOffsetFromWindowStartMs = (-1).minutes.inWholeMilliseconds.toInt(), + shouldImmediatelyFire = true, + ) + } + + @Test + fun `notification should not immediately fire if there was a scheduled firing`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = 6.hours.inWholeMilliseconds.toInt(), + lastFiringOffsetFromWindowStartMs = 1.minutes.inWholeMilliseconds.toInt(), + shouldImmediatelyFire = false, + ) + } + + @Test + fun `notification should immediately fire if scheduled firing time is recent and there was no scheduled firing`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = 2.minutes.inWholeMilliseconds.toInt(), + lastFiringOffsetFromWindowStartMs = (-1).minutes.inWholeMilliseconds.toInt(), + shouldImmediatelyFire = true, + ) + } + + @Test + fun `notification should not immediately fire if scheduled firing time is recent and there was a scheduled firing`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = 1.minutes.inWholeMilliseconds.toInt(), + lastFiringOffsetFromWindowStartMs = 0, + shouldImmediatelyFire = false, + ) + } + + @Test + fun `notification should immediately fire if latest firing was a long time ago`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = 0, + lastFiringOffsetFromWindowStartMs = (-2).days.inWholeMilliseconds.toInt(), + shouldImmediatelyFire = true, + ) + } + + @Test + fun `notification should immediately fire even if next window is approaching if there was no scheduled firing`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = (-1).minutes.inWholeMilliseconds.toInt(), + lastFiringOffsetFromWindowStartMs = (-25).hours.inWholeMilliseconds.toInt(), + shouldImmediatelyFire = true, + ) + } + + @Test + fun `notification should not immediately fire if scheduled firing was just late`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = 23.hours.inWholeMilliseconds.toInt(), + lastFiringOffsetFromWindowStartMs = 22.hours.inWholeMilliseconds.toInt(), + shouldImmediatelyFire = false, + ) + } + + @Test + fun `notification should immediately fire if scheduled firing is now and latest firing was in the past`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = 0, + lastFiringOffsetFromWindowStartMs = (-1).minutes.inWholeMilliseconds.toInt(), + shouldImmediatelyFire = true, + ) + } + + @Test + fun `notification should not immediately fire if scheduled firing is now and latest firing was just now`() { + shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs = 0, + lastFiringOffsetFromWindowStartMs = 0, + shouldImmediatelyFire = false, + ) + } + + private fun shouldImmediatelyFireTest( + currentTimeOffsetFromWindowStartMs: Int, + lastFiringOffsetFromWindowStartMs: Int, + shouldImmediatelyFire: Boolean, + ) { + val reviewReminder = ReviewReminder.createReviewReminder(time = ReviewReminderTime(hour = 1, minute = 0)) + + val currentTime = mockTime.calendar().clone() as Calendar + currentTime.apply { + set(Calendar.HOUR_OF_DAY, 1) + set(Calendar.MINUTE, 0) + set(Calendar.SECOND, 0) + add(Calendar.MILLISECOND, currentTimeOffsetFromWindowStartMs) + } + TimeManager.resetWith(MockTime(currentTime.timeInMillis, step = 1000)) + + val lastFiring = mockTime.calendar().clone() as Calendar + lastFiring.apply { + set(Calendar.HOUR_OF_DAY, 1) + set(Calendar.MINUTE, 0) + set(Calendar.SECOND, 0) + add(Calendar.MILLISECOND, lastFiringOffsetFromWindowStartMs) + } + reviewReminder.latestNotifTime = lastFiring.timeInMillis + + assertThat(reviewReminder.shouldImmediatelyFire(), equalTo(shouldImmediatelyFire)) + + TimeManager.reset() + } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabaseTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabaseTest.kt index b1f3a3c74066..364c039e0aad 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabaseTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/reviewreminders/ReviewRemindersDatabaseTest.kt @@ -19,15 +19,15 @@ package com.ichi2.anki.reviewreminders import androidx.core.content.edit import androidx.test.ext.junit.runners.AndroidJUnit4 import com.ichi2.anki.RobolectricTest -import com.ichi2.anki.libanki.DeckId -import kotlinx.serialization.Serializable -import kotlinx.serialization.SerializationException +import com.ichi2.anki.libanki.EpochMilliseconds +import kotlinx.serialization.InternalSerializationApi +import kotlinx.serialization.KSerializer +import kotlinx.serialization.builtins.MapSerializer import kotlinx.serialization.json.Json -import org.hamcrest.CoreMatchers +import kotlinx.serialization.serializer import org.hamcrest.Description import org.hamcrest.Matcher import org.hamcrest.MatcherAssert.assertThat -import org.hamcrest.Matchers.anEmptyMap import org.hamcrest.Matchers.equalTo import org.hamcrest.Matchers.hasItem import org.hamcrest.Matchers.not @@ -38,77 +38,6 @@ import org.junit.Test import org.junit.runner.RunWith import kotlin.reflect.full.memberProperties -/** - * Schema migration settings for testing purposes. - * Consult this as an example of how to save old schemas and define their [ReviewReminderSchema.migrate] methods. - */ -object TestingReviewReminderMigrationSettings { - /** - * A sample old review reminder schema. Perhaps this was how the [ReviewReminder] data class was originally implemented. - * We would like to test the code that checks if review reminders stored on the device adhere to an old, outdated schema. - * In particular, does the code correctly migrate the serialized data class strings to the updated, current version of [ReviewReminder]? - */ - @Serializable - data class ReviewReminderSchemaVersionOne( - override val id: ReviewReminderId, - val hour: Int, - val minute: Int, - val cardTriggerThreshold: Int, - val did: DeckId, - val enabled: Boolean = true, - ) : ReviewReminderSchema { - override fun migrate(): ReviewReminderSchema = - ReviewReminderSchemaVersionTwo( - id = this.id, - time = VersionTwoDataClasses.ReviewReminderTime(hour, minute), - snoozeAmount = 1, - cardTriggerThreshold = this.cardTriggerThreshold, - did = this.did, - enabled = enabled, - ) - } - - /** - * Here's an example of how you can handle renamed fields in a data class stored as part of a [ReviewReminder]. - * Otherwise, there's a namespace collision with [ReviewReminderTime]. - * - * This class will be serialized into "ReviewReminderTime(timeHour=#, timeMinute=#)", which otherwise might conflict - * with the updated definition of [ReviewReminderTime], which is serialized as "ReviewReminderTime(hour=#, minute=#)". - * When we read the outdated schema from the disk, we need to tell the deserializer that it is reading a - * [VersionTwoDataClasses.ReviewReminderTime] rather than a [ReviewReminderTime], even though the names are the same. - * - * @see ReviewReminderSchemaVersionTwo - */ - object VersionTwoDataClasses { - @Serializable - data class ReviewReminderTime( - val timeHour: Int, - val timeMinute: Int, - ) - } - - /** - * Another example of an old review reminder schema. See [ReviewReminderSchemaVersionOne] for more details. - */ - @Serializable - data class ReviewReminderSchemaVersionTwo( - override val id: ReviewReminderId, - val time: VersionTwoDataClasses.ReviewReminderTime, - val snoozeAmount: Int, - val cardTriggerThreshold: Int, - val did: DeckId, - val enabled: Boolean = true, - ) : ReviewReminderSchema { - override fun migrate(): ReviewReminder = - ReviewReminder.createReviewReminder( - time = ReviewReminderTime(this.time.timeHour, this.time.timeMinute), - cardTriggerThreshold = ReviewReminderCardTriggerThreshold(this.cardTriggerThreshold), - scope = if (this.did == -1L) ReviewReminderScope.Global else ReviewReminderScope.DeckSpecific(this.did), - enabled = enabled, - ) - } -} - /** * If tests in this file have failed, it may be because you have updated [ReviewReminder]! * Please read the documentation of [ReviewReminder] carefully and ensure you have implemented @@ -119,8 +48,9 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { private val did1 = 12345L private val did2 = 67890L + private val emptyReminderGroup = ReviewReminderGroup() private val dummyDeckSpecificRemindersForDeckOne = - mapOf( + ReviewReminderGroup( ReviewReminderId(0) to ReviewReminder.createReviewReminder( ReviewReminderTime(9, 0), @@ -136,7 +66,7 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { ), ) private val dummyDeckSpecificRemindersForDeckTwo = - mapOf( + ReviewReminderGroup( ReviewReminderId(2) to ReviewReminder.createReviewReminder( ReviewReminderTime(10, 30), @@ -152,7 +82,7 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { ), ) private val dummyAppWideReminders = - mapOf( + ReviewReminderGroup( ReviewReminderId(4) to ReviewReminder.createReviewReminder( ReviewReminderTime(9, 0), @@ -178,9 +108,9 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { } @Test - fun `getRemindersForDeck should return empty map when no reminders exist`() { + fun `getRemindersForDeck should return empty group when no reminders exist`() { val reminders = ReviewRemindersDatabase.getRemindersForDeck(did1) - assertThat(reminders, anEmptyMap()) + assertThat(reminders, equalTo(emptyReminderGroup)) } @Test @@ -191,9 +121,9 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { } @Test - fun `getAllDeckSpecificReminders should return empty map when no reminders exist`() { + fun `getAllDeckSpecificReminders should return empty group when no reminders exist`() { val reminders = ReviewRemindersDatabase.getAllDeckSpecificReminders() - assertThat(reminders, anEmptyMap()) + assertThat(reminders, equalTo(emptyReminderGroup)) } @Test @@ -208,9 +138,9 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { } @Test - fun `getAllAppWideReminders should return empty map when no reminders exist`() { + fun `getAllAppWideReminders should return empty group when no reminders exist`() { val reminders = ReviewRemindersDatabase.getAllAppWideReminders() - assertThat(reminders, anEmptyMap()) + assertThat(reminders, equalTo(emptyReminderGroup)) } @Test @@ -220,102 +150,154 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { assertThat(storedReminders, equalTo(dummyAppWideReminders)) } - @Test(expected = SerializationException::class) - fun `getRemindersForDeck should throw SerializationException if JSON string for StoredReviewReminder is corrupted`() { + /** + * Helper function to test how the database handles corrupted JSON strings. + * It should delete only the accessed ones, not throw an exception, and return an empty reminder group. + * + * @param corruptedValue the corrupted JSON string to be inserted into SharedPreferences for did1, did2, and the app-wide key + * @param expectedDeletedKeys the set of keys that should no longer be in SharedPreferences after the access + * @param access a lambda that accesses either deck-specific or app-wide reminders, which should trigger the deletion of the corrupted keys + */ + private fun corruptedRemindersTest( + corruptedValue: String, + expectedDeletedKeys: Set, + inputKeys: Set = + setOf( + ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, + ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did2, + ReviewRemindersDatabase.APP_WIDE_KEY, + ), + access: () -> ReviewReminderGroup, + ) { ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, "corrupted_and_invalid_json_string") + inputKeys.forEach { key -> + putString(key, corruptedValue) + } } - ReviewRemindersDatabase.getRemindersForDeck(did1) + val reminders = access() + assertThat(reminders, equalTo(emptyReminderGroup)) + val remainingKeys = ReviewRemindersDatabase.remindersSharedPrefs.all.keys + assertThat(remainingKeys + expectedDeletedKeys, equalTo(inputKeys)) } - @Test(expected = IllegalArgumentException::class) - fun `getRemindersForDeck should throw IllegalArgumentException if JSON string is not a StoredReviewReminder`() { - val randomObject = Pair("not a map of", "review reminders") - ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, Json.encodeToString(randomObject)) + @Test + fun `getRemindersForDeck should delete reminders if JSON string for StoredReviewReminderGroup is corrupted`() { + corruptedRemindersTest( + corruptedValue = "corrupted_and_invalid_json_string", + expectedDeletedKeys = setOf(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1), + ) { + ReviewRemindersDatabase.getRemindersForDeck(did1) } - ReviewRemindersDatabase.getRemindersForDeck(did1) } - @Test(expected = SerializationException::class) - fun `getRemindersForDeck should throw SerializationException if JSON string for review reminder is corrupted`() { - val corruptedStoredReviewReminder = - ReviewRemindersDatabase.StoredReviewRemindersMap( - ReviewRemindersDatabase.schemaVersion, - "corrupted_and_invalid_json_string", - ) - ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, Json.encodeToString(corruptedStoredReviewReminder)) + @Test + fun `getRemindersForDeck should delete reminders if JSON string is not a StoredReviewReminderGroup`() { + corruptedRemindersTest( + corruptedValue = Json.encodeToString(Pair("not a group of", "review reminders")), + expectedDeletedKeys = setOf(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did2), + ) { + ReviewRemindersDatabase.getRemindersForDeck(did2) } - ReviewRemindersDatabase.getRemindersForDeck(did1) } - @Test(expected = SerializationException::class) - fun `getAllAppWideReminders should throw SerializationException if JSON string for StoredReviewReminder is corrupted`() { - ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.APP_WIDE_KEY, "corrupted_and_invalid_json_string") + @Test + fun `getRemindersForDeck should delete reminders if JSON string for review reminder is corrupted`() { + corruptedRemindersTest( + corruptedValue = + Json.encodeToString( + ReviewRemindersDatabase.StoredReviewReminderGroup( + ReviewRemindersDatabase.schemaVersion, + "corrupted_and_invalid_json_string", + ), + ), + expectedDeletedKeys = setOf(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1), + ) { + ReviewRemindersDatabase.getRemindersForDeck(did1) } - ReviewRemindersDatabase.getAllAppWideReminders() } - @Test(expected = IllegalArgumentException::class) - fun `getAllAppWideReminders should throw IllegalArgumentException if JSON string is not a StoredReviewReminder`() { - val randomObject = Pair("not a map of", "review reminders") - ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.APP_WIDE_KEY, Json.encodeToString(randomObject)) + @Test + fun `getAllAppWideReminders should delete reminders if JSON string for StoredReviewReminderGroup is corrupted`() { + corruptedRemindersTest( + corruptedValue = "corrupted_and_invalid_json_string", + expectedDeletedKeys = setOf(ReviewRemindersDatabase.APP_WIDE_KEY), + ) { + ReviewRemindersDatabase.getAllAppWideReminders() } - ReviewRemindersDatabase.getAllAppWideReminders() } - @Test(expected = SerializationException::class) - fun `getAllAppWideReminders should throw SerializationException if JSON string for review reminder is corrupted`() { - val corruptedStoredReviewReminder = - ReviewRemindersDatabase.StoredReviewRemindersMap( - ReviewRemindersDatabase.schemaVersion, - "corrupted_and_invalid_json_string", - ) - ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.APP_WIDE_KEY, Json.encodeToString(corruptedStoredReviewReminder)) + @Test + fun `getAllAppWideReminders should delete reminders if JSON string is not a StoredReviewReminderGroup`() { + corruptedRemindersTest( + corruptedValue = Json.encodeToString(Pair("not a group of", "review reminders")), + expectedDeletedKeys = setOf(ReviewRemindersDatabase.APP_WIDE_KEY), + ) { + ReviewRemindersDatabase.getAllAppWideReminders() } - ReviewRemindersDatabase.getAllAppWideReminders() } - @Test(expected = SerializationException::class) - fun `getAllDeckSpecificReminders should throw SerializationException if JSON string for StoredReviewReminder is corrupted`() { - ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, "corrupted_and_invalid_json_string") + @Test + fun `getAllAppWideReminders should delete reminders if JSON string for review reminder is corrupted`() { + corruptedRemindersTest( + corruptedValue = + Json.encodeToString( + ReviewRemindersDatabase.StoredReviewReminderGroup( + ReviewRemindersDatabase.schemaVersion, + "corrupted_and_invalid_json_string", + ), + ), + expectedDeletedKeys = setOf(ReviewRemindersDatabase.APP_WIDE_KEY), + ) { + ReviewRemindersDatabase.getAllAppWideReminders() } - ReviewRemindersDatabase.getAllDeckSpecificReminders() } - @Test(expected = IllegalArgumentException::class) - fun `getAllDeckSpecificReminders should throw IllegalArgumentException if JSON string is not a StoredReviewReminder`() { - val randomObject = Pair("not a map of", "review reminders") - ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, Json.encodeToString(randomObject)) + @Test + fun `getAllDeckSpecificReminders should delete reminders if JSON string for StoredReviewReminderGroup is corrupted`() { + corruptedRemindersTest( + corruptedValue = "corrupted_and_invalid_json_string", + expectedDeletedKeys = setOf(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did2), + ) { + ReviewRemindersDatabase.getAllDeckSpecificReminders() } - ReviewRemindersDatabase.getAllDeckSpecificReminders() } - @Test(expected = SerializationException::class) - fun `getAllDeckSpecificReminders should throw SerializationException if JSON string for review reminder is corrupted`() { - val corruptedStoredReviewReminder = - ReviewRemindersDatabase.StoredReviewRemindersMap( - ReviewRemindersDatabase.schemaVersion, - "corrupted_and_invalid_json_string", - ) - ReviewRemindersDatabase.remindersSharedPrefs.edit { - putString(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, Json.encodeToString(corruptedStoredReviewReminder)) + @Test + fun `getAllDeckSpecificReminders should delete reminders if JSON string is not a StoredReviewReminderGroup`() { + corruptedRemindersTest( + corruptedValue = Json.encodeToString(Pair("not a group of", "review reminders")), + expectedDeletedKeys = setOf(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did2), + ) { + ReviewRemindersDatabase.getAllDeckSpecificReminders() + } + } + + @Test + fun `getAllDeckSpecificReminders should delete reminders if JSON string for review reminder is corrupted`() { + corruptedRemindersTest( + corruptedValue = + Json.encodeToString( + ReviewRemindersDatabase.StoredReviewReminderGroup( + ReviewRemindersDatabase.schemaVersion, + "corrupted_and_invalid_json_string", + ), + ), + expectedDeletedKeys = + setOf( + ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, + ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did2, + ), + ) { + ReviewRemindersDatabase.getAllDeckSpecificReminders() } - ReviewRemindersDatabase.getAllDeckSpecificReminders() } @Test fun `editRemindersForDeck should delete SharedPreferences key if no reminders are returned`() { ReviewRemindersDatabase.editRemindersForDeck(did1) { dummyDeckSpecificRemindersForDeckOne } - ReviewRemindersDatabase.editRemindersForDeck(did1) { emptyMap() } + ReviewRemindersDatabase.editRemindersForDeck(did1) { ReviewReminderGroup() } val attemptedRetrieval = ReviewRemindersDatabase.getRemindersForDeck(did1) - assertThat(attemptedRetrieval, anEmptyMap()) + assertThat(attemptedRetrieval, equalTo(emptyReminderGroup)) assertThat( ReviewRemindersDatabase.remindersSharedPrefs.all.keys, not(hasItem(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1)), @@ -325,33 +307,52 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { @Test fun `editAllAppWideReminders should delete SharedPreferences key if no reminders are returned`() { ReviewRemindersDatabase.editAllAppWideReminders { dummyAppWideReminders } - ReviewRemindersDatabase.editAllAppWideReminders { emptyMap() } + ReviewRemindersDatabase.editAllAppWideReminders { ReviewReminderGroup() } val attemptedRetrieval = ReviewRemindersDatabase.getAllAppWideReminders() - assertThat(attemptedRetrieval, anEmptyMap()) + assertThat(attemptedRetrieval, equalTo(emptyReminderGroup)) assertThat( ReviewRemindersDatabase.remindersSharedPrefs.all.keys, not(hasItem(ReviewRemindersDatabase.APP_WIDE_KEY)), ) } + @Test(expected = IllegalArgumentException::class) + fun `editAllAppWideReminders with deck specific reminders should throw IllegalArgumentException`() { + ReviewRemindersDatabase.editAllAppWideReminders { dummyDeckSpecificRemindersForDeckOne } + } + + @Test(expected = IllegalArgumentException::class) + fun `editRemindersForDeck with app wide reminders should throw IllegalArgumentException`() { + ReviewRemindersDatabase.editRemindersForDeck(did1) { dummyAppWideReminders } + } + + @Test(expected = IllegalArgumentException::class) + fun `editRemindersForDeck with reminders for different deck should throw IllegalArgumentException`() { + ReviewRemindersDatabase.editRemindersForDeck(did1) { dummyDeckSpecificRemindersForDeckTwo } + } + /** - * When review reminders are migrated to the new schema, the reminders' IDs will be recreated from scratch. - * Thus, validation that our tests succeeded should ignore [ReviewReminder.id]. + * When review reminders are migrated to the new schema, the reminders' IDs and latestNotifTimes will be recreated from scratch. + * Thus, validation that our tests succeeded should ignore these fields. * This custom Hamcrest matcher performs this validation using reflection. */ - private fun containsEqualReviewRemindersInAnyOrderIgnoringId(expected: Collection): Matcher> = + private fun containsEqualReviewRemindersExcludingVolatileFields( + expected: Collection, + ): Matcher> = object : TypeSafeMatcher>() { override fun describeTo(description: Description) { description.appendValue(expected) } override fun matchesSafely(actual: Iterable): Boolean { + val volatileFields = setOf("id", "latestNotifTime") + val expectedSet = expected .map { e -> ReviewReminder::class .memberProperties - .filterNot { it.name == "id" } + .filterNot { it.name in volatileFields } .associateWith { it.get(e) } }.toSet() @@ -360,7 +361,7 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { .map { a -> ReviewReminder::class .memberProperties - .filterNot { it.name == "id" } + .filterNot { it.name in volatileFields } .associateWith { it.get(a) } }.toSet() @@ -370,18 +371,22 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { /** * If this test has failed, please ensure the review reminder schema version and old schemas in the review reminder - * migration chain are set correctly. + * migration chain are set correctly. If you've written a new migration, please also write a new test in this file + * to prove your migration works! + * + * This test is designed to fail and be updated every time the schema is changed + * to ensure developers know what they are doing and to remind them to write migration tests. */ @Test fun `current schema version points to ReviewReminder`() { - assertThat(ReviewRemindersDatabase.schemaVersion.value, equalTo(1)) + assertThat(ReviewRemindersDatabase.schemaVersion.value, equalTo(3)) assertThat( ReviewRemindersDatabase .oldReviewReminderSchemasForMigration .keys .last() .value, - equalTo(1), + equalTo(3), ) assertThat( ReviewRemindersDatabase @@ -392,6 +397,151 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { ) } + /** + * If this test has failed, you have likely updated [ReviewReminder] without writing a migration! + * Please write a migration (see [ReviewReminder] and the tests in this file for more information), + * update the latest schema version (see [ReviewRemindersDatabase]), add a new test in this file + * to prove your migration works, and update this test to the new latest schema version. + * + * To get a raw string, add a log to [ReviewRemindersDatabase.decodeJson] to print out its input string. + * + * This test is designed to fail and be updated every time the schema is changed + * to ensure developers know what they are doing and to remind them to write migration tests. + */ + @Test + fun `raw ReviewReminder string can be deserialized without throwing`() { + val rawString = + """ + { + "version":3, + "remindersMapJson":"{\"22\":{\"id\":22,\"time\":{\"hour\":14,\"minute\":16},\"cardTriggerThreshold\":1,\"scope\":{\"type\":\"com.ichi2.anki.reviewreminders.ReviewReminderScope.Global\"},\"enabled\":true,\"latestNotifTime\":1771193761002,\"profileID\":\"\",\"onlyNotifyIfNoReviews\":false}}" + } + """.trimIndent() + + val storedReviewReminderGroup = Json.decodeFromString(rawString) + val mapSerializer = MapSerializer(ReviewReminderId.serializer(), ReviewReminder.serializer()) + Json.decodeFromString(mapSerializer, storedReviewReminderGroup.remindersMapJson) + } + + /** + * If this test has failed, you have likely updated [ReviewReminder]'s schema! Please ensure you've written + * a migration for this schema change (see [ReviewReminder]) and write a test in this file to prove your migration works. + * + * This test is designed to fail and be updated every time the schema is changed + * to ensure developers know what they are doing and to remind them to write migration tests. + */ + @Test + fun `ReviewReminder properties and types are the expected values`() { + val expectedPropertiesWithTypes = + mapOf( + "id" to ReviewReminderId::class, + "time" to ReviewReminderTime::class, + "cardTriggerThreshold" to ReviewReminderCardTriggerThreshold::class, + "scope" to ReviewReminderScope::class, + "enabled" to Boolean::class, + "latestNotifTime" to EpochMilliseconds::class, + "profileID" to String::class, + "onlyNotifyIfNoReviews" to Boolean::class, + ) + + val actualPropertiesWithTypes = + ReviewReminder::class + .memberProperties + .associate { it.name to it.returnType.classifier } + + assertThat(actualPropertiesWithTypes, equalTo(expectedPropertiesWithTypes)) + } + + /** + * A single test case for migration testing. + * @see assertMigrationsWork + */ + private data class MigrationTestCase( + val inputVersion: ReviewReminderSchemaVersion, + val input: ReviewReminderSchema, + val expectedOutput: ReviewReminder, + ) + + /** + * Helper function for performing migrations and asserting they work as expected. + * + * In order to create a unified helper function for doing this which can accept arbitrary subclasses + * of [ReviewReminderSchema] and get their serializers at runtime, we need to opt into + * the internal serialization API. Since this is a test-only function, this should be acceptable. + */ + @OptIn(InternalSerializationApi::class) + private fun assertMigrationsWork(vararg testCases: MigrationTestCase) { + // Group + val groupedByScope = + testCases.groupBy { + when (val scope = it.expectedOutput.scope) { + is ReviewReminderScope.DeckSpecific -> scope.did + is ReviewReminderScope.Global -> null + } + } + + // Write + groupedByScope.forEach { (did, casesInScope) -> + // Reading and writing is done per scope, so all test cases in a scope will have the same input version + if (casesInScope.map { it.inputVersion }.toSet().size != 1) { + throw IllegalArgumentException("All test cases in a scope must have the same input version and type") + } + val version = casesInScope.first().inputVersion + val inputType = ReviewRemindersDatabase.oldReviewReminderSchemasForMigration[version]!! + + // We need an unchecked runtime cast to allow this helper to operate on arbitrary subclasses of ReviewReminderSchema + @Suppress("UNCHECKED_CAST") + val inputSerializer = inputType.serializer() as KSerializer + val mapSerializer = MapSerializer(ReviewReminderId.serializer(), inputSerializer) + + val inputMap = casesInScope.associate { it.input.id to it.input } + val packagedInput = + ReviewRemindersDatabase.StoredReviewReminderGroup( + version, + Json.encodeToString(mapSerializer, inputMap), + ) + + val key = + if (did != null) { + ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did + } else { + ReviewRemindersDatabase.APP_WIDE_KEY + } + ReviewRemindersDatabase.remindersSharedPrefs.edit(commit = true) { + putString(key, Json.encodeToString(packagedInput)) + } + } + + // Read and assert + groupedByScope.forEach { (did, casesInScope) -> + val retrievedReminders = + if (did != null) { + ReviewRemindersDatabase.getRemindersForDeck(did) + } else { + ReviewRemindersDatabase.getAllAppWideReminders() + } + + // We ignore ID because the migration process will generate new review reminders from scratch during the migration + // ID is a private, inaccessible property + // Instead, we only check that the ID matches the key in the map; all other properties can be compared normally + retrievedReminders.forEach { id, reminder -> + assertThat(id, equalTo(reminder.id)) + } + assertThat( + retrievedReminders.getRemindersList(), + containsEqualReviewRemindersExcludingVolatileFields( + casesInScope.map { it.expectedOutput }, + ), + ) + } + + // Shared Preferences should not contain any random corrupted keys after or due to the migration process + assertThat( + ReviewRemindersDatabase.remindersSharedPrefs.all.size, + equalTo(groupedByScope.size), + ) + } + @Test fun `review reminder schema migration works`() { // Save existing mocks @@ -401,128 +551,192 @@ class ReviewRemindersDatabaseTest : RobolectricTest() { ReviewRemindersDatabase.schemaVersion = ReviewReminderSchemaVersion(3) ReviewRemindersDatabase.oldReviewReminderSchemasForMigration = mapOf( - ReviewReminderSchemaVersion(1) to TestingReviewReminderMigrationSettings.ReviewReminderSchemaVersionOne::class, - ReviewReminderSchemaVersion(2) to TestingReviewReminderMigrationSettings.ReviewReminderSchemaVersionTwo::class, + ReviewReminderSchemaVersion(1) to TestingReviewReminderMigrationSettings.ReviewReminderTestSchemaVersionOne::class, + ReviewReminderSchemaVersion(2) to TestingReviewReminderMigrationSettings.ReviewReminderTestSchemaVersionTwo::class, ReviewReminderSchemaVersion(3) to ReviewReminder::class, ) - // To spice things up, some will be version one... - val versionOneDummyDeckSpecificRemindersForDeckOne = - mapOf( - ReviewReminderId(0) to - TestingReviewReminderMigrationSettings.ReviewReminderSchemaVersionOne( - ReviewReminderId(0), - 9, - 0, - 5, - did1, - false, + + assertMigrationsWork( + // To spice things up, some will be version one... + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(1), + input = + TestingReviewReminderMigrationSettings.ReviewReminderTestSchemaVersionOne( + id = ReviewReminderId(0), + hour = 9, + minute = 0, + cardTriggerThreshold = 5, + did = did1, + enabled = false, ), - ReviewReminderId(1) to - TestingReviewReminderMigrationSettings.ReviewReminderSchemaVersionOne( - ReviewReminderId(1), - 10, - 30, - 10, - did1, + expectedOutput = dummyDeckSpecificRemindersForDeckOne[ReviewReminderId(0)]!!, + ), + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(1), + input = + TestingReviewReminderMigrationSettings.ReviewReminderTestSchemaVersionOne( + id = ReviewReminderId(1), + hour = 10, + minute = 30, + cardTriggerThreshold = 10, + did = did1, ), - ) - // ...and some will be version two - val versionTwoDummyDeckSpecificRemindersForDeckTwo = - mapOf( - ReviewReminderId(2) to - TestingReviewReminderMigrationSettings.ReviewReminderSchemaVersionTwo( - ReviewReminderId(2), - TestingReviewReminderMigrationSettings.VersionTwoDataClasses.ReviewReminderTime(10, 30), - 1, - 10, - did2, + expectedOutput = dummyDeckSpecificRemindersForDeckOne[ReviewReminderId(1)]!!, + ), + // ...and some will be version two... + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(2), + input = + TestingReviewReminderMigrationSettings.ReviewReminderTestSchemaVersionTwo( + id = ReviewReminderId(2), + time = TestingReviewReminderMigrationSettings.VersionTwoDataClasses.ReviewReminderTime(10, 30), + snoozeAmount = 1, + cardTriggerThreshold = 10, + did = did2, + enabled = true, ), - ReviewReminderId(3) to - TestingReviewReminderMigrationSettings.ReviewReminderSchemaVersionTwo( - ReviewReminderId(3), - TestingReviewReminderMigrationSettings.VersionTwoDataClasses.ReviewReminderTime(12, 30), - 1, - 20, - did2, + expectedOutput = dummyDeckSpecificRemindersForDeckTwo[ReviewReminderId(2)]!!, + ), + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(2), + input = + TestingReviewReminderMigrationSettings.ReviewReminderTestSchemaVersionTwo( + id = ReviewReminderId(3), + time = TestingReviewReminderMigrationSettings.VersionTwoDataClasses.ReviewReminderTime(12, 30), + snoozeAmount = 1, + cardTriggerThreshold = 20, + did = did2, ), - ) - val versionOneDummyAppWideReminders = - mapOf( - ReviewReminderId(4) to - TestingReviewReminderMigrationSettings.ReviewReminderSchemaVersionOne( - ReviewReminderId(4), - 9, - 0, - 5, - -1L, + expectedOutput = dummyDeckSpecificRemindersForDeckTwo[ReviewReminderId(3)]!!, + ), + // ...and some will be app-wide for good measure + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(1), + input = + TestingReviewReminderMigrationSettings.ReviewReminderTestSchemaVersionOne( + id = ReviewReminderId(4), + hour = 9, + minute = 0, + cardTriggerThreshold = 5, + did = -1L, ), - ReviewReminderId(5) to - TestingReviewReminderMigrationSettings.ReviewReminderSchemaVersionOne( - ReviewReminderId(5), - 10, - 30, - 10, - -1L, + expectedOutput = dummyAppWideReminders[ReviewReminderId(4)]!!, + ), + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(1), + input = + TestingReviewReminderMigrationSettings.ReviewReminderTestSchemaVersionOne( + id = ReviewReminderId(5), + hour = 10, + minute = 30, + cardTriggerThreshold = 10, + did = -1L, ), - ) - - val packagedDeckOneReminders = - ReviewRemindersDatabase.StoredReviewRemindersMap( - ReviewReminderSchemaVersion(1), - Json.encodeToString(versionOneDummyDeckSpecificRemindersForDeckOne), - ) - val packagedDeckTwoReminders = - ReviewRemindersDatabase.StoredReviewRemindersMap( - ReviewReminderSchemaVersion(2), - Json.encodeToString(versionTwoDummyDeckSpecificRemindersForDeckTwo), - ) - val packagedGlobalReminders = - ReviewRemindersDatabase.StoredReviewRemindersMap( - ReviewReminderSchemaVersion(1), - Json.encodeToString(versionOneDummyAppWideReminders), - ) - - ReviewRemindersDatabase.remindersSharedPrefs.edit(commit = true) { - putString(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did1, Json.encodeToString(packagedDeckOneReminders)) - putString(ReviewRemindersDatabase.DECK_SPECIFIC_KEY + did2, Json.encodeToString(packagedDeckTwoReminders)) - putString(ReviewRemindersDatabase.APP_WIDE_KEY, Json.encodeToString(packagedGlobalReminders)) - } - - val retrievedDeckOneReminders = ReviewRemindersDatabase.getRemindersForDeck(did1) - val retrievedDeckTwoReminders = ReviewRemindersDatabase.getRemindersForDeck(did2) - val retrievedGlobalReminders = ReviewRemindersDatabase.getAllAppWideReminders() - - retrievedDeckOneReminders.forEach { (id, reminder) -> - assertThat(id, equalTo(reminder.id)) - } - retrievedDeckTwoReminders.forEach { (id, reminder) -> - assertThat(id, equalTo(reminder.id)) - } - retrievedGlobalReminders.forEach { (id, reminder) -> - assertThat(id, equalTo(reminder.id)) - } - - // We ignore ID because the migration process will generate new review reminders from scratch during the migration; ID is a private, inaccessible property - assertThat( - retrievedDeckOneReminders.values, - containsEqualReviewRemindersInAnyOrderIgnoringId(dummyDeckSpecificRemindersForDeckOne.values), - ) - assertThat( - retrievedDeckTwoReminders.values, - containsEqualReviewRemindersInAnyOrderIgnoringId(dummyDeckSpecificRemindersForDeckTwo.values), - ) - assertThat( - retrievedGlobalReminders.values, - containsEqualReviewRemindersInAnyOrderIgnoringId(dummyAppWideReminders.values), + expectedOutput = dummyAppWideReminders[ReviewReminderId(5)]!!, + ), ) - // Shared Preferences should not contain any random corrupted keys after or due to the migration process - // There should be three: two for the specific decks, one for app-wide - val sharedPrefsSize = ReviewRemindersDatabase.remindersSharedPrefs.all.size - assertThat(sharedPrefsSize, CoreMatchers.equalTo(3)) - // Reset mocks ReviewRemindersDatabase.schemaVersion = savedSchemaVersion ReviewRemindersDatabase.oldReviewReminderSchemasForMigration = savedOldReviewReminderSchemasForMigration } + + @Test + fun `review reminder v1 to v2 migration works`() { + assertMigrationsWork( + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(1), + input = + ReviewReminderSchemaV1( + id = ReviewReminderId(0), + time = ReviewReminderTime(9, 0), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(5), + scope = ReviewReminderScope.DeckSpecific(did1), + enabled = true, + profileID = "", + ), + expectedOutput = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(9, 0), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(5), + scope = ReviewReminderScope.DeckSpecific(did1), + enabled = true, + profileID = "", + onlyNotifyIfNoReviews = false, + ), + ), + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(1), + input = + ReviewReminderSchemaV1( + id = ReviewReminderId(1), + time = ReviewReminderTime(10, 30), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(10), + scope = ReviewReminderScope.Global, + enabled = false, + profileID = "", + onlyNotifyIfNoReviews = true, + ), + expectedOutput = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(10, 30), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(10), + scope = ReviewReminderScope.Global, + enabled = false, + profileID = "", + onlyNotifyIfNoReviews = true, + ), + ), + ) + } + + @Test + fun `review reminder v2 to v3 migration works`() { + assertMigrationsWork( + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(2), + input = + ReviewReminderSchemaV2( + id = ReviewReminderId(0), + time = ReviewReminderTime(10, 30), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(10), + scope = ReviewReminderScope.Global, + enabled = true, + profileID = "", + onlyNotifyIfNoReviews = false, + ), + expectedOutput = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(10, 30), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(10), + scope = ReviewReminderScope.Global, + enabled = true, + profileID = "", + onlyNotifyIfNoReviews = false, + ), + ), + MigrationTestCase( + inputVersion = ReviewReminderSchemaVersion(2), + input = + ReviewReminderSchemaV2( + id = ReviewReminderId(1), + time = ReviewReminderTime(12, 0), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(20), + scope = ReviewReminderScope.DeckSpecific(did2), + enabled = false, + profileID = "", + onlyNotifyIfNoReviews = true, + ), + expectedOutput = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(12, 0), + cardTriggerThreshold = ReviewReminderCardTriggerThreshold(20), + scope = ReviewReminderScope.DeckSpecific(did2), + enabled = false, + profileID = "", + onlyNotifyIfNoReviews = true, + ), + ), + ) + } } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/services/AlarmManagerServiceTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/services/AlarmManagerServiceTest.kt index 4e5bd96d08e8..0c8561a3d18f 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/services/AlarmManagerServiceTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/services/AlarmManagerServiceTest.kt @@ -30,26 +30,47 @@ import androidx.test.ext.junit.runners.AndroidJUnit4 import com.ichi2.anki.RobolectricTest import com.ichi2.anki.common.time.MockTime import com.ichi2.anki.common.time.TimeManager +import com.ichi2.anki.libanki.EpochMilliseconds import com.ichi2.anki.reviewreminders.ReviewReminder -import com.ichi2.anki.reviewreminders.ReviewReminderId +import com.ichi2.anki.reviewreminders.ReviewReminderScope import com.ichi2.anki.reviewreminders.ReviewReminderTime import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase +import com.ichi2.anki.services.NotificationService.Companion.EXTRA_REVIEW_REMINDER +import com.ichi2.testutils.ext.storeReminders import io.mockk.every import io.mockk.mockk import io.mockk.mockkStatic import io.mockk.unmockkAll import io.mockk.verify +import org.hamcrest.MatcherAssert.assertThat +import org.hamcrest.Matchers.equalTo import org.junit.After import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import java.util.Calendar +import kotlin.time.Duration.Companion.days import kotlin.time.Duration.Companion.minutes @RunWith(AndroidJUnit4::class) class AlarmManagerServiceTest : RobolectricTest() { companion object { - private val mockTime = MockTime(2024, 0, 1, 12, 0, 0, 0, 0) + /** + * Set the mock time to a consistent date and a specific time: noon in the local time zone (not UTC! this is required because + * the subject-under-test calculates times based on the local time zone, hence the convoluted construction of this mock). + */ + private val mockTime = + MockTime( + initTime = + MockTime(2024, 0, 1, 0, 0, 0, 0, 0) + .calendar() + .apply { + set(Calendar.HOUR_OF_DAY, 12) + set(Calendar.MINUTE, 0) + set(Calendar.SECOND, 0) + }.timeInMillis, + step = 0, + ) } private lateinit var context: Context @@ -65,7 +86,10 @@ class AlarmManagerServiceTest : RobolectricTest() { notificationManager = mockk(relaxed = true) every { context.getSystemService() } returns alarmManager every { context.getSystemService() } returns notificationManager + reviewReminder = ReviewReminder.createReviewReminder(time = ReviewReminderTime(20, 0)) + reviewReminder.latestNotifTime = mockTime.intTimeMS() // Ensure the reminder is ready to have its future instance scheduled + TimeManager.resetWith(mockTime) ReviewRemindersDatabase.remindersSharedPrefs.edit { clear() } } @@ -89,7 +113,7 @@ class AlarmManagerServiceTest : RobolectricTest() { alarmManager.setWindow( AlarmManager.RTC_WAKEUP, expectedSchedulingTime.timeInMillis, - 10.minutes.inWholeMilliseconds, + AlarmManagerService.WINDOW_LENGTH_MS, any(), ) } @@ -109,7 +133,7 @@ class AlarmManagerServiceTest : RobolectricTest() { alarmManager.setWindow( AlarmManager.RTC_WAKEUP, expectedSchedulingTime.timeInMillis, - 10.minutes.inWholeMilliseconds, + AlarmManagerService.WINDOW_LENGTH_MS, any(), ) } @@ -133,30 +157,192 @@ class AlarmManagerServiceTest : RobolectricTest() { verify { alarmManager.cancel(pendingIntent) } } + /** + * A single test case for alarm scheduling testing. + * @see scheduleAllNotificationsTest + */ + private data class ScheduleAllNotificationsTestCase( + val reminder: ReviewReminder, + val shouldImmediatelySetAlarm: Boolean, + val shouldImmediatelyFireNotif: Boolean, + val latestNotifTime: EpochMilliseconds? = null, + ) + + /** + * Helper function that tries scheduling multiple reminders and verifies the outcome for each. + */ + private fun scheduleAllNotificationsTest(vararg testCases: ScheduleAllNotificationsTestCase) { + require(testCases.map { it.reminder.time }.toSet().size == testCases.size) { + "All test cases must have unique reminder times to facilitate alarm scheduling verification" + } + + testCases.forEach { case -> + if (case.latestNotifTime != null) { + case.reminder.latestNotifTime = case.latestNotifTime + } + } + val previousLatestNotifTimes = testCases.associate { it.reminder.id to it.reminder.latestNotifTime } + + ReviewRemindersDatabase.storeReminders(*testCases.map { it.reminder }.toTypedArray()) + val currentTimestamp = mockTime.calendar().clone() as Calendar + + AlarmManagerService.scheduleAllNotifications(context) + + testCases.forEach { case -> + val expectedSchedulingTime = mockTime.calendar().clone() as Calendar + expectedSchedulingTime.apply { + set(Calendar.HOUR_OF_DAY, case.reminder.time.hour) + set(Calendar.MINUTE, case.reminder.time.minute) + set(Calendar.SECOND, 0) + if (before(currentTimestamp)) { + add(Calendar.DAY_OF_YEAR, 1) + } + } + + val alarmSettingCallsExpected = if (case.shouldImmediatelySetAlarm) 1 else 0 + verify(exactly = alarmSettingCallsExpected) { + alarmManager.setWindow( + AlarmManager.RTC_WAKEUP, + expectedSchedulingTime.timeInMillis, + 10.minutes.inWholeMilliseconds, + any(), + ) + } + } + + val (testCasesWithFiring, testCasesWithoutFiring) = testCases.partition { it.shouldImmediatelyFireNotif } + val expectedFired = testCasesWithFiring.map { it.reminder }.toSet() + val expectedNotFired = testCasesWithoutFiring.map { it.reminder }.toSet() + + val capturedIntents = mutableListOf() + verify(exactly = expectedFired.size) { + context.sendBroadcast(capture(capturedIntents)) + } + val actuallyFired = + capturedIntents + .map { intent -> + BundleCompat.getParcelable( + intent.extras!!, + EXTRA_REVIEW_REMINDER, + ReviewReminder::class.java, + )!! + }.toSet() + val actuallyFiredIds = actuallyFired.map { it.id }.toSet() + val notFired = testCases.map { it.reminder }.filterNot { it.id in actuallyFiredIds }.toSet() + + // The actually fired reminders have a modified latestNotifTime, so we can't directly compare the reminder objects + assertThat((actuallyFired + notFired).map { it.id }.toSet(), equalTo(testCases.map { it.reminder.id }.toSet())) + assertThat(expectedFired.map { it.id }.toSet(), equalTo(actuallyFired.map { it.id }.toSet())) + + // But we can compare unfired reminders directly since they should be unchanged + assertThat(expectedNotFired, equalTo(notFired)) + + // Validate latestNotifTime has changed for fired reminders + val expectedFiredCreationTimes = expectedFired.associate { it.id to previousLatestNotifTimes[it.id]!! } + val notificationTimes = actuallyFired.associate { it.id to it.latestNotifTime } + assertThat(expectedFiredCreationTimes.keys.toSet(), equalTo(notificationTimes.keys.toSet())) + expectedFiredCreationTimes.forEach { (id, createdAt) -> + val notifTime = notificationTimes[id]!! + assertThat(notifTime > createdAt, equalTo(true)) + } + + // Validate stored reminders + val stored = ReviewRemindersDatabase.getAllAppWideReminders() + ReviewRemindersDatabase.getAllDeckSpecificReminders() + val (storedFired, storedNotFired) = stored.getRemindersList().partition { it.id in actuallyFired.map { r -> r.id } } + assertThat(storedFired.toSet(), equalTo(actuallyFired)) + assertThat(storedNotFired.toSet(), equalTo(notFired)) + } + @Test fun `scheduleAllNotifications schedules reminders for all enabled reminders in database`() = runTest { val did1 = addDeck("Deck1") val did2 = addDeck("Deck2") - val reminder1 = ReviewReminder.createReviewReminder(time = ReviewReminderTime(9, 0)) - val reminder2 = ReviewReminder.createReviewReminder(time = ReviewReminderTime(10, 0)) - val reminder3 = ReviewReminder.createReviewReminder(time = ReviewReminderTime(11, 0)) - val disabledReminder = - ReviewReminder.createReviewReminder( - time = ReviewReminderTime(11, 0), - enabled = false, - ) - ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reminder1) } - ReviewRemindersDatabase.editRemindersForDeck(did2) { mapOf(ReviewReminderId(1) to reminder2) } - ReviewRemindersDatabase.editAllAppWideReminders { - mapOf( - ReviewReminderId(2) to reminder3, - ReviewReminderId(3) to disabledReminder, - ) - } + scheduleAllNotificationsTest( + ScheduleAllNotificationsTestCase( + reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(9, 0), + scope = ReviewReminderScope.DeckSpecific(did1), + ), + shouldImmediatelySetAlarm = true, + shouldImmediatelyFireNotif = false, + ), + ScheduleAllNotificationsTestCase( + reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(10, 0), + scope = ReviewReminderScope.DeckSpecific(did2), + ), + shouldImmediatelySetAlarm = true, + shouldImmediatelyFireNotif = false, + ), + ScheduleAllNotificationsTestCase( + reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(11, 0), + ), + shouldImmediatelySetAlarm = true, + shouldImmediatelyFireNotif = false, + ), + ScheduleAllNotificationsTestCase( + reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(12, 0), + enabled = false, + ), + shouldImmediatelySetAlarm = false, + shouldImmediatelyFireNotif = false, + ), + ) + } - AlarmManagerService.scheduleAllNotifications(context) - verify(exactly = 3) { alarmManager.setWindow(AlarmManager.RTC_WAKEUP, any(), 10.minutes.inWholeMilliseconds, any()) } + @Test + fun `scheduleAllNotifications immediately fires notification for reminders which missed scheduled firings`() = + runTest { + val did1 = addDeck("Deck1") + scheduleAllNotificationsTest( + ScheduleAllNotificationsTestCase( + reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(11, 58), // Last scheduled at 2 minutes earlier + scope = ReviewReminderScope.DeckSpecific(did1), + ), + shouldImmediatelySetAlarm = false, + shouldImmediatelyFireNotif = true, // Should fire immediately, because... + latestNotifTime = mockTime.intTimeMS() - 10.minutes.inWholeMilliseconds, // ...latest firing was 10 minutes ago + ), + ScheduleAllNotificationsTestCase( + reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(11, 59), // Last scheduled at 1 minute earlier + scope = ReviewReminderScope.Global, + ), + shouldImmediatelySetAlarm = true, + shouldImmediatelyFireNotif = false, // Should not fire immediately, because... + latestNotifTime = mockTime.intTimeMS(), // ...a notification just fired + ), + ScheduleAllNotificationsTestCase( + reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(12, 1), // Last scheduled almost a full day ago + scope = ReviewReminderScope.Global, + ), + shouldImmediatelySetAlarm = true, + shouldImmediatelyFireNotif = false, // Should not fire immediately, because... + latestNotifTime = mockTime.intTimeMS() - 10.minutes.inWholeMilliseconds, // ...latest firing was 10 minutes ago + ), + ScheduleAllNotificationsTestCase( + reminder = + ReviewReminder.createReviewReminder( + time = ReviewReminderTime(12, 2), // Last scheduled almost a full day ago + scope = ReviewReminderScope.DeckSpecific(did1), + ), + shouldImmediatelySetAlarm = false, + shouldImmediatelyFireNotif = true, // Should fire immediately, because... + latestNotifTime = mockTime.intTimeMS() - 2.days.inWholeMilliseconds, // ...latest firing was 2 days ago + ), + ) } @Test @@ -173,7 +359,7 @@ class AlarmManagerServiceTest : RobolectricTest() { alarmManager.setWindow( AlarmManager.RTC_WAKEUP, mockTime.intTimeMS() + 5.minutes.inWholeMilliseconds, - 10.minutes.inWholeMilliseconds, + AlarmManagerService.WINDOW_LENGTH_MS, any(), ) } diff --git a/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt b/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt index f0791f601f1f..8efe0e28484f 100644 --- a/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt +++ b/AnkiDroid/src/test/java/com/ichi2/anki/services/NotificationServiceTest.kt @@ -28,13 +28,16 @@ import com.ichi2.anki.RobolectricTest import com.ichi2.anki.common.time.MockTime import com.ichi2.anki.common.time.TimeManager import com.ichi2.anki.libanki.DeckId +import com.ichi2.anki.libanki.EpochMilliseconds import com.ichi2.anki.reviewreminders.ReviewReminder import com.ichi2.anki.reviewreminders.ReviewReminderCardTriggerThreshold -import com.ichi2.anki.reviewreminders.ReviewReminderId -import com.ichi2.anki.reviewreminders.ReviewReminderScope +import com.ichi2.anki.reviewreminders.ReviewReminderScope.DeckSpecific +import com.ichi2.anki.reviewreminders.ReviewReminderScope.Global import com.ichi2.anki.reviewreminders.ReviewReminderTime import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase import com.ichi2.anki.settings.Prefs +import com.ichi2.testutils.ext.storeReminders +import io.mockk.CapturingSlot import io.mockk.every import io.mockk.mockk import io.mockk.mockkObject @@ -44,6 +47,7 @@ import io.mockk.unmockkAll import io.mockk.verify import org.hamcrest.MatcherAssert.assertThat import org.hamcrest.Matchers.equalTo +import org.hamcrest.Matchers.not import org.junit.After import org.junit.Before import org.junit.Test @@ -53,8 +57,8 @@ import kotlin.time.Duration.Companion.days @RunWith(AndroidJUnit4::class) class NotificationServiceTest : RobolectricTest() { companion object { - private val yesterday = MockTime(TimeManager.time.intTimeMS() - 1.days.inWholeMilliseconds) - private val today = MockTime(TimeManager.time.intTimeMS()) + private val yesterday = MockTime(TimeManager.time.intTimeMS() - 1.days.inWholeMilliseconds, step = 1000) + private val today = MockTime(TimeManager.time.intTimeMS(), step = 1000) } private lateinit var context: Context @@ -80,192 +84,126 @@ class NotificationServiceTest : RobolectricTest() { ReviewRemindersDatabase.remindersSharedPrefs.edit { clear() } } - private fun createAndSaveDummyDeckSpecificReminder(did: DeckId): ReviewReminder { - val reviewReminder = createTestReminder(deckId = did, thresholdInt = 1) - ReviewRemindersDatabase.editRemindersForDeck(did) { mapOf(ReviewReminderId(0) to reviewReminder) } - return reviewReminder - } - - private fun createAndSaveDummyAppWideReminder(): ReviewReminder { - val reviewReminder = createTestReminder(thresholdInt = 1) - ReviewRemindersDatabase.editAllAppWideReminders { mapOf(ReviewReminderId(1) to reviewReminder) } - return reviewReminder - } - - private fun triggerDummyReminderNotification(reviewReminder: ReviewReminder) { - val intent = - NotificationService.getIntent( - context, - reviewReminder, - NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, - ) - NotificationService().onReceive(context, intent) - } - @Test fun `onReceive with less cards than card threshold should not fire notification but schedule next`() = runTest { - val did1 = addDeck("Deck", setAsSelected = true) - addNotes(2).forEach { - it.firstCard().update { did = did1 } - } + val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2) val reviewReminderDeckSpecific = createTestReminder(deckId = did1, thresholdInt = 3) val reviewReminderAppWide = createTestReminder(thresholdInt = 3) - ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminderDeckSpecific) } - ReviewRemindersDatabase.editAllAppWideReminders { mapOf(ReviewReminderId(1) to reviewReminderAppWide) } + val deckSpecificCreationTime = reviewReminderDeckSpecific.latestNotifTime + val appWideCreationTime = reviewReminderAppWide.latestNotifTime + ReviewRemindersDatabase.storeReminders(reviewReminderDeckSpecific, reviewReminderAppWide) triggerDummyReminderNotification(reviewReminderDeckSpecific) triggerDummyReminderNotification(reviewReminderAppWide) - verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } - verify( - exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminderDeckSpecific) } - verify( - exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminderAppWide) } + verifyNoNotifsSent() + verifyNextNotifScheduled(reviewReminderDeckSpecific) + verifyNextNotifScheduled(reviewReminderAppWide) + reviewReminderDeckSpecific.verifyLatestNotifTime(previousTime = deckSpecificCreationTime, shouldHaveUpdated = true) + reviewReminderAppWide.verifyLatestNotifTime(previousTime = appWideCreationTime, shouldHaveUpdated = true) } @Test fun `onReceive with happy path for single deck should fire notification and schedule next`() = runTest { - val did1 = addDeck("Deck", setAsSelected = true) - addNotes(2).forEach { - it.firstCard().update { did = did1 } - } + val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2) val reviewReminder = createAndSaveDummyDeckSpecificReminder(did1) + val creationTime = reviewReminder.latestNotifTime triggerDummyReminderNotification(reviewReminder) - verify( - exactly = 1, - ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminder.id.value, any()) } - verify( - exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } + verifyNotifSent(reviewReminder) + verifyNextNotifScheduled(reviewReminder) + reviewReminder.verifyLatestNotifTime(previousTime = creationTime, shouldHaveUpdated = true) } @Test fun `onReceive with happy path for global reminder should fire notification and schedule next`() = runTest { - val did1 = addDeck("Deck1") - val did2 = addDeck("Deck2") - addNotes(2).forEach { - it.firstCard().update { did = did1 } - } - addNotes(2).forEach { - it.firstCard().update { did = did2 } - } + addDeck("Deck1").withNotes(count = 2) + addDeck("Deck2").withNotes(count = 2) val reviewReminder = createTestReminder(thresholdInt = 4) + val creationTime = reviewReminder.latestNotifTime triggerDummyReminderNotification(reviewReminder) - verify( - exactly = 1, - ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminder.id.value, any()) } - verify( - exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } + verifyNotifSent(reviewReminder) + verifyNextNotifScheduled(reviewReminder) + reviewReminder.verifyLatestNotifTime(previousTime = creationTime, shouldHaveUpdated = true) } @Test fun `onReceive with non-existent deck should not fire notification but schedule next`() = runTest { val reviewReminder = createTestReminder(deckId = 9999) + val creationTime = reviewReminder.latestNotifTime triggerDummyReminderNotification(reviewReminder) - verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } - verify( - exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminder) } + verifyNoNotifsSent() + verifyNextNotifScheduled(reviewReminder) + reviewReminder.verifyLatestNotifTime(previousTime = creationTime, shouldHaveUpdated = true) } @Test fun `onReceive with reviews today and onlyNotifyIfNoReviews is true should not fire notification`() = runTest { - val did1 = addDeck("Deck", setAsSelected = true) - addNotes(1).forEach { - it.firstCard().update { did = did1 } - } + val did1 = addDeck("Deck", setAsSelected = true).withNote() col.sched.answerCard(col.sched.card!!, CardAnswer.Rating.GOOD) val reviewReminderDeckSpecific = createTestReminder(deckId = did1, thresholdInt = 1, onlyNotifyIfNoReviews = true) val reviewReminderAppWide = createTestReminder(thresholdInt = 1, onlyNotifyIfNoReviews = true) - ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminderDeckSpecific) } - ReviewRemindersDatabase.editAllAppWideReminders { mapOf(ReviewReminderId(1) to reviewReminderAppWide) } + ReviewRemindersDatabase.storeReminders(reviewReminderDeckSpecific, reviewReminderAppWide) triggerDummyReminderNotification(reviewReminderDeckSpecific) triggerDummyReminderNotification(reviewReminderAppWide) - verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } + + verifyNoNotifsSent() } @Test fun `onReceive with no reviews ever and onlyNotifyIfNoReviews is true should fire notification`() = runTest { - val did1 = addDeck("Deck", setAsSelected = true) - addNotes(1).forEach { - it.firstCard().update { did = did1 } - } + val did1 = addDeck("Deck", setAsSelected = true).withNote() val reviewReminderDeckSpecific = createTestReminder(deckId = did1, thresholdInt = 1, onlyNotifyIfNoReviews = true) val reviewReminderAppWide = createTestReminder(thresholdInt = 1, onlyNotifyIfNoReviews = true) - ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminderDeckSpecific) } - ReviewRemindersDatabase.editAllAppWideReminders { mapOf(ReviewReminderId(1) to reviewReminderAppWide) } + ReviewRemindersDatabase.storeReminders(reviewReminderDeckSpecific, reviewReminderAppWide) triggerDummyReminderNotification(reviewReminderDeckSpecific) triggerDummyReminderNotification(reviewReminderAppWide) - verify( - exactly = 1, - ) { - notificationManager.notify( - NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, - reviewReminderDeckSpecific.id.value, - any(), - ) - } - verify( - exactly = 1, - ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminderAppWide.id.value, any()) } + + verifyNotifSent(reviewReminderDeckSpecific) + verifyNotifSent(reviewReminderAppWide) } @Test fun `onReceive with review yesterday but none today and onlyNotifyIfNoReviews is true should fire notification`() = runTest { TimeManager.resetWith(yesterday) // Wind back time and perform the review - val did1 = addDeck("Deck", setAsSelected = true) - addNotes(1).forEach { - it.firstCard().update { did = did1 } - } + val did1 = addDeck("Deck", setAsSelected = true).withNote() col.sched.answerCard(col.sched.card!!, CardAnswer.Rating.GOOD) TimeManager.resetWith(today) // Reset time to present - val reviewReminderDeckSpecific = createTestReminder(deckId = did1, thresholdInt = 1, onlyNotifyIfNoReviews = true) - val reviewReminderAppWide = createTestReminder(thresholdInt = 1, onlyNotifyIfNoReviews = true) - ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminderDeckSpecific) } - ReviewRemindersDatabase.editAllAppWideReminders { mapOf(ReviewReminderId(1) to reviewReminderAppWide) } + val reviewReminderDeckSpecific = + createTestReminder(deckId = did1, thresholdInt = 1, onlyNotifyIfNoReviews = true) + val reviewReminderAppWide = + createTestReminder(thresholdInt = 1, onlyNotifyIfNoReviews = true) + ReviewRemindersDatabase.storeReminders( + reviewReminderDeckSpecific, + reviewReminderAppWide, + ) triggerDummyReminderNotification(reviewReminderDeckSpecific) triggerDummyReminderNotification(reviewReminderAppWide) - verify( - exactly = 1, - ) { - notificationManager.notify( - NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, - reviewReminderDeckSpecific.id.value, - any(), - ) - } - verify( - exactly = 1, - ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminderAppWide.id.value, any()) } + + verifyNotifSent(reviewReminderDeckSpecific) + verifyNotifSent(reviewReminderAppWide) } @Test fun `onReceive with onlyNotifyIfNoReviews is false should always fire notification`() = runTest { - val did1 = addDeck("Deck", setAsSelected = true) - addNotes(1).forEach { - it.firstCard().update { did = did1 } - } + val did1 = addDeck("Deck", setAsSelected = true).withNote() val reviewReminderDeckSpecific = createAndSaveDummyDeckSpecificReminder(did1) val reviewReminderAppWide = createAndSaveDummyAppWideReminder() @@ -275,136 +213,71 @@ class NotificationServiceTest : RobolectricTest() { triggerDummyReminderNotification(reviewReminderDeckSpecific) triggerDummyReminderNotification(reviewReminderAppWide) - verify( - exactly = 2, - ) { - notificationManager.notify( - NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, - reviewReminderDeckSpecific.id.value, - any(), - ) - } - verify( - exactly = 2, - ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminderAppWide.id.value, any()) } + verifyNotifSent(reviewReminderDeckSpecific, times = 2) + verifyNotifSent(reviewReminderAppWide, times = 2) } @Test fun `onReceive with blocked collection should not fire notification but schedule next`() = runTest { - val did1 = addDeck("Deck", setAsSelected = true) - addNotes(2).forEach { - it.firstCard().update { did = did1 } - } + val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2) val reviewReminderDeckSpecific = createAndSaveDummyDeckSpecificReminder(did1) val reviewReminderAppWide = createAndSaveDummyAppWideReminder() + val deckSpecificCreationTime = reviewReminderDeckSpecific.latestNotifTime + val appWideCreationTime = reviewReminderAppWide.latestNotifTime CollectionManager.emulatedOpenFailure = CollectionManager.CollectionOpenFailure.LOCKED triggerDummyReminderNotification(reviewReminderDeckSpecific) triggerDummyReminderNotification(reviewReminderAppWide) - verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } - verify( - exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminderDeckSpecific) } - verify( - exactly = 1, - ) { AlarmManagerService.scheduleReviewReminderNotification(context, reviewReminderAppWide) } + verifyNoNotifsSent() + verifyNextNotifScheduled(reviewReminderDeckSpecific) + verifyNextNotifScheduled(reviewReminderAppWide) + reviewReminderDeckSpecific.verifyLatestNotifTime(previousTime = deckSpecificCreationTime, shouldHaveUpdated = true) + reviewReminderAppWide.verifyLatestNotifTime(previousTime = appWideCreationTime, shouldHaveUpdated = true) } @Test fun `onReceive with snoozed notification should fire notification but not schedule next`() = runTest { - val did1 = addDeck("Deck", setAsSelected = true) - addNotes(2).forEach { - it.firstCard().update { did = did1 } - } + val did1 = addDeck("Deck", setAsSelected = true).withNotes(count = 2) val reviewReminderDeckSpecific = createAndSaveDummyDeckSpecificReminder(did1) val reviewReminderAppWide = createAndSaveDummyAppWideReminder() + val deckSpecificCreationTime = reviewReminderDeckSpecific.latestNotifTime + val appWideCreationTime = reviewReminderAppWide.latestNotifTime - val intentDeckSpecific = - NotificationService.getIntent( - context, - reviewReminderDeckSpecific, - NotificationService.NotificationServiceAction.SnoozeNotification, - ) - val intentAppWide = - NotificationService.getIntent( - context, - reviewReminderAppWide, - NotificationService.NotificationServiceAction.SnoozeNotification, - ) + val intentDeckSpecific = reviewReminderDeckSpecific.getNotifIntent(NotifIntent.SNOOZE) + val intentAppWide = reviewReminderAppWide.getNotifIntent(NotifIntent.SNOOZE) NotificationService().onReceive(context, intentDeckSpecific) NotificationService().onReceive(context, intentAppWide) - verify( - exactly = 1, - ) { - notificationManager.notify( - NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, - reviewReminderDeckSpecific.id.value, - any(), - ) - } - verify( - exactly = 1, - ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reviewReminderAppWide.id.value, any()) } - verify(exactly = 0) { AlarmManagerService.scheduleReviewReminderNotification(context, any()) } + verifyNotifSent(reviewReminderDeckSpecific) + verifyNotifSent(reviewReminderAppWide) + verifyNextNotifNotScheduled() + reviewReminderDeckSpecific.verifyLatestNotifTime(previousTime = deckSpecificCreationTime, shouldHaveUpdated = false) + reviewReminderAppWide.verifyLatestNotifTime(previousTime = appWideCreationTime, shouldHaveUpdated = false) } @Test fun `snooze actions of different notifications and different intervals should be different`() = runTest { - val did1 = addDeck("Deck1") - val did2 = addDeck("Deck2") - addNotes(2).forEach { - it.firstCard().update { did = did1 } - } - addNotes(2).forEach { - it.firstCard().update { did = did2 } - } + val did1 = addDeck("Deck1").withNotes(count = 2) + val did2 = addDeck("Deck2").withNotes(count = 2) val reviewReminderOne = createTestReminder(deckId = did1, thresholdInt = 1) val reviewReminderTwo = createTestReminder(deckId = did2, thresholdInt = 1) - ReviewRemindersDatabase.editRemindersForDeck(did1) { mapOf(ReviewReminderId(0) to reviewReminderOne) } - ReviewRemindersDatabase.editRemindersForDeck(did2) { mapOf(ReviewReminderId(1) to reviewReminderTwo) } + ReviewRemindersDatabase.storeReminders(reviewReminderOne, reviewReminderTwo) val slotOne = slot() val slotTwo = slot() - val intentOne = - NotificationService.getIntent( - context, - reviewReminderOne, - NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, - ) + val intentOne = reviewReminderOne.getNotifIntent(NotifIntent.RECURRING) NotificationService().onReceive(context, intentOne) - val intentTwo = - NotificationService.getIntent( - context, - reviewReminderTwo, - NotificationService.NotificationServiceAction.ScheduleRecurringNotifications, - ) + val intentTwo = reviewReminderTwo.getNotifIntent(NotifIntent.RECURRING) NotificationService().onReceive(context, intentTwo) - verify( - exactly = 1, - ) { - notificationManager.notify( - NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, - reviewReminderOne.id.value, - capture(slotOne), - ) - } - verify( - exactly = 1, - ) { - notificationManager.notify( - NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, - reviewReminderTwo.id.value, - capture(slotTwo), - ) - } + verifyNotifSent(reviewReminderOne, slot = slotOne) + verifyNotifSent(reviewReminderTwo, slot = slotTwo) val snoozeIntents = setOf( @@ -416,12 +289,25 @@ class NotificationServiceTest : RobolectricTest() { assertThat(snoozeIntents.size, equalTo(4)) } + private fun createAndSaveDummyDeckSpecificReminder(did: DeckId): ReviewReminder { + val reviewReminder = createTestReminder(deckId = did, thresholdInt = 1) + ReviewRemindersDatabase.storeReminders(reviewReminder) + return reviewReminder + } + + private fun createAndSaveDummyAppWideReminder(): ReviewReminder { + val reviewReminder = createTestReminder(thresholdInt = 1) + ReviewRemindersDatabase.storeReminders(reviewReminder) + return reviewReminder + } + + private fun triggerDummyReminderNotification(reviewReminder: ReviewReminder) { + val intent = reviewReminder.getNotifIntent(NotifIntent.RECURRING) + NotificationService().onReceive(context, intent) + } + /** * Helper method for creating a review reminder to minimize verbosity in this file. - * - * @param deckId If specified, the reminder will be deck-specific to this deck ID. If null, it will be app-wide. - * @param thresholdInt The card trigger threshold as an integer. - * @param onlyNotifyIfNoReviews Whether the reminder should only notify if there are no reviews today. */ private fun createTestReminder( deckId: DeckId? = null, @@ -430,7 +316,71 @@ class NotificationServiceTest : RobolectricTest() { ) = ReviewReminder.createReviewReminder( time = ReviewReminderTime(hour = 12, minute = 0), cardTriggerThreshold = ReviewReminderCardTriggerThreshold(thresholdInt), - scope = if (deckId != null) ReviewReminderScope.DeckSpecific(deckId) else ReviewReminderScope.Global, + scope = if (deckId != null) DeckSpecific(deckId) else Global, onlyNotifyIfNoReviews = onlyNotifyIfNoReviews, ) + + private fun verifyNoNotifsSent() { + verify(exactly = 0) { notificationManager.notify(any(), any(), any()) } + } + + private fun verifyNotifSent( + reminder: ReviewReminder, + times: Int = 1, + slot: CapturingSlot? = null, + ) { + if (slot != null) { + verify( + exactly = times, + ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reminder.id.value, capture(slot)) } + } else { + verify( + exactly = times, + ) { notificationManager.notify(NotificationService.REVIEW_REMINDER_NOTIFICATION_TAG, reminder.id.value, any()) } + } + } + + private fun verifyNextNotifNotScheduled() { + verify(exactly = 0) { AlarmManagerService.scheduleReviewReminderNotification(any(), any()) } + } + + private fun verifyNextNotifScheduled(reminder: ReviewReminder) { + verify( + exactly = 1, + ) { AlarmManagerService.scheduleReviewReminderNotification(context, reminder) } + } + + private fun ReviewReminder.verifyLatestNotifTime( + previousTime: EpochMilliseconds, + shouldHaveUpdated: Boolean, + ) { + val storedReminderGroup = + when (scope) { + is DeckSpecific -> ReviewRemindersDatabase.getRemindersForDeck(scope.did) + is Global -> ReviewRemindersDatabase.getAllAppWideReminders() + } + val storedReminder = storedReminderGroup.getRemindersList().find { it.id == id }!! + + if (shouldHaveUpdated) { + assertThat(previousTime, not(equalTo(storedReminder.latestNotifTime))) + } else { + assertThat(previousTime, equalTo(storedReminder.latestNotifTime)) + } + } + + /** + * Convenience enum class to minimize verbosity in test methods when using [getNotifIntent]. + */ + private enum class NotifIntent { + RECURRING, + SNOOZE, + } + + private fun ReviewReminder.getNotifIntent(action: NotifIntent) = + when (action) { + NotifIntent.RECURRING -> NotificationService.NotificationServiceAction.ScheduleRecurringNotifications + NotifIntent.SNOOZE -> NotificationService.NotificationServiceAction.SnoozeNotification + }.let { action -> + NotificationService.getIntent(context, this, action) + } } diff --git a/AnkiDroid/src/test/java/com/ichi2/testutils/ext/ReviewRemindersDatabase.kt b/AnkiDroid/src/test/java/com/ichi2/testutils/ext/ReviewRemindersDatabase.kt new file mode 100644 index 000000000000..f9b507a409ed --- /dev/null +++ b/AnkiDroid/src/test/java/com/ichi2/testutils/ext/ReviewRemindersDatabase.kt @@ -0,0 +1,40 @@ +/* + * Copyright (c) 2026 Eric Li + * + * This program is free software; you can redistribute it and/or modify it under + * the terms of the GNU General Public License as published by the Free Software + * Foundation; either version 3 of the License, or (at your option) any later + * version. + * + * This program is distributed in the hope that it will be useful, but WITHOUT ANY + * WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A + * PARTICULAR PURPOSE. See the GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License along with + * this program. If not, see . + */ + +package com.ichi2.testutils.ext + +import com.ichi2.anki.reviewreminders.ReviewReminder +import com.ichi2.anki.reviewreminders.ReviewReminderGroup +import com.ichi2.anki.reviewreminders.ReviewReminderScope.DeckSpecific +import com.ichi2.anki.reviewreminders.ReviewReminderScope.Global +import com.ichi2.anki.reviewreminders.ReviewRemindersDatabase + +fun ReviewRemindersDatabase.storeReminders(vararg reminders: ReviewReminder) { + reminders.forEach { reminder -> + when (reminder.scope) { + is DeckSpecific -> { + editRemindersForDeck(reminder.scope.did) { reminders -> + reminders + ReviewReminderGroup(reminder.id to reminder) + } + } + is Global -> { + editAllAppWideReminders { reminders -> + reminders + ReviewReminderGroup(reminder.id to reminder) + } + } + } + } +} diff --git a/libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt b/libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt index 9e0ef2f52dbc..000f62a9e9c6 100644 --- a/libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt +++ b/libanki/testutils/src/main/java/com/ichi2/anki/libanki/testutils/AnkiTest.kt @@ -229,6 +229,43 @@ interface AnkiTest { front: String = "Front", ): List = List(count) { addBasicNote(front = front) } + /** + * Adds [count] notes into the specified [queueType] of the provided deck. + */ + fun addNoteToDeck( + deckId: DeckId, + count: Int = 1, + queueType: QueueType = QueueType.New, + ) = addNotes(count).forEach { + it.firstCard().update { + did = deckId + queue = queueType + } + } + + /** + * Convenience method for chaining [addDeck] and [addNoteToDeck]. + * + * Usage: `val deckId = addDeck("My Deck").withNotes(count = 5, queueType = QueueType.New)` + */ + fun DeckId.withNotes( + count: Int, + queueType: QueueType = QueueType.New, + ): DeckId = + this.apply { + addNoteToDeck(this, count = count, queueType = queueType) + } + + /** + * Convenience method for chaining [addDeck] and [addNoteToDeck]. + * + * Usage: `val deckId = addDeck("My Deck").withNote(queueType = QueueType.New)` + */ + fun DeckId.withNote(queueType: QueueType = QueueType.New): DeckId = + this.apply { + addNoteToDeck(this, count = 1, queueType = queueType) + } + fun Note.moveToDeck( deckName: String, createDeckIfMissing: Boolean = true,