From bd11ed9f175425219605cb8cb58a8866d1a9fb9d Mon Sep 17 00:00:00 2001 From: Greyson Parrelli Date: Mon, 12 Dec 2022 11:54:05 -0500 Subject: [PATCH] Fix table drop order during backup import. Fixes #12671 --- .../securesms/backup/FullBackupImporter.java | 89 +++++++--- .../backup/FullBackupImporterTest.kt | 162 ++++++++++++++++++ .../main/java/org/signal/core/util/SqlUtil.kt | 11 ++ 3 files changed, 239 insertions(+), 23 deletions(-) create mode 100644 app/src/test/java/org/thoughtcrime/securesms/backup/FullBackupImporterTest.kt diff --git a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupImporter.java b/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupImporter.java index 3d31374a0a0..1a5c22889a2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupImporter.java +++ b/app/src/main/java/org/thoughtcrime/securesms/backup/FullBackupImporter.java @@ -5,11 +5,11 @@ import android.content.ContentValues; import android.content.Context; import android.content.SharedPreferences; -import android.database.Cursor; import android.net.Uri; import android.util.Pair; import androidx.annotation.NonNull; +import androidx.annotation.VisibleForTesting; import net.zetetic.database.sqlcipher.SQLiteDatabase; @@ -42,29 +42,24 @@ import java.io.IOException; import java.io.InputStream; import java.io.OutputStream; +import java.util.ArrayList; import java.util.Collections; import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.LinkedHashSet; import java.util.LinkedList; import java.util.List; +import java.util.Map; import java.util.Objects; +import java.util.Queue; +import java.util.Set; +import java.util.stream.Collectors; public class FullBackupImporter extends FullBackupBase { @SuppressWarnings("unused") private static final String TAG = Log.tag(FullBackupImporter.class); - private static final String[] TABLES_TO_DROP_FIRST = { - "distribution_list_member", - "distribution_list", - "message_send_log_recipients", - "msl_recipient", - "msl_message", - "reaction", - "notification_profile_schedule", - "notification_profile_allowed_members", - "story_sends" - }; - public static void importFile(@NonNull Context context, @NonNull AttachmentSecret attachmentSecret, @NonNull SQLiteDatabase db, @NonNull Uri uri, @NonNull String passphrase) throws IOException @@ -268,21 +263,69 @@ private static void processPreference(@NonNull Context context, SharedPreference } private static void dropAllTables(@NonNull SQLiteDatabase db) { - for (String name : TABLES_TO_DROP_FIRST) { - db.execSQL("DROP TABLE IF EXISTS " + name); + for (String trigger : SqlUtil.getAllTriggers(db)) { + Log.i(TAG, "Dropping trigger: " + trigger); + db.execSQL("DROP TRIGGER IF EXISTS " + trigger); + } + for (String table : getTablesToDropInOrder(db)) { + Log.i(TAG, "Dropping table: " + table); + db.execSQL("DROP TABLE IF EXISTS " + table); + } + } + + /** + * Returns the list of tables we should drop, in the order they should be dropped in. + * The order is chosen to ensure we won't violate any foreign key constraints when we import them. + */ + private static List getTablesToDropInOrder(@NonNull SQLiteDatabase input) { + List tables = SqlUtil.getAllTables(input) + .stream() + .filter(table -> !table.startsWith("sqlite_")) + .sorted() + .collect(Collectors.toList()); + + + Map> dependsOn = new LinkedHashMap<>(); + for (String table : tables) { + dependsOn.put(table, SqlUtil.getForeignKeyDependencies(input, table)); + } + + for (String table : tables) { + Set dependsOnTable = dependsOn.keySet().stream().filter(t -> dependsOn.get(t).contains(table)).collect(Collectors.toSet()); + Log.i(TAG, "Tables that depend on " + table + ": " + dependsOnTable); } - try (Cursor cursor = db.rawQuery("SELECT name, type FROM sqlite_master", null)) { - while (cursor != null && cursor.moveToNext()) { - String name = cursor.getString(0); - String type = cursor.getString(1); + return computeTableDropOrder(dependsOn); + } + + @VisibleForTesting + static List computeTableDropOrder(@NonNull Map> dependsOn) { + List rootNodes = dependsOn.keySet() + .stream() + .filter(table -> { + boolean nothingDependsOnIt = dependsOn.values().stream().noneMatch(it -> it.contains(table)); + return nothingDependsOnIt; + }) + .sorted() + .collect(Collectors.toList()); + + LinkedHashSet dropOrder = new LinkedHashSet<>(); + + Queue processOrder = new LinkedList<>(rootNodes); - if ("table".equals(type) && !name.startsWith("sqlite_")) { - Log.i(TAG, "Dropping table: " + name); - db.execSQL("DROP TABLE IF EXISTS " + name); - } + while (!processOrder.isEmpty()) { + String head = processOrder.remove(); + + dropOrder.remove(head); + dropOrder.add(head); + + Set dependencies = dependsOn.get(head); + if (dependencies != null) { + processOrder.addAll(dependencies); } } + + return new ArrayList<>(dropOrder); } public static class DatabaseDowngradeException extends IOException { diff --git a/app/src/test/java/org/thoughtcrime/securesms/backup/FullBackupImporterTest.kt b/app/src/test/java/org/thoughtcrime/securesms/backup/FullBackupImporterTest.kt new file mode 100644 index 00000000000..e439dc65834 --- /dev/null +++ b/app/src/test/java/org/thoughtcrime/securesms/backup/FullBackupImporterTest.kt @@ -0,0 +1,162 @@ +package org.thoughtcrime.securesms.backup + +import org.junit.Assert.assertEquals +import org.junit.Test + +class FullBackupImporterTest { + + @Test + fun `computeTableOrder - empty`() { + val order = FullBackupImporter.computeTableDropOrder(mapOf()) + + assertEquals(listOf(), order) + } + + /** + * A B C + */ + @Test + fun `computeTableOrder - no dependencies`() { + val order = FullBackupImporter.computeTableDropOrder( + mapOf( + "A" to setOf(), + "B" to setOf(), + "C" to setOf(), + ) + ) + + assertEquals(listOf("A", "B", "C"), order) + } + + /** + * A + * | + * B + * | + * C + */ + @Test + fun `computeTableOrder - single chain`() { + val order = FullBackupImporter.computeTableDropOrder( + mapOf( + "A" to setOf("B"), + "B" to setOf("C"), + ) + ) + + assertEquals(listOf("A", "B", "C"), order) + } + + /** + * ┌──A──┐ B C + * ┌─D ┌─E─┐ + * F G H + */ + @Test + fun `computeTableOrder - complex 1`() { + val order = FullBackupImporter.computeTableDropOrder( + mapOf( + "A" to setOf("D", "E"), + "B" to setOf(), + "C" to setOf(), + "D" to setOf("F"), + "E" to setOf("G", "H"), + "F" to setOf(), + "G" to setOf(), + "H" to setOf(), + ) + ) + + assertEquals(listOf("A", "B", "C", "D", "E", "F", "G", "H"), order) + } + + /** + * ┌────A────┐ + * │ | │ + * ┌─B─┐ C ┌─D─┐ + * │ │ | │ │ + * E F G H I + */ + @Test + fun `computeTableOrder - complex 2`() { + val order = FullBackupImporter.computeTableDropOrder( + mapOf( + "A" to setOf("B", "C", "D"), + "B" to setOf("E", "F"), + "C" to setOf("G"), + "D" to setOf("H", "I"), + "E" to setOf(), + "F" to setOf(), + "G" to setOf(), + "H" to setOf(), + "I" to setOf(), + ) + ) + + assertEquals(listOf("A", "B", "C", "D", "E", "F", "G", "H", "I"), order) + } + + /** + * ┌─A─┐ B ┌─C─┐ + * │ │ | │ │ + * D E F G H + */ + @Test + fun `computeTableOrder - multiple roots`() { + val order = FullBackupImporter.computeTableDropOrder( + mapOf( + "A" to setOf("D", "E"), + "B" to setOf("F"), + "C" to setOf("G", "H"), + "D" to setOf(), + "E" to setOf(), + "F" to setOf(), + "G" to setOf(), + "H" to setOf(), + ) + ) + + assertEquals(listOf("A", "B", "C", "D", "E", "F", "G", "H"), order) + } + + /** + * ┌─A─┐ B ┌─C─┐ + * │ │ | │ │ + * D E D D E + */ + @Test + fun `computeTableOrder - multiple roots, dupes across graphs`() { + val order = FullBackupImporter.computeTableDropOrder( + mapOf( + "A" to setOf("D", "E"), + "B" to setOf("D"), + "C" to setOf("D", "E"), + "D" to setOf(), + "E" to setOf(), + ) + ) + + assertEquals(listOf("A", "B", "C", "D", "E"), order) + } + + /** + * A B + * │ │ + * D C + * │ + * D + */ + @Test + fun `computeTableOrder - multiple roots, new dependencies across roots`() { + val order = FullBackupImporter.computeTableDropOrder( + mapOf( + "A" to setOf("D"), + "B" to setOf("C"), + "C" to setOf("D"), + "D" to setOf(), + ) + ) + + assertEquals(listOf("A", "B", "C", "D"), order) + } +} diff --git a/core-util/src/main/java/org/signal/core/util/SqlUtil.kt b/core-util/src/main/java/org/signal/core/util/SqlUtil.kt index b1164f925f2..d4cbd98d965 100644 --- a/core-util/src/main/java/org/signal/core/util/SqlUtil.kt +++ b/core-util/src/main/java/org/signal/core/util/SqlUtil.kt @@ -33,6 +33,17 @@ object SqlUtil { return tables } + @JvmStatic + fun getAllTriggers(db: SupportSQLiteDatabase): List { + val tables: MutableList = LinkedList() + db.query("SELECT name FROM sqlite_master WHERE type=?", arrayOf("trigger")).use { cursor -> + while (cursor.moveToNext()) { + tables.add(cursor.getString(0)) + } + } + return tables + } + /** * Given a table, this will return a set of tables that it has a foreign key dependency on. */