am ff91ec35: Tolerate crashes during re-aggregation.

* commit 'ff91ec356f1b17dea095a880f61b8bc4ff333b1e':
  Tolerate crashes during re-aggregation.

4 files changed, 80 insertions, 7 deletions

diff --git a/src/com/android/providers/contacts/ContactsProvider2.java b/src/com/android/providers/contacts/ContactsProvider2.java
index c321489..26aac63 100644
--- a/src/com/android/providers/contacts/ContactsProvider2.java
+++ b/src/com/android/providers/contacts/ContactsProvider2.java
@@ -7954,40 +7954,66 @@ public class ContactsProvider2 extends AbstractContactsProvider
             int count = 0;
             SQLiteDatabase db = null;
             boolean success = false;
+            boolean transactionStarted = false;
             try {
-                // Re-aggregation os only for the contacts DB.
+                // Re-aggregation is only for the contacts DB.
                 switchToContactMode();
                 db = mContactsHelper.getWritableDatabase();
                 mActiveDb.set(db);
 
                 // Start the actual process.
                 db.beginTransaction();
+                transactionStarted = true;
 
                 count = mContactAggregator.markAllVisibleForAggregation(db);
                 mContactAggregator.aggregateInTransaction(mTransactionContext.get(), db);
 
                 updateSearchIndexInTransaction();
 
-                mContactsHelper.setProperty(DbProperties.AGGREGATION_ALGORITHM,
-                        String.valueOf(PROPERTY_AGGREGATION_ALGORITHM_VERSION));
+                updateAggregationAlgorithmVersion();
 
                 db.setTransactionSuccessful();
 
                 success = true;
             } finally {
                 mTransactionContext.get().clearAll();
-                if (db != null) {
+                if (transactionStarted) {
                     db.endTransaction();
                 }
                 final long end = SystemClock.elapsedRealtime();
                 Log.i(TAG, "Aggregation algorithm upgraded for " + count + " raw contacts"
                         + (success ? (" in " + (end - start) + "ms") : " failed"));
             }
+        } catch (RuntimeException e) {
+            Log.e(TAG, "Failed to upgrade aggregation algorithm; continuing anyway.", e);
+
+            // Got some exception during re-aggregation.  Re-aggregation isn't that important, so
+            // just bump the aggregation algorithm version and let the provider start normally.
+            try {
+                final SQLiteDatabase db =  mContactsHelper.getWritableDatabase();
+                db.beginTransaction();
+                try {
+                    updateAggregationAlgorithmVersion();
+                    db.setTransactionSuccessful();
+                } finally {
+                    db.endTransaction();
+                }
+            } catch (RuntimeException e2) {
+                // Couldn't even update the algorithm version...  There's really nothing we can do
+                // here, so just go ahead and start the provider.  Next time the provider starts
+                // it'll try re-aggregation again, which may or may not succeed.
+                Log.e(TAG, "Failed to bump aggregation algorithm version; continuing anyway.", e2);
+            }
         } finally { // Need one more finally because endTransaction() may fail.
             setProviderStatus(ProviderStatus.STATUS_NORMAL);
         }
     }
 
+    private void updateAggregationAlgorithmVersion() {
+        mContactsHelper.setProperty(DbProperties.AGGREGATION_ALGORITHM,
+                String.valueOf(PROPERTY_AGGREGATION_ALGORITHM_VERSION));
+    }
+
     @VisibleForTesting
     boolean isPhone() {
         if (!mIsPhoneInitialized) {
diff --git a/src/com/android/providers/contacts/aggregation/util/ContactMatcher.java b/src/com/android/providers/contacts/aggregation/util/ContactMatcher.java
index a29735d..2e552e9 100644
--- a/src/com/android/providers/contacts/aggregation/util/ContactMatcher.java
+++ b/src/com/android/providers/contacts/aggregation/util/ContactMatcher.java
@@ -18,6 +18,8 @@ package com.android.providers.contacts.aggregation.util;
 import com.android.providers.contacts.ContactsDatabaseHelper.NameLookupType;
 import com.android.providers.contacts.util.Hex;
 
+import android.util.Log;
+
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -27,6 +29,7 @@ import java.util.List;
  * Logic for matching contacts' data and accumulating match scores.
  */
 public class ContactMatcher {
+    private static final String TAG = "ContactMatcher";
 
     // Best possible match score
     public static final int MAX_SCORE = 100;
@@ -296,8 +299,16 @@ public class ContactMatcher {
             return;
         }
 
-        byte[] decodedCandidateName = Hex.decodeHex(candidateName);
-        byte[] decodedName = Hex.decodeHex(name);
+        final byte[] decodedCandidateName;
+        final byte[] decodedName;
+        try {
+            decodedCandidateName = Hex.decodeHex(candidateName);
+            decodedName = Hex.decodeHex(name);
+        } catch (RuntimeException e) {
+            // How could this happen??  See bug 6827136
+            Log.e(TAG, "Failed to decode normalized name.  Skipping.", e);
+            return;
+        }
 
         NameDistance nameDistance = algorithm == MATCHING_ALGORITHM_CONSERVATIVE ?
                 mNameDistanceConservative : mNameDistanceApproximate;
diff --git a/src/com/android/providers/contacts/util/Hex.java b/src/com/android/providers/contacts/util/Hex.java
index ad26f4b..c3e5e74 100644
--- a/src/com/android/providers/contacts/util/Hex.java
+++ b/src/com/android/providers/contacts/util/Hex.java
@@ -74,12 +74,16 @@ public class Hex {
 
     /**
      * Quickly converts a hexadecimal string to a byte array.
+     *
+     * TODO Use checked exceptions instead of RuntimeException.  Apparently normalized names *may*
+     * contain non-hex strings and we want to make sure the provider won't crash even with such
+     * input.
      */
     public static byte[] decodeHex(String hexString) {
         int length = hexString.length();
 
         if ((length & 0x01) != 0) {
-            throw new IllegalArgumentException("Odd number of characters.");
+            throw new IllegalArgumentException("Odd number of characters: " + hexString);
         }
 
         boolean badHex = false;
diff --git a/tests/src/com/android/providers/contacts/aggregation/util/ContactMatcherTest.java b/tests/src/com/android/providers/contacts/aggregation/util/ContactMatcherTest.java
new file mode 100644
index 0000000..97faacd
--- /dev/null
+++ b/tests/src/com/android/providers/contacts/aggregation/util/ContactMatcherTest.java
@@ -0,0 +1,32 @@
+/*
+ * Copyright (C) 2012 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License
+ */
+package com.android.providers.contacts.aggregation.util;
+
+import com.android.providers.contacts.ContactsDatabaseHelper.NameLookupType;
+
+import android.test.AndroidTestCase;
+
+public class ContactMatcherTest extends AndroidTestCase {
+
+    public void testMatchName_invalidHexDecimal() {
+        final ContactMatcher matcher = new ContactMatcher();
+
+        // This shouldn't throw.  Bug 6827136
+        matcher.matchName(1, NameLookupType.NAME_COLLATION_KEY, "InvalidHex",
+                NameLookupType.NAME_COLLATION_KEY, "InvalidHex2",
+                ContactMatcher.MATCHING_ALGORITHM_CONSERVATIVE);
+    }
+}