Skip to content

Commit

Permalink
Fix issues with RecipientDatabase#update().
Browse files Browse the repository at this point in the history
  • Loading branch information
@greyson-signal
greyson-signal committed Dec 5, 2019
1 parent acfba9a commit a16242b
Show file tree
Hide file tree
Showing 3 changed files with 147 additions and 21 deletions.
29 changes: 8 additions & 21 deletions src/org/thoughtcrime/securesms/database/RecipientDatabase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@

import androidx.annotation.NonNull;
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;

import com.annimon.stream.Stream;
import com.google.android.gms.common.util.ArrayUtils;
Expand All @@ -26,9 +27,11 @@
import org.thoughtcrime.securesms.util.FeatureFlags;
import org.thoughtcrime.securesms.util.GroupUtil;
import org.thoughtcrime.securesms.util.IdentityUtil;
import org.thoughtcrime.securesms.util.SqlUtil;
import org.thoughtcrime.securesms.util.Util;
import org.whispersystems.libsignal.IdentityKey;
import org.whispersystems.libsignal.InvalidKeyException;
import org.whispersystems.libsignal.util.Pair;
import org.whispersystems.libsignal.util.guava.Optional;
import org.whispersystems.signalservice.api.push.SignalServiceAddress;
import org.whispersystems.signalservice.api.util.UuidUtil;
Expand Down Expand Up @@ -1164,29 +1167,13 @@ void markDirty(@NonNull RecipientId recipientId, @NonNull DirtyState dirtyState)
* query such that this will only return true if a row was *actually* updated.
*/
private boolean update(@NonNull RecipientId id, ContentValues contentValues) {
SQLiteDatabase database = databaseHelper.getWritableDatabase();
StringBuilder qualifier = new StringBuilder();
Set<Map.Entry<String, Object>> valueSet = contentValues.valueSet();
String[] args = new String[valueSet.size() + 1];

args[0] = id.serialize();

int i = 0;

for (Map.Entry<String, Object> entry : valueSet) {
qualifier.append(entry.getKey()).append(" != ?");

if (i != valueSet.size() - 1) {
qualifier.append(" OR ");
}

args[i + 1] = String.valueOf(entry.getValue());

i++;
}
SQLiteDatabase database = databaseHelper.getWritableDatabase();
String selection = ID + " = ?";
String[] args = new String[]{id.serialize()};

Pair<String, String[]> result = SqlUtil.buildTrueUpdateQuery(selection, args, contentValues);

return database.update(TABLE_NAME, contentValues, ID + " = ? AND (" + qualifier + ")", args) > 0;
return database.update(TABLE_NAME, contentValues, result.first(), result.second()) > 0;
}

private @NonNull Optional<RecipientId> getByColumn(@NonNull String column, String value) {
Expand Down
44 changes: 44 additions & 0 deletions src/org/thoughtcrime/securesms/util/SqlUtil.java
View file
Original file line number Diff line number Diff line change
@@ -1,11 +1,20 @@
package org.thoughtcrime.securesms.util;

import android.content.ContentValues;
import android.database.Cursor;

import androidx.annotation.NonNull;

import net.sqlcipher.database.SQLiteDatabase;

import org.whispersystems.libsignal.util.Pair;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Set;

public final class SqlUtil {
private SqlUtil() {}

Expand All @@ -31,4 +40,39 @@ public static boolean columnExists(@NonNull SQLiteDatabase db, @NonNull String t

return false;
}

/**
* Returns an updated query and args pairing that will only update rows that would *actually*
* change. In other words, if {@link SQLiteDatabase#update(String, ContentValues, String, String[])}
* returns > 0, then you know something *actually* changed.
*/
public static @NonNull Pair<String, String[]> buildTrueUpdateQuery(@NonNull String selection,
@NonNull String[] args,
@NonNull ContentValues contentValues)
{
StringBuilder qualifier = new StringBuilder();
Set<Map.Entry<String, Object>> valueSet = contentValues.valueSet();
List<String> fullArgs = new ArrayList<>(args.length + valueSet.size());

fullArgs.addAll(Arrays.asList(args));

int i = 0;

for (Map.Entry<String, Object> entry : valueSet) {
if (entry.getValue() != null) {
qualifier.append(entry.getKey()).append(" != ? OR ").append(entry.getKey()).append(" IS NULL");
fullArgs.add(String.valueOf(entry.getValue()));
} else {
qualifier.append(entry.getKey()).append(" NOT NULL");
}

if (i != valueSet.size() - 1) {
qualifier.append(" OR ");
}

i++;
}

return new Pair<>("(" + selection + ") AND (" + qualifier + ")", fullArgs.toArray(new String[0]));
}
}
95 changes: 95 additions & 0 deletions test/unitTest/java/org/thoughtcrime/securesms/database/SqliteUtilTest.java
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
package org.thoughtcrime.securesms.database;

import android.app.Application;
import android.content.ContentValues;

import org.junit.Test;
import org.junit.runner.RunWith;
import org.robolectric.RobolectricTestRunner;
import org.robolectric.annotation.Config;
import org.thoughtcrime.securesms.util.SqlUtil;
import org.whispersystems.libsignal.util.Pair;

import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;

@RunWith(RobolectricTestRunner.class)
@Config(manifest = Config.NONE, application = Application.class)
public class SqliteUtilTest {

@Test
public void buildTrueUpdateQuery_simple() {
String selection = "_id = ?";
String[] args = new String[]{"1"};

ContentValues values = new ContentValues();
values.put("a", 2);

Pair<String, String[]> result = SqlUtil.buildTrueUpdateQuery(selection, args, values);

assertEquals("(_id = ?) AND (a != ? OR a IS NULL)", result.first());
assertArrayEquals(new String[] { "1", "2" }, result.second());
}

@Test
public void buildTrueUpdateQuery_complexSelection() {
String selection = "_id = ? AND (foo = ? OR bar != ?)";
String[] args = new String[]{"1", "2", "3"};

ContentValues values = new ContentValues();
values.put("a", 4);

Pair<String, String[]> result = SqlUtil.buildTrueUpdateQuery(selection, args, values);

assertEquals("(_id = ? AND (foo = ? OR bar != ?)) AND (a != ? OR a IS NULL)", result.first());
assertArrayEquals(new String[] { "1", "2", "3", "4" }, result.second());
}

@Test
public void buildTrueUpdateQuery_multipleContentValues() {
String selection = "_id = ?";
String[] args = new String[]{"1"};

ContentValues values = new ContentValues();
values.put("a", 2);
values.put("b", 3);
values.put("c", 4);

Pair<String, String[]> result = SqlUtil.buildTrueUpdateQuery(selection, args, values);

assertEquals("(_id = ?) AND (a != ? OR a IS NULL OR b != ? OR b IS NULL OR c != ? OR c IS NULL)", result.first());
assertArrayEquals(new String[] { "1", "2", "3", "4"}, result.second());
}

@Test
public void buildTrueUpdateQuery_nullContentValue() {
String selection = "_id = ?";
String[] args = new String[]{"1"};

ContentValues values = new ContentValues();
values.put("a", (String) null);

Pair<String, String[]> result = SqlUtil.buildTrueUpdateQuery(selection, args, values);

assertEquals("(_id = ?) AND (a NOT NULL)", result.first());
assertArrayEquals(new String[] { "1" }, result.second());
}

@Test
public void buildTrueUpdateQuery_complexContentValue() {
String selection = "_id = ?";
String[] args = new String[]{"1"};

ContentValues values = new ContentValues();
values.put("a", (String) null);
values.put("b", 2);
values.put("c", 3);
values.put("d", (String) null);
values.put("e", (String) null);

Pair<String, String[]> result = SqlUtil.buildTrueUpdateQuery(selection, args, values);

assertEquals("(_id = ?) AND (a NOT NULL OR b != ? OR b IS NULL OR c != ? OR c IS NULL OR d NOT NULL OR e NOT NULL)", result.first());
assertArrayEquals(new String[] { "1", "2", "3" }, result.second());
}
}

0 comments on commit a16242b

Please sign in to comment.