Autogenerate IDs for inserts and consolidate loader into one transaction.
Bug: 36841782,77724716,77725859
Test: implemented
PiperOrigin-RevId: 192191296
Change-Id: I7a22367b33c7555d014a29a2af2942f2eb76c0a5
diff --git a/java/com/android/dialer/speeddial/SpeedDialUiItemLoader.java b/java/com/android/dialer/speeddial/SpeedDialUiItemLoader.java
index 257c74f..13e5f87 100644
--- a/java/com/android/dialer/speeddial/SpeedDialUiItemLoader.java
+++ b/java/com/android/dialer/speeddial/SpeedDialUiItemLoader.java
@@ -97,15 +97,9 @@
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 @@
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 @@
}
}
- // 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 f636194..5b54b79 100644
--- a/java/com/android/dialer/speeddial/database/SpeedDialEntry.java
+++ b/java/com/android/dialer/speeddial/database/SpeedDialEntry.java
@@ -27,8 +27,13 @@
@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 @@
@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 0efc110..ce771c3 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 @@
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 @@
*
* <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 01d49c3..7c823bd 100644
--- a/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java
+++ b/java/com/android/dialer/speeddial/database/SpeedDialEntryDatabaseHelper.java
@@ -25,6 +25,7 @@
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 @@
}
@Override
- public List<SpeedDialEntry> getAllEntries() {
+ public ImmutableList<SpeedDialEntry> getAllEntries() {
List<SpeedDialEntry> entries = new ArrayList<>();
String query = "SELECT * FROM " + TABLE_NAME;
@@ -118,25 +119,24 @@
.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 @@
}
}
+ 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 @@
}
@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 @@
}
}
- 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 @@
}
@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();
}
}