Autogenerate IDs for inserts and consolidate loader into one transaction.

Bug: 36841782,77724716,77725859
Test: implemented
PiperOrigin-RevId: 192191296
Change-Id: I7a22367b33c7555d014a29a2af2942f2eb76c0a5

4 files changed, 123 insertions, 56 deletions

diff --git a/java/com/android/dialer/speeddial/SpeedDialUiItemLoader.java b/java/com/android/dialer/speeddial/SpeedDialUiItemLoader.java
index 257c74f58..13e5f8744 100644
--- a/java/com/android/dialer/speeddial/SpeedDialUiItemLoader.java
+++ b/java/com/android/dialer/speeddial/SpeedDialUiItemLoader.java
@@ -97,15 +97,9 @@ public final class SpeedDialUiItemLoader {
     List<SpeedDialEntry> entriesToUpdate = new ArrayList<>();
     List<Long> entriesToDelete = new ArrayList<>();
 
-    // Track the highest entry ID
-    // TODO(a bug): use auto-generated IDs
-    long maxId = 0L;
-
     // Get all SpeedDialEntries and mark them to be updated or deleted
     List<SpeedDialEntry> entries = db.getAllEntries();
     for (SpeedDialEntry entry : entries) {
-      maxId = Math.max(entry.id(), maxId);
-
       SpeedDialUiItem contact = getSpeedDialContact(entry);
       // Remove contacts that no longer exist or are no longer starred
       if (contact == null || !contact.isStarred()) {
@@ -138,11 +132,8 @@ public final class SpeedDialUiItemLoader {
         speedDialUiItems.add(contact);
 
       } else if (speedDialUiItems.stream().noneMatch(c -> c.contactId() == contact.contactId())) {
-        // Increment the ID so there aren't any collisions
-        maxId += 1;
         entriesToInsert.add(
             SpeedDialEntry.builder()
-                .setId(maxId)
                 .setLookupKey(contact.lookupKey())
                 .setContactId(contact.contactId())
                 .setDefaultChannel(contact.defaultChannel())
@@ -153,10 +144,10 @@ public final class SpeedDialUiItemLoader {
       }
     }
 
-    // TODO(a bug): use a single db transaction
-    db.delete(entriesToDelete);
-    db.update(entriesToUpdate);
-    db.insert(entriesToInsert);
+    db.insertUpdateAndDelete(
+        ImmutableList.copyOf(entriesToInsert),
+        ImmutableList.copyOf(entriesToUpdate),
+        ImmutableList.copyOf(entriesToDelete));
     return ImmutableList.copyOf(speedDialUiItems);
   }
 
diff --git a/java/com/android/dialer/speeddial/database/SpeedDialEntry.java b/java/com/android/dialer/speeddial/database/SpeedDialEntry.java
index f63619480..5b54b79c8 100644
--- a/java/com/android/dialer/speeddial/database/SpeedDialEntry.java
+++ b/java/com/android/dialer/speeddial/database/SpeedDialEntry.java
@@ -27,8 +27,13 @@ import java.lang.annotation.RetentionPolicy;
 @AutoValue
 public abstract class SpeedDialEntry {
 
-  /** Unique ID */
-  public abstract long id();
+  /**
+   * Unique ID
+   *
+   * <p>Must be null when inserting, and an ID will be generated and returned after inserting.
+   */
+  @Nullable
+  public abstract Long id();
 
   /** @see {@link Contacts#_ID} */
   public abstract long contactId();
@@ -55,7 +60,7 @@ public abstract class SpeedDialEntry {
   @AutoValue.Builder
   public abstract static class Builder {
 
-    public abstract Builder setId(long id);
+    public abstract Builder setId(Long id);
 
     public abstract Builder setContactId(long contactId);
 
diff --git a/java/com/android/dialer/speeddial/database/SpeedDialEntryDao.java b/java/com/android/dialer/speeddial/database/SpeedDialEntryDao.java
index 0efc110f4..ce771c3c8 100644
--- a/java/com/android/dialer/speeddial/database/SpeedDialEntryDao.java
+++ b/java/com/android/dialer/speeddial/database/SpeedDialEntryDao.java
@@ -16,7 +16,7 @@
 
 package com.android.dialer.speeddial.database;
 
-import java.util.List;
+import com.google.common.collect.ImmutableList;
 
 /**
  * Interface that databases support speed dial entries should implement.
@@ -26,19 +26,19 @@ import java.util.List;
 public interface SpeedDialEntryDao {
 
   /** Return all entries in the database */
-  List<SpeedDialEntry> getAllEntries();
+  ImmutableList<SpeedDialEntry> getAllEntries();
 
   /**
    * Insert new entries.
    *
-   * <p>Fails if any of the {@link SpeedDialEntry#id()} already exist.
+   * <p>{@link SpeedDialEntry#id() ids} must be null.
    */
-  void insert(List<SpeedDialEntry> entries);
+  void insert(ImmutableList<SpeedDialEntry> entries);
 
   /**
    * Insert a new entry.
    *
-   * <p>Fails if the {@link SpeedDialEntry#id()} already exists.
+   * <p>{@link SpeedDialEntry#id() ids} must be null.
    */
   long insert(SpeedDialEntry entry);
 
@@ -47,14 +47,26 @@ public interface SpeedDialEntryDao {
    *
    * <p>Fails if the {@link SpeedDialEntry#id()} doesn't exist.
    */
-  void update(List<SpeedDialEntry> entries);
+  void update(ImmutableList<SpeedDialEntry> entries);
 
   /**
    * Delete the passed in entries based on {@link SpeedDialEntry#id}.
    *
    * <p>Fails if the {@link SpeedDialEntry#id()} doesn't exist.
    */
-  void delete(List<Long> entries);
+  void delete(ImmutableList<Long> entries);
+
+  /**
+   * Inserts, updates and deletes rows all in on transaction.
+   *
+   * @see #insert(ImmutableList)
+   * @see #update(ImmutableList)
+   * @see #delete(ImmutableList)
+   */
+  void insertUpdateAndDelete(
+      ImmutableList<SpeedDialEntry> entriesToInsert,
+      ImmutableList<SpeedDialEntry> entriesToUpdate,
+      ImmutableList<Long> entriesToDelete);
 
   /** Delete all entries in the database. */
   void deleteAll();
diff --git a/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java b/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java
index 01d49c3d7..7c823bd63 100644
--- a/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java
+++ b/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java
@@ -25,6 +25,7 @@ import android.text.TextUtils;
 import com.android.dialer.common.Assert;
 import com.android.dialer.common.database.Selection;
 import com.android.dialer.speeddial.database.SpeedDialEntry.Channel;
+import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
 import java.util.List;
 
@@ -94,7 +95,7 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper
   }
 
   @Override
-  public List<SpeedDialEntry> getAllEntries() {
+  public ImmutableList<SpeedDialEntry> getAllEntries() {
     List<SpeedDialEntry> entries = new ArrayList<>();
 
     String query = "SELECT * FROM " + TABLE_NAME;
@@ -118,25 +119,24 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper
                 .setDefaultChannel(channel)
                 .setContactId(cursor.getLong(POSITION_CONTACT_ID))
                 .setLookupKey(cursor.getString(POSITION_LOOKUP_KEY))
-                .setId(cursor.getInt(POSITION_ID))
+                .setId(cursor.getLong(POSITION_ID))
                 .build();
         entries.add(entry);
       }
     }
-    return entries;
+    return ImmutableList.copyOf(entries);
   }
 
   @Override
-  public void insert(List<SpeedDialEntry> entries) {
+  public void insert(ImmutableList<SpeedDialEntry> entries) {
+    if (entries.isEmpty()) {
+      return;
+    }
+
     SQLiteDatabase db = getWritableDatabase();
     db.beginTransaction();
     try {
-      for (SpeedDialEntry entry : entries) {
-        if (db.insert(TABLE_NAME, null, buildContentValues(entry)) == -1L) {
-          throw Assert.createUnsupportedOperationFailException(
-              "Attempted to insert a row that already exists.");
-        }
-      }
+      insert(db, entries);
       db.setTransactionSuccessful();
     } finally {
       db.endTransaction();
@@ -144,11 +144,21 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper
     }
   }
 
+  private void insert(SQLiteDatabase writeableDatabase, ImmutableList<SpeedDialEntry> entries) {
+    for (SpeedDialEntry entry : entries) {
+      Assert.checkArgument(entry.id() == null);
+      if (writeableDatabase.insert(TABLE_NAME, null, buildContentValuesWithoutId(entry)) == -1L) {
+        throw Assert.createUnsupportedOperationFailException(
+            "Attempted to insert a row that already exists.");
+      }
+    }
+  }
+
   @Override
   public long insert(SpeedDialEntry entry) {
     long updateRowId;
     try (SQLiteDatabase db = getWritableDatabase()) {
-      updateRowId = db.insert(TABLE_NAME, null, buildContentValues(entry));
+      updateRowId = db.insert(TABLE_NAME, null, buildContentValuesWithoutId(entry));
     }
     if (updateRowId == -1) {
       throw Assert.createUnsupportedOperationFailException(
@@ -158,22 +168,15 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper
   }
 
   @Override
-  public void update(List<SpeedDialEntry> entries) {
+  public void update(ImmutableList<SpeedDialEntry> entries) {
+    if (entries.isEmpty()) {
+      return;
+    }
+
     SQLiteDatabase db = getWritableDatabase();
     db.beginTransaction();
     try {
-      for (SpeedDialEntry entry : entries) {
-        int count =
-            db.update(
-                TABLE_NAME,
-                buildContentValues(entry),
-                ID + " = ?",
-                new String[] {Long.toString(entry.id())});
-        if (count != 1) {
-          throw Assert.createUnsupportedOperationFailException(
-              "Attempted to update an undetermined number of rows: " + count);
-        }
-      }
+      update(db, entries);
       db.setTransactionSuccessful();
     } finally {
       db.endTransaction();
@@ -181,9 +184,34 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper
     }
   }
 
-  private ContentValues buildContentValues(SpeedDialEntry entry) {
+  private void update(SQLiteDatabase writeableDatabase, ImmutableList<SpeedDialEntry> entries) {
+    for (SpeedDialEntry entry : entries) {
+      int count =
+          writeableDatabase.update(
+              TABLE_NAME,
+              buildContentValuesWithId(entry),
+              ID + " = ?",
+              new String[] {Long.toString(entry.id())});
+      if (count != 1) {
+        throw Assert.createUnsupportedOperationFailException(
+            "Attempted to update an undetermined number of rows: " + count);
+      }
+    }
+  }
+
+  private ContentValues buildContentValuesWithId(SpeedDialEntry entry) {
+    return buildContentValues(entry, true);
+  }
+
+  private ContentValues buildContentValuesWithoutId(SpeedDialEntry entry) {
+    return buildContentValues(entry, false);
+  }
+
+  private ContentValues buildContentValues(SpeedDialEntry entry, boolean includeId) {
     ContentValues values = new ContentValues();
-    values.put(ID, entry.id());
+    if (includeId) {
+      values.put(ID, entry.id());
+    }
     values.put(CONTACT_ID, entry.contactId());
     values.put(LOOKUP_KEY, entry.lookupKey());
     if (entry.defaultChannel() != null) {
@@ -195,19 +223,50 @@ public final class SpeedDialEntryDatabaseHelper extends SQLiteOpenHelper
   }
 
   @Override
-  public void delete(List<Long> ids) {
+  public void delete(ImmutableList<Long> ids) {
+    if (ids.isEmpty()) {
+      return;
+    }
+
+    try (SQLiteDatabase db = getWritableDatabase()) {
+      delete(db, ids);
+    }
+  }
+
+  private void delete(SQLiteDatabase writeableDatabase, ImmutableList<Long> ids) {
     List<String> idStrings = new ArrayList<>();
     for (Long id : ids) {
       idStrings.add(Long.toString(id));
     }
 
     Selection selection = Selection.builder().and(Selection.column(ID).in(idStrings)).build();
-    try (SQLiteDatabase db = getWritableDatabase()) {
-      int count = db.delete(TABLE_NAME, selection.getSelection(), selection.getSelectionArgs());
-      if (count != ids.size()) {
-        throw Assert.createUnsupportedOperationFailException(
-            "Attempted to delete an undetermined number of rows: " + count);
-      }
+    int count =
+        writeableDatabase.delete(
+            TABLE_NAME, selection.getSelection(), selection.getSelectionArgs());
+    if (count != ids.size()) {
+      throw Assert.createUnsupportedOperationFailException(
+          "Attempted to delete an undetermined number of rows: " + count);
+    }
+  }
+
+  @Override
+  public void insertUpdateAndDelete(
+      ImmutableList<SpeedDialEntry> entriesToInsert,
+      ImmutableList<SpeedDialEntry> entriesToUpdate,
+      ImmutableList<Long> entriesToDelete) {
+    if (entriesToInsert.isEmpty() && entriesToUpdate.isEmpty() && entriesToDelete.isEmpty()) {
+      return;
+    }
+    SQLiteDatabase db = getWritableDatabase();
+    db.beginTransaction();
+    try {
+      insert(db, entriesToInsert);
+      update(db, entriesToUpdate);
+      delete(db, entriesToDelete);
+      db.setTransactionSuccessful();
+    } finally {
+      db.endTransaction();
+      db.close();
     }
   }