From 346a0cc1e00fcfde05df66eb37433172c96b1c36 Mon Sep 17 00:00:00 2001 From: Andrew Stadler Date: Wed, 20 Jan 2010 17:24:33 -0800 Subject: [PATCH] Backup/Restore accounts - bugfix - DO NOT MERGE * Followup to 85d765f4 * Workaround for (HTC bug: 2275383) & (Moto bug: 2226582) * Restores mSyncKey as null instead of empty string, which is how a new account is initialized. Bug: 2385980 * Cleanup synchronized logic in backup & restore * Minor cleanups & improved comments Cherry-picked from master d612717340b2670776a75a1d2e9e3df81fd4c052 --- .../android/email/AccountBackupRestore.java | 21 +++++++++---------- src/com/android/email/LegacyConversions.java | 14 ++++++++----- .../activity/setup/AccountSetupOutgoing.java | 2 +- .../email/AccountBackupRestoreTests.java | 16 ++++++++++++++ .../android/email/LegacyConversionsTests.java | 2 +- 5 files changed, 37 insertions(+), 18 deletions(-) diff --git a/src/com/android/email/AccountBackupRestore.java b/src/com/android/email/AccountBackupRestore.java index 376c29f5c..d84f95aa8 100644 --- a/src/com/android/email/AccountBackupRestore.java +++ b/src/com/android/email/AccountBackupRestore.java @@ -49,9 +49,7 @@ public class AccountBackupRestore { new Thread() { @Override public void run() { - synchronized(AccountBackupRestore.class) { - doBackupAccounts(context, Preferences.getPreferences(context)); - } + doBackupAccounts(context, Preferences.getPreferences(context)); } }.start(); } @@ -62,10 +60,7 @@ public class AccountBackupRestore { */ public static void restoreAccountsIfNeeded(final Context context) { // Don't log here; This is called often. - boolean restored; - synchronized(AccountBackupRestore.class) { - restored = doRestoreAccounts(context, Preferences.getPreferences(context)); - } + boolean restored = doRestoreAccounts(context, Preferences.getPreferences(context)); if (restored) { // after restoring accounts, register services appropriately Log.w(Email.LOG_TAG, "Register services after restoring accounts"); @@ -81,7 +76,8 @@ public class AccountBackupRestore { * @param context used to access the provider * @param preferences used to access the backups (provided separately for testability) */ - /* package */ static void doBackupAccounts(Context context, Preferences preferences) { + /* package */ synchronized static void doBackupAccounts(Context context, + Preferences preferences) { // 1. Wipe any existing backup accounts Account[] oldBackups = preferences.getAccounts(); for (Account backup : oldBackups) { @@ -91,6 +87,8 @@ public class AccountBackupRestore { // 2. Identify the default account (if any). This is required because setting // the default account flag is lazy,and sometimes we don't have any flags set. We'll // use this to make it explicit (see loop, below). + // This is also the quick check for "no accounts" (the only case in which the returned + // value is -1) and if so, we can exit immediately. long defaultAccountId = EmailContent.Account.getDefaultAccountId(context); if (defaultAccountId == -1) { return; @@ -102,7 +100,7 @@ public class AccountBackupRestore { try { while (c.moveToNext()) { EmailContent.Account fromAccount = - EmailContent.getContent(c, EmailContent.Account.class); + EmailContent.getContent(c, EmailContent.Account.class); if (Email.DEBUG) { Log.v(Email.LOG_TAG, "Backing up account:" + fromAccount.getDisplayName()); } @@ -142,7 +140,8 @@ public class AccountBackupRestore { * @param preferences used to access the backups (provided separately for testability) * @return true if accounts were restored (meaning services should be restarted, etc.) */ - /* package */ static boolean doRestoreAccounts(Context context, Preferences preferences) { + /* package */ synchronized static boolean doRestoreAccounts(Context context, + Preferences preferences) { boolean result = false; // 1. Quick check - if we have any accounts, get out @@ -165,7 +164,7 @@ public class AccountBackupRestore { continue; } // Restore the account - Log.v(Email.LOG_TAG, "Restoring account:" + backupAccount.getDescription()); + Log.w(Email.LOG_TAG, "Restoring account:" + backupAccount.getDescription()); EmailContent.Account toAccount = LegacyConversions.makeAccount(context, backupAccount); diff --git a/src/com/android/email/LegacyConversions.java b/src/com/android/email/LegacyConversions.java index 6c424df51..80a4de19a 100644 --- a/src/com/android/email/LegacyConversions.java +++ b/src/com/android/email/LegacyConversions.java @@ -529,7 +529,7 @@ public class LegacyConversions { * * @param context application context * @param fromAccount the provider account to be backed up (including transient hostauth's) - * @result a legacy Account object ready to be committed to preferences + * @return a legacy Account object ready to be committed to preferences */ /* package */ static Account makeLegacyAccount(Context context, EmailContent.Account fromAccount) { @@ -572,6 +572,10 @@ public class LegacyConversions { * Conversion from legacy account to provider account * * Used for backup/restore and for account migration. + * + * @param context application context + * @param fromAccount the legacy account to convert to modern format + * @return an Account ready to be committed to provider */ /* package */ static EmailContent.Account makeAccount(Context context, Account fromAccount) { @@ -579,17 +583,17 @@ public class LegacyConversions { result.setDisplayName(fromAccount.getDescription()); result.setEmailAddress(fromAccount.getEmail()); - result.mSyncKey = ""; + result.mSyncKey = null; result.setSyncLookback(fromAccount.getSyncWindow()); result.setSyncInterval(fromAccount.getAutomaticCheckIntervalMinutes()); - // result.mHostAuthKeyRecv - // result.mHostAuthKeySend; + // result.mHostAuthKeyRecv; -- will be set when object is saved + // result.mHostAuthKeySend; -- will be set when object is saved int flags = 0; if (fromAccount.isNotifyNewMail()) flags |= EmailContent.Account.FLAGS_NOTIFY_NEW_MAIL; if (fromAccount.isVibrate()) flags |= EmailContent.Account.FLAGS_VIBRATE; result.setFlags(flags); result.setDeletePolicy(fromAccount.getDeletePolicy()); - // result.setDefaultAccount(); + // result.setDefaultAccount(); -- will be set by caller, if neededf result.mCompatibilityUuid = fromAccount.getUuid(); result.setSenderName(fromAccount.getName()); result.setRingtone(fromAccount.getRingtone()); diff --git a/src/com/android/email/activity/setup/AccountSetupOutgoing.java b/src/com/android/email/activity/setup/AccountSetupOutgoing.java index 40e369efd..f51a36dbe 100644 --- a/src/com/android/email/activity/setup/AccountSetupOutgoing.java +++ b/src/com/android/email/activity/setup/AccountSetupOutgoing.java @@ -253,7 +253,7 @@ public class AccountSetupOutgoing extends Activity implements OnClickListener, if (mAccount.isSaved()) { mAccount.update(this, mAccount.toContentValues()); mAccount.mHostAuthSend.update(this, mAccount.mHostAuthSend.toContentValues()); - } else { + } else { mAccount.save(this); } // Update the backup (side copy) of the accounts diff --git a/tests/src/com/android/email/AccountBackupRestoreTests.java b/tests/src/com/android/email/AccountBackupRestoreTests.java index 47466e716..e88efb1e2 100644 --- a/tests/src/com/android/email/AccountBackupRestoreTests.java +++ b/tests/src/com/android/email/AccountBackupRestoreTests.java @@ -231,6 +231,7 @@ public class AccountBackupRestoreTests extends ProviderTestCase2 } else { fail("Unexpected restore account name=" + restored.getDisplayName()); } + checkRestoredTransientValues(restored); } } finally { c.close(); @@ -266,12 +267,27 @@ public class AccountBackupRestoreTests extends ProviderTestCase2 } else { fail("Unexpected restore account name=" + restored.getDisplayName()); } + checkRestoredTransientValues(restored); } } finally { c.close(); } } + /** + * Check a given restored account to make sure that transient (non-backed-up) values + * are initialized to reasonable values. + */ + private void checkRestoredTransientValues(EmailContent.Account restored) { + // sync key == null + assertNull(restored.mSyncKey); + // hostauth id's are no longer zero or -1 + assertTrue(restored.mHostAuthKeyRecv > 0); + assertTrue(restored.mHostAuthKeySend > 0); + // protocol version == null or non-empty string + assertTrue(restored.mProtocolVersion == null || restored.mProtocolVersion.length() > 0); + } + /** * TODO: Test restore EAS accounts, with and without contacts sync * diff --git a/tests/src/com/android/email/LegacyConversionsTests.java b/tests/src/com/android/email/LegacyConversionsTests.java index e487ffeac..d7133276c 100644 --- a/tests/src/com/android/email/LegacyConversionsTests.java +++ b/tests/src/com/android/email/LegacyConversionsTests.java @@ -555,7 +555,7 @@ public class LegacyConversionsTests extends ProviderTestCase2 { throws MessagingException { assertEquals(tag + " description", expect.getDescription(), actual.mDisplayName); assertEquals(tag + " email", expect.getEmail(), actual.mEmailAddress); - assertEquals(tag + " sync key", "", actual.mSyncKey); + assertEquals(tag + " sync key", null, actual.mSyncKey); assertEquals(tag + " lookback", expect.getSyncWindow(), actual.mSyncLookback); assertEquals(tag + " sync intvl", expect.getAutomaticCheckIntervalMinutes(), actual.mSyncInterval);