From afe097f318a7eeac9d240d2bedbc5caba7640ea8 Mon Sep 17 00:00:00 2001 From: Yu Ping Hu Date: Fri, 26 Jul 2013 14:18:39 -0700 Subject: [PATCH] Simplify account reconciliation. - Rather than handle by type, do them all at once. - Simplify when reconciliation happens. Bug: 9056861 Change-Id: If264678c82c63090246ef8ff857c8e46f6672c85 --- .../email/provider/AccountReconciler.java | 153 +++++++++++------- .../android/email/service/AccountService.java | 33 +--- .../EmailBroadcastProcessorService.java | 3 +- .../email/service/EmailServiceStub.java | 3 +- .../android/email/service/MailService.java | 82 +--------- .../email/service/MailServiceTests.java | 44 ----- 6 files changed, 99 insertions(+), 219 deletions(-) diff --git a/src/com/android/email/provider/AccountReconciler.java b/src/com/android/email/provider/AccountReconciler.java index 93c510e6a..e888b78cb 100644 --- a/src/com/android/email/provider/AccountReconciler.java +++ b/src/com/android/email/provider/AccountReconciler.java @@ -21,42 +21,64 @@ import android.accounts.AccountManagerFuture; import android.accounts.AuthenticatorException; import android.accounts.OperationCanceledException; import android.content.Context; -import android.net.Uri; +import android.database.Cursor; import com.android.email.NotificationController; +import com.android.email.R; import com.android.emailcommon.Logging; import com.android.emailcommon.provider.Account; import com.android.emailcommon.provider.EmailContent; import com.android.emailcommon.provider.Mailbox; import com.android.mail.utils.LogUtils; -import com.google.common.annotations.VisibleForTesting; +import com.google.common.collect.ImmutableList; import java.io.IOException; +import java.util.Arrays; +import java.util.Collections; import java.util.List; public class AccountReconciler { - // AccountManager accounts with a name beginning with this constant are ignored for purposes - // of reconcilation. This is for unit test purposes only; the caller may NOT be in the same - // package as this class, so we make the constant public. - @VisibleForTesting - static final String ACCOUNT_MANAGER_ACCOUNT_TEST_PREFIX = " _"; + /** + * Get all AccountManager accounts for all email types. + * @param context Our {@link Context}. + * @return A list of all {@link android.accounts.Account}s created by our app. + */ + private static List getAllAmAccounts(final Context context) { + final AccountManager am = AccountManager.get(context); + final ImmutableList.Builder builder = ImmutableList.builder(); + // TODO: Consider getting the types programmatically, in case we add more types. + builder.addAll(Arrays.asList(am.getAccountsByType( + context.getString(R.string.account_manager_type_legacy_imap)))); + builder.addAll(Arrays.asList(am.getAccountsByType( + context.getString(R.string.account_manager_type_pop3)))); + builder.addAll(Arrays.asList(am.getAccountsByType( + context.getString(R.string.account_manager_type_exchange)))); + return builder.build(); + } /** - * Checks two account lists to see if there is any reconciling to be done. Can be done on the - * UI thread. - * @param context the app context - * @param emailProviderAccounts accounts as reported in the Email provider - * @param accountManagerAccounts accounts as reported by the system account manager, for the - * particular protocol types that match emailProviderAccounts + * Get a all {@link Account} objects from the {@link EmailProvider}. + * @param context Our {@link Context}. + * @return A list of all {@link Account}s from the {@link EmailProvider}. */ - public static boolean accountsNeedReconciling( - final Context context, - List emailProviderAccounts, - android.accounts.Account[] accountManagerAccounts) { + private static List getAllEmailProviderAccounts(final Context context) { + final Cursor c = context.getContentResolver().query(Account.CONTENT_URI, + Account.CONTENT_PROJECTION, null, null, null); + if (c == null) { + return Collections.emptyList(); + } - return reconcileAccountsInternal( - context, emailProviderAccounts, accountManagerAccounts, - context, false /* performReconciliation */); + final ImmutableList.Builder builder = ImmutableList.builder(); + try { + while (c.moveToNext()) { + final Account account = new Account(); + account.restore(c); + builder.add(account); + } + } finally { + c.close(); + } + return builder.build(); } /** @@ -71,18 +93,42 @@ public class AccountReconciler { * into the account manager. * * @param context The context in which to operate - * @param emailProviderAccounts the exchange provider accounts to work from - * @param accountManagerAccounts The account manager accounts to work from - * @param providerContext application provider context */ - public static void reconcileAccounts( - Context context, - List emailProviderAccounts, - android.accounts.Account[] accountManagerAccounts, - Context providerContext) { - reconcileAccountsInternal( - context, emailProviderAccounts, accountManagerAccounts, - providerContext, true /* performReconciliation */); + public static void reconcileAccounts(final Context context) { + final List amAccounts = getAllAmAccounts(context); + final List providerAccounts = getAllEmailProviderAccounts(context); + reconcileAccountsInternal(context, providerAccounts, amAccounts, true); + } + + /** + * Check if the AccountManager accounts list contains a specific account. + * @param accounts The list of {@link android.accounts.Account} objects. + * @param name The name of the account to find. + * @return Whether the account is in the list. + */ + private static boolean hasAmAccount(final List accounts, + final String name) { + for (final android.accounts.Account account : accounts) { + if (account.name.equalsIgnoreCase(name)) { + return true; + } + } + return false; + } + + /** + * Check if the EmailProvider accounts list contains a specific account. + * @param accounts The list of {@link Account} objects. + * @param name The name of the account to find. + * @return Whether the account is in the list. + */ + private static boolean hasEpAccount(final List accounts, final String name) { + for (final Account account : accounts) { + if (account.mEmailAddress.equalsIgnoreCase(name)) { + return true; + } + } + return false; } /** @@ -91,31 +137,23 @@ public class AccountReconciler { * {@code performReconciliation}. */ private static boolean reconcileAccountsInternal( - Context context, - List emailProviderAccounts, - android.accounts.Account[] accountManagerAccounts, - Context providerContext, - boolean performReconciliation) { + final Context context, + final List emailProviderAccounts, + final List accountManagerAccounts, + final boolean performReconciliation) { boolean needsReconciling = false; // First, look through our EmailProvider accounts to make sure there's a corresponding // AccountManager account - for (Account providerAccount: emailProviderAccounts) { - String providerAccountName = providerAccount.mEmailAddress; - boolean found = false; - for (android.accounts.Account accountManagerAccount: accountManagerAccounts) { - if (accountManagerAccount.name.equalsIgnoreCase(providerAccountName)) { - found = true; - break; - } - } - if (!found) { + for (final Account providerAccount : emailProviderAccounts) { + final String providerAccountName = providerAccount.mEmailAddress; + if (!hasAmAccount(accountManagerAccounts, providerAccountName)) { if ((providerAccount.mFlags & Account.FLAGS_INCOMPLETE) != 0) { // Do another test before giving up; an incomplete account shouldn't have // any mailboxes (the incomplete flag is used to prevent reconciliation // between the time the EP account is created and when the AM account is // asynchronously created) - if (EmailContent.count(providerContext, Mailbox.CONTENT_URI, + if (EmailContent.count(context, Mailbox.CONTENT_URI, Mailbox.ACCOUNT_KEY + "=?", new String[] { Long.toString(providerAccount.mId) } ) > 0) { LogUtils.w(Logging.LOG_TAG, @@ -133,8 +171,8 @@ public class AccountReconciler { LogUtils.d(Logging.LOG_TAG, "Account deleted in AccountManager; deleting from provider: " + providerAccountName); - Uri uri = EmailProvider.uiUri("uiaccount", providerAccount.mId); - context.getContentResolver().delete(uri, null, null); + context.getContentResolver().delete( + EmailProvider.uiUri("uiaccount", providerAccount.mId), null, null); // Cancel all notifications for this account NotificationController.cancelNotifications(context, providerAccount); @@ -143,18 +181,9 @@ public class AccountReconciler { } // Now, look through AccountManager accounts to make sure we have a corresponding cached EAS // account from EmailProvider - for (android.accounts.Account accountManagerAccount: accountManagerAccounts) { - String accountManagerAccountName = accountManagerAccount.name; - boolean found = false; - for (Account cachedEasAccount: emailProviderAccounts) { - if (cachedEasAccount.mEmailAddress.equalsIgnoreCase(accountManagerAccountName)) { - found = true; - } - } - if (accountManagerAccountName.startsWith(ACCOUNT_MANAGER_ACCOUNT_TEST_PREFIX)) { - found = true; - } - if (!found) { + for (final android.accounts.Account accountManagerAccount : accountManagerAccounts) { + final String accountManagerAccountName = accountManagerAccount.name; + if (!hasEpAccount(emailProviderAccounts, accountManagerAccountName)) { // This account has been deleted from the EmailProvider database needsReconciling = true; @@ -164,7 +193,7 @@ public class AccountReconciler { accountManagerAccountName); // Delete the account AccountManagerFuture blockingResult = AccountManager.get(context) - .removeAccount(accountManagerAccount, null, null); + .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. diff --git a/src/com/android/email/service/AccountService.java b/src/com/android/email/service/AccountService.java index f89645874..fa0eef46d 100644 --- a/src/com/android/email/service/AccountService.java +++ b/src/com/android/email/service/AccountService.java @@ -16,27 +16,22 @@ package com.android.email.service; -import android.accounts.AccountManager; import android.app.Service; import android.content.Context; import android.content.Intent; -import android.database.Cursor; import android.os.Bundle; import android.os.IBinder; import com.android.email.NotificationController; import com.android.email.ResourceHelper; -import com.android.email.provider.AccountReconciler; import com.android.email2.ui.MailActivityEmail; import com.android.emailcommon.Configuration; import com.android.emailcommon.Device; import com.android.emailcommon.VendorPolicyLoader; -import com.android.emailcommon.provider.Account; import com.android.emailcommon.service.IAccountService; import com.android.emailcommon.utility.EmailAsyncTask; import java.io.IOException; -import java.util.ArrayList; public class AccountService extends Service { @@ -56,33 +51,9 @@ public class AccountService extends Service { NotificationController.getInstance(mContext).cancelLoginFailedNotification(accountId); } - private ArrayList getAccountList(String forProtocol) { - ArrayList providerAccounts = new ArrayList(); - Cursor c = mContext.getContentResolver().query(Account.CONTENT_URI, - Account.ID_PROJECTION, null, null, null); - try { - while (c.moveToNext()) { - long accountId = c.getLong(Account.CONTENT_ID_COLUMN); - String protocol = Account.getProtocol(mContext, accountId); - if ((protocol != null) && forProtocol.equals(protocol)) { - Account account = Account.restoreAccountWithId(mContext, accountId); - if (account != null) { - providerAccounts.add(account); - } - } - } - } finally { - c.close(); - } - return providerAccounts; - } - @Override public void reconcileAccounts(String protocol, String accountManagerType) { - ArrayList providerList = getAccountList(protocol); - android.accounts.Account[] accountMgrList = - AccountManager.get(mContext).getAccountsByType(accountManagerType); - AccountReconciler.reconcileAccounts(mContext, providerList, accountMgrList, mContext); + // TODO: No longer used, delete this. } @Override @@ -128,4 +99,4 @@ public class AccountService extends Service { } return mBinder; } -} \ No newline at end of file +} diff --git a/src/com/android/email/service/EmailBroadcastProcessorService.java b/src/com/android/email/service/EmailBroadcastProcessorService.java index e12a97b35..092d7e8a3 100644 --- a/src/com/android/email/service/EmailBroadcastProcessorService.java +++ b/src/com/android/email/service/EmailBroadcastProcessorService.java @@ -32,6 +32,7 @@ import com.android.email.Preferences; import com.android.email.R; import com.android.email.SecurityPolicy; import com.android.email.activity.setup.AccountSettings; +import com.android.email.provider.AccountReconciler; import com.android.emailcommon.Logging; import com.android.emailcommon.VendorPolicyLoader; import com.android.emailcommon.provider.Account; @@ -184,7 +185,7 @@ public class EmailBroadcastProcessorService extends IntentService { private void reconcileAndStartServices() { // Reconcile accounts - MailService.reconcileLocalAccountsSync(this); + AccountReconciler.reconcileAccounts(this); // Starts remote services, if any EmailServiceUtils.startRemoteServices(this); } diff --git a/src/com/android/email/service/EmailServiceStub.java b/src/com/android/email/service/EmailServiceStub.java index b59aebbb2..9089f5026 100644 --- a/src/com/android/email/service/EmailServiceStub.java +++ b/src/com/android/email/service/EmailServiceStub.java @@ -29,6 +29,7 @@ import android.text.TextUtils; import com.android.email.NotificationController; import com.android.email.mail.Sender; import com.android.email.mail.Store; +import com.android.email.provider.AccountReconciler; import com.android.email.provider.Utilities; import com.android.email.service.EmailServiceUtils.EmailServiceInfo; import com.android.email2.ui.MailActivityEmail; @@ -477,7 +478,7 @@ public abstract class EmailServiceStub extends IEmailService.Stub implements IEm @Override public void deleteAccountPIMData(final String emailAddress) throws RemoteException { - MailService.reconcileLocalAccountsSync(mContext); + AccountReconciler.reconcileAccounts(mContext); } @Override diff --git a/src/com/android/email/service/MailService.java b/src/com/android/email/service/MailService.java index 0926bd366..e626cc68b 100644 --- a/src/com/android/email/service/MailService.java +++ b/src/com/android/email/service/MailService.java @@ -22,7 +22,6 @@ import android.accounts.AccountManagerFuture; import android.app.Service; import android.content.Context; import android.content.Intent; -import android.database.Cursor; import android.os.Bundle; import android.os.IBinder; @@ -31,22 +30,17 @@ import com.android.email.service.EmailServiceUtils.EmailServiceInfo; import com.android.email2.ui.MailActivityEmail; import com.android.emailcommon.provider.Account; import com.android.emailcommon.provider.HostAuth; -import com.android.emailcommon.utility.EmailAsyncTask; -import com.android.mail.utils.LogUtils; -import com.google.common.annotations.VisibleForTesting; - -import java.util.ArrayList; -import java.util.List; /** * Legacy service, now used mainly for account reconciliation + * TODO: Eliminate this service, since it doesn't actually do anything. */ public class MailService extends Service { @Override public int onStartCommand(final Intent intent, int flags, final int startId) { super.onStartCommand(intent, flags, startId); - reconcileLocalAccountsSync(this); + AccountReconciler.reconcileAccounts(this); // Make sure our services are running, if necessary MailActivityEmail.setServicesEnabledAsync(this); return START_STICKY; @@ -57,78 +51,6 @@ public class MailService extends Service { return null; } - public static ArrayList getAccountList(Context context, String protocol) { - ArrayList providerAccounts = new ArrayList(); - Cursor c = context.getContentResolver().query(Account.CONTENT_URI, Account.ID_PROJECTION, - null, null, null); - try { - while (c.moveToNext()) { - long accountId = c.getLong(Account.CONTENT_ID_COLUMN); - if (protocol.equals(Account.getProtocol(context, accountId))) { - Account account = Account.restoreAccountWithId(context, accountId); - if (account != null) { - providerAccounts.add(account); - } - } - } - } finally { - c.close(); - } - return providerAccounts; - } - - /** - * Reconcile local (i.e. non-remote) accounts. - */ - public static void reconcileLocalAccountsSync(Context context) { - List serviceList = EmailServiceUtils.getServiceInfoList(context); - for (EmailServiceInfo info: serviceList) { - if (info.klass != null) { - new AccountReconcilerTask(context, info).runAsync(); - } - } - } - - static class AccountReconcilerTask implements Runnable { - private final Context mContext; - private final EmailServiceInfo mInfo; - - AccountReconcilerTask(Context context, EmailServiceInfo info) { - mContext = context; - mInfo = info; - } - - public void runAsync() { - EmailAsyncTask.runAsyncSerial(this); - } - - @Override - public void run() { - LogUtils.d("MailService", "Reconciling accounts of type " + mInfo.accountType + - ", protocol " + mInfo.protocol); - android.accounts.Account[] accountManagerAccounts = AccountManager.get(mContext) - .getAccountsByType(mInfo.accountType); - ArrayList providerAccounts = getAccountList(mContext, mInfo.protocol); - reconcileAccountsWithAccountManager(mContext, providerAccounts, - accountManagerAccounts, mContext); - } - } - - /** - * See Utility.reconcileAccounts for details - * @param context The context in which to operate - * @param emailProviderAccounts the exchange provider accounts to work from - * @param accountManagerAccounts The account manager accounts to work from - * @param providerContext the provider's context (in unit tests, this may differ from context) - */ - @VisibleForTesting - public static void reconcileAccountsWithAccountManager(Context context, - List emailProviderAccounts, android.accounts.Account[] accountManagerAccounts, - Context providerContext) { - AccountReconciler.reconcileAccounts(context, emailProviderAccounts, accountManagerAccounts, - providerContext); - } - public static AccountManagerFuture setupAccountManagerAccount(Context context, Account account, boolean email, boolean calendar, boolean contacts, AccountManagerCallback callback) { diff --git a/tests/src/com/android/email/service/MailServiceTests.java b/tests/src/com/android/email/service/MailServiceTests.java index f716e314a..90f2d3060 100644 --- a/tests/src/com/android/email/service/MailServiceTests.java +++ b/tests/src/com/android/email/service/MailServiceTests.java @@ -167,50 +167,6 @@ public class MailServiceTests extends AccountTestCase { assertEquals(getTestAccountEmailAddress("3"), accountManagerAccounts[0].name); } - public void testReconcileDetection() { - Context context = getContext(); - List providerAccounts; - android.accounts.Account[] accountManagerAccounts; - - android.accounts.Account[] baselineAccounts = - AccountManager.get(context).getAccountsByType(TEST_ACCOUNT_TYPE); - - // Empty lists match. - providerAccounts = new ArrayList(); - accountManagerAccounts = new android.accounts.Account[0]; - assertFalse(AccountReconciler.accountsNeedReconciling( - context, providerAccounts, accountManagerAccounts)); - - setupProviderAndAccountManagerAccount(getTestAccountName("1")); - accountManagerAccounts = getAccountManagerAccounts(baselineAccounts); - providerAccounts = makeExchangeServiceAccountList(); - - // A single account, but empty list on the other side is detected as needing reconciliation - assertTrue(AccountReconciler.accountsNeedReconciling( - context, new ArrayList(), accountManagerAccounts)); - assertTrue(AccountReconciler.accountsNeedReconciling( - context, providerAccounts, new android.accounts.Account[0])); - - // Note that no reconciliation should have happened though - we just wanted to detect it. - assertEquals(1, makeExchangeServiceAccountList().size()); - assertEquals(1, getAccountManagerAccounts(baselineAccounts).length); - - // Single account matches - no reconciliation should be detected. - assertFalse(AccountReconciler.accountsNeedReconciling( - context, providerAccounts, accountManagerAccounts)); - - // Provider: 1,2,3. AccountManager: 1, 3. - String username = getTestAccountName("2"); - ProviderTestUtils.setupAccount(getTestAccountName("2"), true, getMockContext()); - setupProviderAndAccountManagerAccount(getTestAccountName("3")); - - accountManagerAccounts = getAccountManagerAccounts(baselineAccounts); - providerAccounts = makeExchangeServiceAccountList(); - assertTrue(AccountReconciler.accountsNeedReconciling( - context, providerAccounts, accountManagerAccounts)); - } - - /** * Lightweight subclass of the Controller class allows injection of mock context */