Ensure that pinned_order is unique.

This commit is contained in:
Greyson Parrelli
2025-02-21 16:07:10 -05:00
committed by GitHub
parent 46e303ffca
commit 464ffbabdb
8 changed files with 223 additions and 33 deletions

View File

@@ -22,7 +22,7 @@ fun ThreadTable.getThreadsForBackup(db: SignalDatabase, includeImageWallpapers:
SELECT
${ThreadTable.TABLE_NAME}.${ThreadTable.ID},
${ThreadTable.RECIPIENT_ID},
${ThreadTable.PINNED},
${ThreadTable.PINNED_ORDER},
${ThreadTable.READ},
${ThreadTable.ARCHIVED},
${RecipientTable.TABLE_NAME}.${RecipientTable.MESSAGE_EXPIRATION_TIME},

View File

@@ -10,6 +10,7 @@ import org.signal.core.util.decodeOrNull
import org.signal.core.util.requireBlob
import org.signal.core.util.requireBoolean
import org.signal.core.util.requireInt
import org.signal.core.util.requireIntOrNull
import org.signal.core.util.requireLong
import org.thoughtcrime.securesms.backup.v2.proto.Chat
import org.thoughtcrime.securesms.backup.v2.util.ChatStyleConverter
@@ -54,7 +55,7 @@ class ChatArchiveExporter(private val cursor: Cursor, private val db: SignalData
id = cursor.requireLong(ThreadTable.ID),
recipientId = cursor.requireLong(ThreadTable.RECIPIENT_ID),
archived = cursor.requireBoolean(ThreadTable.ARCHIVED),
pinnedOrder = cursor.requireInt(ThreadTable.PINNED).takeIf { it > 0 },
pinnedOrder = cursor.requireIntOrNull(ThreadTable.PINNED_ORDER),
expirationTimerMs = cursor.requireLong(RecipientTable.MESSAGE_EXPIRATION_TIME).seconds.inWholeMilliseconds.takeIf { it > 0 },
expireTimerVersion = cursor.requireInt(RecipientTable.MESSAGE_EXPIRATION_TIME_VERSION),
muteUntilMs = cursor.requireLong(RecipientTable.MUTE_UNTIL).takeIf { it > 0 },

View File

@@ -43,7 +43,7 @@ object ChatArchiveImporter {
.insertInto(ThreadTable.TABLE_NAME)
.values(
ThreadTable.RECIPIENT_ID to recipientId.serialize(),
ThreadTable.PINNED to (chat.pinnedOrder ?: 0),
ThreadTable.PINNED_ORDER to chat.pinnedOrder,
ThreadTable.ARCHIVED to chat.archived.toInt(),
ThreadTable.READ to if (chat.markedUnread) ThreadTable.ReadStatus.FORCED_UNREAD.serialize() else ThreadTable.ReadStatus.READ.serialize(),
ThreadTable.ACTIVE to 1

View File

@@ -20,6 +20,7 @@ import org.signal.core.util.or
import org.signal.core.util.readToList
import org.signal.core.util.readToSingleBoolean
import org.signal.core.util.readToSingleInt
import org.signal.core.util.readToSingleIntOrNull
import org.signal.core.util.readToSingleLong
import org.signal.core.util.requireBoolean
import org.signal.core.util.requireInt
@@ -114,7 +115,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
const val LAST_SEEN = "last_seen"
const val HAS_SENT = "has_sent"
const val LAST_SCROLLED = "last_scrolled"
const val PINNED = "pinned"
const val PINNED_ORDER = "pinned_order"
const val UNREAD_SELF_MENTION_COUNT = "unread_self_mention_count"
const val ACTIVE = "active"
@@ -144,7 +145,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
$LAST_SEEN INTEGER DEFAULT 0,
$HAS_SENT INTEGER DEFAULT 0,
$LAST_SCROLLED INTEGER DEFAULT 0,
$PINNED INTEGER DEFAULT 0,
$PINNED_ORDER INTEGER UNIQUE DEFAULT NULL,
$UNREAD_SELF_MENTION_COUNT INTEGER DEFAULT 0,
$ACTIVE INTEGER DEFAULT 0,
$SNIPPET_MESSAGE_EXTRAS BLOB DEFAULT NULL
@@ -154,8 +155,8 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
@JvmField
val CREATE_INDEXS = arrayOf(
"CREATE INDEX IF NOT EXISTS thread_recipient_id_index ON $TABLE_NAME ($RECIPIENT_ID, $ACTIVE);",
"CREATE INDEX IF NOT EXISTS archived_count_index ON $TABLE_NAME ($ACTIVE, $ARCHIVED, $MEANINGFUL_MESSAGES, $PINNED);",
"CREATE INDEX IF NOT EXISTS thread_pinned_index ON $TABLE_NAME ($PINNED);",
"CREATE INDEX IF NOT EXISTS archived_count_index ON $TABLE_NAME ($ACTIVE, $ARCHIVED, $MEANINGFUL_MESSAGES, $PINNED_ORDER);",
"CREATE INDEX IF NOT EXISTS thread_pinned_index ON $TABLE_NAME ($PINNED_ORDER);",
"CREATE INDEX IF NOT EXISTS thread_read ON $TABLE_NAME ($READ);",
"CREATE INDEX IF NOT EXISTS thread_active ON $TABLE_NAME ($ACTIVE);"
)
@@ -182,7 +183,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
LAST_SEEN,
HAS_READ_RECEIPT,
LAST_SCROLLED,
PINNED,
PINNED_ORDER,
UNREAD_SELF_MENTION_COUNT
)
@@ -936,7 +937,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
for (threadId in threadIds) {
val values = ContentValues().apply {
if (archive) {
put(PINNED, "0")
put(PINNED_ORDER, null as Int?)
put(ARCHIVED, "1")
} else {
put(ARCHIVED, "0")
@@ -992,13 +993,13 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
val folderQuery = chatFolder.toQuery()
val filterQuery = conversationFilter.toQuery()
val where = if (pinned) {
"$ARCHIVED = 0 AND $PINNED != 0 $filterQuery $folderQuery"
"$ARCHIVED = 0 AND $PINNED_ORDER NOT NULL $filterQuery $folderQuery"
} else {
"$ARCHIVED = 0 AND $PINNED = 0 AND $MEANINGFUL_MESSAGES != 0 $filterQuery $folderQuery"
"$ARCHIVED = 0 AND $PINNED_ORDER IS NULL AND $MEANINGFUL_MESSAGES != 0 $filterQuery $folderQuery"
}
val query = if (pinned) {
createQuery(where, PINNED + " ASC", offset, limit)
createQuery(where, PINNED_ORDER + " ASC", offset, limit)
} else {
createQuery(where, offset, limit, preferPinned = false)
}
@@ -1029,7 +1030,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
readableDatabase
.select("COUNT(*)")
.from(TABLE_NAME)
.where("$ACTIVE = 1 AND $ARCHIVED = 0 AND $PINNED != 0 $filterQuery")
.where("$ACTIVE = 1 AND $ARCHIVED = 0 AND $PINNED_ORDER NOT NULL $filterQuery")
.run()
.readToSingleInt(0)
} else {
@@ -1042,7 +1043,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
WHERE
$ACTIVE = 1 AND
$ARCHIVED = 0 AND
$PINNED != 0
$PINNED_ORDER NOT NULL
$filterQuery
$folderQuery
"""
@@ -1057,7 +1058,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
readableDatabase
.select("COUNT(*)")
.from(TABLE_NAME)
.where("$ACTIVE = 1 AND $ARCHIVED = 0 AND ($MEANINGFUL_MESSAGES != 0 OR $PINNED != 0) $filterQuery")
.where("$ACTIVE = 1 AND $ARCHIVED = 0 AND ($MEANINGFUL_MESSAGES != 0 OR $PINNED_ORDER NOT NULL) $filterQuery")
.run()
.readToSingleInt(0)
} else {
@@ -1071,7 +1072,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
WHERE
$ACTIVE = 1 AND
$ARCHIVED = 0 AND
($MEANINGFUL_MESSAGES != 0 OR $PINNED != 0)
($MEANINGFUL_MESSAGES != 0 OR $PINNED_ORDER NOT NULL)
$filterQuery
$folderQuery
"""
@@ -1129,7 +1130,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
return readableDatabase
.select(ID, RECIPIENT_ID)
.from(TABLE_NAME)
.where("$PINNED > 0")
.where("$PINNED_ORDER NOT NULL")
.run()
.readToList { cursor ->
RecipientId.from(cursor.requireLong(RECIPIENT_ID))
@@ -1143,7 +1144,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
return readableDatabase
.select(ID)
.from(TABLE_NAME)
.where("$PINNED > 0")
.where("$PINNED_ORDER NOT NULL")
.run()
.readToList { cursor ->
cursor.requireLong(ID)
@@ -1164,19 +1165,24 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
writableDatabase.withinTransaction { db ->
if (clearFirst) {
db.update(TABLE_NAME)
.values(PINNED to 0)
.where("$PINNED > 0")
.values(PINNED_ORDER to null)
.where("$PINNED_ORDER NOT NULL")
.run()
}
var pinnedCount = getPinnedConversationListCount(ConversationFilter.OFF)
val maxPinnedOrder = db.select("MAX($PINNED_ORDER)")
.from(TABLE_NAME)
.run()
.readToSingleIntOrNull() ?: 0
var pinnedOrder = maxPinnedOrder + 1
for (threadId in threadIds) {
pinnedCount++
db.update(TABLE_NAME)
.values(PINNED to pinnedCount, ACTIVE to 1)
.values(PINNED_ORDER to pinnedOrder, ACTIVE to 1)
.where("$ID = ?", threadId)
.run()
pinnedOrder++
}
}
@@ -1189,13 +1195,13 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
writableDatabase.withinTransaction { db ->
val query: SqlUtil.Query = SqlUtil.buildSingleCollectionQuery(ID, threadIds)
db.update(TABLE_NAME)
.values(PINNED to 0)
.values(PINNED_ORDER to null)
.where(query.where, *query.whereArgs)
.run()
getPinnedThreadIds().forEachIndexed { index: Int, threadId: Long ->
db.update(TABLE_NAME)
.values(PINNED to index + 1)
.values(PINNED_ORDER to index + 1)
.where("$ID = ?", threadId)
.run()
}
@@ -1550,7 +1556,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
applyStorageSyncUpdate(recipientId, record.proto.noteToSelfArchived, record.proto.noteToSelfMarkedUnread)
db.updateAll(TABLE_NAME)
.values(PINNED to 0)
.values(PINNED_ORDER to null)
.run()
var pinnedPosition = 1
@@ -1584,7 +1590,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
if (pinnedRecipient != null) {
db.update(TABLE_NAME)
.values(PINNED to pinnedPosition, ACTIVE to 1)
.values(PINNED_ORDER to pinnedPosition, ACTIVE to 1)
.where("$RECIPIENT_ID = ?", pinnedRecipient.id)
.run()
}
@@ -1958,7 +1964,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
LAST_SEEN to 0,
HAS_SENT to 0,
LAST_SCROLLED to 0,
PINNED to 0,
PINNED_ORDER to null,
UNREAD_SELF_MENTION_COUNT to 0,
ACTIVE to 0
)
@@ -2074,7 +2080,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
private fun createQuery(where: String, offset: Long, limit: Long, preferPinned: Boolean): String {
val orderBy = if (preferPinned) {
"$TABLE_NAME.$PINNED DESC, $TABLE_NAME.$DATE DESC"
"$TABLE_NAME.$PINNED_ORDER DESC, $TABLE_NAME.$DATE DESC"
} else {
"$TABLE_NAME.$DATE DESC"
}
@@ -2258,7 +2264,7 @@ class ThreadTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTa
.setMeaningfulMessages(cursor.requireLong(MEANINGFUL_MESSAGES) > 0)
.setUnreadCount(cursor.requireInt(UNREAD_COUNT))
.setForcedUnread(cursor.requireInt(READ) == ReadStatus.FORCED_UNREAD.serialize())
.setPinned(cursor.requireBoolean(PINNED))
.setPinned(cursor.requireBoolean(PINNED_ORDER))
.setUnreadSelfMentionsCount(cursor.requireInt(UNREAD_SELF_MENTION_COUNT))
.setExtra(extra)
.setSnippetMessageExtras(messageExtras)

View File

@@ -121,6 +121,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V261_RemapCallRinge
import org.thoughtcrime.securesms.database.helpers.migration.V263_InAppPaymentsSubscriberTableRebuild
import org.thoughtcrime.securesms.database.helpers.migration.V264_FixGroupAddMemberUpdate
import org.thoughtcrime.securesms.database.helpers.migration.V265_FixFtsTriggers
import org.thoughtcrime.securesms.database.helpers.migration.V266_UniqueThreadPinOrder
/**
* Contains all of the database migrations for [SignalDatabase]. Broken into a separate file for cleanliness.
@@ -245,10 +246,11 @@ object SignalDatabaseMigrations {
// V263 was originally V262, but a typo in the version mapping caused it not to be run.
263 to V263_InAppPaymentsSubscriberTableRebuild,
264 to V264_FixGroupAddMemberUpdate,
265 to V265_FixFtsTriggers
265 to V265_FixFtsTriggers,
266 to V266_UniqueThreadPinOrder
)
const val DATABASE_VERSION = 265
const val DATABASE_VERSION = 266
@JvmStatic
fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {

View File

@@ -0,0 +1,161 @@
/*
* Copyright 2025 Signal Messenger, LLC
* SPDX-License-Identifier: AGPL-3.0-only
*/
package org.thoughtcrime.securesms.database.helpers.migration
import android.app.Application
import androidx.core.content.contentValuesOf
import net.zetetic.database.sqlcipher.SQLiteDatabase
import org.signal.core.util.Stopwatch
import org.signal.core.util.logging.Log
import org.signal.core.util.readToList
import org.signal.core.util.requireInt
import org.signal.core.util.requireLong
/**
* Somehow we have a bug where pinned orders aren't always unique. Could be from some old bug, not clear.
* Regardless, we'll add some guarantees in the schema itself.
*
* We want to add a unique constraint on pinned order. To do that, we need to move to using NULL instead of 0
* for the unset state. That includes changing the default value, which requires a table copy.
*
* While we're at it, we'll also change the column to pinned_order, since it's an order and not a boolean.
*/
@Suppress("ClassName")
object V266_UniqueThreadPinOrder : SignalDatabaseMigration {
private val TAG = Log.tag(V266_UniqueThreadPinOrder::class)
override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
val stopwatch = Stopwatch("migration")
// 1. Convert all 0's to nulls
db.execSQL(
"""
UPDATE thread
SET pinned = null
WHERE pinned = 0
"""
)
stopwatch.split("nulls")
// 2. Correct any duplicate pinned columns
val pinnedThreads = db.rawQuery(
"""
SELECT _id, pinned, last_seen
FROM thread
WHERE pinned NOT NULL
ORDER BY pinned DESC
"""
).readToList { cursor ->
ThreadPinnedData(
id = cursor.requireLong("_id"),
pinned = cursor.requireInt("pinned"),
lastSeen = cursor.requireLong("last_seen")
)
}
if (pinnedThreads.isNotEmpty()) {
if (pinnedThreads.distinctBy { it.pinned }.size != pinnedThreads.size) {
Log.w(TAG, "There's a duplicate pinned value! Correcting.")
pinnedThreads
.sortedBy { it.lastSeen }
.sortedBy { it.pinned }
.mapIndexed { i, thread -> thread.copy(pinned = i + 1) }
.forEach { thread ->
val values = contentValuesOf("pinned" to thread.pinned)
db.update("thread", values, "_id = ${thread.id}", null)
}
}
}
stopwatch.split("fix-dupes")
// 3. Create the new schema and copy everything over
db.execSQL(
"""
CREATE TABLE thread_tmp (
_id INTEGER PRIMARY KEY AUTOINCREMENT,
date INTEGER DEFAULT 0,
meaningful_messages INTEGER DEFAULT 0,
recipient_id INTEGER NOT NULL UNIQUE REFERENCES recipient (_id) ON DELETE CASCADE,
read INTEGER DEFAULT 1,
type INTEGER DEFAULT 0,
error INTEGER DEFAULT 0,
snippet TEXT,
snippet_type INTEGER DEFAULT 0,
snippet_uri TEXT DEFAULT NULL,
snippet_content_type TEXT DEFAULT NULL,
snippet_extras TEXT DEFAULT NULL,
unread_count INTEGER DEFAULT 0,
archived INTEGER DEFAULT 0,
status INTEGER DEFAULT 0,
has_delivery_receipt INTEGER DEFAULT 0,
has_read_receipt INTEGER DEFAULT 0,
expires_in INTEGER DEFAULT 0,
last_seen INTEGER DEFAULT 0,
has_sent INTEGER DEFAULT 0,
last_scrolled INTEGER DEFAULT 0,
pinned_order INTEGER UNIQUE DEFAULT NULL,
unread_self_mention_count INTEGER DEFAULT 0,
active INTEGER DEFAULT 0,
snippet_message_extras BLOB DEFAULT NULL
)
"""
)
stopwatch.split("table-create")
db.execSQL(
"""
INSERT INTO thread_tmp
SELECT
_id,
date,
meaningful_messages,
recipient_id,
read,
type,
error,
snippet,
snippet_type,
snippet_uri,
snippet_content_type,
snippet_extras,
unread_count,
archived,
status,
has_delivery_receipt,
has_read_receipt,
expires_in,
last_seen,
has_sent,
last_scrolled,
pinned,
unread_self_mention_count,
active,
snippet_message_extras
FROM thread
"""
)
stopwatch.split("table-copy")
db.execSQL("DROP TABLE thread")
db.execSQL("ALTER TABLE thread_tmp RENAME TO thread")
stopwatch.split("replace")
db.execSQL("CREATE INDEX IF NOT EXISTS thread_recipient_id_index ON thread (recipient_id, active);")
db.execSQL("CREATE INDEX IF NOT EXISTS archived_count_index ON thread (active, archived, meaningful_messages, pinned_order);")
db.execSQL("CREATE INDEX IF NOT EXISTS thread_pinned_index ON thread (pinned_order);")
db.execSQL("CREATE INDEX IF NOT EXISTS thread_read ON thread (read);")
db.execSQL("CREATE INDEX IF NOT EXISTS thread_active ON thread (active);")
stopwatch.split("indexes")
stopwatch.stop(TAG)
}
data class ThreadPinnedData(
val id: Long,
val pinned: Int,
val lastSeen: Long
)
}

View File

@@ -171,9 +171,10 @@ public class ApplicationMigrations {
// static final int FIX_INACTIVE_GROUPS = 127;
static final int DUPLICATE_E164_FIX = 128;
static final int FTS_TRIGGER_FIX = 129;
static final int THREAD_TABLE_PINNED_MIGRATION = 130;
}
public static final int CURRENT_VERSION = 129;
public static final int CURRENT_VERSION = 130;
/**
* This *must* be called after the {@link JobManager} has been instantiated, but *before* the call
@@ -788,6 +789,10 @@ public class ApplicationMigrations {
jobs.put(Version.FTS_TRIGGER_FIX, new DatabaseMigrationJob());
}
if (lastSeenVersion < Version.THREAD_TABLE_PINNED_MIGRATION) {
jobs.put(Version.THREAD_TABLE_PINNED_MIGRATION, new DatabaseMigrationJob());
}
return jobs;
}

View File

@@ -1,6 +1,7 @@
package org.signal.core.util
import android.database.Cursor
import androidx.core.database.getIntOrNull
import androidx.core.database.getLongOrNull
import androidx.core.database.getStringOrNull
import java.util.Optional
@@ -21,6 +22,10 @@ fun Cursor.requireInt(column: String): Int {
return CursorUtil.requireInt(this, column)
}
fun Cursor.requireIntOrNull(column: String): Int? {
return this.getIntOrNull(this.getColumnIndexOrThrow(column))
}
fun Cursor.optionalInt(column: String): Optional<Int> {
return CursorUtil.getInt(this, column)
}
@@ -123,6 +128,16 @@ fun Cursor.readToSingleInt(defaultValue: Int = 0): Int {
}
}
fun Cursor.readToSingleIntOrNull(): Int? {
return use {
if (it.moveToFirst()) {
it.getIntOrNull(0)
} else {
null
}
}
}
fun Cursor.readToSingleBoolean(defaultValue: Boolean = false): Boolean {
return use {
if (it.moveToFirst()) {