If and account can't be loaded, do not perform an operation

b/17939382
Now, all operations take an Account, rather than an
accountId. The account must be loaded prior to performing
the operation. If it cannot be for some reason, we won't
even start. This solves the problem of calling syncStart(),
and then being unable to call syncEnd() because we don't
have an account object.
I think this could still be cleaner. I'd rather not pass
the account in the constructor, and instead set it in
doOperation(). That's too big of a change for right now
though, maybe after the next release.

Change-Id: I460b7a55e545a5822d6424e5db9b344d50a55fa4

11 files changed, 130 insertions, 150 deletions

diff --git a/src/com/android/exchange/eas/EasAutoDiscover.java b/src/com/android/exchange/eas/EasAutoDiscover.java
index cbcb032b..d3927acf 100644
--- a/src/com/android/exchange/eas/EasAutoDiscover.java
+++ b/src/com/android/exchange/eas/EasAutoDiscover.java
@@ -68,22 +68,30 @@ public class EasAutoDiscover extends EasOperation {
     private HostAuth mHostAuth;
     private String mRedirectUri;
 
+
+    private static Account makeAccount(final String username, final String password) {
+        final HostAuth hostAuth = new HostAuth();
+        hostAuth.mLogin = username;
+        hostAuth.mPassword = password;
+        hostAuth.mPort = 443;
+        hostAuth.mProtocol = Eas.PROTOCOL;
+        hostAuth.mFlags = HostAuth.FLAG_SSL | HostAuth.FLAG_AUTHENTICATE;
+        final Account account = new Account();
+        account.mEmailAddress = username;
+        account.mHostAuthRecv = hostAuth;
+        return account;
+    }
+
     public EasAutoDiscover(final Context context, final String uri, final int attemptNumber,
                            final String username, final String password) {
         // We don't actually need an account or a hostAuth, but the EasServerConnection requires
         // one. Just create dummy values.
-        super(context, -1);
+        super(context, makeAccount(username, password));
         mAttemptNumber = attemptNumber;
         mUri = uri;
         mUsername = username;
         mPassword = password;
-        mHostAuth = new HostAuth();
-        mHostAuth.mLogin = mUsername;
-        mHostAuth.mPassword = mPassword;
-        mHostAuth.mPort = 443;
-        mHostAuth.mProtocol = Eas.PROTOCOL;
-        mHostAuth.mFlags = HostAuth.FLAG_SSL | HostAuth.FLAG_AUTHENTICATE;
-        setAccount(new Account(), mHostAuth);
+        mHostAuth = mAccount.mHostAuthRecv;
     }
 
     public static String genUri(final String domain, final int attemptNumber) {
diff --git a/src/com/android/exchange/eas/EasFolderSync.java b/src/com/android/exchange/eas/EasFolderSync.java
index f9853b8c..b5e28c14 100644
--- a/src/com/android/exchange/eas/EasFolderSync.java
+++ b/src/com/android/exchange/eas/EasFolderSync.java
@@ -29,7 +29,6 @@ import com.android.exchange.EasResponse;
 import com.android.exchange.adapter.FolderSyncParser;
 import com.android.exchange.adapter.Serializer;
 import com.android.exchange.adapter.Tags;
-import com.android.exchange.service.EasService;
 import com.android.mail.utils.LogUtils;
 
 import org.apache.http.HttpEntity;
@@ -65,17 +64,6 @@ public class EasFolderSync extends EasOperation {
     private Bundle mValidationResult;
 
     /**
-     * Constructor for use with {@link EasService} when performing an actual sync.
-     * @param context
-     * @param accountId
-     */
-    public EasFolderSync(final Context context, final long accountId) {
-        super(context, accountId);
-        mStatusOnly = false;
-        mPolicy = null;
-    }
-
-    /**
      * Constructor for actually doing folder sync.
      * @param context
      * @param account
@@ -86,14 +74,20 @@ public class EasFolderSync extends EasOperation {
         mPolicy = null;
     }
 
+    private static Account makeAccount(final HostAuth hostAuth) {
+        final Account account = new Account();
+        account.mHostAuthRecv = hostAuth;
+        account.mEmailAddress = hostAuth.mLogin;
+        return account;
+    }
+
     /**
      * Constructor for account validation.
      * @param context
      * @param hostAuth
      */
     public EasFolderSync(final Context context, final HostAuth hostAuth) {
-        super(context, -1);
-        setDummyAccount(hostAuth);
+        super(context, makeAccount(hostAuth), hostAuth);
         mStatusOnly = true;
     }
 
diff --git a/src/com/android/exchange/eas/EasFullSyncOperation.java b/src/com/android/exchange/eas/EasFullSyncOperation.java
index ae28a42a..76ddf9b3 100644
--- a/src/com/android/exchange/eas/EasFullSyncOperation.java
+++ b/src/com/android/exchange/eas/EasFullSyncOperation.java
@@ -56,9 +56,9 @@ public class EasFullSyncOperation extends EasOperation {
     final Bundle mSyncExtras;
     Set<String> mAuthsToSync;
 
-    public EasFullSyncOperation(final Context context, final long accountId,
+    public EasFullSyncOperation(final Context context, final Account account,
                                 final Bundle syncExtras) {
-        super(context, accountId);
+        super(context, account);
         mSyncExtras = syncExtras;
     }
 
@@ -94,12 +94,12 @@ public class EasFullSyncOperation extends EasOperation {
 
     @Override
     public int performOperation() {
-        // Make sure the account is loaded if it hasn't already been.
-        if (!init(false)) {
-            LogUtils.i(LOG_TAG, "Failed to initialize %d before operation EasFullSyncOperation",
-                    getAccountId());
+        if (!init()) {
+            LogUtils.i(LOG_TAG, "Failed to initialize %d before sending request for operation %s",
+                    getAccountId(), getCommand());
             return RESULT_INITIALIZATION_FAILURE;
         }
+
         final android.accounts.Account amAccount = new android.accounts.Account(
                 mAccount.mEmailAddress, Eas.EXCHANGE_ACCOUNT_MANAGER_TYPE);
         mAuthsToSync = EasService.getAuthoritiesToSync(amAccount, AUTHORITIES_TO_SYNC);
diff --git a/src/com/android/exchange/eas/EasLoadAttachment.java b/src/com/android/exchange/eas/EasLoadAttachment.java
index 9527e51e..16e4d0ee 100644
--- a/src/com/android/exchange/eas/EasLoadAttachment.java
+++ b/src/com/android/exchange/eas/EasLoadAttachment.java
@@ -19,6 +19,7 @@ package com.android.exchange.eas;
 import android.content.Context;
 import android.os.RemoteException;
 
+import com.android.emailcommon.provider.Account;
 import com.android.emailcommon.provider.EmailContent;
 import com.android.emailcommon.provider.EmailContent.Attachment;
 import com.android.emailcommon.service.EmailServiceStatus;
@@ -69,15 +70,15 @@ public final class EasLoadAttachment extends EasOperation {
     /**
      * Constructor for use with {@link EasService} when performing an actual sync.
      * @param context Our {@link Context}.
-     * @param accountId The id of the account in question (i.e. its id in the database).
+     * @param account The account we're loading the attachment for.
      * @param attachmentId The local id of the attachment (i.e. its id in the database).
      * @param callback The callback for any status updates.
      */
-    public EasLoadAttachment(final Context context, final long accountId, final long attachmentId,
+    public EasLoadAttachment(final Context context, final Account account, final long attachmentId,
             final IEmailServiceCallback callback) {
         // The account is loaded before performOperation but it is not guaranteed to be available
         // before then.
-        super(context, accountId);
+        super(context, account);
         mCallback = callback;
         mAttachmentId = attachmentId;
     }
diff --git a/src/com/android/exchange/eas/EasOperation.java b/src/com/android/exchange/eas/EasOperation.java
index b032bb74..c6330506 100644
--- a/src/com/android/exchange/eas/EasOperation.java
+++ b/src/com/android/exchange/eas/EasOperation.java
@@ -24,6 +24,7 @@ import android.content.SyncResult;
 import android.net.Uri;
 import android.os.Build;
 import android.os.Bundle;
+import android.support.annotation.NonNull;
 import android.telephony.TelephonyManager;
 import android.text.TextUtils;
 import android.text.format.DateUtils;
@@ -149,11 +150,8 @@ public abstract class EasOperation {
 
     protected final Context mContext;
 
-    /** The provider id for the account this operation is on. */
-    private final long mAccountId;
-
     /** The cached {@link Account} state; can be null if it hasn't been loaded yet. */
-    protected Account mAccount;
+    protected final Account mAccount;
 
     /** The connection to use for this operation. This is created when {@link #mAccount} is set. */
     protected EasServerConnection mConnection;
@@ -164,30 +162,18 @@ public abstract class EasOperation {
         }
     }
 
-    @VisibleForTesting
-    public void replaceEasServerConnection(EasServerConnection connection) {
-        mConnection = connection;
-    }
-
     public static boolean isFatal(int result) {
         return result < RESULT_MIN_OK_RESULT;
     }
 
-    /**
-     * Constructor which defers loading of account and connection info.
-     * @param context
-     * @param accountId
-     */
-    protected EasOperation(final Context context, final long accountId) {
-        mContext = context;
-        mAccountId = accountId;
-    }
-
-    protected EasOperation(final Context context, final Account account,
+    protected EasOperation(final Context context, @NonNull final Account account,
             final EasServerConnection connection) {
-        this(context, account.mId);
+        mContext = context;
         mAccount = account;
         mConnection = connection;
+        if (account == null) {
+            throw new IllegalStateException("Null account in EasOperation");
+        }
     }
 
     protected EasOperation(final Context context, final Account account, final HostAuth hostAuth) {
@@ -205,59 +191,21 @@ public abstract class EasOperation {
      */
     protected EasOperation(final EasOperation parentOperation) {
         mContext = parentOperation.mContext;
-        mAccountId = parentOperation.mAccountId;
         mAccount = parentOperation.mAccount;
         mConnection = parentOperation.mConnection;
     }
 
     /**
-     * Some operations happen before the account exists (e.g. account validation).
-     * These operations cannot use {@link #init}, so instead we make a dummy account and
-     * supply a temporary {@link HostAuth}.
-     * @param hostAuth
+     * This will always be called at the begining of performOperation and can be overridden
+     * to do whatever setup is needed.
+     * @return true if initialization succeeded, false otherwise.
      */
-    protected final void setDummyAccount(final HostAuth hostAuth) {
-        mAccount = new Account();
-        mAccount.mEmailAddress = hostAuth.mLogin;
-        mConnection = new EasServerConnection(mContext, mAccount, hostAuth);
-    }
-
-    /**
-     * Loads (or reloads) the {@link Account} for this operation, and sets up our connection to the
-     * server. This can be overridden to add additional functionality, but child implementations
-     * should always call super().
-     * @param allowReload If false, do not perform a load if we already have an {@link Account}
-     *                    (i.e. just keep the existing one); otherwise allow replacement of the
-     *                    account. Note that this can result in a valid Account being replaced with
-     *                    null if the account no longer exists.
-     * @return Whether we now have a valid {@link Account} object.
-     */
-    public boolean init(final boolean allowReload) {
-        if (mAccount == null || allowReload) {
-            mAccount = Account.restoreAccountWithId(mContext, getAccountId());
-            if (mAccount != null) {
-                mConnection = new EasServerConnection(mContext, mAccount,
-                        mAccount.getOrCreateHostAuthRecv(mContext));
-            }
-        }
-        return (mAccount != null);
-    }
-
-    /**
-     * Sets the account. This is for use in cases where the account is not available upon
-     * construction. This will also create the EasServerConnection.
-     * @param account
-     * @param hostAuth
-     */
-    protected void setAccount(final Account account, final HostAuth hostAuth) {
-        mAccount = account;
-        if (mAccount != null) {
-            mConnection = new EasServerConnection(mContext, mAccount, hostAuth);
-        }
+    public boolean init() {
+        return true;
     }
 
     public final long getAccountId() {
-        return mAccountId;
+        return mAccount.getId();
     }
 
     public final Account getAccount() {
@@ -307,13 +255,11 @@ public abstract class EasOperation {
      * @return A result code for the outcome of this operation, as described above.
      */
     public int performOperation() {
-        // Make sure the account is loaded if it hasn't already been.
-        if (!init(false)) {
+        if (!init()) {
             LogUtils.i(LOG_TAG, "Failed to initialize %d before sending request for operation %s",
                     getAccountId(), getCommand());
             return RESULT_INITIALIZATION_FAILURE;
         }
-
         try {
             return performOperationInternal();
         } finally {
diff --git a/src/com/android/exchange/eas/EasOutboxSync.java b/src/com/android/exchange/eas/EasOutboxSync.java
index ba259712..049fd19d 100644
--- a/src/com/android/exchange/eas/EasOutboxSync.java
+++ b/src/com/android/exchange/eas/EasOutboxSync.java
@@ -83,10 +83,9 @@ public class EasOutboxSync extends EasOperation {
      * need to reset any derived values (eg, mIsEas14).
      */
     @Override
-    public boolean init(final boolean allowReload) {
-        final boolean haveValidAccount = super.init(allowReload);
+    public boolean init() {
         initEas14();
-        return haveValidAccount;
+        return true;
     }
 
     private void initEas14() {
diff --git a/src/com/android/exchange/eas/EasSearch.java b/src/com/android/exchange/eas/EasSearch.java
index 8020ee6a..64100f41 100644
--- a/src/com/android/exchange/eas/EasSearch.java
+++ b/src/com/android/exchange/eas/EasSearch.java
@@ -5,6 +5,7 @@ import android.content.Context;
 import android.content.SyncResult;
 
 import com.android.emailcommon.Logging;
+import com.android.emailcommon.provider.Account;
 import com.android.emailcommon.provider.Mailbox;
 import com.android.emailcommon.service.SearchParams;
 import com.android.exchange.CommandStatusException;
@@ -41,9 +42,9 @@ public class EasSearch extends EasOperation {
     int mTotalResults;
     Mailbox mSearchMailbox;
 
-    public EasSearch(final Context context, final long accountId, final SearchParams searchParams,
+    public EasSearch(final Context context, final Account account, final SearchParams searchParams,
         final long destMailboxId) {
-        super(context, accountId);
+        super(context, account);
         mSearchParams = searchParams;
         mDestMailboxId = destMailboxId;
     }
diff --git a/src/com/android/exchange/eas/EasSearchGal.java b/src/com/android/exchange/eas/EasSearchGal.java
index 0dbadd2b..ea3c27c8 100644
--- a/src/com/android/exchange/eas/EasSearchGal.java
+++ b/src/com/android/exchange/eas/EasSearchGal.java
@@ -2,6 +2,7 @@ package com.android.exchange.eas;
 
 import android.content.Context;
 
+import com.android.emailcommon.provider.Account;
 import com.android.exchange.CommandStatusException;
 import com.android.exchange.EasResponse;
 import com.android.exchange.adapter.GalParser;
@@ -25,9 +26,9 @@ public class EasSearchGal extends EasOperation {
     final private int mLimit;
     private GalResult mResult;
 
-    public EasSearchGal(Context context, final long accountId, final String filter,
+    public EasSearchGal(Context context, final Account account, final String filter,
                         final int limit) {
-        super(context, accountId);
+        super(context, account);
         mFilter = filter;
         mLimit = limit;
     }
diff --git a/src/com/android/exchange/eas/EasSendMeetingResponse.java b/src/com/android/exchange/eas/EasSendMeetingResponse.java
index 7897bb53..ba1411f4 100644
--- a/src/com/android/exchange/eas/EasSendMeetingResponse.java
+++ b/src/com/android/exchange/eas/EasSendMeetingResponse.java
@@ -52,9 +52,9 @@ public class EasSendMeetingResponse extends EasOperation {
     private final int mMeetingResponse;
     private int mEasResponse;
 
-    public EasSendMeetingResponse(final Context context, final long accountId,
+    public EasSendMeetingResponse(final Context context, final Account account,
                                   final EmailContent.Message message, final int meetingResponse) {
-        super(context, accountId);
+        super(context, account);
         mMessage = message;
         mMeetingResponse = meetingResponse;
     }
diff --git a/src/com/android/exchange/eas/EasSyncBase.java b/src/com/android/exchange/eas/EasSyncBase.java
index e8925188..8e77e26d 100644
--- a/src/com/android/exchange/eas/EasSyncBase.java
+++ b/src/com/android/exchange/eas/EasSyncBase.java
@@ -65,18 +65,15 @@ public class EasSyncBase extends EasOperation {
     }
 
     @Override
-    public boolean init(final boolean allowReload) {
-        final boolean result = super.init(allowReload);
-        if (result) {
-            mCollectionTypeHandler = getCollectionTypeHandler(mMailbox.mType);
-            if (mCollectionTypeHandler == null) {
-                return false;
-            }
-            // Set up traffic stats bookkeeping.
-            final int trafficFlags = TrafficFlags.getSyncFlags(mContext, mAccount);
-            TrafficStats.setThreadStatsTag(trafficFlags | mCollectionTypeHandler.getTrafficFlag());
+    public boolean init() {
+        mCollectionTypeHandler = getCollectionTypeHandler(mMailbox.mType);
+        if (mCollectionTypeHandler == null) {
+            return false;
         }
-        return result;
+        // Set up traffic stats bookkeeping.
+        final int trafficFlags = TrafficFlags.getSyncFlags(mContext, mAccount);
+        TrafficStats.setThreadStatsTag(trafficFlags | mCollectionTypeHandler.getTrafficFlag());
+        return true;
     }
 
     @Override
diff --git a/src/com/android/exchange/service/EasService.java b/src/com/android/exchange/service/EasService.java
index ceb686ba..cf0a983b 100644
--- a/src/com/android/exchange/service/EasService.java
+++ b/src/com/android/exchange/service/EasService.java
@@ -100,15 +100,22 @@ public class EasService extends Service {
         public void loadAttachment(final IEmailServiceCallback callback, final long accountId,
                 final long attachmentId, final boolean background) {
             LogUtils.d(TAG, "IEmailService.loadAttachment: %d", attachmentId);
-            final EasLoadAttachment operation = new EasLoadAttachment(EasService.this, accountId,
-                    attachmentId, callback);
-            doOperation(operation, "IEmailService.loadAttachment");
+            final Account account = loadAccount(EasService.this, accountId);
+            if (account != null) {
+                final EasLoadAttachment operation = new EasLoadAttachment(EasService.this, account,
+                        attachmentId, callback);
+                doOperation(operation, "IEmailService.loadAttachment");
+            }
         }
 
         @Override
         public void updateFolderList(final long accountId) {
-            final EasFolderSync operation = new EasFolderSync(EasService.this, accountId);
-            doOperation(operation, "IEmailService.updateFolderList");
+            LogUtils.d(TAG, "IEmailService.updateFolderList: %d", accountId);
+            final Account account = loadAccount(EasService.this, accountId);
+            if (account != null) {
+                final EasFolderSync operation = new EasFolderSync(EasService.this, account);
+                doOperation(operation, "IEmailService.updateFolderList");
+            }
         }
 
         public void sendMail(final long accountId) {
@@ -117,13 +124,20 @@ public class EasService extends Service {
         }
 
         public int sync(final long accountId, Bundle syncExtras) {
-            EasFullSyncOperation op = new EasFullSyncOperation(EasService.this, accountId, syncExtras);
-            final int result = doOperation(op, "IEmailService.sync");
-            if (result == EasFullSyncOperation.RESULT_SECURITY_HOLD) {
-                LogUtils.i(LogUtils.TAG, "Security Hold trying to sync");
+            LogUtils.d(TAG, "IEmailService.updateFolderList: %d", accountId);
+            final Account account = loadAccount(EasService.this, accountId);
+            if (account != null) {
+                EasFullSyncOperation op = new EasFullSyncOperation(EasService.this, account,
+                        syncExtras);
+                final int result = doOperation(op, "IEmailService.sync");
+                if (result == EasFullSyncOperation.RESULT_SECURITY_HOLD) {
+                    LogUtils.i(LogUtils.TAG, "Security Hold trying to sync");
+                    return EmailServiceStatus.INTERNAL_ERROR;
+                }
+                return convertToEmailServiceStatus(result);
+            } else {
                 return EmailServiceStatus.INTERNAL_ERROR;
             }
-            return convertToEmailServiceStatus(result);
         }
 
         @Override
@@ -139,6 +153,7 @@ public class EasService extends Service {
 
         @Override
         public Bundle validate(final HostAuthCompat hostAuthCom) {
+            LogUtils.d(TAG, "IEmailService.validate");
             final HostAuth hostAuth = hostAuthCom.toHostAuth();
             final EasFolderSync operation = new EasFolderSync(EasService.this, hostAuth);
             doOperation(operation, "IEmailService.validate");
@@ -148,24 +163,33 @@ public class EasService extends Service {
         @Override
         public int searchMessages(final long accountId, final SearchParams searchParams,
                 final long destMailboxId) {
-            final EasSearch operation = new EasSearch(EasService.this, accountId, searchParams,
-                    destMailboxId);
-            doOperation(operation, "IEmailService.searchMessages");
-            return operation.getTotalResults();
+            LogUtils.d(TAG, "IEmailService.searchMessages");
+            final Account account = loadAccount(EasService.this, accountId);
+            if (account != null) {
+                final EasSearch operation = new EasSearch(EasService.this, account, searchParams,
+                        destMailboxId);
+                doOperation(operation, "IEmailService.searchMessages");
+                return operation.getTotalResults();
+            } else {
+                return 0;
+            }
         }
 
         @Override
         public void sendMeetingResponse(final long messageId, final int response) {
             EmailContent.Message msg = EmailContent.Message.restoreMessageWithId(EasService.this,
                     messageId);
+            LogUtils.d(TAG, "IEmailService.sendMeetingResponse");
             if (msg == null) {
                 LogUtils.e(TAG, "Could not load message %d in sendMeetingResponse", messageId);
                 return;
             }
-
-            final EasSendMeetingResponse operation = new EasSendMeetingResponse(EasService.this,
-                    msg.mAccountKey, msg, response);
-            doOperation(operation, "IEmailService.sendMeetingResponse");
+            final Account account = loadAccount(EasService.this, msg.mAccountKey);
+            if (account != null) {
+                final EasSendMeetingResponse operation = new EasSendMeetingResponse(EasService.this,
+                        account, msg, response);
+                doOperation(operation, "IEmailService.sendMeetingResponse");
+            }
         }
 
         @Override
@@ -224,8 +248,6 @@ public class EasService extends Service {
 
         @Override
         public void setLogging(final int flags) {
-            // TODO: This isn't persisted. If Exchange goes down and restarts, debugging will
-            // be turned off.
             sProtocolLogging = ((flags & EmailServiceProxy.DEBUG_EXCHANGE_BIT) != 0);
             sFileLogging = ((flags & EmailServiceProxy.DEBUG_FILE_BIT) != 0);
             SharedPreferences sharedPrefs = EasService.this.getSharedPreferences(PREFERENCES_FILE,
@@ -251,6 +273,14 @@ public class EasService extends Service {
         }
     };
 
+    private static Account loadAccount(final Context context, final long accountId) {
+        Account account = Account.restoreAccountWithId(context, accountId);
+        if (account == null) {
+            LogUtils.e(TAG, "Could not load account %d", accountId);
+        }
+        return account;
+    }
+
     /**
      * Content selection string for getting all accounts that are configured for push.
      * TODO: Add protocol check so that we don't get e.g. IMAP accounts here.
@@ -439,18 +469,21 @@ public class EasService extends Service {
 
     static public GalResult searchGal(final Context context, final long accountId,
                                       final String filter, final int limit) {
-        final EasSearchGal operation = new EasSearchGal(context, accountId, filter, limit);
-        // We don't use doOperation() here for two reasons:
-        // 1. This is a static function, doOperation is not, and we don't have an instance of
-        // EasService.
-        // 2. All doOperation() does besides this is stop the ping and then restart it. This is
-        // required during syncs, but not for GalSearches.
-        final int result = operation.performOperation();
-        if (result == EasSearchGal.RESULT_OK) {
-            return operation.getResult();
-        } else {
-            return null;
+        GalResult galResult = null;
+        final Account account = loadAccount(context, accountId);
+        if (account != null) {
+            final EasSearchGal operation = new EasSearchGal(context, account, filter, limit);
+            // We don't use doOperation() here for two reasons:
+            // 1. This is a static function, doOperation is not, and we don't have an instance of
+            // EasService.
+            // 2. All doOperation() does besides this is stop the ping and then restart it. This is
+            // required during syncs, but not for GalSearches.
+            final int result = operation.performOperation();
+            if (result == EasSearchGal.RESULT_OK) {
+                galResult = operation.getResult();
+            }
         }
+        return galResult;
     }
 
     /**