Don't use sendMailCallback() -- don't track outbox status
There're differences between how SMTP and EAS use this callback. We should eventually unify the behavior, but till then let's not use sendMailCallback(). Bug 3116377 Change-Id: Ic5ecf16251c11ab2bd2e16e29bd417f1ece67f14
This commit is contained in:
parent
9a9c725174
commit
be1aa37dc5
|
@ -1072,6 +1072,26 @@ public class Controller {
|
||||||
* Callback for sending pending messages. This will be called once to start the
|
* Callback for sending pending messages. This will be called once to start the
|
||||||
* group, multiple times for messages, and once to complete the group.
|
* group, multiple times for messages, and once to complete the group.
|
||||||
*
|
*
|
||||||
|
* Unfortunately this callback works differently on SMTP and EAS.
|
||||||
|
*
|
||||||
|
* On SMTP:
|
||||||
|
*
|
||||||
|
* First, we get this.
|
||||||
|
* result == null, messageId == -1, progress == 0: start batch send
|
||||||
|
*
|
||||||
|
* Then we get these callbacks per message.
|
||||||
|
* (Exchange backend may skip "start sending one message".)
|
||||||
|
* result == null, messageId == xx, progress == 0: start sending one message
|
||||||
|
* result == xxxx, messageId == xx, progress == 0; failed sending one message
|
||||||
|
*
|
||||||
|
* Finally we get this.
|
||||||
|
* result == null, messageId == -1, progres == 100; finish sending batch
|
||||||
|
*
|
||||||
|
* On EAS: Almost same as above, except:
|
||||||
|
*
|
||||||
|
* - There's no first ("start batch send") callback.
|
||||||
|
* - accountId is always -1.
|
||||||
|
*
|
||||||
* @param result If null, the operation completed without error
|
* @param result If null, the operation completed without error
|
||||||
* @param accountId The account being operated on
|
* @param accountId The account being operated on
|
||||||
* @param messageId The being sent (may be unknown at start)
|
* @param messageId The being sent (may be unknown at start)
|
||||||
|
|
|
@ -62,7 +62,22 @@ public class RefreshManager {
|
||||||
private String mErrorMessage;
|
private String mErrorMessage;
|
||||||
|
|
||||||
public interface Listener {
|
public interface Listener {
|
||||||
|
/**
|
||||||
|
* Refresh status of a mailbox list or a message list has changed.
|
||||||
|
*
|
||||||
|
* @param accountId ID of the account.
|
||||||
|
* @param mailboxId -1 if it's about the mailbox list, or the ID of the mailbox list in
|
||||||
|
* question.
|
||||||
|
*/
|
||||||
public void onRefreshStatusChanged(long accountId, long mailboxId);
|
public void onRefreshStatusChanged(long accountId, long mailboxId);
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Error callback.
|
||||||
|
*
|
||||||
|
* @param accountId ID of the account, or -1 if unknown.
|
||||||
|
* @param mailboxId ID of the mailbox, or -1 if unknown.
|
||||||
|
* @param message error message which can be shown to the user.
|
||||||
|
*/
|
||||||
public void onMessagingError(long accountId, long mailboxId, String message);
|
public void onMessagingError(long accountId, long mailboxId, String message);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -145,7 +160,6 @@ public class RefreshManager {
|
||||||
|
|
||||||
private final RefreshStatusMap mMailboxListStatus = new RefreshStatusMap();
|
private final RefreshStatusMap mMailboxListStatus = new RefreshStatusMap();
|
||||||
private final RefreshStatusMap mMessageListStatus = new RefreshStatusMap();
|
private final RefreshStatusMap mMessageListStatus = new RefreshStatusMap();
|
||||||
private final RefreshStatusMap mOutboxStatus = new RefreshStatusMap();
|
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* @return the singleton instance.
|
* @return the singleton instance.
|
||||||
|
@ -248,11 +262,7 @@ public class RefreshManager {
|
||||||
* Send pending messages.
|
* Send pending messages.
|
||||||
*/
|
*/
|
||||||
public boolean sendPendingMessages(long accountId) {
|
public boolean sendPendingMessages(long accountId) {
|
||||||
final Status status = mOutboxStatus.get(accountId);
|
|
||||||
if (!status.canRefresh()) return false;
|
|
||||||
|
|
||||||
Log.i(Email.LOG_TAG, "sendPendingMessages " + accountId);
|
Log.i(Email.LOG_TAG, "sendPendingMessages " + accountId);
|
||||||
status.onRefreshRequested();
|
|
||||||
notifyRefreshStatusChanged(accountId, -1);
|
notifyRefreshStatusChanged(accountId, -1);
|
||||||
mController.sendPendingMessages(accountId);
|
mController.sendPendingMessages(accountId);
|
||||||
return true;
|
return true;
|
||||||
|
@ -285,10 +295,6 @@ public class RefreshManager {
|
||||||
return mMessageListStatus.get(mailboxId).getLastRefreshTime();
|
return mMessageListStatus.get(mailboxId).getLastRefreshTime();
|
||||||
}
|
}
|
||||||
|
|
||||||
public long getLastSendMessageTime(long accountId) {
|
|
||||||
return mOutboxStatus.get(accountId).getLastRefreshTime();
|
|
||||||
}
|
|
||||||
|
|
||||||
public boolean isMailboxListRefreshing(long accountId) {
|
public boolean isMailboxListRefreshing(long accountId) {
|
||||||
return mMailboxListStatus.get(accountId).isRefreshing();
|
return mMailboxListStatus.get(accountId).isRefreshing();
|
||||||
}
|
}
|
||||||
|
@ -297,10 +303,6 @@ public class RefreshManager {
|
||||||
return mMessageListStatus.get(mailboxId).isRefreshing();
|
return mMessageListStatus.get(mailboxId).isRefreshing();
|
||||||
}
|
}
|
||||||
|
|
||||||
public boolean isSendingMessage(long accountId) {
|
|
||||||
return mOutboxStatus.get(accountId).isRefreshing();
|
|
||||||
}
|
|
||||||
|
|
||||||
public boolean isRefreshingAnyMailboxList() {
|
public boolean isRefreshingAnyMailboxList() {
|
||||||
return mMailboxListStatus.isRefreshingAny();
|
return mMailboxListStatus.isRefreshingAny();
|
||||||
}
|
}
|
||||||
|
@ -309,13 +311,8 @@ public class RefreshManager {
|
||||||
return mMessageListStatus.isRefreshingAny();
|
return mMessageListStatus.isRefreshingAny();
|
||||||
}
|
}
|
||||||
|
|
||||||
public boolean isSendingAnyMessage() {
|
public boolean isRefreshingAny() {
|
||||||
return mOutboxStatus.isRefreshingAny();
|
return isRefreshingAnyMailboxList() || isRefreshingAnyMessageList();
|
||||||
}
|
|
||||||
|
|
||||||
public boolean isRefreshingOrSendingAny() {
|
|
||||||
return isRefreshingAnyMailboxList() || isRefreshingAnyMessageList()
|
|
||||||
|| isSendingAnyMessage();
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public String getErrorMessage() {
|
public String getErrorMessage() {
|
||||||
|
@ -347,10 +344,6 @@ public class RefreshManager {
|
||||||
return mMessageListStatus.get(mailboxId);
|
return mMessageListStatus.get(mailboxId);
|
||||||
}
|
}
|
||||||
|
|
||||||
/* package */ Status getOutboxStatusForTest(long acountId) {
|
|
||||||
return mOutboxStatus.get(acountId);
|
|
||||||
}
|
|
||||||
|
|
||||||
private class ControllerResult extends Controller.Result {
|
private class ControllerResult extends Controller.Result {
|
||||||
private boolean mSendMailExceptionReported = false;
|
private boolean mSendMailExceptionReported = false;
|
||||||
|
|
||||||
|
@ -425,20 +418,8 @@ public class RefreshManager {
|
||||||
/**
|
/**
|
||||||
* Send message progress callback.
|
* Send message progress callback.
|
||||||
*
|
*
|
||||||
* This callback is overly overloaded:
|
* We don't keep track of the status of outboxes, but we monitor this to catch
|
||||||
*
|
* errors.
|
||||||
* First, we get this.
|
|
||||||
* result == null, messageId == -1, progress == 0: start batch send
|
|
||||||
*
|
|
||||||
* Then we get these callbacks per message.
|
|
||||||
* (Exchange backend may skip "start sending one message".)
|
|
||||||
* result == null, messageId == xx, progress == 0: start sending one message
|
|
||||||
* result == xxxx, messageId == xx, progress == 0; failed sending one message
|
|
||||||
*
|
|
||||||
* Finally we get this.
|
|
||||||
* result == null, messageId == -1, progres == 100; finish sending batch
|
|
||||||
*
|
|
||||||
* So, let's just report the first exception we get, and ignore the rest.
|
|
||||||
*/
|
*/
|
||||||
@Override
|
@Override
|
||||||
public void sendMailCallback(MessagingException exception, long accountId, long messageId,
|
public void sendMailCallback(MessagingException exception, long accountId, long messageId,
|
||||||
|
@ -450,17 +431,14 @@ public class RefreshManager {
|
||||||
if (progress == 0 && messageId == -1) {
|
if (progress == 0 && messageId == -1) {
|
||||||
mSendMailExceptionReported = false;
|
mSendMailExceptionReported = false;
|
||||||
}
|
}
|
||||||
if (messageId == -1) {
|
|
||||||
// Update the status only for the batch start/end.
|
|
||||||
// (i.e. don't report for each message.)
|
|
||||||
mOutboxStatus.get(accountId).onCallback(exception, progress, mClock);
|
|
||||||
notifyRefreshStatusChanged(accountId, -1);
|
|
||||||
}
|
|
||||||
if (exception != null && !mSendMailExceptionReported) {
|
if (exception != null && !mSendMailExceptionReported) {
|
||||||
// Only the first error in a batch will be reported.
|
// Only the first error in a batch will be reported.
|
||||||
mSendMailExceptionReported = true;
|
mSendMailExceptionReported = true;
|
||||||
reportError(accountId, messageId, exception.getUiErrorMessage(mContext));
|
reportError(accountId, messageId, exception.getUiErrorMessage(mContext));
|
||||||
}
|
}
|
||||||
|
if (progress == 100) {
|
||||||
|
mSendMailExceptionReported = false;
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
@ -534,13 +534,15 @@ public class MessageListXL extends Activity implements
|
||||||
Utility.runAsync(new Runnable() {
|
Utility.runAsync(new Runnable() {
|
||||||
@Override
|
@Override
|
||||||
public void run() {
|
public void run() {
|
||||||
Account account = Account.restoreAccountWithId(mContext, accountId);
|
String msg = message;
|
||||||
String msg = message;
|
if (accountId != -1) {
|
||||||
if (account != null) {
|
Account account = Account.restoreAccountWithId(mContext, accountId);
|
||||||
msg = account.mDisplayName + ": " + msg;
|
if (account != null) {
|
||||||
}
|
msg = account.mDisplayName + ": " + msg;
|
||||||
Utility.showToast(MessageListXL.this, msg);
|
}
|
||||||
}});
|
}
|
||||||
|
Utility.showToast(MessageListXL.this, msg);
|
||||||
|
}});
|
||||||
// END STOPSHIP
|
// END STOPSHIP
|
||||||
updateProgressIcon();
|
updateProgressIcon();
|
||||||
}
|
}
|
||||||
|
|
|
@ -329,22 +329,10 @@ public class RefreshManagerTest extends InstrumentationTestCase {
|
||||||
assertEquals(ACCOUNT_1, mController.mAccountId);
|
assertEquals(ACCOUNT_1, mController.mAccountId);
|
||||||
assertEquals(-1, mController.mMailboxId);
|
assertEquals(-1, mController.mMailboxId);
|
||||||
mController.reset();
|
mController.reset();
|
||||||
assertTrue(mTarget.isSendingMessage(ACCOUNT_1));
|
|
||||||
assertTrue(mTarget.isSendingAnyMessage());
|
|
||||||
|
|
||||||
// Request again -- shouldn't be accepted.
|
|
||||||
assertFalse(mTarget.sendPendingMessages(ACCOUNT_1));
|
|
||||||
|
|
||||||
assertFalse(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertFalse(mListener.mCalledOnConnectionError);
|
|
||||||
mListener.reset();
|
|
||||||
assertFalse(mController.mCalledSendPendingMessages);
|
|
||||||
mController.reset();
|
|
||||||
|
|
||||||
// request sending for account 2
|
// request sending for account 2
|
||||||
assertTrue(mTarget.sendPendingMessages(ACCOUNT_2));
|
assertTrue(mTarget.sendPendingMessages(ACCOUNT_2));
|
||||||
|
|
||||||
assertTrue(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertFalse(mListener.mCalledOnConnectionError);
|
assertFalse(mListener.mCalledOnConnectionError);
|
||||||
assertEquals(ACCOUNT_2, mListener.mAccountId);
|
assertEquals(ACCOUNT_2, mListener.mAccountId);
|
||||||
assertEquals(-1, mListener.mMailboxId);
|
assertEquals(-1, mListener.mMailboxId);
|
||||||
|
@ -353,33 +341,24 @@ public class RefreshManagerTest extends InstrumentationTestCase {
|
||||||
assertEquals(ACCOUNT_2, mController.mAccountId);
|
assertEquals(ACCOUNT_2, mController.mAccountId);
|
||||||
assertEquals(-1, mController.mMailboxId);
|
assertEquals(-1, mController.mMailboxId);
|
||||||
mController.reset();
|
mController.reset();
|
||||||
assertTrue(mTarget.isSendingMessage(ACCOUNT_2));
|
|
||||||
assertTrue(mTarget.isSendingAnyMessage());
|
|
||||||
|
|
||||||
// sending for account 1...
|
// Sending start for account 1...
|
||||||
|
// batch send start. (message id == -1, progress == 0)
|
||||||
mController.mListener.sendMailCallback(null, ACCOUNT_1, -1, 0);
|
mController.mListener.sendMailCallback(null, ACCOUNT_1, -1, 0);
|
||||||
|
|
||||||
assertTrue(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertFalse(mListener.mCalledOnConnectionError);
|
assertFalse(mListener.mCalledOnConnectionError);
|
||||||
assertEquals(ACCOUNT_1, mListener.mAccountId);
|
|
||||||
assertEquals(-1, mListener.mMailboxId);
|
|
||||||
mListener.reset();
|
mListener.reset();
|
||||||
assertTrue(mTarget.isSendingMessage(ACCOUNT_1));
|
|
||||||
assertEquals(0, mTarget.getOutboxStatusForTest(ACCOUNT_1).getLastRefreshTime());
|
|
||||||
|
|
||||||
// Per message callback (1)
|
// Per message callback
|
||||||
mController.mListener.sendMailCallback(null, ACCOUNT_1, 100, 0);
|
mController.mListener.sendMailCallback(null, ACCOUNT_1, 100, 0);
|
||||||
mController.mListener.sendMailCallback(null, ACCOUNT_1, 101, 0);
|
mController.mListener.sendMailCallback(null, ACCOUNT_1, 101, 0);
|
||||||
|
|
||||||
// No callback per message
|
|
||||||
assertFalse(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertFalse(mListener.mCalledOnConnectionError);
|
assertFalse(mListener.mCalledOnConnectionError);
|
||||||
mListener.reset();
|
mListener.reset();
|
||||||
|
|
||||||
// Exception -- first error will be reported.
|
// Exception -- first error will be reported.
|
||||||
mController.mListener.sendMailCallback(EXCEPTION, ACCOUNT_1, 102, 0);
|
mController.mListener.sendMailCallback(EXCEPTION, ACCOUNT_1, 102, 0);
|
||||||
|
|
||||||
assertFalse(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertTrue(mListener.mCalledOnConnectionError);
|
assertTrue(mListener.mCalledOnConnectionError);
|
||||||
assertEquals(EXCEPTION.getUiErrorMessage(mContext), mListener.mMessage);
|
assertEquals(EXCEPTION.getUiErrorMessage(mContext), mListener.mMessage);
|
||||||
mListener.reset();
|
mListener.reset();
|
||||||
|
@ -388,7 +367,6 @@ public class RefreshManagerTest extends InstrumentationTestCase {
|
||||||
mController.mListener.sendMailCallback(null, ACCOUNT_1, 103, 0);
|
mController.mListener.sendMailCallback(null, ACCOUNT_1, 103, 0);
|
||||||
mController.mListener.sendMailCallback(EXCEPTION, ACCOUNT_1, 104, 0);
|
mController.mListener.sendMailCallback(EXCEPTION, ACCOUNT_1, 104, 0);
|
||||||
|
|
||||||
assertFalse(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertFalse(mListener.mCalledOnConnectionError);
|
assertFalse(mListener.mCalledOnConnectionError);
|
||||||
mListener.reset();
|
mListener.reset();
|
||||||
|
|
||||||
|
@ -396,46 +374,8 @@ public class RefreshManagerTest extends InstrumentationTestCase {
|
||||||
Log.w(Email.LOG_TAG, "" + mController.mListener.getClass());
|
Log.w(Email.LOG_TAG, "" + mController.mListener.getClass());
|
||||||
mController.mListener.sendMailCallback(null, ACCOUNT_1, -1, 100);
|
mController.mListener.sendMailCallback(null, ACCOUNT_1, -1, 100);
|
||||||
|
|
||||||
assertTrue(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertFalse(mListener.mCalledOnConnectionError);
|
assertFalse(mListener.mCalledOnConnectionError);
|
||||||
assertEquals(ACCOUNT_1, mListener.mAccountId);
|
|
||||||
assertEquals(-1, mListener.mMailboxId);
|
|
||||||
mListener.reset();
|
mListener.reset();
|
||||||
assertFalse(mTarget.isSendingMessage(ACCOUNT_1));
|
|
||||||
assertEquals(mClock.mTime, mTarget.getOutboxStatusForTest(ACCOUNT_1)
|
|
||||||
.getLastRefreshTime());
|
|
||||||
|
|
||||||
// Check "any" method.
|
|
||||||
assertTrue(mTarget.isSendingAnyMessage()); // still sending for account 2
|
|
||||||
|
|
||||||
// sending for account 2...
|
|
||||||
mClock.advance();
|
|
||||||
|
|
||||||
mController.mListener.sendMailCallback(null, ACCOUNT_2, -1, 0);
|
|
||||||
|
|
||||||
assertTrue(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertFalse(mListener.mCalledOnConnectionError);
|
|
||||||
assertEquals(ACCOUNT_2, mListener.mAccountId);
|
|
||||||
assertEquals(-1, mListener.mMailboxId);
|
|
||||||
mListener.reset();
|
|
||||||
assertTrue(mTarget.isSendingMessage(ACCOUNT_2));
|
|
||||||
assertEquals(0, mTarget.getOutboxStatusForTest(ACCOUNT_2).getLastRefreshTime());
|
|
||||||
|
|
||||||
// Done with exception.
|
|
||||||
mController.mListener.sendMailCallback(EXCEPTION, ACCOUNT_2, -1, 0);
|
|
||||||
|
|
||||||
assertTrue(mListener.mCalledOnRefreshStatusChanged);
|
|
||||||
assertTrue(mListener.mCalledOnConnectionError);
|
|
||||||
assertEquals(ACCOUNT_2, mListener.mAccountId);
|
|
||||||
assertEquals(-1, mListener.mMailboxId);
|
|
||||||
assertEquals(EXCEPTION.getUiErrorMessage(mContext), mListener.mMessage);
|
|
||||||
mListener.reset();
|
|
||||||
assertFalse(mTarget.isSendingMessage(ACCOUNT_2));
|
|
||||||
assertEquals(mClock.mTime, mTarget.getOutboxStatusForTest(ACCOUNT_2)
|
|
||||||
.getLastRefreshTime());
|
|
||||||
|
|
||||||
// Check "any" method.
|
|
||||||
assertFalse(mTarget.isSendingAnyMessage());
|
|
||||||
}
|
}
|
||||||
|
|
||||||
public void testSendPendingMessagesForAllAccounts() throws Throwable {
|
public void testSendPendingMessagesForAllAccounts() throws Throwable {
|
||||||
|
|
Loading…
Reference in New Issue