diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java index 1a02e64b3c..fabae7cb13 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/AttachmentDatabase.java @@ -278,7 +278,7 @@ public class AttachmentDatabase extends Database { } SQLiteDatabase database = databaseHelper.getSignalReadableDatabase(); - SqlUtil.Query query = SqlUtil.buildCollectionQuery(MMS_ID, mmsIds); + SqlUtil.Query query = SqlUtil.buildSingleCollectionQuery(MMS_ID, mmsIds); Map> output = new HashMap<>(); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/CdsDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/CdsDatabase.kt index 605957b927..b91c12c795 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/CdsDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/CdsDatabase.kt @@ -68,13 +68,10 @@ class CdsDatabase(context: Context, databaseHelper: SignalDatabase) : Database(c SqlUtil.buildBulkInsert(TABLE_NAME, arrayOf(E164), insertValues) .forEach { writableDatabase.execSQL(it.where, it.whereArgs) } - for (e164 in seenE164s) { - writableDatabase - .update(TABLE_NAME) - .values(LAST_SEEN_AT to lastSeen) - .where("$E164 = ?", e164) - .run() - } + val contentValues = contentValuesOf(LAST_SEEN_AT to lastSeen) + + SqlUtil.buildCollectionQuery(E164, seenE164s) + .forEach { query -> writableDatabase.update(TABLE_NAME, contentValues, query.where, query.whereArgs) } writableDatabase.setTransactionSuccessful() } finally { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/MessageDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/MessageDatabase.java index ebcc41370c..8d4bee04b8 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/MessageDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/MessageDatabase.java @@ -330,7 +330,7 @@ public abstract class MessageDatabase extends Database implements MmsSmsColumns } SQLiteDatabase db = databaseHelper.getSignalWritableDatabase(); - SqlUtil.Query where = SqlUtil.buildCollectionQuery(ID, ids); + SqlUtil.Query where = SqlUtil.buildSingleCollectionQuery(ID, ids); ContentValues values = new ContentValues(); values.put(NOTIFIED_TIMESTAMP, timestamp); diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt index 48c7caf30c..3b328d5c78 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/RecipientDatabase.kt @@ -1378,7 +1378,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : db.beginTransaction() try { - val query = SqlUtil.buildCollectionQuery(ID, ids) + val query = SqlUtil.buildSingleCollectionQuery(ID, ids) val values = ContentValues().apply { put(MUTE_UNTIL, until) } @@ -2518,7 +2518,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : } if (Util.hasItems(idsToUpdate)) { - val query = SqlUtil.buildCollectionQuery(ID, idsToUpdate) + val query = SqlUtil.buildSingleCollectionQuery(ID, idsToUpdate) val values = ContentValues(1).apply { put(PROFILE_SHARING, 1) } @@ -2536,7 +2536,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : return } - var query = SqlUtil.buildCollectionQuery(ID, recipientIds) + var query = SqlUtil.buildSingleCollectionQuery(ID, recipientIds) val db = writableDatabase db.query(TABLE_NAME, arrayOf(ID), "${query.where} AND $GROUPS_IN_COMMON = 0", query.whereArgs, null, null, null).use { cursor -> @@ -2547,7 +2547,7 @@ open class RecipientDatabase(context: Context, databaseHelper: SignalDatabase) : } if (Util.hasItems(idsToUpdate)) { - query = SqlUtil.buildCollectionQuery(ID, idsToUpdate) + query = SqlUtil.buildSingleCollectionQuery(ID, idsToUpdate) val values = ContentValues().apply { put(GROUPS_IN_COMMON, 1) } diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/SessionDatabase.kt b/app/src/main/java/org/thoughtcrime/securesms/database/SessionDatabase.kt index 04878035eb..efa0e18138 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/SessionDatabase.kt +++ b/app/src/main/java/org/thoughtcrime/securesms/database/SessionDatabase.kt @@ -119,7 +119,7 @@ class SessionDatabase(context: Context, databaseHelper: SignalDatabase) : Databa } fun getAllFor(serviceId: ServiceId, addressNames: List): List { - val query: SqlUtil.Query = SqlUtil.buildCollectionQuery(ADDRESS, addressNames) + val query: SqlUtil.Query = SqlUtil.buildSingleCollectionQuery(ADDRESS, addressNames) val results: MutableList = LinkedList() val queryString = "$ACCOUNT_ID = ? AND (${query.where})" diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java index dcdbafd93d..637ddbff41 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/ThreadDatabase.java @@ -472,7 +472,7 @@ public class ThreadDatabase extends Database { db.beginTransaction(); try { - SqlUtil.Query query = SqlUtil.buildCollectionQuery(ID, threadIds); + SqlUtil.Query query = SqlUtil.buildSingleCollectionQuery(ID, threadIds); ContentValues contentValues = new ContentValues(); contentValues.put(READ, ReadStatus.FORCED_UNREAD.serialize()); @@ -1062,7 +1062,7 @@ public class ThreadDatabase extends Database { public Map getThreadIdsIfExistsFor(@NonNull RecipientId ... recipientIds) { SQLiteDatabase db = databaseHelper.getSignalReadableDatabase(); - SqlUtil.Query query = SqlUtil.buildCollectionQuery(RECIPIENT_ID, Arrays.asList(recipientIds)); + SqlUtil.Query query = SqlUtil.buildSingleCollectionQuery(RECIPIENT_ID, Arrays.asList(recipientIds)); Map results = new HashMap<>(); try (Cursor cursor = db.query(TABLE_NAME, new String[]{ ID, RECIPIENT_ID }, query.getWhere(), query.getWhereArgs(), null, null, null, "1")) { @@ -1140,7 +1140,7 @@ public class ThreadDatabase extends Database { public @NonNull List getRecipientIdsForThreadIds(Collection threadIds) { SQLiteDatabase db = databaseHelper.getSignalReadableDatabase(); - SqlUtil.Query query = SqlUtil.buildCollectionQuery(ID, threadIds); + SqlUtil.Query query = SqlUtil.buildSingleCollectionQuery(ID, threadIds); List ids = new ArrayList<>(threadIds.size()); try (Cursor cursor = db.query(TABLE_NAME, new String[] { RECIPIENT_ID }, query.getWhere(), query.getWhereArgs(), null, null, null)) { diff --git a/app/src/main/java/org/thoughtcrime/securesms/database/UnknownStorageIdDatabase.java b/app/src/main/java/org/thoughtcrime/securesms/database/UnknownStorageIdDatabase.java index 76252877f9..58885055e2 100644 --- a/app/src/main/java/org/thoughtcrime/securesms/database/UnknownStorageIdDatabase.java +++ b/app/src/main/java/org/thoughtcrime/securesms/database/UnknownStorageIdDatabase.java @@ -7,8 +7,6 @@ import android.database.Cursor; import androidx.annotation.NonNull; import androidx.annotation.Nullable; -import com.annimon.stream.Stream; - import org.signal.core.util.CursorUtil; import org.thoughtcrime.securesms.util.Base64; import org.signal.core.util.SqlUtil; @@ -67,7 +65,7 @@ public class UnknownStorageIdDatabase extends Database { */ public List getAllWithTypes(List types) { List ids = new ArrayList<>(); - SqlUtil.Query query = SqlUtil.buildCollectionQuery(TYPE, types); + SqlUtil.Query query = SqlUtil.buildSingleCollectionQuery(TYPE, types); try (Cursor cursor = databaseHelper.getSignalReadableDatabase().query(TABLE_NAME, null, query.getWhere(), query.getWhereArgs(), null, null, null)) { while (cursor != null && cursor.moveToNext()) { 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 5c834e9539..db35dd5230 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 @@ -183,8 +183,33 @@ object SqlUtil { return Query("($selection) AND ($qualifier)", fullArgs.toTypedArray()) } + /** + * A convenient way of making queries in the form: WHERE [column] IN (?, ?, ..., ?) + * Handles breaking it + */ @JvmStatic - fun buildCollectionQuery(column: String, values: Collection): Query { + fun buildCollectionQuery(column: String, values: Collection): List { + return buildCollectionQuery(column, values, MAX_QUERY_ARGS) + } + + @VisibleForTesting + @JvmStatic + fun buildCollectionQuery(column: String, values: Collection, maxSize: Int): List { + require(!values.isEmpty()) { "Must have values!" } + + return values + .chunked(maxSize) + .map { batch -> buildSingleCollectionQuery(column, batch) } + } + + /** + * A convenient way of making queries in the form: WHERE [column] IN (?, ?, ..., ?) + * + * Important: Should only be used if you know the number of values is < 1000. Otherwise you risk creating a SQL statement this is too large. + * Prefer [buildCollectionQuery] when possible. + */ + @JvmStatic + fun buildSingleCollectionQuery(column: String, values: Collection): Query { require(!values.isEmpty()) { "Must have values!" } val query = StringBuilder() diff --git a/core-util/src/test/java/org/signal/core/util/SqlUtilTest.java b/core-util/src/test/java/org/signal/core/util/SqlUtilTest.java index a604380d63..62c4b12fa6 100644 --- a/core-util/src/test/java/org/signal/core/util/SqlUtilTest.java +++ b/core-util/src/test/java/org/signal/core/util/SqlUtilTest.java @@ -117,26 +117,42 @@ public final class SqlUtilTest { @Test public void buildCollectionQuery_single() { - SqlUtil.Query updateQuery = SqlUtil.buildCollectionQuery("a", Arrays.asList(1)); + List updateQuery = SqlUtil.buildCollectionQuery("a", Arrays.asList(1)); - assertEquals("a IN (?)", updateQuery.getWhere()); - assertArrayEquals(new String[] { "1" }, updateQuery.getWhereArgs()); + assertEquals(1, updateQuery.size()); + assertEquals("a IN (?)", updateQuery.get(0).getWhere()); + assertArrayEquals(new String[] { "1" }, updateQuery.get(0).getWhereArgs()); } @Test public void buildCollectionQuery_multiple() { - SqlUtil.Query updateQuery = SqlUtil.buildCollectionQuery("a", Arrays.asList(1, 2, 3)); + List updateQuery = SqlUtil.buildCollectionQuery("a", Arrays.asList(1, 2, 3)); - assertEquals("a IN (?, ?, ?)", updateQuery.getWhere()); - assertArrayEquals(new String[] { "1", "2", "3" }, updateQuery.getWhereArgs()); + assertEquals(1, updateQuery.size()); + assertEquals("a IN (?, ?, ?)", updateQuery.get(0).getWhere()); + assertArrayEquals(new String[] { "1", "2", "3" }, updateQuery.get(0).getWhereArgs()); + } + + @Test + public void buildCollectionQuery_multiple_twoBatches() { + List updateQuery = SqlUtil.buildCollectionQuery("a", Arrays.asList(1, 2, 3), 2); + + assertEquals(2, updateQuery.size()); + + assertEquals("a IN (?, ?)", updateQuery.get(0).getWhere()); + assertArrayEquals(new String[] { "1", "2" }, updateQuery.get(0).getWhereArgs()); + + assertEquals("a IN (?)", updateQuery.get(1).getWhere()); + assertArrayEquals(new String[] { "3" }, updateQuery.get(1).getWhereArgs()); } @Test public void buildCollectionQuery_multipleRecipientIds() { - SqlUtil.Query updateQuery = SqlUtil.buildCollectionQuery("a", Arrays.asList(new TestId(1), new TestId(2), new TestId(3))); + List updateQuery = SqlUtil.buildCollectionQuery("a", Arrays.asList(new TestId(1), new TestId(2), new TestId(3))); - assertEquals("a IN (?, ?, ?)", updateQuery.getWhere()); - assertArrayEquals(new String[] { "1", "2", "3" }, updateQuery.getWhereArgs()); + assertEquals(1, updateQuery.size()); + assertEquals("a IN (?, ?, ?)", updateQuery.get(0).getWhere()); + assertArrayEquals(new String[] { "1", "2", "3" }, updateQuery.get(0).getWhereArgs()); } @Test(expected = IllegalArgumentException.class)