Fix issues with RecipientDatabase#update().

src/org/thoughtcrime/securesms/database/RecipientDatabase.java

@@ -8,6 +8,7 @@ import android.text.TextUtils;
 
 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;
@@ -26,9 +27,11 @@ import org.thoughtcrime.securesms.util.Base64;
 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;
@@ -1165,28 +1168,12 @@ public class RecipientDatabase extends Database {
    */
   private boolean update(@NonNull RecipientId id, ContentValues contentValues) {
     SQLiteDatabase database  = databaseHelper.getWritableDatabase();
-    StringBuilder                  qualifier = new StringBuilder();
+    String         selection = ID + " = ?";
-    Set<Map.Entry<String, Object>> valueSet  = contentValues.valueSet();
+    String[]       args      = new String[]{id.serialize()};
-    String[]                       args      = new String[valueSet.size() + 1];
 
-    args[0] = id.serialize();
+    Pair<String, String[]> result = SqlUtil.buildTrueUpdateQuery(selection, args, contentValues);
 
-    int i = 0;
+    return database.update(TABLE_NAME, contentValues, result.first(), result.second()) > 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++;
-    }
-
-
-    return database.update(TABLE_NAME, contentValues, ID + " = ? AND (" + qualifier + ")", args) > 0;
   }
 
   private @NonNull Optional<RecipientId> getByColumn(@NonNull String column, String value) {

src/org/thoughtcrime/securesms/util/SqlUtil.java

@@ -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() {}
 
@@ -31,4 +40,39 @@ public final class SqlUtil {
 
     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]));
+  }
 }

test/unitTest/java/org/thoughtcrime/securesms/database/SqliteUtilTest.java

@@ -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());
+  }
+}