Correctly change aggregation mode in all cases, unit tests.
Fixed broken unit tests, and in the process corrected an
issue where we changed aggregation mode in some cases where
is wasn't needed. (When updating but no values change.)
diff --git a/src/com/android/contacts/model/EntityDelta.java b/src/com/android/contacts/model/EntityDelta.java
index cb4a8c9..69e1e51 100644
--- a/src/com/android/contacts/model/EntityDelta.java
+++ b/src/com/android/contacts/model/EntityDelta.java
@@ -288,18 +288,13 @@
final boolean isContactInsert = mValues.isInsert();
final boolean isContactDelete = mValues.isDelete();
+ final boolean isContactUpdate = !isContactInsert && !isContactDelete;
final Long beforeId = mValues.getId();
final Long beforeVersion = mValues.getAsLong(RawContacts.VERSION);
Builder builder;
- // Suspend aggregation while persisting edits
- if (mValues.beforeExists()) {
- builder = buildSetAggregationMode(beforeId, RawContacts.AGGREGATION_MODE_SUSPENDED);
- possibleAdd(diff, builder);
- }
-
// Build possible operation at Contact level
builder = mValues.buildDiff(RawContacts.CONTENT_URI);
possibleAdd(diff, builder);
@@ -337,14 +332,20 @@
possibleAdd(diff, builder);
}
- // Enable aggregation when finished with updates
- if (mValues.beforeExists()) {
+ final boolean hasOperations = diff.size() > 0;
+
+ if (hasOperations && isContactUpdate) {
+ // Suspend aggregation while persisting updates
+ builder = buildSetAggregationMode(beforeId, RawContacts.AGGREGATION_MODE_SUSPENDED);
+ diff.add(0, builder.build());
+
+ // Restore aggregation as last operation
builder = buildSetAggregationMode(beforeId, RawContacts.AGGREGATION_MODE_DEFAULT);
- possibleAdd(diff, builder);
+ diff.add(builder.build());
}
- // If any operations, assert that version is identical so we bail if changed
- if (diff.size() > 0 && beforeVersion != null && beforeId != null) {
+ if (hasOperations && (isContactUpdate || isContactDelete)) {
+ // Assert version is consistent while persisting changes
builder = ContentProviderOperation.newAssertQuery(RawContacts.CONTENT_URI);
builder.withSelection(RawContacts._ID + "=" + beforeId, null);
builder.withValue(RawContacts.VERSION, beforeVersion);
diff --git a/tests/src/com/android/contacts/EntityDeltaTests.java b/tests/src/com/android/contacts/EntityDeltaTests.java
index dee242d..be2dd35 100644
--- a/tests/src/com/android/contacts/EntityDeltaTests.java
+++ b/tests/src/com/android/contacts/EntityDeltaTests.java
@@ -226,16 +226,26 @@
// Assert two operations: insert Data row and enforce version
final ArrayList<ContentProviderOperation> diff = source.buildDiff();
- assertEquals("Unexpected operations", 2, diff.size());
+ assertEquals("Unexpected operations", 4, diff.size());
{
final ContentProviderOperation oper = diff.get(0);
assertEquals("Expected version enforcement", TYPE_ASSERT, oper.getType());
}
{
final ContentProviderOperation oper = diff.get(1);
+ assertEquals("Expected aggregation mode change", TYPE_UPDATE, oper.getType());
+ assertEquals("Incorrect target", RawContacts.CONTENT_URI, oper.getUri());
+ }
+ {
+ final ContentProviderOperation oper = diff.get(2);
assertEquals("Incorrect type", TYPE_INSERT, oper.getType());
assertEquals("Incorrect target", Data.CONTENT_URI, oper.getUri());
}
+ {
+ final ContentProviderOperation oper = diff.get(3);
+ assertEquals("Expected aggregation mode change", TYPE_UPDATE, oper.getType());
+ assertEquals("Incorrect target", RawContacts.CONTENT_URI, oper.getUri());
+ }
}
public void testEntityDiffUpdateInsert() {
@@ -254,21 +264,31 @@
// Assert three operations: update Contact, insert Data row, enforce version
final ArrayList<ContentProviderOperation> diff = source.buildDiff();
- assertEquals("Unexpected operations", 3, diff.size());
+ assertEquals("Unexpected operations", 5, diff.size());
{
final ContentProviderOperation oper = diff.get(0);
assertEquals("Expected version enforcement", TYPE_ASSERT, oper.getType());
}
{
final ContentProviderOperation oper = diff.get(1);
- assertEquals("Incorrect type", TYPE_UPDATE, oper.getType());
+ assertEquals("Expected aggregation mode change", TYPE_UPDATE, oper.getType());
assertEquals("Incorrect target", RawContacts.CONTENT_URI, oper.getUri());
}
{
final ContentProviderOperation oper = diff.get(2);
+ assertEquals("Incorrect type", TYPE_UPDATE, oper.getType());
+ assertEquals("Incorrect target", RawContacts.CONTENT_URI, oper.getUri());
+ }
+ {
+ final ContentProviderOperation oper = diff.get(3);
assertEquals("Incorrect type", TYPE_INSERT, oper.getType());
assertEquals("Incorrect target", Data.CONTENT_URI, oper.getUri());
}
+ {
+ final ContentProviderOperation oper = diff.get(4);
+ assertEquals("Expected aggregation mode change", TYPE_UPDATE, oper.getType());
+ assertEquals("Incorrect target", RawContacts.CONTENT_URI, oper.getUri());
+ }
}
public void testEntityDiffNoneUpdate() {
@@ -281,16 +301,26 @@
// Assert two operations: update Data and enforce version
final ArrayList<ContentProviderOperation> diff = source.buildDiff();
- assertEquals("Unexpected operations", 2, diff.size());
+ assertEquals("Unexpected operations", 4, diff.size());
{
final ContentProviderOperation oper = diff.get(0);
assertEquals("Expected version enforcement", TYPE_ASSERT, oper.getType());
}
{
final ContentProviderOperation oper = diff.get(1);
+ assertEquals("Expected aggregation mode change", TYPE_UPDATE, oper.getType());
+ assertEquals("Incorrect target", RawContacts.CONTENT_URI, oper.getUri());
+ }
+ {
+ final ContentProviderOperation oper = diff.get(2);
assertEquals("Incorrect type", TYPE_UPDATE, oper.getType());
assertEquals("Incorrect target", Data.CONTENT_URI, oper.getUri());
}
+ {
+ final ContentProviderOperation oper = diff.get(3);
+ assertEquals("Expected aggregation mode change", TYPE_UPDATE, oper.getType());
+ assertEquals("Incorrect target", RawContacts.CONTENT_URI, oper.getUri());
+ }
}
public void testEntityDiffDelete() {