Decouple RemoteMessageStore from MessagingListener

This commit is contained in:
cketti 2018-05-27 18:46:51 +02:00
parent 84d2e680ef
commit dfcc4376a5
6 changed files with 168 additions and 94 deletions

View file

@ -92,6 +92,7 @@ import com.fsck.k9.notification.NotificationController;
import com.fsck.k9.search.LocalSearch;
import com.fsck.k9.search.SearchAccount;
import com.fsck.k9.search.SearchSpecification;
import org.jetbrains.annotations.NotNull;
import timber.log.Timber;
import static com.fsck.k9.K9.MAX_SEND_ATTEMPTS;
@ -739,7 +740,8 @@ public class MessagingController {
Folder providedRemoteFolder) {
RemoteMessageStore remoteMessageStore = getRemoteMessageStore(account);
if (remoteMessageStore != null) {
remoteMessageStore.sync(account, folder, listener, providedRemoteFolder);
ControllerSyncListener syncListener = new ControllerSyncListener(account, listener);
remoteMessageStore.sync(account, folder, syncListener, providedRemoteFolder);
} else {
synchronizeMailboxSynchronousLegacy(account, folder, listener);
}
@ -4093,4 +4095,88 @@ public class MessagingController {
private interface MessageActor {
void act(Account account, LocalFolder messageFolder, List<LocalMessage> messages);
}
class ControllerSyncListener implements SyncListener {
private final Account account;
private final MessagingListener listener;
ControllerSyncListener(Account account, MessagingListener listener) {
this.account = account;
this.listener = listener;
}
@Override
public void syncStarted(@NotNull String folderServerId, @NotNull String folderName) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxStarted(account, folderServerId, folderName);
}
}
@Override
public void syncHeadersStarted(@NotNull String folderServerId, @NotNull String folderName) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxHeadersStarted(account, folderServerId, folderName);
}
}
@Override
public void syncHeadersProgress(@NotNull String folderServerId, int completed, int total) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxHeadersProgress(account, folderServerId, completed, total);
}
}
@Override
public void syncHeadersFinished(@NotNull String folderServerId, int totalMessagesInMailbox,
int numNewMessages) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxHeadersFinished(account, folderServerId, totalMessagesInMailbox,
numNewMessages);
}
}
@Override
public void syncProgress(@NotNull String folderServerId, int completed, int total) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxProgress(account, folderServerId, completed, total);
}
}
@Override
public void syncNewMessage(@NotNull String folderServerId, @NotNull Message message) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxNewMessage(account, folderServerId, message);
}
}
@Override
public void syncRemovedMessage(@NotNull String folderServerId, @NotNull Message message) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxRemovedMessage(account, folderServerId, message);
}
}
@Override
public void syncFinished(@NotNull String folderServerId, int totalMessagesInMailbox, int numNewMessages) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxFinished(account, folderServerId, totalMessagesInMailbox,
numNewMessages);
}
}
@Override
public void syncFailed(@NotNull String folderServerId, @NotNull String message) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.synchronizeMailboxFailed(account, folderServerId, message);
}
}
@Override
public void folderStatusChanged(@NotNull String folderServerId, int unreadMessageCount) {
for (MessagingListener messagingListener : getListeners(listener)) {
messagingListener.folderStatusChanged(account, folderServerId, unreadMessageCount);
}
}
}
}

View file

@ -9,7 +9,7 @@ public interface RemoteMessageStore {
// TODO: Nicer interface
// Instead of using Account pass in "remote store config", "sync config", "local mail store" (like LocalStore
// only with an interface/implementation optimized for sync; eventually this can replace LocalStore which does
// many things we don't need and does badly some of the things we do need), "folder id", "sync listener"
// many things we don't need and does badly some of the things we do need), "folder id"
// TODO: Add a way to cancel the sync process
void sync(Account account, String folder, MessagingListener listener, Folder providedRemoteFolder);
void sync(Account account, String folder, SyncListener listener, Folder providedRemoteFolder);
}

View file

@ -0,0 +1,20 @@
package com.fsck.k9.controller
import com.fsck.k9.mail.Message
interface SyncListener {
fun syncStarted(folderServerId: String, folderName: String)
fun syncHeadersStarted(folderServerId: String, folderName: String)
fun syncHeadersProgress(folderServerId: String, completed: Int, total: Int)
fun syncHeadersFinished(folderServerId: String, totalMessagesInMailbox: Int, numNewMessages: Int)
fun syncProgress(folderServerId: String, completed: Int, total: Int)
fun syncNewMessage(folderServerId: String, message: Message)
fun syncRemovedMessage(folderServerId: String, message: Message)
fun syncFinished(folderServerId: String, totalMessagesInMailbox: Int, numNewMessages: Int)
fun syncFailed(folderServerId: String, message: String)
fun folderStatusChanged(folderServerId: String, unreadMessageCount: Int)
}

View file

@ -7,6 +7,7 @@ import com.fsck.k9.Account;
import com.fsck.k9.controller.MessagingController;
import com.fsck.k9.controller.MessagingListener;
import com.fsck.k9.controller.RemoteMessageStore;
import com.fsck.k9.controller.SyncListener;
import com.fsck.k9.mail.Folder;
import com.fsck.k9.notification.NotificationController;
@ -21,7 +22,7 @@ public class ImapMessageStore implements RemoteMessageStore {
}
@Override
public void sync(Account account, String folder, MessagingListener listener, Folder providedRemoteFolder) {
public void sync(Account account, String folder, SyncListener listener, Folder providedRemoteFolder) {
imapSync.sync(account, folder, listener, providedRemoteFolder);
}
}

View file

@ -20,7 +20,7 @@ import com.fsck.k9.AccountStats;
import com.fsck.k9.K9;
import com.fsck.k9.activity.MessageReference;
import com.fsck.k9.controller.MessagingController;
import com.fsck.k9.controller.MessagingListener;
import com.fsck.k9.controller.SyncListener;
import com.fsck.k9.controller.UidReverseComparator;
import com.fsck.k9.mail.AuthenticationFailedException;
import com.fsck.k9.mail.BodyFactory;
@ -28,7 +28,6 @@ import com.fsck.k9.mail.DefaultBodyFactory;
import com.fsck.k9.mail.FetchProfile;
import com.fsck.k9.mail.Flag;
import com.fsck.k9.mail.Folder;
import com.fsck.k9.mail.Folder.FolderType;
import com.fsck.k9.mail.Message;
import com.fsck.k9.mail.MessageRetrievalListener;
import com.fsck.k9.mail.MessagingException;
@ -59,11 +58,11 @@ class ImapSync {
this.context = context;
}
void sync(Account account, String folder, MessagingListener listener, Folder providedRemoteFolder) {
void sync(Account account, String folder, SyncListener listener, Folder providedRemoteFolder) {
synchronizeMailboxSynchronous(account, folder, listener, providedRemoteFolder);
}
void synchronizeMailboxSynchronous(final Account account, final String folder, final MessagingListener listener,
void synchronizeMailboxSynchronous(final Account account, final String folder, final SyncListener listener,
Folder providedRemoteFolder) {
Folder remoteFolder = null;
LocalFolder tLocalFolder = null;
@ -86,9 +85,7 @@ class ImapSync {
localFolder.open(Folder.OPEN_MODE_RW);
String folderName = localFolder.getName();
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxStarted(account, folder, folderName);
}
listener.syncStarted(folder, folderName);
try {
processPendingCommandsSynchronous(account);
@ -180,9 +177,7 @@ class ImapSync {
remoteStart, remoteMessageCount, folder);
final AtomicInteger headerProgress = new AtomicInteger(0);
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxHeadersStarted(account, folder, folderName);
}
listener.syncHeadersStarted(folder, folderName);
List<? extends Message> remoteMessageArray =
@ -192,9 +187,8 @@ class ImapSync {
for (Message thisMess : remoteMessageArray) {
headerProgress.incrementAndGet();
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxHeadersProgress(account, folder, headerProgress.get(), messageCount);
}
listener.syncHeadersProgress(folder, headerProgress.get(), messageCount);
Long localMessageTimestamp = localUidMap.get(thisMess.getUid());
if (localMessageTimestamp == null || localMessageTimestamp >= earliestTimestamp) {
remoteMessages.add(thisMess);
@ -204,9 +198,7 @@ class ImapSync {
Timber.v("SYNC: Got %d messages for folder %s", remoteUidMap.size(), folder);
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxHeadersFinished(account, folder, headerProgress.get(), remoteUidMap.size());
}
listener.syncHeadersFinished(folder, headerProgress.get(), remoteUidMap.size());
} else if (remoteMessageCount < 0) {
throw new Exception("Message count " + remoteMessageCount + " for folder " + folder);
@ -231,9 +223,7 @@ class ImapSync {
localFolder.destroyMessages(destroyMessages);
for (Message destroyMessage : destroyMessages) {
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxRemovedMessage(account, folder, destroyMessage);
}
listener.syncRemovedMessage(folder, destroyMessage);
}
}
}
@ -247,12 +237,11 @@ class ImapSync {
/*
* Now we download the actual content of messages.
*/
int newMessages = downloadMessages(account, remoteFolder, localFolder, remoteMessages, false, true);
int newMessages = downloadMessages(account, remoteFolder, localFolder, remoteMessages, false,
true, listener);
int unreadMessageCount = localFolder.getUnreadMessageCount();
for (MessagingListener l : getListeners()) {
l.folderStatusChanged(account, folder, unreadMessageCount);
}
listener.folderStatusChanged(folder, unreadMessageCount);
/* Notify listeners that we're finally done. */
@ -265,9 +254,7 @@ class ImapSync {
System.currentTimeMillis(),
newMessages);
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxFinished(account, folder, remoteMessageCount, newMessages);
}
listener.syncFinished(folder, remoteMessageCount, newMessages);
if (commandException != null) {
@ -275,9 +262,7 @@ class ImapSync {
Timber.e("Root cause failure in %s:%s was '%s'",
account.getDescription(), tLocalFolder.getServerId(), rootMessage);
localFolder.setStatus(rootMessage);
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxFailed(account, folder, rootMessage);
}
listener.syncFailed(folder, rootMessage);
}
Timber.i("Done synchronizing folder %s:%s", account.getDescription(), folder);
@ -285,9 +270,7 @@ class ImapSync {
} catch (AuthenticationFailedException e) {
handleAuthenticationFailure(account, true);
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxFailed(account, folder, "Authentication failure");
}
listener.syncFailed(folder, "Authentication failure");
} catch (Exception e) {
Timber.e(e, "synchronizeMailbox");
// If we don't set the last checked, it can try too often during
@ -303,9 +286,8 @@ class ImapSync {
}
}
for (MessagingListener l : getListeners(listener)) {
l.synchronizeMailboxFailed(account, folder, rootMessage);
}
listener.syncFailed(folder, rootMessage);
notifyUserIfCertificateProblem(account, e, true);
Timber.e("Failed synchronizing folder %s:%s @ %tc", account.getDescription(), folder,
System.currentTimeMillis());
@ -343,7 +325,7 @@ class ImapSync {
*/
int downloadMessages(final Account account, final Folder remoteFolder,
final LocalFolder localFolder, List<Message> inputMessages,
boolean flagSyncOnly, boolean purgeToVisibleLimit) throws MessagingException {
boolean flagSyncOnly, boolean purgeToVisibleLimit, final SyncListener listener) throws MessagingException {
final Date earliestDate = account.getEarliestPollDate();
Date downloadStarted = new Date(); // now
@ -370,14 +352,12 @@ class ImapSync {
for (Message message : messages) {
evaluateMessageForDownload(message, folder, localFolder, remoteFolder, account, unsyncedMessages,
syncFlagMessages, flagSyncOnly);
syncFlagMessages, flagSyncOnly, listener);
}
final AtomicInteger progress = new AtomicInteger(0);
final int todo = unsyncedMessages.size() + syncFlagMessages.size();
for (MessagingListener l : getListeners()) {
l.synchronizeMailboxProgress(account, folder, progress.get(), todo);
}
listener.syncProgress(folder, progress.get(), todo);
Timber.d("SYNC: Have %d unsynced messages", unsyncedMessages.size());
@ -400,7 +380,7 @@ class ImapSync {
Timber.d("SYNC: About to fetch %d unsynced messages for folder %s", unsyncedMessages.size(), folder);
fetchUnsyncedMessages(account, remoteFolder, unsyncedMessages, smallMessages, largeMessages, progress, todo,
fp);
fp, listener);
String updatedPushState = localFolder.getPushState();
for (Message message : unsyncedMessages) {
@ -429,7 +409,7 @@ class ImapSync {
// fp.add(FetchProfile.Item.FLAGS);
// fp.add(FetchProfile.Item.ENVELOPE);
downloadSmallMessages(account, remoteFolder, localFolder, smallMessages, progress, unreadBeforeStart,
newMessages, todo, fp);
newMessages, todo, fp, listener);
smallMessages.clear();
/*
* Now do the large messages that require more round trips.
@ -437,7 +417,7 @@ class ImapSync {
fp = new FetchProfile();
fp.add(FetchProfile.Item.STRUCTURE);
downloadLargeMessages(account, remoteFolder, localFolder, largeMessages, progress, unreadBeforeStart,
newMessages, todo, fp);
newMessages, todo, fp, listener);
largeMessages.clear();
/*
@ -445,7 +425,7 @@ class ImapSync {
* download.
*/
refreshLocalMessageFlags(account, remoteFolder, localFolder, syncFlagMessages, progress, todo);
refreshLocalMessageFlags(account, remoteFolder, localFolder, syncFlagMessages, progress, todo, listener);
Timber.d("SYNC: Synced remote messages for folder %s, %d new messages", folder, newMessages.get());
@ -453,9 +433,7 @@ class ImapSync {
localFolder.purgeToVisibleLimit(new MessageRemovalListener() {
@Override
public void messageRemoved(Message message) {
for (MessagingListener l : getListeners()) {
l.synchronizeMailboxRemovedMessage(account, folder, message);
}
listener.syncRemovedMessage(folder, message);
}
});
@ -470,7 +448,8 @@ class ImapSync {
final Account account,
final List<Message> unsyncedMessages,
final List<Message> syncFlagMessages,
boolean flagSyncOnly) throws MessagingException {
boolean flagSyncOnly,
SyncListener listener) throws MessagingException {
if (message.isSet(Flag.DELETED)) {
Timber.v("Message with uid %s is marked as deleted", message.getUid());
@ -497,10 +476,8 @@ class ImapSync {
localMessage.setFlag(Flag.X_DOWNLOADED_FULL, message.isSet(Flag.X_DOWNLOADED_FULL));
localMessage.setFlag(Flag.X_DOWNLOADED_PARTIAL, message.isSet(Flag.X_DOWNLOADED_PARTIAL));
for (MessagingListener l : getListeners()) {
if (!localMessage.isSet(Flag.SEEN)) {
l.synchronizeMailboxNewMessage(account, folder, localMessage);
}
if (!localMessage.isSet(Flag.SEEN)) {
listener.syncNewMessage(folder, localMessage);
}
}
}
@ -529,7 +506,8 @@ class ImapSync {
final List<Message> largeMessages,
final AtomicInteger progress,
final int todo,
FetchProfile fp) throws MessagingException {
FetchProfile fp,
final SyncListener listener) throws MessagingException {
final String folder = remoteFolder.getServerId();
final Date earliestDate = account.getEarliestPollDate();
@ -549,10 +527,10 @@ class ImapSync {
}
}
progress.incrementAndGet();
for (MessagingListener l : getListeners()) {
//TODO: This might be the source of poll count errors in the UI. Is todo always the same as ofTotal
l.synchronizeMailboxProgress(account, folder, progress.get(), todo);
}
//TODO: This might be the source of poll count errors in the UI. Is todo always the same as ofTotal
listener.syncProgress(folder, progress.get(), todo);
return;
}
@ -586,7 +564,8 @@ class ImapSync {
final int unreadBeforeStart,
final AtomicInteger newMessages,
final int todo,
FetchProfile fp) throws MessagingException {
FetchProfile fp,
final SyncListener listener) throws MessagingException {
final String folder = remoteFolder.getServerId();
final Date earliestDate = account.getEarliestPollDate();
@ -623,12 +602,11 @@ class ImapSync {
account, folder, message.getUid());
// Update the listener with what we've found
for (MessagingListener l : getListeners()) {
l.synchronizeMailboxProgress(account, folder, progress.get(), todo);
if (!localMessage.isSet(Flag.SEEN)) {
l.synchronizeMailboxNewMessage(account, folder, localMessage);
}
listener.syncProgress(folder, progress.get(), todo);
if (!localMessage.isSet(Flag.SEEN)) {
listener.syncNewMessage(folder, localMessage);
}
// Send a notification of this message
if (shouldNotifyForMessage(account, localFolder, message)) {
@ -660,7 +638,8 @@ class ImapSync {
final int unreadBeforeStart,
final AtomicInteger newMessages,
final int todo,
FetchProfile fp) throws MessagingException {
FetchProfile fp,
SyncListener listener) throws MessagingException {
final String folder = remoteFolder.getServerId();
final Date earliestDate = account.getEarliestPollDate();
@ -692,12 +671,12 @@ class ImapSync {
if (!localMessage.isSet(Flag.SEEN)) {
newMessages.incrementAndGet();
}
for (MessagingListener l : getListeners()) {
l.synchronizeMailboxProgress(account, folder, progress.get(), todo);
if (!localMessage.isSet(Flag.SEEN)) {
l.synchronizeMailboxNewMessage(account, folder, localMessage);
}
listener.syncProgress(folder, progress.get(), todo);
if (!localMessage.isSet(Flag.SEEN)) {
listener.syncNewMessage(folder, localMessage);
}
// Send a notification of this message
if (shouldNotifyForMessage(account, localFolder, message)) {
// Notify with the localMessage so that we don't have to recalculate the content preview.
@ -712,7 +691,8 @@ class ImapSync {
final LocalFolder localFolder,
List<Message> syncFlagMessages,
final AtomicInteger progress,
final int todo
final int todo,
SyncListener listener
) throws MessagingException {
final String folder = remoteFolder.getServerId();
@ -735,9 +715,7 @@ class ImapSync {
if (messageChanged) {
boolean shouldBeNotifiedOf = false;
if (localMessage.isSet(Flag.DELETED) || isMessageSuppressed(localMessage)) {
for (MessagingListener l : getListeners()) {
l.synchronizeMailboxRemovedMessage(account, folder, localMessage);
}
listener.syncRemovedMessage(folder, localMessage);
} else {
if (shouldNotifyForMessage(account, localFolder, localMessage)) {
shouldBeNotifiedOf = true;
@ -751,9 +729,7 @@ class ImapSync {
}
}
progress.incrementAndGet();
for (MessagingListener l : getListeners()) {
l.synchronizeMailboxProgress(account, folder, progress.get(), todo);
}
listener.syncProgress(folder, progress.get(), todo);
}
}
@ -855,14 +831,6 @@ class ImapSync {
controller.processPendingCommandsSynchronous(account);
}
private Set<MessagingListener> getListeners() {
return controller.getListeners();
}
private Set<MessagingListener> getListeners(MessagingListener listener) {
return controller.getListeners(listener);
}
private void updateMoreMessages(Folder remoteFolder, LocalFolder localFolder, Date earliestDate, int remoteStart)
throws IOException, MessagingException {
controller.updateMoreMessages(remoteFolder, localFolder, earliestDate, remoteStart);

View file

@ -14,7 +14,7 @@ import com.fsck.k9.AccountStats;
import com.fsck.k9.RobolectricTest;
import com.fsck.k9.controller.MessagingController;
import com.fsck.k9.controller.MessagingListener;
import com.fsck.k9.controller.SimpleMessagingListener;
import com.fsck.k9.controller.SyncListener;
import com.fsck.k9.mail.FetchProfile;
import com.fsck.k9.mail.Folder;
import com.fsck.k9.mail.Message;
@ -65,7 +65,7 @@ public class ImapSyncTest extends RobolectricTest {
@Mock
private AccountStats accountStats;
@Mock
private SimpleMessagingListener listener;
private SyncListener listener;
@Mock
private LocalFolder localFolder;
@Mock
@ -103,7 +103,7 @@ public class ImapSyncTest extends RobolectricTest {
imapSync.sync(account, FOLDER_NAME, listener, remoteFolder);
verify(listener).synchronizeMailboxFinished(account, FOLDER_NAME, 1, 0);
verify(listener).syncFinished(FOLDER_NAME, 1, 0);
}
@Test
@ -112,7 +112,7 @@ public class ImapSyncTest extends RobolectricTest {
imapSync.sync(account, FOLDER_NAME, listener, remoteFolder);
verify(listener).synchronizeMailboxFinished(account, FOLDER_NAME, 0, 0);
verify(listener).syncFinished(FOLDER_NAME, 0, 0);
}
@Test
@ -121,8 +121,7 @@ public class ImapSyncTest extends RobolectricTest {
imapSync.sync(account, FOLDER_NAME, listener, remoteFolder);
verify(listener).synchronizeMailboxFailed(account, FOLDER_NAME,
"Exception: Message count -1 for folder Folder");
verify(listener).syncFailed(FOLDER_NAME, "Exception: Message count -1 for folder Folder");
}
@Test