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 (file)
--- a/src/com/android/contacts/model/EntityDelta.java
+++ b/src/com/android/contacts/model/EntityDelta.java
@@ -288,18 +288,13 @@ public class EntityDelta implements Parcelable {
 
         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 @@ public class EntityDelta implements Parcelable {
             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 (file)
--- a/tests/src/com/android/contacts/EntityDeltaTests.java
+++ b/tests/src/com/android/contacts/EntityDeltaTests.java
@@ -226,16 +226,26 @@ public class EntityDeltaTests extends AndroidTestCase {
 
         // 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 @@ public class EntityDeltaTests extends AndroidTestCase {
 
         // 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 @@ public class EntityDeltaTests extends AndroidTestCase {
 
         // 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() {