From d2f6ad6d50b5570327f8cca3b2d2bdcaec36ea90 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Mon, 1 Oct 2012 10:14:14 -0700 Subject: Make sure to rebuild search index on locale changes... so that search will still work even after changing the locale. - Rebuild the search index upon locale changes, as it contains locale-sensitive data. - Also make sure to update the in-memory collator in NameNormalizer upon locale changes. - Rebuild the search index on the next db upgrade in order to fix the search index which already contains invalid data. Bug 7251461 Change-Id: Id579a67de792a52a0091bf76d7c5d374f76f1639 --- .../providers/contacts/ContactsDatabaseHelper.java | 12 +++-- .../providers/contacts/ContactsProvider2.java | 3 +- .../android/providers/contacts/NameNormalizer.java | 51 ++++++++++++++++----- .../providers/contacts/SearchIndexManager.java | 12 +++-- .../providers/contacts/NameNormalizerTest.java | 52 ++++++++++++++++++++++ 5 files changed, 112 insertions(+), 18 deletions(-) diff --git a/src/com/android/providers/contacts/ContactsDatabaseHelper.java b/src/com/android/providers/contacts/ContactsDatabaseHelper.java index 95008d7..5baf9dc 100644 --- a/src/com/android/providers/contacts/ContactsDatabaseHelper.java +++ b/src/com/android/providers/contacts/ContactsDatabaseHelper.java @@ -107,7 +107,7 @@ public class ContactsDatabaseHelper extends SQLiteOpenHelper { * 700-799 Jelly Bean * */ - static final int DATABASE_VERSION = 704; + static final int DATABASE_VERSION = 705; private static final String DATABASE_NAME = "contacts2.db"; private static final String DATABASE_PRESENCE = "presence_db"; @@ -2375,6 +2375,14 @@ public class ContactsDatabaseHelper extends SQLiteOpenHelper { oldVersion = 704; } + if (oldVersion < 705) { + // Before this version, we didn't rebuild the search index on locale changes, so + // if the locale has changed after sync, the index contains gets stale. + // To correct the issue we have to rebuild the index here. + upgradeSearchIndex = true; + oldVersion = 705; + } + if (upgradeViewsAndTriggers) { createContactsViews(db); createGroupsView(db); @@ -2536,8 +2544,6 @@ public class ContactsDatabaseHelper extends SQLiteOpenHelper { + " ADD " + RawContacts.SORT_KEY_ALTERNATIVE + " TEXT COLLATE " + ContactsProvider2.PHONEBOOK_COLLATOR_NAME + ";"); - final Locale locale = Locale.getDefault(); - NameSplitter splitter = createNameSplitter(); SQLiteStatement rawContactUpdate = db.compileStatement( diff --git a/src/com/android/providers/contacts/ContactsProvider2.java b/src/com/android/providers/contacts/ContactsProvider2.java index a225b4d..410aaf4 100644 --- a/src/com/android/providers/contacts/ContactsProvider2.java +++ b/src/com/android/providers/contacts/ContactsProvider2.java @@ -1638,6 +1638,7 @@ public class ContactsProvider2 extends AbstractContactsProvider setProviderStatus(ProviderStatus.STATUS_CHANGING_LOCALE); mContactsHelper.setLocale(this, currentLocale); mProfileHelper.setLocale(this, currentLocale); + mSearchIndexManager.updateIndex(true); prefs.edit().putString(PREF_LOCALE, currentLocale.toString()).apply(); invalidateFastScrollingIndexCache(); setProviderStatus(providerStatus); @@ -1668,7 +1669,7 @@ public class ContactsProvider2 extends AbstractContactsProvider } protected void updateSearchIndexInBackground() { - mSearchIndexManager.updateIndex(); + mSearchIndexManager.updateIndex(false); } protected void updateDirectoriesInBackground(boolean rescan) { diff --git a/src/com/android/providers/contacts/NameNormalizer.java b/src/com/android/providers/contacts/NameNormalizer.java index c2b945e..e3f98a8 100644 --- a/src/com/android/providers/contacts/NameNormalizer.java +++ b/src/com/android/providers/contacts/NameNormalizer.java @@ -16,6 +16,7 @@ package com.android.providers.contacts; import com.android.providers.contacts.util.Hex; +import com.google.common.annotations.VisibleForTesting; import java.text.CollationKey; import java.text.Collator; @@ -28,17 +29,45 @@ import java.util.Locale; */ public class NameNormalizer { - private static final RuleBasedCollator sCompressingCollator; - static { - sCompressingCollator = (RuleBasedCollator)Collator.getInstance(Locale.getDefault()); - sCompressingCollator.setStrength(Collator.PRIMARY); - sCompressingCollator.setDecomposition(Collator.CANONICAL_DECOMPOSITION); + private static final Object sCollatorLock = new Object(); + + private static Locale sCollatorLocale; + + private static RuleBasedCollator sCachedCompressingCollator; + private static RuleBasedCollator sCachedComplexityCollator; + + /** + * Ensure that the cached collators are for the current locale. + */ + private static void ensureCollators() { + final Locale locale = Locale.getDefault(); + if (locale.equals(sCollatorLocale)) { + return; + } + sCollatorLocale = locale; + + sCachedCompressingCollator = (RuleBasedCollator) Collator.getInstance(locale); + sCachedCompressingCollator.setStrength(Collator.PRIMARY); + sCachedCompressingCollator.setDecomposition(Collator.CANONICAL_DECOMPOSITION); + + sCachedComplexityCollator = (RuleBasedCollator) Collator.getInstance(locale); + sCachedComplexityCollator.setStrength(Collator.SECONDARY); + } + + @VisibleForTesting + static RuleBasedCollator getCompressingCollator() { + synchronized (sCollatorLock) { + ensureCollators(); + return sCachedCompressingCollator; + } } - private static final RuleBasedCollator sComplexityCollator; - static { - sComplexityCollator = (RuleBasedCollator)Collator.getInstance(Locale.getDefault()); - sComplexityCollator.setStrength(Collator.SECONDARY); + @VisibleForTesting + static RuleBasedCollator getComplexityCollator() { + synchronized (sCollatorLock) { + ensureCollators(); + return sCachedComplexityCollator; + } } /** @@ -46,7 +75,7 @@ public class NameNormalizer { * of names. It ignores non-letter, non-digit characters, and removes accents. */ public static String normalize(String name) { - CollationKey key = sCompressingCollator.getCollationKey(lettersAndDigitsOnly(name)); + CollationKey key = getCompressingCollator().getCollationKey(lettersAndDigitsOnly(name)); return Hex.encodeHex(key.toByteArray(), true); } @@ -57,7 +86,7 @@ public class NameNormalizer { public static int compareComplexity(String name1, String name2) { String clean1 = lettersAndDigitsOnly(name1); String clean2 = lettersAndDigitsOnly(name2); - int diff = sComplexityCollator.compare(clean1, clean2); + int diff = getComplexityCollator().compare(clean1, clean2); if (diff != 0) { return diff; } diff --git a/src/com/android/providers/contacts/SearchIndexManager.java b/src/com/android/providers/contacts/SearchIndexManager.java index 361b5d8..20fd16b 100644 --- a/src/com/android/providers/contacts/SearchIndexManager.java +++ b/src/com/android/providers/contacts/SearchIndexManager.java @@ -243,13 +243,19 @@ public class SearchIndexManager { mDbHelper = (ContactsDatabaseHelper) mContactsProvider.getDatabaseHelper(); } - public void updateIndex() { - if (getSearchIndexVersion() == SEARCH_INDEX_VERSION) { - return; + public void updateIndex(boolean force) { + if (force) { + setSearchIndexVersion(0); + } else { + if (getSearchIndexVersion() == SEARCH_INDEX_VERSION) { + return; + } } SQLiteDatabase db = mDbHelper.getWritableDatabase(); db.beginTransaction(); try { + // We do a version check again, because the version might have been modified after + // the first check. We need to do the check again in a transaction to make sure. if (getSearchIndexVersion() != SEARCH_INDEX_VERSION) { rebuildIndex(db); setSearchIndexVersion(SEARCH_INDEX_VERSION); diff --git a/tests/src/com/android/providers/contacts/NameNormalizerTest.java b/tests/src/com/android/providers/contacts/NameNormalizerTest.java index 9e4aaac..2872962 100644 --- a/tests/src/com/android/providers/contacts/NameNormalizerTest.java +++ b/tests/src/com/android/providers/contacts/NameNormalizerTest.java @@ -16,8 +16,12 @@ package com.android.providers.contacts; +import android.test.MoreAsserts; import android.test.suitebuilder.annotation.SmallTest; +import java.text.RuleBasedCollator; +import java.util.Locale; + import junit.framework.TestCase; /** @@ -32,6 +36,25 @@ import junit.framework.TestCase; @SmallTest public class NameNormalizerTest extends TestCase { + private Locale mOriginalLocale; + + + @Override + protected void setUp() throws Exception { + super.setUp(); + + mOriginalLocale = Locale.getDefault(); + + // Run all test in en_US + Locale.setDefault(Locale.US); + } + + @Override + protected void tearDown() throws Exception { + Locale.setDefault(mOriginalLocale); + super.tearDown(); + } + public void testDifferent() { final String name1 = NameNormalizer.normalize("Helene"); final String name2 = NameNormalizer.normalize("Francesca"); @@ -69,4 +92,33 @@ public class NameNormalizerTest extends TestCase { public void testComplexityLength() { assertTrue(NameNormalizer.compareComplexity("helene2009", "helene") > 0); } + + public void testGetCollators() { + final RuleBasedCollator compressing1 = NameNormalizer.getCompressingCollator(); + final RuleBasedCollator complexity1 = NameNormalizer.getComplexityCollator(); + + assertNotNull(compressing1); + assertNotNull(complexity1); + assertNotSame(compressing1, complexity1); + + // Get again. Should be cached. + final RuleBasedCollator compressing2 = NameNormalizer.getCompressingCollator(); + final RuleBasedCollator complexity2 = NameNormalizer.getComplexityCollator(); + + assertSame(compressing1, compressing2); + assertSame(complexity1, complexity2); + + // Change locale -- now new collators should be returned. + Locale.setDefault(Locale.FRANCE); + + final RuleBasedCollator compressing3 = NameNormalizer.getCompressingCollator(); + final RuleBasedCollator complexity3 = NameNormalizer.getComplexityCollator(); + + assertNotNull(compressing3); + assertNotNull(complexity3); + assertNotSame(compressing3, complexity3); + + assertNotSame(compressing1, compressing3); + assertNotSame(complexity1, complexity3); + } } -- cgit v1.1