From de7d21c10a6e829dae1b3a00618b871313d5a433 Mon Sep 17 00:00:00 2001 From: Andrew Stadler Date: Fri, 19 Feb 2010 15:50:04 -0800 Subject: [PATCH] Fix various problems with SyncManagerAccountTests 1. Destructive of existing user accounts in device 2. Can get confused (miscounting) due to existing user accounts 3. Cleaned up use of context and mock context 4. Disallow account backup and account security updates when testing 5. Make account manager removeAccount() calls blocking, so the test does not proceed until the accounts are really deleted. Bug: 2454870 --- src/com/android/exchange/SyncManager.java | 66 +++++++++++---- .../exchange/SyncManagerAccountTests.java | 81 ++++++++++++++++--- 2 files changed, 121 insertions(+), 26 deletions(-) diff --git a/src/com/android/exchange/SyncManager.java b/src/com/android/exchange/SyncManager.java index d2e7121cf..8ecbb675c 100644 --- a/src/com/android/exchange/SyncManager.java +++ b/src/com/android/exchange/SyncManager.java @@ -48,7 +48,10 @@ import org.apache.http.params.BasicHttpParams; import org.apache.http.params.HttpParams; import android.accounts.AccountManager; +import android.accounts.AccountManagerFuture; +import android.accounts.AuthenticatorException; import android.accounts.OnAccountsUpdateListener; +import android.accounts.OperationCanceledException; import android.app.AlarmManager; import android.app.PendingIntent; import android.app.Service; @@ -85,16 +88,10 @@ import java.io.File; import java.io.FileReader; import java.io.FileWriter; import java.io.IOException; -import java.security.KeyManagementException; -import java.security.NoSuchAlgorithmException; import java.util.ArrayList; import java.util.HashMap; import java.util.List; -import javax.net.ssl.SSLContext; -import javax.net.ssl.TrustManager; -import javax.net.ssl.X509TrustManager; - /** * The SyncManager handles all aspects of starting, maintaining, and stopping the various sync * adapters used by Exchange. However, it is capable of handing any kind of email sync, and it @@ -858,12 +855,24 @@ public class SyncManager extends Service implements Runnable { } } + /** + * The reconciler (which is called from this listener) can make blocking calls back into + * the account manager. So, in this callback we spin up a worker thread to call the + * reconciler. + */ public class EasAccountsUpdatedListener implements OnAccountsUpdateListener { - public void onAccountsUpdated(android.accounts.Account[] accounts) { - reconcileAccountsWithAccountManager(INSTANCE, getAccountList(), - AccountManager.get(INSTANCE).getAccountsByType( - Email.EXCHANGE_ACCOUNT_MANAGER_TYPE)); - } + public void onAccountsUpdated(android.accounts.Account[] accounts) { + new Thread() { + @Override + public void run() { + android.accounts.Account[] accountMgrList = AccountManager.get(INSTANCE) + .getAccountsByType(Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); + AccountList providerList = getAccountList(); + reconcileAccountsWithAccountManager(INSTANCE, + providerList, accountMgrList, false, mResolver); + } + }.start(); + } } static public void smLog(String str) { @@ -1365,9 +1374,19 @@ public class SyncManager extends Service implements Runnable { * * Note that the duplication of account information is caused by the Email application's * incomplete integration with AccountManager. + * + * This function may not be called from the main/UI thread, because it makes blocking calls + * into the account manager. + * + * @param context The context in which to operate + * @param cachedEasAccounts the exchange provider accounts to work from + * @param accountManagerAccounts The account manager accounts to work from + * @param blockExternalChanges FOR TESTING ONLY - block backups, security changes, etc. + * @param resolver the content resolver for making provider updates (injected for testability) */ - /*package*/ void reconcileAccountsWithAccountManager(Context context, - List cachedEasAccounts, android.accounts.Account[] accountManagerAccounts) { + void reconcileAccountsWithAccountManager(Context context, + List cachedEasAccounts, android.accounts.Account[] accountManagerAccounts, + boolean blockExternalChanges, ContentResolver resolver) { // First, look through our cached EAS Accounts (from EmailProvider) to make sure there's a // corresponding AccountManager account boolean accountsDeleted = false; @@ -1389,7 +1408,7 @@ public class SyncManager extends Service implements Runnable { alwaysLog("Account deleted in AccountManager; deleting from provider: " + providerAccountName); // TODO This will orphan downloaded attachments; need to handle this - mResolver.delete(ContentUris.withAppendedId(Account.CONTENT_URI, + resolver.delete(ContentUris.withAppendedId(Account.CONTENT_URI, providerAccount.mId), null, null); accountsDeleted = true; } @@ -1409,13 +1428,26 @@ public class SyncManager extends Service implements Runnable { alwaysLog("Account deleted from provider; deleting from AccountManager: " + accountManagerAccountName); // Delete the account - AccountManager.get(context).removeAccount(accountManagerAccount, null, null); + AccountManagerFuture blockingResult; + blockingResult = AccountManager.get(context) + .removeAccount(accountManagerAccount, null, null); + try { + // Note: All of the potential errors from removeAccount() are simply logged + // here, as there is nothing to actually do about them. + blockingResult.getResult(); + } catch (OperationCanceledException e) { + Log.d(Email.LOG_TAG, e.toString()); + } catch (AuthenticatorException e) { + Log.d(Email.LOG_TAG, e.toString()); + } catch (IOException e) { + Log.d(Email.LOG_TAG, e.toString()); + } accountsDeleted = true; } } // If we changed the list of accounts, refresh the backup & security settings - if (accountsDeleted) { - AccountBackupRestore.backupAccounts(getContext()); + if (!blockExternalChanges && accountsDeleted) { + AccountBackupRestore.backupAccounts(context); SecurityPolicy.getInstance(context).reducePolicies(); } } diff --git a/tests/src/com/android/exchange/SyncManagerAccountTests.java b/tests/src/com/android/exchange/SyncManagerAccountTests.java index 75fa6cf6d..6dce51393 100644 --- a/tests/src/com/android/exchange/SyncManagerAccountTests.java +++ b/tests/src/com/android/exchange/SyncManagerAccountTests.java @@ -38,12 +38,12 @@ import android.test.ProviderTestCase2; import java.io.IOException; import java.util.HashMap; +import java.util.HashSet; /** * You can run this entire test case with: * runtest -c com.android.exchange.SyncManagerAccountTests email */ - public class SyncManagerAccountTests extends ProviderTestCase2 { private static final String TEST_ACCOUNT_PREFIX = "__test"; @@ -83,7 +83,7 @@ public class SyncManagerAccountTests extends ProviderTestCase2 { private Account setupProviderAndAccountManagerAccount(String username) { // Note that setupAccount creates the email address username@android.com, so that's what // we need to use for the account manager - createAccountManagerAccount(username + "@android.com"); + createAccountManagerAccount(username + TEST_ACCOUNT_SUFFIX); return ProviderTestUtils.setupAccount(username, true, mMockContext); } @@ -131,11 +131,76 @@ public class SyncManagerAccountTests extends ProviderTestCase2 { return TEST_ACCOUNT_PREFIX + name + TEST_ACCOUNT_SUFFIX; } + /** + * Confirm that the test below is functional (and non-destructive) when there are + * prexisting (non-test) accounts in the account manager. + */ + public void testTestReconcileAccounts() { + Account firstAccount = null; + final String TEST_USER_ACCOUNT = "__user_account_test_1"; + Context context = getContext(); + try { + // Note: Unlike calls to setupProviderAndAccountManagerAccount(), we are creating + // *real* accounts here (not in the mock provider) + createAccountManagerAccount(TEST_USER_ACCOUNT + TEST_ACCOUNT_SUFFIX); + firstAccount = ProviderTestUtils.setupAccount(TEST_USER_ACCOUNT, true, context); + // Now run the test with the "user" accounts in place + testReconcileAccounts(); + } finally { + if (firstAccount != null) { + boolean firstAccountFound = false; + // delete the provider account + context.getContentResolver().delete(firstAccount.getUri(), null, null); + // delete the account manager account + android.accounts.Account[] accountManagerAccounts = AccountManager.get(context) + .getAccountsByType(Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); + for (android.accounts.Account accountManagerAccount: accountManagerAccounts) { + if ((TEST_USER_ACCOUNT + TEST_ACCOUNT_SUFFIX) + .equals(accountManagerAccount.name)) { + deleteAccountManagerAccount(context, accountManagerAccount); + firstAccountFound = true; + } + } + assertTrue(firstAccountFound); + } + } + } + + /** + * Helper to retrieve account manager accounts *and* remove any preexisting accounts + * from the list, to "hide" them from the reconciler. + */ + private android.accounts.Account[] getAccountManagerAccounts(Context context, + android.accounts.Account[] baseline) { + android.accounts.Account[] rawList = + AccountManager.get(context).getAccountsByType(Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); + if (baseline.length == 0) { + return rawList; + } + HashSet set = new HashSet(); + for (android.accounts.Account addAccount : rawList) { + set.add(addAccount); + } + for (android.accounts.Account removeAccount : baseline) { + set.remove(removeAccount); + } + return set.toArray(new android.accounts.Account[0]); + } + + /** + * Note, there is some inherent risk in this test, as it creates *real* accounts in the + * system (it cannot use the mock context with the Account Manager). + */ public void testReconcileAccounts() { // Note that we can't use mMockContext for AccountManager interactions, as it isn't a fully // functional Context. Context context = getContext(); + // Capture the baseline (account manager accounts) so we can measure the changes + // we're making, irrespective of the number of actual accounts, and not destroy them + android.accounts.Account[] baselineAccounts = + AccountManager.get(context).getAccountsByType(Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); + // Set up three accounts, both in AccountManager and in EmailProvider Account firstAccount = setupProviderAndAccountManagerAccount(getTestAccountName("1")); setupProviderAndAccountManagerAccount(getTestAccountName("2")); @@ -144,7 +209,7 @@ public class SyncManagerAccountTests extends ProviderTestCase2 { // Check that they're set up properly assertEquals(3, EmailContent.count(mMockContext, Account.CONTENT_URI, null, null)); android.accounts.Account[] accountManagerAccounts = - AccountManager.get(context).getAccountsByType(Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); + getAccountManagerAccounts(context, baselineAccounts); assertEquals(3, accountManagerAccounts.length); // Delete account "2" from AccountManager @@ -153,8 +218,7 @@ public class SyncManagerAccountTests extends ProviderTestCase2 { deleteAccountManagerAccount(context, removedAccount); // Confirm it's deleted - accountManagerAccounts = - AccountManager.get(context).getAccountsByType(Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); + accountManagerAccounts = getAccountManagerAccounts(context, baselineAccounts); assertEquals(2, accountManagerAccounts.length); // Run the reconciler @@ -162,7 +226,7 @@ public class SyncManagerAccountTests extends ProviderTestCase2 { ContentResolver resolver = mMockContext.getContentResolver(); syncManager.mResolver = resolver; syncManager.reconcileAccountsWithAccountManager(context, - makeSyncManagerAccountList(), accountManagerAccounts); + makeSyncManagerAccountList(), accountManagerAccounts, true, resolver); // There should now be only two EmailProvider accounts assertEquals(2, EmailContent.count(mMockContext, Account.CONTENT_URI, null, null)); @@ -175,11 +239,10 @@ public class SyncManagerAccountTests extends ProviderTestCase2 { // Run the reconciler syncManager.reconcileAccountsWithAccountManager(context, - makeSyncManagerAccountList(), accountManagerAccounts); + makeSyncManagerAccountList(), accountManagerAccounts, true, resolver); // There should now be only one AccountManager account - accountManagerAccounts = AccountManager.get(getContext()).getAccountsByType( - Email.EXCHANGE_ACCOUNT_MANAGER_TYPE); + accountManagerAccounts = getAccountManagerAccounts(context, baselineAccounts); assertEquals(1, accountManagerAccounts.length); // ... and it should be account "3" assertEquals(getTestAccountEmailAddress("3"), accountManagerAccounts[0].name);