From 9dad9ad973ccf8255228a41acc0ee78af989a651 Mon Sep 17 00:00:00 2001 From: Makoto Onuki Date: Mon, 20 Jun 2011 18:10:10 -0700 Subject: [PATCH] Move restore account to EmailProvider. In order to reduce the UI initialization code, move let EmailProvider restore accounts when it opens the DB. Backup can be moved too, but I just leave it as a TODO. Change-Id: Id5c1810904db6abaecbfecbaa8d2d53834ebf07b --- src/com/android/email/activity/Welcome.java | 7 -- .../email/provider/AccountBackupRestore.java | 48 +--------- .../android/email/provider/EmailProvider.java | 89 +++++++++++++++---- .../android/email/service/AccountService.java | 5 +- .../android/email/service/MailService.java | 3 - .../provider/AccountBackupRestoreTests.java | 6 +- 6 files changed, 81 insertions(+), 77 deletions(-) diff --git a/src/com/android/email/activity/Welcome.java b/src/com/android/email/activity/Welcome.java index 78cfe9195..6770394c1 100644 --- a/src/com/android/email/activity/Welcome.java +++ b/src/com/android/email/activity/Welcome.java @@ -162,13 +162,6 @@ public class Welcome extends Activity { // Reset the "accounts changed" notification, now that we're here Email.setNotifyUiAccountsChanged(false); - // Restore accounts, if it has not happened already - // NOTE: This is blocking, which it should not be (in the UI thread) - // We're going to live with this for the short term and replace with something - // smarter. Long-term fix: Move this, and most of the code below, to an AsyncTask - // and do the DB work in a thread. Then post handler to finish() as appropriate. - AccountBackupRestore.restoreIfNeeded(this); - // Because the app could be reloaded (for debugging, etc.), we need to make sure that // ExchangeService gets a chance to start. There is no harm to starting it if it has // already been started diff --git a/src/com/android/email/provider/AccountBackupRestore.java b/src/com/android/email/provider/AccountBackupRestore.java index 16c16002b..f9217fb2b 100644 --- a/src/com/android/email/provider/AccountBackupRestore.java +++ b/src/com/android/email/provider/AccountBackupRestore.java @@ -16,16 +16,8 @@ package com.android.email.provider; -import com.android.email.Email; -import com.android.email.Preferences; -import com.android.emailcommon.Logging; -import com.android.emailcommon.provider.Account; -import com.android.emailcommon.provider.EmailContent; - import android.content.ContentResolver; import android.content.Context; -import android.text.TextUtils; -import android.util.Log; /** * Helper class to facilitate EmailProvider's account backup/restore facility. @@ -52,45 +44,11 @@ public class AccountBackupRestore { /** * Backup user Account and HostAuth data into our backup database + * + * TODO Make EmailProvider do this automatically. */ public static void backup(Context context) { ContentResolver resolver = context.getContentResolver(); - int numBackedUp = resolver.update(EmailProvider.ACCOUNT_BACKUP_URI, null, null, null); - if (numBackedUp < 0) { - Log.e(Logging.LOG_TAG, "Account backup failed!"); - } else if (Email.DEBUG) { - Log.d(Logging.LOG_TAG, "Backed up " + numBackedUp + " accounts..."); - } - } - - /** - * Restore user Account and HostAuth data from our backup database - */ - public static void restoreIfNeeded(Context context) { - if (sBackupsChecked) return; - - // Check for legacy backup - String legacyBackup = Preferences.getLegacyBackupPreference(context); - // If there's a legacy backup, create a new-style backup and delete the legacy backup - // In the 1:1000000000 chance that the user gets an app update just as his database becomes - // corrupt, oh well... - if (!TextUtils.isEmpty(legacyBackup)) { - backup(context); - Preferences.clearLegacyBackupPreference(context); - Log.w(Logging.LOG_TAG, "Created new EmailProvider backup database"); - } - - // If we have accounts, we're done - if (EmailContent.count(context, Account.CONTENT_URI) > 0) return; - ContentResolver resolver = context.getContentResolver(); - int numRecovered = resolver.update(EmailProvider.ACCOUNT_RESTORE_URI, null, null, null); - if (numRecovered > 0) { - Log.e(Logging.LOG_TAG, "Recovered " + numRecovered + " accounts!"); - } else if (numRecovered < 0) { - Log.e(Logging.LOG_TAG, "Account recovery failed?"); - } else if (Email.DEBUG) { - Log.d(Logging.LOG_TAG, "No accounts to restore..."); - } - sBackupsChecked = true; + resolver.update(EmailProvider.ACCOUNT_BACKUP_URI, null, null, null); } } diff --git a/src/com/android/email/provider/EmailProvider.java b/src/com/android/email/provider/EmailProvider.java index d3d043d25..dfc90cc10 100644 --- a/src/com/android/email/provider/EmailProvider.java +++ b/src/com/android/email/provider/EmailProvider.java @@ -17,6 +17,7 @@ package com.android.email.provider; import com.android.email.Email; +import com.android.email.Preferences; import com.android.email.provider.ContentCache.CacheToken; import com.android.email.service.AttachmentDownloadService; import com.android.emailcommon.AccountManagerTypes; @@ -60,6 +61,7 @@ import android.database.sqlite.SQLiteException; import android.database.sqlite.SQLiteOpenHelper; import android.net.Uri; import android.provider.ContactsContract; +import android.text.TextUtils; import android.util.Log; import java.io.File; @@ -86,8 +88,6 @@ public class EmailProvider extends ContentProvider { Uri.parse("content://" + EmailContent.AUTHORITY + "/integrityCheck"); public static final Uri ACCOUNT_BACKUP_URI = Uri.parse("content://" + EmailContent.AUTHORITY + "/accountBackup"); - public static final Uri ACCOUNT_RESTORE_URI = - Uri.parse("content://" + EmailContent.AUTHORITY + "/accountRestore"); /** Appended to the notification URI for delete operations */ public static final String NOTIFICATION_OP_DELETE = "delete"; @@ -724,7 +724,8 @@ public class EmailProvider extends ContentProvider { private SQLiteDatabase mDatabase; private SQLiteDatabase mBodyDatabase; - public synchronized SQLiteDatabase getDatabase(Context context) { + @VisibleForTesting + synchronized SQLiteDatabase getDatabase(Context context) { // Always return the cached database, if we've got one if (mDatabase != null) { return mDatabase; @@ -745,6 +746,11 @@ public class EmailProvider extends ContentProvider { String bodyFileName = mBodyDatabase.getPath(); mDatabase.execSQL("attach \"" + bodyFileName + "\" as BodyDatabase"); } + + // Restore accounts if the database is corrupted... + restoreIfNeeded(context, mDatabase); + } else { + Log.w(TAG, "getWritableDatabase returned null!"); } // Check for any orphaned Messages in the updated/deleted tables @@ -755,10 +761,41 @@ public class EmailProvider extends ContentProvider { } /*package*/ static SQLiteDatabase getReadableDatabase(Context context) { - DatabaseHelper helper = new EmailProvider().new DatabaseHelper(context, DATABASE_NAME); + DatabaseHelper helper = new DatabaseHelper(context, DATABASE_NAME); return helper.getReadableDatabase(); } + /** + * Restore user Account and HostAuth data from our backup database + */ + public static void restoreIfNeeded(Context context, SQLiteDatabase mainDatabase) { + if (Email.DEBUG) { + Log.w(TAG, "restoreIfNeeded..."); + } + // Check for legacy backup + String legacyBackup = Preferences.getLegacyBackupPreference(context); + // If there's a legacy backup, create a new-style backup and delete the legacy backup + // In the 1:1000000000 chance that the user gets an app update just as his database becomes + // corrupt, oh well... + if (!TextUtils.isEmpty(legacyBackup)) { + backupAccounts(context, mainDatabase); + Preferences.clearLegacyBackupPreference(context); + Log.w(TAG, "Created new EmailProvider backup database"); + return; + } + + // If we have accounts, we're done + Cursor c = mainDatabase.query(Account.TABLE_NAME, EmailContent.ID_PROJECTION, null, null, + null, null, null); + if (c.moveToFirst()) { + if (Email.DEBUG) { + Log.w(TAG, "restoreIfNeeded: Account exists."); + } + return; // At least one account exists. + } + restoreAccounts(context, mainDatabase); + } + /** {@inheritDoc} */ @Override public void shutdown() { @@ -841,7 +878,7 @@ public class EmailProvider extends ContentProvider { } } - private class DatabaseHelper extends SQLiteOpenHelper { + private static class DatabaseHelper extends SQLiteOpenHelper { Context mContext; DatabaseHelper(Context context, String name) { @@ -1577,7 +1614,7 @@ public class EmailProvider extends ContentProvider { * @param id the unique id (_id) of the row * @return a fully populated HostAuth or null if the row does not exist */ - private HostAuth restoreHostAuth(SQLiteDatabase db, long id) { + private static HostAuth restoreHostAuth(SQLiteDatabase db, long id) { Cursor c = db.query(HostAuth.TABLE_NAME, HostAuth.CONTENT_PROJECTION, HostAuth.RECORD_ID + "=?", new String[] {Long.toString(id)}, null, null, null); try { @@ -1598,7 +1635,7 @@ public class EmailProvider extends ContentProvider { * @param toDatabase the destination database * @return the number of accounts copied, or -1 if an error occurred */ - private int copyAccountTables(SQLiteDatabase fromDatabase, SQLiteDatabase toDatabase) { + private static int copyAccountTables(SQLiteDatabase fromDatabase, SQLiteDatabase toDatabase) { if (fromDatabase == null || toDatabase == null) return -1; int copyCount = 0; try { @@ -1664,7 +1701,7 @@ public class EmailProvider extends ContentProvider { return copyCount; } - private SQLiteDatabase getBackupDatabase(Context context) { + private static SQLiteDatabase getBackupDatabase(Context context) { DatabaseHelper helper = new DatabaseHelper(context, BACKUP_DATABASE_NAME); return helper.getWritableDatabase(); } @@ -1672,11 +1709,19 @@ public class EmailProvider extends ContentProvider { /** * Backup account data, returning the number of accounts backed up */ - private int backupAccounts() { - Context context = getContext(); + private static int backupAccounts(Context context, SQLiteDatabase mainDatabase) { + if (Email.DEBUG) { + Log.d(TAG, "backupAccounts..."); + } SQLiteDatabase backupDatabase = getBackupDatabase(context); try { - return copyAccountTables(getDatabase(context), backupDatabase); + int numBackedUp = copyAccountTables(mainDatabase, backupDatabase); + if (numBackedUp < 0) { + Log.e(TAG, "Account backup failed!"); + } else if (Email.DEBUG) { + Log.d(TAG, "Backed up " + numBackedUp + " accounts..."); + } + return numBackedUp; } finally { if (backupDatabase != null) { backupDatabase.close(); @@ -1687,11 +1732,21 @@ public class EmailProvider extends ContentProvider { /** * Restore account data, returning the number of accounts restored */ - private int restoreAccounts() { - Context context = getContext(); + private static int restoreAccounts(Context context, SQLiteDatabase mainDatabase) { + if (Email.DEBUG) { + Log.d(TAG, "restoreAccounts..."); + } SQLiteDatabase backupDatabase = getBackupDatabase(context); try { - return copyAccountTables(backupDatabase, getDatabase(context)); + int numRecovered = copyAccountTables(backupDatabase, mainDatabase); + if (numRecovered > 0) { + Log.e(TAG, "Recovered " + numRecovered + " accounts!"); + } else if (numRecovered < 0) { + Log.e(TAG, "Account recovery failed?"); + } else if (Email.DEBUG) { + Log.d(TAG, "No accounts to restore..."); + } + return numRecovered; } finally { if (backupDatabase != null) { backupDatabase.close(); @@ -1706,9 +1761,7 @@ public class EmailProvider extends ContentProvider { checkDatabases(); return 0; } else if (uri == ACCOUNT_BACKUP_URI) { - return backupAccounts(); - } else if (uri == ACCOUNT_RESTORE_URI) { - return restoreAccounts(); + return backupAccounts(getContext(), getDatabase(getContext())); } // Notify all existing cursors, except for ACCOUNT_RESET_NEW_COUNT(_ID) @@ -1963,7 +2016,7 @@ public class EmailProvider extends ContentProvider { @VisibleForTesting @SuppressWarnings("deprecation") - void convertPolicyFlagsToPolicyTable(SQLiteDatabase db) { + static void convertPolicyFlagsToPolicyTable(SQLiteDatabase db) { Cursor c = db.query(Account.TABLE_NAME, new String[] {EmailContent.RECORD_ID /*0*/, AccountColumns.SECURITY_FLAGS /*1*/}, AccountColumns.SECURITY_FLAGS + ">0", null, null, null, null); diff --git a/src/com/android/email/service/AccountService.java b/src/com/android/email/service/AccountService.java index 34b90bf91..57f8a6e94 100644 --- a/src/com/android/email/service/AccountService.java +++ b/src/com/android/email/service/AccountService.java @@ -21,7 +21,6 @@ import com.android.email.ExchangeUtils; import com.android.email.NotificationController; import com.android.email.ResourceHelper; import com.android.email.VendorPolicyLoader; -import com.android.email.provider.AccountBackupRestore; import com.android.emailcommon.Configuration; import com.android.emailcommon.Device; import com.android.emailcommon.service.IAccountService; @@ -54,7 +53,9 @@ public class AccountService extends Service { @Override public void restoreAccountsIfNeeded() { - AccountBackupRestore.restoreIfNeeded(mContext); + // 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.) } @Override diff --git a/src/com/android/email/service/MailService.java b/src/com/android/email/service/MailService.java index e1da6e285..318783380 100644 --- a/src/com/android/email/service/MailService.java +++ b/src/com/android/email/service/MailService.java @@ -141,9 +141,6 @@ public class MailService extends Service { public int onStartCommand(final Intent intent, int flags, final int startId) { super.onStartCommand(intent, flags, startId); - // Restore accounts, if it has not happened already - AccountBackupRestore.restoreIfNeeded(this); - EmailAsyncTask.runAsyncParallel(new Runnable() { @Override public void run() { diff --git a/tests/src/com/android/email/provider/AccountBackupRestoreTests.java b/tests/src/com/android/email/provider/AccountBackupRestoreTests.java index 6c4175366..b1e06fe96 100644 --- a/tests/src/com/android/email/provider/AccountBackupRestoreTests.java +++ b/tests/src/com/android/email/provider/AccountBackupRestoreTests.java @@ -114,8 +114,10 @@ public class AccountBackupRestoreTests extends ProviderTestCase2 assertEquals(0, EmailContent.count(mMockContext, Account.CONTENT_URI)); assertEquals(0, EmailContent.count(mMockContext, HostAuth.CONTENT_URI)); - // Restore the accounts - AccountBackupRestore.restoreIfNeeded(mMockContext); + // Because we restore accounts at the db open time, we first need to close the db + // explicitly here. + // Accounts will be restored next time we touch the db. + getProvider().shutdown(); // Make sure there are two accounts and four host auths assertEquals(2, EmailContent.count(mMockContext, Account.CONTENT_URI));