Controller.Result callbacks should all have accountId

- Added accountId to loadAttachmentCallback/loadMessageForViewCallback

- Cleaned up LegacyListener/MessagingListener.
  Removed the constructors which take messageId and attachmentId, which
  are used to bridge loadAttachmentProgress, which the callsite doesn't know
  these IDs.  The inconsistency (only loadAttachmentProgress() uses the member
  messageId) doesn't look too good, so extracted this into a separate class,
  MessageRetrievalListenerBridge.

Change-Id: I46303e50df2b0e1fe8616e7c9cef632ac14f23aa
This commit is contained in:
Makoto Onuki 2010-12-10 13:36:18 -08:00
parent 9d2e36412d
commit aef9515ee7
9 changed files with 97 additions and 106 deletions

View File

@ -395,9 +395,10 @@ public class Controller {
cv.put(MessageColumns.FLAG_LOADED, Message.FLAG_LOADED_COMPLETE);
mProviderContext.getContentResolver().update(uri, cv, null, null);
Log.d(Email.LOG_TAG, "Unexpected loadMessageForView() for service-based message.");
final long accountId = Account.getAccountIdForMessageId(mProviderContext, messageId);
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadMessageForViewCallback(null, messageId, 100);
listener.loadMessageForViewCallback(null, accountId, messageId, 100);
}
}
} else {
@ -871,10 +872,10 @@ public class Controller {
// This presumably is for POP3 messages
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadAttachmentCallback(null, messageId, attachmentId, 0);
listener.loadAttachmentCallback(null, accountId, messageId, attachmentId, 0);
}
for (Result listener : mListeners) {
listener.loadAttachmentCallback(null, messageId, attachmentId, 100);
listener.loadAttachmentCallback(null, accountId, messageId, attachmentId, 100);
}
}
return;
@ -1080,8 +1081,8 @@ public class Controller {
* @param messageId the message which contains the attachment
* @param progress 0 for "starting", 1..99 for updates (if needed in UI), 100 for complete
*/
public void loadMessageForViewCallback(MessagingException result, long messageId,
int progress) {
public void loadMessageForViewCallback(MessagingException result, long accountId,
long messageId, int progress) {
}
/**
@ -1092,8 +1093,8 @@ public class Controller {
* @param attachmentId the attachment being loaded
* @param progress 0 for "starting", 1..99 for updates (if needed in UI), 100 for complete
*/
public void loadAttachmentCallback(MessagingException result, long messageId,
long attachmentId, int progress) {
public void loadAttachmentCallback(MessagingException result, long accountId,
long messageId, long attachmentId, int progress) {
}
/**
@ -1151,15 +1152,41 @@ public class Controller {
}
}
/**
* Bridge to intercept {@link MessageRetrievalListener#loadAttachmentProgress} and
* pass down to {@link Result}.
*/
public class MessageRetrievalListenerBridge implements MessageRetrievalListener {
private final long mMessageId;
private final long mAttachmentId;
private final long mAccountId;
public MessageRetrievalListenerBridge(long messageId, long attachmentId) {
mMessageId = messageId;
mAttachmentId = attachmentId;
mAccountId = Account.getAccountIdForMessageId(mProviderContext, mMessageId);
}
@Override
public void loadAttachmentProgress(int progress) {
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadAttachmentCallback(null, mAccountId, mMessageId, mAttachmentId,
progress);
}
}
}
@Override
public void messageRetrieved(com.android.email.mail.Message message) {
}
}
/**
* Support for receiving callbacks from MessagingController and dealing with UI going
* out of scope.
*/
public class LegacyListener extends MessagingListener implements MessageRetrievalListener {
public LegacyListener(long messageId, long attachmentId) {
super(messageId, attachmentId);
}
public class LegacyListener extends MessagingListener {
public LegacyListener() {
}
@ -1244,28 +1271,31 @@ public class Controller {
@Override
public void loadMessageForViewStarted(long messageId) {
final long accountId = Account.getAccountIdForMessageId(mProviderContext, messageId);
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadMessageForViewCallback(null, messageId, 0);
listener.loadMessageForViewCallback(null, accountId, messageId, 0);
}
}
}
@Override
public void loadMessageForViewFinished(long messageId) {
final long accountId = Account.getAccountIdForMessageId(mProviderContext, messageId);
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadMessageForViewCallback(null, messageId, 100);
listener.loadMessageForViewCallback(null, accountId, messageId, 100);
}
}
}
@Override
public void loadMessageForViewFailed(long messageId, String message) {
final long accountId = Account.getAccountIdForMessageId(mProviderContext, messageId);
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadMessageForViewCallback(new MessagingException(message),
messageId, 0);
accountId, messageId, 0);
}
}
}
@ -1280,7 +1310,7 @@ public class Controller {
}
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadAttachmentCallback(null, messageId, attachmentId, 0);
listener.loadAttachmentCallback(null, accountId, messageId, attachmentId, 0);
}
}
}
@ -1294,20 +1324,11 @@ public class Controller {
}
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadAttachmentCallback(null, messageId, attachmentId, 100);
listener.loadAttachmentCallback(null, accountId, messageId, attachmentId, 100);
}
}
}
@Override
public void loadAttachmentProgress(int progress) {
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadAttachmentCallback(null, messageId, attachmentId, progress);
}
}
}
@Override
public void loadAttachmentFailed(long accountId, long messageId, long attachmentId,
MessagingException me) {
@ -1324,7 +1345,7 @@ public class Controller {
}
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadAttachmentCallback(me, messageId, attachmentId, 0);
listener.loadAttachmentCallback(me, accountId, messageId, attachmentId, 0);
}
}
}
@ -1362,10 +1383,6 @@ public class Controller {
}
}
}
@Override
public void messageRetrieved(com.android.email.mail.Message message) {
}
}
/**
@ -1393,9 +1410,11 @@ public class Controller {
}
break;
}
final long accountId = Account.getAccountIdForMessageId(mProviderContext, messageId);
synchronized (mListeners) {
for (Result listener : mListeners) {
listener.loadAttachmentCallback(result, messageId, attachmentId, progress);
listener.loadAttachmentCallback(result, accountId, messageId, attachmentId,
progress);
}
}
}

View File

@ -57,7 +57,7 @@ public class ControllerResultUiThreadWrapper<T extends Result> extends Result {
@Override
public void loadAttachmentCallback(final MessagingException result, final long messageId,
final long attachmentId, final int progress) {
final long accountId, final long attachmentId, final int progress) {
run(new Runnable() {
public void run() {
/* It's possible this callback is unregistered after this Runnable was posted and
@ -65,18 +65,19 @@ public class ControllerResultUiThreadWrapper<T extends Result> extends Result {
* on the UI thread.
*/
if (!isRegistered()) return;
mWrappee.loadAttachmentCallback(result, messageId, attachmentId, progress);
mWrappee.loadAttachmentCallback(result, accountId, messageId, attachmentId,
progress);
}
});
}
@Override
public void loadMessageForViewCallback(final MessagingException result,
public void loadMessageForViewCallback(final MessagingException result, final long accountId,
final long messageId, final int progress) {
run(new Runnable() {
public void run() {
if (!isRegistered()) return;
mWrappee.loadMessageForViewCallback(result, messageId, progress);
mWrappee.loadMessageForViewCallback(result, accountId, messageId, progress);
}
});
}

View File

@ -26,7 +26,7 @@ import java.util.concurrent.ConcurrentHashMap;
public class GroupMessagingListener extends MessagingListener {
/* The synchronization of the methods in this class
is not needed because we use ConcurrentHashMap.
Nevertheless, let's keep the "synchronized" for a while in the case
we may want to change the implementation to use something else
than ConcurrentHashMap.
@ -171,14 +171,6 @@ public class GroupMessagingListener extends MessagingListener {
}
}
@Override
synchronized public void loadAttachmentProgress(
int progress) {
for (MessagingListener l : mListeners) {
l.loadAttachmentProgress(progress);
}
}
@Override
synchronized public void loadAttachmentFinished(
long accountId,

View File

@ -1936,7 +1936,8 @@ public class MessagingController implements Runnable {
FetchProfile fp = new FetchProfile();
fp.add(storePart);
remoteFolder.fetch(new Message[] { storeMessage }, fp,
mController.new LegacyListener(messageId, attachmentId));
mController.new MessageRetrievalListenerBridge(
messageId, attachmentId));
// If we failed to load the attachment, throw an Exception here, so that
// AttachmentDownloadService knows that we failed

View File

@ -28,14 +28,6 @@ import android.content.Context;
* changes in this class.
*/
public class MessagingListener {
long messageId = -1;
long attachmentId = -1;
public MessagingListener(long _messageId, long _attachmentId) {
messageId = _messageId;
attachmentId = _attachmentId;
}
public MessagingListener() {
}
@ -94,8 +86,6 @@ public class MessagingListener {
boolean requiresDownload) {
}
public void loadAttachmentProgress(int progress) {}
public void loadAttachmentFinished(
long accountId,
long messageId,

View File

@ -847,56 +847,18 @@ public class MessageListXL extends Activity implements
}
@Override
public void loadAttachmentCallback(
MessagingException result, long messageId, long attachmentId, int progress) {
new AccountFinder(result, messageId, progress).execute();
public void loadAttachmentCallback(MessagingException result, long accountId,
long messageId, long attachmentId, int progress) {
handleError(result, accountId, progress);
}
@Override
public void loadMessageForViewCallback(
MessagingException result, long messageId, int progress) {
new AccountFinder(result, messageId, progress).execute();
}
/**
* AsyncTask to determine the account id from a message id. Used for
* {@link #loadAttachmentCallback} and {@link #loadMessageForViewCallback}, which don't
* report the underlying account ID.
*/
private class AccountFinder extends AsyncTask<Void, Void, Long> {
private final MessagingException mException;
private final long mMessageId;
private final int mProgress;
public AccountFinder(MessagingException exception, long messageId, int progress) {
mException = exception;
mMessageId = messageId;
mProgress = progress;
}
@Override
protected Long doInBackground(Void... params) {
if (mMessageId == -1) {
return null; // Message ID unknown
}
Message m = Message.restoreMessageWithId(MessageListXL.this, mMessageId);
return m != null ? m.mAccountKey : null;
}
@Override
protected void onPostExecute(Long accountId) {
if ((accountId == null) || isCancelled()) {
return;
}
handleError(mException, accountId, mProgress);
}
public void loadMessageForViewCallback(MessagingException result, long accountId,
long messageId, int progress) {
handleError(result, accountId, progress);
}
private void handleError(MessagingException result, long accountId, int progress) {
if (!isRegistered()) {
// This ControllerResult may be already unregistered, because of the asynctask.
return;
}
if (accountId == -1) {
return;
}

View File

@ -1374,8 +1374,8 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O
}
@Override
public void loadMessageForViewCallback(MessagingException result, long messageId,
int progress) {
public void loadMessageForViewCallback(MessagingException result, long accountId,
long messageId, int progress) {
if (messageId != mWaitForLoadMessageId) {
// We are not waiting for this message to load, so exit quickly
return;
@ -1414,8 +1414,8 @@ public abstract class MessageViewFragmentBase extends Fragment implements View.O
}
@Override
public void loadAttachmentCallback(MessagingException result, long messageId,
long attachmentId, int progress) {
public void loadAttachmentCallback(MessagingException result, long accountId,
long messageId, long attachmentId, int progress) {
if (messageId == mMessageId) {
if (result == null) {
showAttachmentProgress(attachmentId, progress);

View File

@ -1507,6 +1507,17 @@ public abstract class EmailContent {
return null;
}
/**
* Return the account ID for a message with a given id
*
* @param context the caller's context
* @param messageId the id of the message
* @return the account ID, or -1 if the account doesn't exist
*/
public static long getAccountIdForMessageId(Context context, long messageId) {
return Message.getKeyColumnLong(context, messageId, MessageColumns.ACCOUNT_KEY);
}
/**
* Return the account for a message with a given id
* @param context the caller's context
@ -1514,8 +1525,7 @@ public abstract class EmailContent {
* @return the account, or null if the account doesn't exist
*/
public static Account getAccountForMessageId(Context context, long messageId) {
long accountId = Message.getKeyColumnLong(context, messageId,
MessageColumns.ACCOUNT_KEY);
long accountId = getAccountIdForMessageId(context, messageId);
if (accountId != -1) {
return Account.restoreAccountWithId(context, accountId);
}

View File

@ -2069,6 +2069,22 @@ public class ProviderTests extends ProviderTestCase2<EmailProvider> {
assertEquals(b2.mId, Message.getKeyColumnLong(c, m2.mId, MessageColumns.MAILBOX_KEY));
}
public void testGetAccountIdForMessageId() {
final Context c = mMockContext;
Account a1 = ProviderTestUtils.setupAccount("acct1", true, c);
Account a2 = ProviderTestUtils.setupAccount("acct2", true, c);
Mailbox b1 = ProviderTestUtils.setupMailbox("box1", a1.mId, true, c, Mailbox.TYPE_MAIL);
Mailbox b2 = ProviderTestUtils.setupMailbox("box2", a2.mId, true, c, Mailbox.TYPE_MAIL);
Message m1 = createMessage(c, b1, false, false);
Message m2 = createMessage(c, b2, false, false);
assertEquals(a1.mId, Account.getAccountIdForMessageId(c, m1.mId));
assertEquals(a2.mId, Account.getAccountIdForMessageId(c, m2.mId));
// message desn't exist
assertEquals(-1, Account.getAccountIdForMessageId(c, 12345));
}
public void testGetAccountMailboxFromMessageId() {
final Context c = mMockContext;
Account a = ProviderTestUtils.setupAccount("acct", true, c);