empathicqubit/Signal-Android
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

Add some more missing indexes for foreign keys and create test.

Browse source
This commit is contained in:
Greyson Parrelli 2023-05-03 14:46:57 -04:00
parent 5e86cca277
commit 63ce2de3bf
9 changed files with 122 additions and 34 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

31
app/src/androidTest/java/org/thoughtcrime/securesms/database/DatabaseConsistencyTest.kt
View file

@ -8,6 +8,10 @@ import net.zetetic.database.sqlcipher.SQLiteOpenHelper
import org.junit.Rule
import org.junit.Test
import org.junit.runner.RunWith
import org.signal.core.util.ForeignKeyConstraint
import org.signal.core.util.Index
import org.signal.core.util.getForeignKeys
import org.signal.core.util.getIndexes
import org.signal.core.util.readToList
import org.signal.core.util.requireNonNullString
import org.thoughtcrime.securesms.database.helpers.SignalDatabaseMigrations
@ -24,7 +28,7 @@ class DatabaseConsistencyTest {
val harness = SignalActivityRule()
@Test
fun test() {
fun testUpgradeConsistency() {
val currentVersionStatements = SignalDatabase.rawDatabase.getAllCreateStatements()
val testHelper = InMemoryTestHelper(ApplicationDependencies.getApplication()).also {
it.onUpgrade(it.writableDatabase, 181, SignalDatabaseMigrations.DATABASE_VERSION)
@ -61,6 +65,30 @@ class DatabaseConsistencyTest {
}
}
@Test
fun testForeignKeyIndexCoverage() {
/** We may deem certain indexes non-critical if deletion frequency is low or table size is small. */
val ignoredColumns: List<Pair<String, String>> = listOf(
StorySendTable.TABLE_NAME to StorySendTable.DISTRIBUTION_ID
)
val foreignKeys: List<ForeignKeyConstraint> = SignalDatabase.rawDatabase.getForeignKeys()
val indexesByFirstColumn: List<Index> = SignalDatabase.rawDatabase.getIndexes()
val notFound: List<Pair<String, String>> = foreignKeys
.filterNot { ignoredColumns.contains(it.table to it.column) }
.filterNot { foreignKey ->
indexesByFirstColumn.hasPrimaryIndexFor(foreignKey.table, foreignKey.column)
}
.map { it.table to it.column }
assertTrue("Missing indexes to cover: $notFound", notFound.isEmpty())
}
private fun List<Index>.hasPrimaryIndexFor(table: String, column: String): Boolean {
return this.any { index -> index.table == table && index.columns[0] == column }
}
private data class Statement(
val name: String,
val sql: String
@ -74,6 +102,7 @@ class DatabaseConsistencyTest {
sql = cursor.requireNonNullString("sql").normalizeSql()
)
}
.filterNot { it.name.startsWith("sqlite_stat") }
.sortedBy { it.name }
}

4
app/src/main/java/org/thoughtcrime/securesms/database/CallTable.kt
View file

@ -81,7 +81,9 @@ class CallTable(context: Context, databaseHelper: SignalDatabase) : DatabaseTabl
val CREATE_INDEXES = arrayOf(
"CREATE INDEX call_call_id_index ON $TABLE_NAME ($CALL_ID)",
"CREATE INDEX call_message_id_index ON $TABLE_NAME ($MESSAGE_ID)"
"CREATE INDEX call_message_id_index ON $TABLE_NAME ($MESSAGE_ID)",
"CREATE INDEX call_call_link_index ON $TABLE_NAME ($CALL_LINK)",
"CREATE INDEX call_peer_index ON $TABLE_NAME ($PEER)"
)
}

7
app/src/main/java/org/thoughtcrime/securesms/database/DistributionListTables.kt
View file

@ -41,7 +41,7 @@ class DistributionListTables constructor(context: Context?, databaseHelper: Sign
val CREATE_TABLE: Array<String> = arrayOf(ListTable.CREATE_TABLE, MembershipTable.CREATE_TABLE)
@JvmField
val CREATE_INDEXES: Array<String> = arrayOf(MembershipTable.CREATE_INDEX)
val CREATE_INDEXES: Array<String> = MembershipTable.CREATE_INDEXES
const val RECIPIENT_ID = ListTable.RECIPIENT_ID
const val DISTRIBUTION_ID = ListTable.DISTRIBUTION_ID
@ -123,7 +123,10 @@ class DistributionListTables constructor(context: Context?, databaseHelper: Sign
)
"""
const val CREATE_INDEX = "CREATE UNIQUE INDEX distribution_list_member_list_id_recipient_id_privacy_mode_index ON $TABLE_NAME ($LIST_ID, $RECIPIENT_ID, $PRIVACY_MODE)"
val CREATE_INDEXES = arrayOf(
"CREATE UNIQUE INDEX distribution_list_member_list_id_recipient_id_privacy_mode_index ON $TABLE_NAME ($LIST_ID, $RECIPIENT_ID, $PRIVACY_MODE)",
"CREATE INDEX distribution_list_member_recipient_id ON $TABLE_NAME ($RECIPIENT_ID)"
)
}
/**

3
app/src/main/java/org/thoughtcrime/securesms/database/MessageSendLogTables.kt
View file

@ -143,7 +143,8 @@ class MessageSendLogTables constructor(context: Context?, databaseHelper: Signal
/** Created for [MslPayloadTable.CREATE_TRIGGERS] and [deleteAllRelatedToMessage] */
val CREATE_INDEXES = arrayOf(
"CREATE INDEX msl_message_message_index ON $TABLE_NAME ($MESSAGE_ID, $PAYLOAD_ID)"
"CREATE INDEX msl_message_message_index ON $TABLE_NAME ($MESSAGE_ID, $PAYLOAD_ID)",
"CREATE INDEX msl_message_payload_index ON $TABLE_NAME ($PAYLOAD_ID)"
)
}

7
app/src/main/java/org/thoughtcrime/securesms/database/helpers/SignalDatabaseMigrations.kt
View file

@ -42,6 +42,7 @@ import org.thoughtcrime.securesms.database.helpers.migration.V183_CallLinkTableM
import org.thoughtcrime.securesms.database.helpers.migration.V184_CallLinkReplaceIndexMigration
import org.thoughtcrime.securesms.database.helpers.migration.V185_MessageRecipientsAndEditMessageMigration
import org.thoughtcrime.securesms.database.helpers.migration.V186_ForeignKeyIndicesMigration
import org.thoughtcrime.securesms.database.helpers.migration.V187_MoreForeignKeyIndexesMigration
/**
* Contains all of the database migrations for [SignalDatabase]. Broken into a separate file for cleanliness.
@ -50,7 +51,7 @@ object SignalDatabaseMigrations {
val TAG: String = Log.tag(SignalDatabaseMigrations.javaClass)
const val DATABASE_VERSION = 186
const val DATABASE_VERSION = 187
@JvmStatic
fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
@ -205,6 +206,10 @@ object SignalDatabaseMigrations {
if (oldVersion < 186) {
V186_ForeignKeyIndicesMigration.migrate(context, db, oldVersion, newVersion)
}
if (oldVersion < 187) {
V187_MoreForeignKeyIndexesMigration.migrate(context, db, oldVersion, newVersion)
}
}
@JvmStatic

32
app/src/main/java/org/thoughtcrime/securesms/database/helpers/migration/V187_MoreForeignKeyIndexesMigration.kt Normal file
View file

@ -0,0 +1,32 @@
package org.thoughtcrime.securesms.database.helpers.migration
import android.app.Application
import net.zetetic.database.sqlcipher.SQLiteDatabase
import org.signal.core.util.Stopwatch
import org.signal.core.util.logging.Log
/**
* I found some other tables that didn't have the proper indexes setup to correspond with their foreign keys.
*/
object V187_MoreForeignKeyIndexesMigration : SignalDatabaseMigration {
private val TAG = Log.tag(V187_MoreForeignKeyIndexesMigration::class.java)
override fun migrate(context: Application, db: SQLiteDatabase, oldVersion: Int, newVersion: Int) {
val stopwatch = Stopwatch("migration")
db.execSQL("CREATE INDEX IF NOT EXISTS call_call_link_index ON call (call_link)")
stopwatch.split("call_link")
db.execSQL("CREATE INDEX IF NOT EXISTS call_peer_index ON call (peer)")
stopwatch.split("call_peer")
db.execSQL("CREATE INDEX IF NOT EXISTS distribution_list_member_recipient_id ON distribution_list_member (recipient_id)")
stopwatch.split("dlist_member")
db.execSQL("CREATE INDEX IF NOT EXISTS msl_message_payload_index ON msl_message (payload_id)")
stopwatch.split("msl_payload")
stopwatch.stop(TAG)
}
}

42
core-util/src/main/java/org/signal/core/util/SQLiteDatabaseExtensions.kt
View file

@ -35,6 +35,34 @@ fun SupportSQLiteDatabase.getTableRowCount(table: String): Int {
}
}
fun SupportSQLiteDatabase.getForeignKeys(): List<ForeignKeyConstraint> {
return SqlUtil.getAllTables(this)
.map { table ->
this.query("PRAGMA foreign_key_list($table)").readToList { cursor ->
ForeignKeyConstraint(
table = table,
column = cursor.requireNonNullString("from"),
dependsOnTable = cursor.requireNonNullString("table"),
dependsOnColumn = cursor.requireNonNullString("to"),
onDelete = cursor.requireString("on_delete") ?: "NOTHING"
)
}
}
.flatten()
}
fun SupportSQLiteDatabase.getIndexes(): List<Index> {
return this.query("SELECT name, tbl_name FROM sqlite_master WHERE type='index' ORDER BY name ASC").readToList { cursor ->
val indexName = cursor.requireNonNullString("name")
Index(
name = indexName,
table = cursor.requireNonNullString("tbl_name"),
columns = this.query("PRAGMA index_info($indexName)").readToList { it.requireNonNullString("name") }
)
}
}
/**
* Checks if a row exists that matches the query.
*/
@ -345,3 +373,17 @@ class InsertBuilderPart2(
return db.insert(tableName, conflictStrategy, values)
}
}
data class ForeignKeyConstraint(
val table: String,
val column: String,
val dependsOnTable: String,
val dependsOnColumn: String,
val onDelete: String
)
data class Index(
val name: String,
val table: String,
val columns: List<String>
)

28
spinner/lib/src/main/java/org/signal/spinner/DatabaseUtil.kt
View file

@ -2,10 +2,6 @@ package org.signal.spinner
import android.database.Cursor
import androidx.sqlite.db.SupportSQLiteDatabase
import org.signal.core.util.SqlUtil
import org.signal.core.util.readToList
import org.signal.core.util.requireNonNullString
import org.signal.core.util.requireString
fun SupportSQLiteDatabase.getTableNames(): List<String> {
val out = mutableListOf<String>()
@ -30,22 +26,6 @@ fun SupportSQLiteDatabase.getTriggers(): Cursor {
return this.query("SELECT * FROM sqlite_master WHERE type='trigger' ORDER BY name ASC")
}
fun SupportSQLiteDatabase.getForeignKeys(): List<ForeignKeyConstraint> {
return SqlUtil.getAllTables(this)
.map { table ->
this.query("PRAGMA foreign_key_list($table)").readToList { cursor ->
ForeignKeyConstraint(
table = table,
column = cursor.requireNonNullString("from"),
dependsOnTable = cursor.requireNonNullString("table"),
dependsOnColumn = cursor.requireNonNullString("to"),
onDelete = cursor.requireString("on_delete") ?: "NOTHING"
)
}
}
.flatten()
}
fun SupportSQLiteDatabase.getTableRowCount(table: String): Int {
return this.query("SELECT COUNT(*) FROM $table").use {
if (it.moveToFirst()) {
@ -55,11 +35,3 @@ fun SupportSQLiteDatabase.getTableRowCount(table: String): Int {
}
}
}
data class ForeignKeyConstraint(
val table: String,
val column: String,
val dependsOnTable: String,
val dependsOnColumn: String,
val onDelete: String
)

2
spinner/lib/src/main/java/org/signal/spinner/SpinnerServer.kt
View file

@ -8,6 +8,8 @@ import com.github.jknack.handlebars.Template
import com.github.jknack.handlebars.helper.ConditionalHelpers
import fi.iki.elonen.NanoHTTPD
import org.signal.core.util.ExceptionUtil
import org.signal.core.util.ForeignKeyConstraint
import org.signal.core.util.getForeignKeys
import org.signal.core.util.logging.Log
import org.signal.spinner.Spinner.DatabaseConfig
import java.lang.IllegalArgumentException