diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt index 0f2bea40e..fea43c5d8 100644 --- a/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt +++ b/app/src/test/kotlin/com/android/messaging/datamodel/DatabaseUpgradeHelperTest.kt @@ -13,6 +13,7 @@ import io.mockk.unmockkAll import org.junit.After import org.junit.Assert.assertEquals import org.junit.Assert.assertFalse +import org.junit.Assert.assertNotEquals import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test @@ -115,6 +116,37 @@ class DatabaseUpgradeHelperTest { ) } + /** + * Conversation ids name notification channels, shortcuts and per-conversation prefs, which + * outlive the database - restored without it, or left behind by a rebuild. Recreating a + * deleted channel id undeletes its old settings, so counting from 1 again would hand each new + * conversation the settings of whichever old one had its number. + */ + @Test + fun rebuildTables_startsEachDatabasesConversationIdsAtItsOwnPointFarPastOne() { + SQLiteDatabase.create(null).use { db -> + DatabaseHelper.rebuildTables(db) + val firstId = db.insertConversation() + DatabaseHelper.rebuildTables(db) + val secondId = db.insertConversation() + + assertTrue(firstId >= DatabaseHelper.CONVERSATION_ID_SEED_MIN) + assertTrue(secondId >= DatabaseHelper.CONVERSATION_ID_SEED_MIN) + assertNotEquals("a rebuilt database reused the old id space", firstId, secondId) + } + } + + @Test + fun onCreate_startsConversationIdsFarPastOne() { + val context = RuntimeEnvironment.getApplication().applicationContext + + SQLiteDatabase.create(null).use { db -> + DatabaseHelper.getInstance(context).onCreate(db) + + assertTrue(db.insertConversation() >= DatabaseHelper.CONVERSATION_ID_SEED_MIN) + } + } + @Test fun upgradeToVersion3_createsPinnedColumnAndIndex() { val table = DatabaseHelper.CONVERSATIONS_TABLE @@ -159,6 +191,14 @@ class DatabaseUpgradeHelperTest { } } + private fun SQLiteDatabase.insertConversation(): Long { + return insert( + DatabaseHelper.CONVERSATIONS_TABLE, + null, + contentValuesOf(ConversationColumns.NAME to "Weekend plan"), + ) + } + private fun SQLiteDatabase.hasColumn(table: String, column: String): Boolean { return rawQuery("SELECT * FROM $table LIMIT 0", null).use { cursor -> cursor.getColumnIndex(column) != -1 diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/StaleConversationStateTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/StaleConversationStateTest.kt new file mode 100644 index 000000000..6777f178f --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/datamodel/StaleConversationStateTest.kt @@ -0,0 +1,108 @@ +package com.android.messaging.datamodel + +import android.app.Application +import android.app.NotificationChannel +import android.app.NotificationManager +import android.content.Context +import android.content.ContextWrapper +import com.android.messaging.util.BuglePrefs +import com.android.messaging.util.BugleWidgetPrefs +import com.android.messaging.util.NotificationChannelUtil +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +/** + * A restore brings back per-conversation state without the database it was keyed against, and a + * rebuilt database counts its ids again. Whatever survives would attach itself to an unrelated + * conversation, or at best clutter the system settings with conversations that no longer exist. + */ +@RunWith(RobolectricTestRunner::class) +class StaleConversationStateTest { + + private val context: Application = RuntimeEnvironment.getApplication() + + @Test + fun clearsConversationPrefsAndKeepsTheRest() { + val preferences = context.getSharedPreferences( + BuglePrefs.SHARED_PREFERENCES_NAME, + Context.MODE_PRIVATE, + ) + preferences.edit() + .putLong("conversation_snooze_until_7", Long.MAX_VALUE) + .putString("conversation_sim_selection_7", "3") + .putBoolean("conversation_notification_channels_migrated_v1", true) + .putBoolean("some_app_setting", true) + .commit() + + clearStaleConversationState(context) + + assertEquals( + setOf("conversation_notification_channels_migrated_v1", "some_app_setting"), + preferences.all.keys, + ) + } + + @Test + fun clearsWidgetConversations() { + val widgetPreferences = context.getSharedPreferences( + BugleWidgetPrefs.SHARED_PREFERENCES_WIDGET_NAME, + Context.MODE_PRIVATE, + ) + widgetPreferences.edit().putString("conversation_id12", "7").commit() + + clearStaleConversationState(context) + + assertEquals(emptyMap(), widgetPreferences.all) + } + + @Test + fun deletesConversationChannelsAndKeepsTheAppChannels() { + val notificationManager = context.getSystemService(NotificationManager::class.java) + listOf(NotificationChannelUtil.INCOMING_MESSAGES, NotificationChannelUtil.ALERTS_CHANNEL) + .forEach { id -> + notificationManager.createNotificationChannel( + NotificationChannel(id, id, NotificationManager.IMPORTANCE_HIGH), + ) + } + val conversationChannel = NotificationChannel( + "7", + "Weekend plan", + NotificationManager.IMPORTANCE_HIGH, + ) + conversationChannel.setConversationId(NotificationChannelUtil.INCOMING_MESSAGES, "7") + notificationManager.createNotificationChannel(conversationChannel) + + clearStaleConversationState(context) + + assertEquals( + setOf( + NotificationChannelUtil.INCOMING_MESSAGES, + NotificationChannelUtil.ALERTS_CHANNEL, + ), + notificationManager.notificationChannels.map { it.id }.toSet(), + ) + } + + /** It runs while the database is being created, so throwing here would fail that too. */ + @Test + fun withoutSystemServices_stillClearsPrefsAndDoesNotThrow() { + val preferences = context.getSharedPreferences( + BuglePrefs.SHARED_PREFERENCES_NAME, + Context.MODE_PRIVATE, + ) + preferences.edit().putLong("conversation_snooze_until_7", Long.MAX_VALUE).commit() + val contextWithoutServices = object : ContextWrapper(context) { + override fun getSystemService(name: String): Any? { + return null + } + } + + clearStaleConversationState(contextWithoutServices) + + assertFalse(preferences.contains("conversation_snooze_until_7")) + } +} diff --git a/app/src/test/kotlin/com/android/messaging/datamodel/action/SyncMessagesActionMmsReplacementRaceTest.kt b/app/src/test/kotlin/com/android/messaging/datamodel/action/SyncMessagesActionMmsReplacementRaceTest.kt index f90e8ce77..f4b427278 100644 --- a/app/src/test/kotlin/com/android/messaging/datamodel/action/SyncMessagesActionMmsReplacementRaceTest.kt +++ b/app/src/test/kotlin/com/android/messaging/datamodel/action/SyncMessagesActionMmsReplacementRaceTest.kt @@ -82,10 +82,6 @@ class SyncMessagesActionMmsReplacementRaceTest { @After fun tearDown() { ShadowContentResolver.reset() - if (::database.isInitialized) { - deleteSyntheticRows() - } - FactoryTestAccess.reset() } diff --git a/app/src/test/kotlin/com/android/messaging/widget/WidgetConversationProviderTest.kt b/app/src/test/kotlin/com/android/messaging/widget/WidgetConversationProviderTest.kt new file mode 100644 index 000000000..3367d3c43 --- /dev/null +++ b/app/src/test/kotlin/com/android/messaging/widget/WidgetConversationProviderTest.kt @@ -0,0 +1,81 @@ +package com.android.messaging.widget + +import android.app.Application +import android.appwidget.AppWidgetManager +import android.content.ContentResolver +import android.content.ContextWrapper +import android.view.View +import android.widget.FrameLayout +import android.widget.RemoteViews +import com.android.messaging.FactoryTestAccess +import com.android.messaging.R +import com.android.messaging.testutil.installTestFactory +import io.mockk.Runs +import io.mockk.every +import io.mockk.just +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.unmockkAll +import kotlin.concurrent.thread +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment + +@RunWith(RobolectricTestRunner::class) +class WidgetConversationProviderTest { + + private val context: Application = RuntimeEnvironment.getApplication() + private val appWidgetManager = mockk(relaxed = true) + + @Before + fun setUp() { + installTestFactory(context = context) + mockkStatic(AppWidgetManager::class) + every { AppWidgetManager.getInstance(any()) } returns appWidgetManager + } + + @After + fun tearDown() { + unmockkAll() + FactoryTestAccess.reset() + } + + /** + * Reading the conversation can create the database, and creating it resets every widget. A + * rebuild already past its check for the conversation must not put that conversation back + * over the reset, or the widget sits on a conversation that no longer exists. + */ + @Test + fun rebuildWidget_whenReadingTheConversationResetsTheWidget_leavesItToBeConfigured() { + WidgetConversationPrefs.saveConversationIdPref( + appWidgetId = WIDGET_ID, + conversationId = "7", + ) + val contentResolver = mockk() + every { contentResolver.query(any(), any(), any(), any(), any()) } answers { + WidgetConversationPrefs.deleteConversationIdPref(WIDGET_ID) + null + } + val resettingContext = object : ContextWrapper(context) { + override fun getContentResolver(): ContentResolver { + return contentResolver + } + } + val updates = mutableListOf() + every { appWidgetManager.updateAppWidget(WIDGET_ID, capture(updates)) } just Runs + + // Off the main thread, as only there does it read the conversation + thread { WidgetConversationProvider.rebuildWidget(resettingContext, WIDGET_ID) }.join() + + val widget = updates.last().apply(context, FrameLayout(context)) + assertEquals(View.VISIBLE, widget.findViewById(R.id.widget_configuration).visibility) + } + + companion object { + private const val WIDGET_ID = 3 + } +} diff --git a/res/xml/backup_rules.xml b/res/xml/backup_rules.xml index 3cb6be5c4..22c065458 100644 --- a/res/xml/backup_rules.xml +++ b/res/xml/backup_rules.xml @@ -1,5 +1,19 @@ - - + + + + + + + + diff --git a/src/com/android/messaging/data/conversationsettings/repository/ConversationNotificationRepository.kt b/src/com/android/messaging/data/conversationsettings/repository/ConversationNotificationRepository.kt index 46ee4155a..4921081a6 100644 --- a/src/com/android/messaging/data/conversationsettings/repository/ConversationNotificationRepository.kt +++ b/src/com/android/messaging/data/conversationsettings/repository/ConversationNotificationRepository.kt @@ -99,8 +99,8 @@ internal class ConversationNotificationRepositoryImpl @Inject constructor( } } - private companion object { + companion object { const val SNOOZE_KEY_PREFIX = "conversation_snooze_until_" - const val SNOOZE_NOT_SET = 0L + private const val SNOOZE_NOT_SET = 0L } } diff --git a/src/com/android/messaging/data/subscription/repository/ConversationSimSelectionRepository.kt b/src/com/android/messaging/data/subscription/repository/ConversationSimSelectionRepository.kt index 2a9d3c158..a26b33c74 100644 --- a/src/com/android/messaging/data/subscription/repository/ConversationSimSelectionRepository.kt +++ b/src/com/android/messaging/data/subscription/repository/ConversationSimSelectionRepository.kt @@ -58,7 +58,7 @@ internal class ConversationSimSelectionRepositoryImpl @Inject constructor() : return "$PREF_KEY_PREFIX${conversationId.value}" } - private companion object { + companion object { const val PREF_KEY_PREFIX = "conversation_sim_selection_" } } diff --git a/src/com/android/messaging/datamodel/DataModelImpl.java b/src/com/android/messaging/datamodel/DataModelImpl.java index 4c9f21708..9d0193237 100644 --- a/src/com/android/messaging/datamodel/DataModelImpl.java +++ b/src/com/android/messaging/datamodel/DataModelImpl.java @@ -114,6 +114,7 @@ void onCreateTables(final SQLiteDatabase db) { LogUtil.w(LogUtil.BUGLE_TAG, "Rebuilt databases: reseting related state"); // Clear other things that implicitly reference the DB SyncManager.resetLastSyncTimestamps(); + StaleConversationState.clearStaleConversationState(mContext); } @Override diff --git a/src/com/android/messaging/datamodel/DatabaseHelper.java b/src/com/android/messaging/datamodel/DatabaseHelper.java index de36ca801..fefbaaf29 100644 --- a/src/com/android/messaging/datamodel/DatabaseHelper.java +++ b/src/com/android/messaging/datamodel/DatabaseHelper.java @@ -36,6 +36,7 @@ import java.io.File; import java.util.ArrayList; import java.util.List; +import java.util.concurrent.ThreadLocalRandom; /** * TODO: Open Issues: @@ -68,6 +69,10 @@ private static final int getDatabaseVersion(final Context context) { public static final String PARTICIPANTS_TABLE = "participants"; public static final String CONVERSATION_PARTICIPANTS_TABLE = "conversation_participants"; + @VisibleForTesting + static final long CONVERSATION_ID_SEED_MIN = 1L << 40; + private static final long CONVERSATION_ID_SEED_MAX = 1L << 52; + // Views static final String DRAFT_PARTS_VIEW = "draft_parts_view"; @@ -835,6 +840,8 @@ private static void createDatabase(final SQLiteDatabase db) { db.execSQL(sql); } + seedConversationIds(db); + for (final String sql : CREATE_INDEX_SQLS) { db.execSQL(sql); } @@ -854,6 +861,25 @@ private static void createDatabase(final SQLiteDatabase db) { DataModel.get().onCreateTables(db); } + /** + * conversations._id names each conversation's notification channel, shortcut, widget and + * per-conversation prefs, and all of those outlive the database: a restore brings them back + * without it, and the OS restores notification channels on its own. A table counting from 1 + * again would hand every new conversation the settings of whichever old one had its number, + * and deleting the old channels doesn't help, because recreating a deleted channel id brings + * its settings back. So each database starts its ids at a random point, far past any id + * counted from 1 and far from any other database's. Random rather than the clock, which can + * be wrong on first boot. + */ + private static void seedConversationIds(final SQLiteDatabase db) { + final long seed = ThreadLocalRandom + .current() + .nextLong(CONVERSATION_ID_SEED_MIN, CONVERSATION_ID_SEED_MAX); + + db.execSQL("INSERT INTO sqlite_sequence (name, seq) VALUES (?, ?)", + new Object[] {CONVERSATIONS_TABLE, seed}); + } + @Override public void onConfigure(SQLiteDatabase db) { // The schema declares ON DELETE CASCADE, but foreign keys are enforced per connection and diff --git a/src/com/android/messaging/datamodel/StaleConversationState.kt b/src/com/android/messaging/datamodel/StaleConversationState.kt new file mode 100644 index 000000000..ceb8b73d5 --- /dev/null +++ b/src/com/android/messaging/datamodel/StaleConversationState.kt @@ -0,0 +1,98 @@ +@file:JvmName("StaleConversationState") + +package com.android.messaging.datamodel + +import android.app.NotificationManager +import android.appwidget.AppWidgetManager +import android.content.ComponentName +import android.content.Context +import android.content.pm.ShortcutManager +import androidx.core.content.edit +import com.android.messaging.data.conversationsettings.repository.ConversationNotificationRepositoryImpl +import com.android.messaging.data.subscription.repository.ConversationSimSelectionRepositoryImpl +import com.android.messaging.util.BuglePrefs +import com.android.messaging.util.BugleWidgetPrefs +import com.android.messaging.util.LogUtil +import com.android.messaging.widget.WidgetConversationProvider + +private const val SHORTCUT_MATCH_FLAGS = ShortcutManager.FLAG_MATCH_DYNAMIC or + ShortcutManager.FLAG_MATCH_CACHED or + ShortcutManager.FLAG_MATCH_PINNED + +private val CONVERSATION_PREF_KEY_PREFIXES = listOf( + ConversationNotificationRepositoryImpl.SNOOZE_KEY_PREFIX, + ConversationSimSelectionRepositoryImpl.PREF_KEY_PREFIX, +) + +/** + * Clears what a new database leaves behind keyed by the conversation ids of the one before it: + * a restore brings this back without the database, and a rebuild doesn't touch it. Seeded + * conversation ids already keep it from reaching new conversations, this takes it out of the + * user's way. Each step is best effort, as a failure here must not fail creating the database. + */ +internal fun clearStaleConversationState(context: Context) { + bestEffort(step = "conversation prefs") { clearConversationPrefs(context) } + + bestEffort(step = "widgets") { resetWidgets(context) } + bestEffort(step = "notifications") { clearConversationNotifications(context) } + bestEffort(step = "shortcuts") { removeConversationShortcuts(context) } +} + +private fun clearConversationPrefs(context: Context) { + val preferences = context.getSharedPreferences( + BuglePrefs.SHARED_PREFERENCES_NAME, + Context.MODE_PRIVATE, + ) + preferences.edit { + preferences + .all + .keys + .filter { key -> + CONVERSATION_PREF_KEY_PREFIXES.any(key::startsWith) + } + .forEach(::remove) + } +} + +private fun resetWidgets(context: Context) { + context + .getSharedPreferences( + BugleWidgetPrefs.SHARED_PREFERENCES_WIDGET_NAME, + Context.MODE_PRIVATE, + ) + .edit { + clear() + } + + val provider = ComponentName(context, WidgetConversationProvider::class.java) + + AppWidgetManager.getInstance(context)?.getAppWidgetIds(provider)?.forEach { appWidgetId -> + WidgetConversationProvider.rebuildWidget(context, appWidgetId) + } +} + +private fun clearConversationNotifications(context: Context) { + val notificationManager = context.getSystemService(NotificationManager::class.java) + notificationManager.cancelAll() + notificationManager.notificationChannels + .filter { it.conversationId != null } + .forEach { notificationManager.deleteNotificationChannel(it.id) } +} + +private fun removeConversationShortcuts(context: Context) { + val shortcutManager = context.getSystemService(ShortcutManager::class.java) + val shortcutIds = shortcutManager.getShortcuts(SHORTCUT_MATCH_FLAGS) + .filterNot { it.isImmutable } + .map { it.id } + + shortcutManager.removeLongLivedShortcuts(shortcutIds) + + // Pinned shortcuts can't be removed, only disabled + shortcutManager.disableShortcuts(shortcutIds) +} + +private inline fun bestEffort(step: String, block: () -> Unit) { + runCatching(block).onFailure { + LogUtil.w(LogUtil.BUGLE_TAG, "Could not clear stale conversation $step", it) + } +} diff --git a/src/com/android/messaging/widget/WidgetConversationProvider.java b/src/com/android/messaging/widget/WidgetConversationProvider.java index d250ca7ce..55fbdef97 100644 --- a/src/com/android/messaging/widget/WidgetConversationProvider.java +++ b/src/com/android/messaging/widget/WidgetConversationProvider.java @@ -114,6 +114,13 @@ public static void rebuildWidget(final Context context, final int appWidgetId) { // conversation data. We'll do excute this again off of the UI thread. final ConversationListItemData convData = isMainThread ? null : getConversationData(context, conversationId); + if (!TextUtils.equals(conversationId, + WidgetConversationPrefs.getConversationIdPref(appWidgetId))) { + // Reading the conversation can create the database, which resets every widget. + // Rebuild from what the widget shows now rather than put back what it showed. + rebuildWidget(context, appWidgetId); + return; + } // Launch an intent to avoid ANRs final Intent intent = new Intent(context, WidgetConversationService.class);