Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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<String, Any>(), 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"))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -82,10 +82,6 @@ class SyncMessagesActionMmsReplacementRaceTest {
@After
fun tearDown() {
ShadowContentResolver.reset()
if (::database.isInitialized) {
deleteSyntheticRows()
}

FactoryTestAccess.reset()
}

Expand Down
Original file line number Diff line number Diff line change
@@ -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<AppWidgetManager>(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<ContentResolver>()
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<RemoteViews>()
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<View>(R.id.widget_configuration).visibility)
}

companion object {
private const val WIDGET_ID = 3
}
}
18 changes: 16 additions & 2 deletions res/xml/backup_rules.xml
Original file line number Diff line number Diff line change
@@ -1,5 +1,19 @@
<?xml version="1.0" encoding="utf-8"?>
<data-extraction-rules>
<cloud-backup disableIfNoEncryptionCapabilities="true" />
<device-transfer/>
<cloud-backup disableIfNoEncryptionCapabilities="true">
<exclude
domain="database"
path="bugle_db" />
<exclude
domain="sharedpref"
path="bugle_widgets.xml" />
</cloud-backup>
<device-transfer>
<exclude
domain="database"
path="bugle_db" />
<exclude
domain="sharedpref"
path="bugle_widgets.xml" />
</device-transfer>
</data-extraction-rules>
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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_"
}
}
1 change: 1 addition & 0 deletions src/com/android/messaging/datamodel/DataModelImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
26 changes: 26 additions & 0 deletions src/com/android/messaging/datamodel/DatabaseHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
import java.io.File;
import java.util.ArrayList;
import java.util.List;
import java.util.concurrent.ThreadLocalRandom;

/**
* TODO: Open Issues:
Expand Down Expand Up @@ -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";

Expand Down Expand Up @@ -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);
}
Expand All @@ -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
Expand Down
Loading
Loading