From 4e4aba9ebc43c6a83190f3a883fa05bb7d5100b3 Mon Sep 17 00:00:00 2001 From: Marc Blank Date: Mon, 18 Jul 2011 17:33:38 -0700 Subject: [PATCH] Clean up account reconciliation * Move AccountReconciler to the Email app (from EmailCommon) * Ensure that Controller.deleteAccountSync() performs ALL actions needed to clean up after an account deletion (delete attachment files, reset policies, refresh the UI, etc.) * Add reconcileAccounts() API to AccountService * Remove accountDeleted() and restoreAccountsIfNeede() from the AccountService API * Remove unused callback Bug: 4883073 Bug: 4767084 Change-Id: I43ffaf009db1a6f306bb0f2a74fb4dd3b2c4b966 --- .../service/AccountServiceProxy.java | 18 ++----- .../emailcommon/service/IAccountService.aidl | 3 +- src/com/android/email/Controller.java | 35 ++++---------- .../ControllerResultUiThreadWrapper.java | 15 ++---- .../email/provider}/AccountReconciler.java | 20 ++++---- .../android/email/service/AccountService.java | 48 ++++++++++++++----- .../android/email/service/MailService.java | 32 +++---------- .../android/email/SecurityPolicyTests.java | 37 ++++---------- .../android/email/provider/ProviderTests.java | 1 - .../email/service/MailServiceTests.java | 9 +--- 10 files changed, 78 insertions(+), 140 deletions(-) rename {emailcommon/src/com/android/emailcommon/utility => src/com/android/email/provider}/AccountReconciler.java (93%) diff --git a/emailcommon/src/com/android/emailcommon/service/AccountServiceProxy.java b/emailcommon/src/com/android/emailcommon/service/AccountServiceProxy.java index 8270e4a57..8f26dbc73 100644 --- a/emailcommon/src/com/android/emailcommon/service/AccountServiceProxy.java +++ b/emailcommon/src/com/android/emailcommon/service/AccountServiceProxy.java @@ -65,25 +65,13 @@ public class AccountServiceProxy extends ServiceProxy implements IAccountService } @Override - public void accountDeleted() { + public void reconcileAccounts(final String protocol, final String accountManagerType) { setTask(new ProxyTask() { @Override public void run() throws RemoteException { - mService.accountDeleted(); + mService.reconcileAccounts(protocol, accountManagerType); } - }, "accountDeleted"); - } - - // The following call is synchronous, and should not be made from the UI thread - @Override - public void restoreAccountsIfNeeded() { - setTask(new ProxyTask() { - @Override - public void run() throws RemoteException { - mService.restoreAccountsIfNeeded(); - } - }, "restoreAccountsIfNeeded"); - waitForCompletion(); + }, "reconcileAccounts"); } // The following call is synchronous, and should not be made from the UI thread diff --git a/emailcommon/src/com/android/emailcommon/service/IAccountService.aidl b/emailcommon/src/com/android/emailcommon/service/IAccountService.aidl index e64d4302b..a29baf58c 100644 --- a/emailcommon/src/com/android/emailcommon/service/IAccountService.aidl +++ b/emailcommon/src/com/android/emailcommon/service/IAccountService.aidl @@ -22,8 +22,7 @@ interface IAccountService { oneway void notifyLoginFailed(long accountId); oneway void notifyLoginSucceeded(long accountId); - void accountDeleted(); - void restoreAccountsIfNeeded(); + void reconcileAccounts(String protocol, String accountManagerType); int getAccountColor(long accountId); diff --git a/src/com/android/email/Controller.java b/src/com/android/email/Controller.java index 0586b4618..015b4c3ff 100644 --- a/src/com/android/email/Controller.java +++ b/src/com/android/email/Controller.java @@ -28,11 +28,13 @@ import android.os.Bundle; import android.os.IBinder; import android.os.RemoteCallbackList; import android.os.RemoteException; +import android.test.IsolatedContext; import android.util.Log; import com.android.email.mail.store.Pop3Store.Pop3Message; import com.android.email.provider.AccountBackupRestore; import com.android.email.service.EmailServiceUtils; +import com.android.email.service.MailService; import com.android.emailcommon.Api; import com.android.emailcommon.Logging; import com.android.emailcommon.mail.AuthenticationFailedException; @@ -1119,22 +1121,14 @@ public class Controller { * Delete an account. */ public void deleteAccount(final long accountId) { - Utility.runAsync(new Runnable() { + EmailAsyncTask.runAsyncParallel(new Runnable() { + @Override public void run() { deleteAccountSync(accountId, mProviderContext); } }); } - /** - * Backup our accounts; define this here so that unit tests can override the behavior - * @param context the caller's context - */ - @VisibleForTesting - protected void backupAccounts(Context context) { - AccountBackupRestore.backup(context); - } - /** * Delete an account synchronously. */ @@ -1154,20 +1148,17 @@ public class Controller { Uri uri = ContentUris.withAppendedId(Account.CONTENT_URI, accountId); context.getContentResolver().delete(uri, null, null); - backupAccounts(context); + // For unit tests, don't run backup, security, and ui pieces + if (context instanceof IsolatedContext) return; - // Release or relax device administration, if relevant + // Clean up + AccountBackupRestore.backup(context); SecurityPolicy.getInstance(context).reducePolicies(); - Email.setServicesEnabledSync(context); + Email.setNotifyUiAccountsChanged(true); + MailService.actionReschedule(context); } catch (Exception e) { Log.w(Logging.LOG_TAG, "Exception while deleting account", e); - } finally { - synchronized (mListeners) { - for (Result l : mListeners) { - l.deleteAccountCallback(accountId); - } - } } } @@ -1329,12 +1320,6 @@ public class Controller { public void sendMailCallback(MessagingException result, long accountId, long messageId, int progress) { } - - /** - * Callback from {@link Controller#deleteAccount}. - */ - public void deleteAccountCallback(long accountId) { - } } /** diff --git a/src/com/android/email/ControllerResultUiThreadWrapper.java b/src/com/android/email/ControllerResultUiThreadWrapper.java index 9a33b929f..f87c0525d 100644 --- a/src/com/android/email/ControllerResultUiThreadWrapper.java +++ b/src/com/android/email/ControllerResultUiThreadWrapper.java @@ -16,13 +16,13 @@ package com.android.email; +import android.os.Handler; + import com.android.email.Controller.Result; import com.android.emailcommon.mail.MessagingException; import java.util.ArrayList; -import android.os.Handler; - /** * A {@link Result} that wraps another {@link Result} and makes sure methods gets called back * on the UI thread. @@ -129,14 +129,5 @@ public class ControllerResultUiThreadWrapper extends Result { } }); } - - @Override - public void deleteAccountCallback(final long accountId) { - run(new Runnable() { - public void run() { - if (!isRegistered()) return; - mWrappee.deleteAccountCallback(accountId); - } - }); - } } + \ No newline at end of file diff --git a/emailcommon/src/com/android/emailcommon/utility/AccountReconciler.java b/src/com/android/email/provider/AccountReconciler.java similarity index 93% rename from emailcommon/src/com/android/emailcommon/utility/AccountReconciler.java rename to src/com/android/email/provider/AccountReconciler.java index dd3865833..5126e73c9 100644 --- a/emailcommon/src/com/android/emailcommon/utility/AccountReconciler.java +++ b/src/com/android/email/provider/AccountReconciler.java @@ -14,21 +14,20 @@ * limitations under the License. */ -package com.android.emailcommon.utility; - -import com.android.emailcommon.Logging; -import com.android.emailcommon.provider.Account; -import com.google.common.annotations.VisibleForTesting; +package com.android.email.provider; import android.accounts.AccountManager; import android.accounts.AccountManagerFuture; import android.accounts.AuthenticatorException; import android.accounts.OperationCanceledException; -import android.content.ContentResolver; -import android.content.ContentUris; import android.content.Context; import android.util.Log; +import com.android.email.Controller; +import com.android.emailcommon.Logging; +import com.android.emailcommon.provider.Account; +import com.google.common.annotations.VisibleForTesting; + import java.io.IOException; import java.util.List; @@ -57,7 +56,7 @@ public class AccountReconciler { */ public static boolean reconcileAccounts(Context context, List emailProviderAccounts, android.accounts.Account[] accountManagerAccounts, - ContentResolver resolver) { + Context providerContext) { // First, look through our EmailProvider accounts to make sure there's a corresponding // AccountManager account boolean accountsDeleted = false; @@ -80,9 +79,8 @@ public class AccountReconciler { Log.d(Logging.LOG_TAG, "Account deleted in AccountManager; deleting from provider: " + providerAccountName); - // TODO This will orphan downloaded attachments; need to handle this - resolver.delete(ContentUris.withAppendedId(Account.CONTENT_URI, - providerAccount.mId), null, null); + Controller.getInstance(context).deleteAccountSync(providerAccount.mId, + providerContext); accountsDeleted = true; } } diff --git a/src/com/android/email/service/AccountService.java b/src/com/android/email/service/AccountService.java index e28ec422b..88bd6461b 100644 --- a/src/com/android/email/service/AccountService.java +++ b/src/com/android/email/service/AccountService.java @@ -16,22 +16,27 @@ 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.Email; import com.android.email.NotificationController; import com.android.email.ResourceHelper; import com.android.email.VendorPolicyLoader; +import com.android.email.provider.AccountReconciler; import com.android.emailcommon.Configuration; import com.android.emailcommon.Device; +import com.android.emailcommon.provider.Account; import com.android.emailcommon.service.IAccountService; import com.android.emailcommon.utility.EmailAsyncTask; -import android.app.Service; -import android.content.Context; -import android.content.Intent; -import android.os.Bundle; -import android.os.IBinder; - import java.io.IOException; +import java.util.ArrayList; public class AccountService extends Service { @@ -50,16 +55,33 @@ public class AccountService extends Service { NotificationController.getInstance(mContext).cancelLoginFailedNotification(accountId); } - @Override - public void restoreAccountsIfNeeded() { - // Obsolete -- this is done 100% transparently in EmailProvider. - // We leave the method because we don't want to change the service interface. - // (It may be okay to remove it, but we're not sure at this point.) + 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 accountDeleted() { - MailService.accountDeleted(mContext); + 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); } @Override diff --git a/src/com/android/email/service/MailService.java b/src/com/android/email/service/MailService.java index 4ed27d8e6..daa09bcf8 100644 --- a/src/com/android/email/service/MailService.java +++ b/src/com/android/email/service/MailService.java @@ -38,16 +38,14 @@ import android.util.Log; import com.android.email.Controller; import com.android.email.Email; import com.android.email.Preferences; -import com.android.email.SecurityPolicy; import com.android.email.SingleRunningTask; -import com.android.email.provider.AccountBackupRestore; +import com.android.email.provider.AccountReconciler; import com.android.emailcommon.AccountManagerTypes; import com.android.emailcommon.mail.MessagingException; import com.android.emailcommon.provider.Account; import com.android.emailcommon.provider.EmailContent; import com.android.emailcommon.provider.HostAuth; import com.android.emailcommon.provider.Mailbox; -import com.android.emailcommon.utility.AccountReconciler; import com.android.emailcommon.utility.EmailAsyncTask; import com.google.common.annotations.VisibleForTesting; @@ -703,7 +701,7 @@ public class MailService extends Service { .getAccountsByType(AccountManagerTypes.TYPE_POP_IMAP); ArrayList providerAccounts = getPopImapAccountList(context); MailService.reconcileAccountsWithAccountManager(context, providerAccounts, - accountManagerAccounts, false, context.getContentResolver()); + accountManagerAccounts, context); } }; @@ -715,38 +713,20 @@ public class MailService extends Service { sReconcilePopImapAccountsSyncExecutor.run(context); } - /** - * Handles a variety of cleanup actions that must be performed when an account has been deleted. - * This includes triggering an account backup, ensuring that security policies are properly - * reset, if necessary, notifying the UI of the change, and resetting scheduled syncs and - * notifications. - * @param context the caller's context - */ - public static void accountDeleted(Context context) { - AccountBackupRestore.backup(context); - SecurityPolicy.getInstance(context).reducePolicies(); - Email.setNotifyUiAccountsChanged(true); - MailService.actionReschedule(context); - } - /** * 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 blockExternalChanges FOR TESTING ONLY - block backups, security changes, etc. - * @param resolver the content resolver for making provider updates (injected for testability) + * @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, - boolean blockExternalChanges, ContentResolver resolver) { - boolean accountsDeleted = AccountReconciler.reconcileAccounts(context, - emailProviderAccounts, accountManagerAccounts, resolver); - // If we changed the list of accounts, refresh the backup & security settings - if (!blockExternalChanges && accountsDeleted) { - accountDeleted(context); - } + Context providerContext) { + AccountReconciler.reconcileAccounts(context, emailProviderAccounts, accountManagerAccounts, + providerContext); } public static void setupAccountManagerAccount(Context context, Account account, diff --git a/tests/src/com/android/email/SecurityPolicyTests.java b/tests/src/com/android/email/SecurityPolicyTests.java index 50c9cbacd..ae1157563 100644 --- a/tests/src/com/android/email/SecurityPolicyTests.java +++ b/tests/src/com/android/email/SecurityPolicyTests.java @@ -307,17 +307,6 @@ public class SecurityPolicyTests extends ProviderTestCase2 { assertEquals(Account.FLAGS_VIBRATE_ALWAYS, a2b.mFlags); } - private static class MockController extends Controller { - protected MockController(Context context) { - super(context); - } - - @Override - protected void backupAccounts(Context context) { - // For testing, we don't want to back up our accounts - } - } - /** * Test the response to disabling DeviceAdmin status */ @@ -352,23 +341,15 @@ public class SecurityPolicyTests extends ProviderTestCase2 { assertNull(a3a.mSecuritySyncKey); assertTrue(a3a.mPolicyKey == 0); - // Simulate revoke of device admin; directly call deleteSecuredAccounts, which is normally - // called from a background thread - MockController mockController = new MockController(mMockContext); - Controller.injectMockControllerForTest(mockController); - try { - mSecurityPolicy.deleteSecuredAccounts(mMockContext); - Policy after2 = mSecurityPolicy.getAggregatePolicy(); - assertEquals(EMPTY_POLICY, after2); - Account a1b = Account.restoreAccountWithId(mMockContext, a1.mId); - assertNull(a1b); - Account a2b = Account.restoreAccountWithId(mMockContext, a2.mId); - assertNull(a2b); - Account a3b = Account.restoreAccountWithId(mMockContext, a3.mId); - assertNull(a3b.mSecuritySyncKey); - } finally { - Controller.injectMockControllerForTest(null); - } + mSecurityPolicy.deleteSecuredAccounts(mMockContext); + Policy after2 = mSecurityPolicy.getAggregatePolicy(); + assertEquals(EMPTY_POLICY, after2); + Account a1b = Account.restoreAccountWithId(mMockContext, a1.mId); + assertNull(a1b); + Account a2b = Account.restoreAccountWithId(mMockContext, a2.mId); + assertNull(a2b); + Account a3b = Account.restoreAccountWithId(mMockContext, a3.mId); + assertNull(a3b.mSecuritySyncKey); } /** diff --git a/tests/src/com/android/email/provider/ProviderTests.java b/tests/src/com/android/email/provider/ProviderTests.java index b268d2ffa..0377a6cb0 100644 --- a/tests/src/com/android/email/provider/ProviderTests.java +++ b/tests/src/com/android/email/provider/ProviderTests.java @@ -50,7 +50,6 @@ import com.android.emailcommon.provider.EmailContent.PolicyColumns; import com.android.emailcommon.provider.HostAuth; import com.android.emailcommon.provider.Mailbox; import com.android.emailcommon.provider.Policy; -import com.android.emailcommon.utility.AccountReconciler; import com.android.emailcommon.utility.TextUtilities; import com.android.emailcommon.utility.Utility; diff --git a/tests/src/com/android/email/service/MailServiceTests.java b/tests/src/com/android/email/service/MailServiceTests.java index a2d93e960..7e7f8c035 100644 --- a/tests/src/com/android/email/service/MailServiceTests.java +++ b/tests/src/com/android/email/service/MailServiceTests.java @@ -68,11 +68,6 @@ public class MailServiceTests extends AccountTestCase { super.tearDown(); // Delete any test accounts we might have created earlier deleteTemporaryAccountManagerAccounts(); - PackageManager pm = getContext().getPackageManager(); - pm.setComponentEnabledSetting( - new ComponentName(getContext(), EasTestAuthenticatorService.class), - PackageManager.COMPONENT_ENABLED_STATE_DISABLED, - PackageManager.DONT_KILL_APP); } /** @@ -147,7 +142,7 @@ public class MailServiceTests extends AccountTestCase { // Run the reconciler ContentResolver resolver = mMockContext.getContentResolver(); MailService.reconcileAccountsWithAccountManager(context, - makeExchangeServiceAccountList(), accountManagerAccounts, true, resolver); + makeExchangeServiceAccountList(), accountManagerAccounts, mMockContext); // There should now be only two EmailProvider accounts assertEquals(2, EmailContent.count(mMockContext, Account.CONTENT_URI, null, null)); @@ -160,7 +155,7 @@ public class MailServiceTests extends AccountTestCase { // Run the reconciler MailService.reconcileAccountsWithAccountManager(context, - makeExchangeServiceAccountList(), accountManagerAccounts, true, resolver); + makeExchangeServiceAccountList(), accountManagerAccounts, mMockContext); // There should now be only one AccountManager account accountManagerAccounts = getAccountManagerAccounts(baselineAccounts);