Merge pull request #5935 from k9mail/callback_cleanup

Callback cleanup
This commit is contained in:
cketti 2022-03-01 16:42:22 +01:00 committed by GitHub
commit 547a87381b
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
17 changed files with 86 additions and 298 deletions

View file

@ -60,7 +60,6 @@ import com.fsck.k9.mail.Flag;
import com.fsck.k9.mail.FolderClass; import com.fsck.k9.mail.FolderClass;
import com.fsck.k9.mail.Message; import com.fsck.k9.mail.Message;
import com.fsck.k9.mail.MessageDownloadState; import com.fsck.k9.mail.MessageDownloadState;
import com.fsck.k9.mail.MessageRetrievalListener;
import com.fsck.k9.mail.MessagingException; import com.fsck.k9.mail.MessagingException;
import com.fsck.k9.mail.Part; import com.fsck.k9.mail.Part;
import com.fsck.k9.mail.ServerSettings; import com.fsck.k9.mail.ServerSettings;
@ -401,56 +400,22 @@ public class MessagingController {
/** /**
* Find all messages in any local account which match the query 'query' * Find all messages in any local account which match the query 'query'
*/ */
public void searchLocalMessages(final LocalSearch search, final MessagingListener listener) { public List<LocalMessage> searchLocalMessages(final LocalSearch search) {
threadPool.execute(new Runnable() {
@Override
public void run() {
searchLocalMessagesSynchronous(search, listener);
}
});
}
@VisibleForTesting
void searchLocalMessagesSynchronous(final LocalSearch search, final MessagingListener listener) {
List<Account> searchAccounts = getAccountsFromLocalSearch(search, preferences); List<Account> searchAccounts = getAccountsFromLocalSearch(search, preferences);
for (final Account account : searchAccounts) {
// Collecting statistics of the search result
MessageRetrievalListener<LocalMessage> retrievalListener = new MessageRetrievalListener<LocalMessage>() {
@Override
public void messageStarted(String message, int number, int ofTotal) {
}
@Override
public void messagesFinished(int number) {
}
@Override
public void messageFinished(LocalMessage message, int number, int ofTotal) {
if (!isMessageSuppressed(message)) {
List<LocalMessage> messages = new ArrayList<>(); List<LocalMessage> messages = new ArrayList<>();
for (final Account account : searchAccounts) {
messages.add(message);
if (listener != null) {
listener.listLocalMessagesAddMessages(account, null, messages);
}
}
}
};
// build and do the query in the localstore
try { try {
LocalStore localStore = localStoreProvider.getInstance(account); LocalStore localStore = localStoreProvider.getInstance(account);
localStore.searchForMessages(retrievalListener, search); List<LocalMessage> localMessages = localStore.searchForMessages(search);
messages.addAll(localMessages);
} catch (Exception e) { } catch (Exception e) {
Timber.e(e); Timber.e(e);
} }
} }
if (listener != null) { return messages;
listener.listLocalMessagesFinished();
}
} }
public Future<?> searchRemoteMessages(String acctUuid, long folderId, String query, Set<Flag> requiredFlags, public Future<?> searchRemoteMessages(String acctUuid, long folderId, String query, Set<Flag> requiredFlags,
@ -1070,7 +1035,7 @@ public class MessagingController {
Timber.i("Marking all messages in %s:%s as read", account, folderServerId); Timber.i("Marking all messages in %s:%s as read", account, folderServerId);
// TODO: Make this one database UPDATE operation // TODO: Make this one database UPDATE operation
List<LocalMessage> messages = localFolder.getMessages(null, false); List<LocalMessage> messages = localFolder.getMessages(false);
for (Message message : messages) { for (Message message : messages) {
if (!message.isSet(Flag.SEEN)) { if (!message.isSet(Flag.SEEN)) {
message.setFlag(Flag.SEEN, true); message.setFlag(Flag.SEEN, true);
@ -1539,7 +1504,7 @@ public class MessagingController {
long outboxFolderId = localFolder.getDatabaseId(); long outboxFolderId = localFolder.getDatabaseId();
List<LocalMessage> localMessages = localFolder.getMessages(null); List<LocalMessage> localMessages = localFolder.getMessages();
int progress = 0; int progress = 0;
int todo = localMessages.size(); int todo = localMessages.size();
for (MessagingListener l : getListeners()) { for (MessagingListener l : getListeners()) {
@ -2198,7 +2163,7 @@ public class MessagingController {
// Remove all messages marked as deleted // Remove all messages marked as deleted
folder.destroyDeletedMessages(); folder.destroyDeletedMessages();
compact(account, null); compact(account);
} }
public void emptyTrash(final Account account, MessagingListener listener) { public void emptyTrash(final Account account, MessagingListener listener) {
@ -2482,22 +2447,14 @@ public class MessagingController {
notificationController.clearFetchingMailNotification(account); notificationController.clearFetchingMailNotification(account);
} }
public void compact(final Account account, final MessagingListener ml) { public void compact(Account account) {
putBackground("compact:" + account, ml, new Runnable() { putBackground("compact:" + account, null, () -> {
@Override
public void run() {
try { try {
MessageStore messageStore = messageStoreManager.getMessageStore(account); MessageStore messageStore = messageStoreManager.getMessageStore(account);
long oldSize = messageStore.getSize();
messageStore.compact(); messageStore.compact();
long newSize = messageStore.getSize();
for (MessagingListener l : getListeners(ml)) {
l.accountSizeChanged(account, oldSize, newSize);
}
} catch (Exception e) { } catch (Exception e) {
Timber.e(e, "Failed to compact account %s", account); Timber.e(e, "Failed to compact account %s", account);
} }
}
}); });
} }

View file

@ -13,11 +13,6 @@ import com.fsck.k9.mailstore.LocalMessage;
public interface MessagingListener { public interface MessagingListener {
void accountSizeChanged(Account account, long oldSize, long newSize);
void listLocalMessagesAddMessages(Account account, String folderServerId, List<LocalMessage> messages);
void listLocalMessagesFinished();
void synchronizeMailboxStarted(Account account, long folderId); void synchronizeMailboxStarted(Account account, long folderId);
void synchronizeMailboxHeadersStarted(Account account, String folderServerId); void synchronizeMailboxHeadersStarted(Account account, String folderServerId);
void synchronizeMailboxHeadersProgress(Account account, String folderServerId, int completed, int total); void synchronizeMailboxHeadersProgress(Account account, String folderServerId, int completed, int total);

View file

@ -9,22 +9,9 @@ import android.content.Context;
import com.fsck.k9.Account; import com.fsck.k9.Account;
import com.fsck.k9.mail.Message; import com.fsck.k9.mail.Message;
import com.fsck.k9.mail.Part; import com.fsck.k9.mail.Part;
import com.fsck.k9.mailstore.LocalMessage;
public abstract class SimpleMessagingListener implements MessagingListener { public abstract class SimpleMessagingListener implements MessagingListener {
@Override
public void accountSizeChanged(Account account, long oldSize, long newSize) {
}
@Override
public void listLocalMessagesAddMessages(Account account, String folderServerId, List<LocalMessage> messages) {
}
@Override
public void listLocalMessagesFinished() {
}
@Override @Override
public void synchronizeMailboxStarted(Account account, long folderId) { public void synchronizeMailboxStarted(Account account, long folderId) {
} }

View file

@ -576,17 +576,16 @@ public class LocalFolder {
}); });
} }
public List<LocalMessage> getMessages(MessageRetrievalListener<LocalMessage> listener) throws MessagingException { public List<LocalMessage> getMessages() throws MessagingException {
return getMessages(listener, true); return getMessages(true);
} }
public List<LocalMessage> getMessages(final MessageRetrievalListener<LocalMessage> listener, public List<LocalMessage> getMessages(final boolean includeDeleted) throws MessagingException {
final boolean includeDeleted) throws MessagingException {
return localStore.getDatabase().execute(false, new DbCallback<List<LocalMessage>>() { return localStore.getDatabase().execute(false, new DbCallback<List<LocalMessage>>() {
@Override @Override
public List<LocalMessage> doDbWork(final SQLiteDatabase db) throws MessagingException { public List<LocalMessage> doDbWork(final SQLiteDatabase db) throws MessagingException {
open(); open();
return LocalFolder.this.localStore.getMessages(listener, LocalFolder.this, return LocalFolder.this.localStore.getMessages(LocalFolder.this,
"SELECT " + LocalStore.GET_MESSAGES_COLS + "SELECT " + LocalStore.GET_MESSAGES_COLS +
"FROM messages " + "FROM messages " +
"LEFT JOIN message_parts ON (message_parts.id = messages.message_part_id) " + "LEFT JOIN message_parts ON (message_parts.id = messages.message_part_id) " +
@ -898,7 +897,7 @@ public class LocalFolder {
public void setFlags(final Set<Flag> flags, boolean value) public void setFlags(final Set<Flag> flags, boolean value)
throws MessagingException { throws MessagingException {
open(); open();
for (LocalMessage message : getMessages(null)) { for (LocalMessage message : getMessages()) {
message.setFlags(flags, value); message.setFlags(flags, value);
} }
} }

View file

@ -43,7 +43,6 @@ import com.fsck.k9.mail.FetchProfile.Item;
import com.fsck.k9.mail.Flag; import com.fsck.k9.mail.Flag;
import com.fsck.k9.mail.FolderClass; import com.fsck.k9.mail.FolderClass;
import com.fsck.k9.mail.FolderType; import com.fsck.k9.mail.FolderType;
import com.fsck.k9.mail.MessageRetrievalListener;
import com.fsck.k9.mail.MessagingException; import com.fsck.k9.mail.MessagingException;
import com.fsck.k9.mail.Multipart; import com.fsck.k9.mail.Multipart;
import com.fsck.k9.mail.Part; import com.fsck.k9.mail.Part;
@ -359,9 +358,7 @@ public class LocalStore {
}); });
} }
public List<LocalMessage> searchForMessages(MessageRetrievalListener<LocalMessage> retrievalListener, public List<LocalMessage> searchForMessages(LocalSearch search) throws MessagingException {
LocalSearch search) throws MessagingException {
StringBuilder query = new StringBuilder(); StringBuilder query = new StringBuilder();
List<String> queryArgs = new ArrayList<>(); List<String> queryArgs = new ArrayList<>();
SqlQueryBuilder.buildWhereClause(account, search.getConditions(), query, queryArgs); SqlQueryBuilder.buildWhereClause(account, search.getConditions(), query, queryArgs);
@ -382,24 +379,20 @@ public class LocalStore {
Timber.d("Query = %s", sqlQuery); Timber.d("Query = %s", sqlQuery);
return getMessages(retrievalListener, null, sqlQuery, selectionArgs); return getMessages(null, sqlQuery, selectionArgs);
} }
/* /*
* Given a query string, actually do the query for the messages and * Given a query string, actually do the query for the messages and
* call the MessageRetrievalListener for each one * call the MessageRetrievalListener for each one
*/ */
List<LocalMessage> getMessages( List<LocalMessage> getMessages(LocalFolder folder, String queryString, String[] placeHolders)
final MessageRetrievalListener<LocalMessage> listener, throws MessagingException {
final LocalFolder folder,
final String queryString, final String[] placeHolders
) throws MessagingException {
final List<LocalMessage> messages = new ArrayList<>(); final List<LocalMessage> messages = new ArrayList<>();
final int j = database.execute(false, new DbCallback<Integer>() { database.execute(false, new DbCallback<Void>() {
@Override @Override
public Integer doDbWork(final SQLiteDatabase db) { public Void doDbWork(final SQLiteDatabase db) {
Cursor cursor = null; Cursor cursor = null;
int i = 0;
try { try {
cursor = db.rawQuery(queryString + " LIMIT 10", placeHolders); cursor = db.rawQuery(queryString + " LIMIT 10", placeHolders);
@ -408,10 +401,6 @@ public class LocalStore {
message.populateFromGetMessageCursor(cursor); message.populateFromGetMessageCursor(cursor);
messages.add(message); messages.add(message);
if (listener != null) {
listener.messageFinished(message, i, -1);
}
i++;
} }
cursor.close(); cursor.close();
cursor = db.rawQuery(queryString + " LIMIT -1 OFFSET 10", placeHolders); cursor = db.rawQuery(queryString + " LIMIT -1 OFFSET 10", placeHolders);
@ -421,22 +410,16 @@ public class LocalStore {
message.populateFromGetMessageCursor(cursor); message.populateFromGetMessageCursor(cursor);
messages.add(message); messages.add(message);
if (listener != null) {
listener.messageFinished(message, i, -1);
}
i++;
} }
} catch (Exception e) { } catch (Exception e) {
Timber.d(e, "Got an exception"); Timber.d(e, "Got an exception");
} finally { } finally {
Utility.closeQuietly(cursor); Utility.closeQuietly(cursor);
} }
return i;
return null;
} }
}); });
if (listener != null) {
listener.messagesFinished(j);
}
return Collections.unmodifiableList(messages); return Collections.unmodifiableList(messages);
@ -448,7 +431,7 @@ public class LocalStore {
LocalSearch search = new LocalSearch(); LocalSearch search = new LocalSearch();
search.and(SearchField.THREAD_ID, rootIdString, Attribute.EQUALS); search.and(SearchField.THREAD_ID, rootIdString, Attribute.EQUALS);
return searchForMessages(null, search); return searchForMessages(search);
} }
public AttachmentInfo getAttachmentInfo(final String attachmentId) throws MessagingException { public AttachmentInfo getAttachmentInfo(final String attachmentId) throws MessagingException {

View file

@ -52,6 +52,7 @@ import org.mockito.stubbing.Answer;
import org.robolectric.RuntimeEnvironment; import org.robolectric.RuntimeEnvironment;
import org.robolectric.shadows.ShadowLog; import org.robolectric.shadows.ShadowLog;
import static com.google.common.truth.Truth.assertThat;
import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.nullable; import static org.mockito.ArgumentMatchers.nullable;
import static org.mockito.ArgumentMatchers.eq; import static org.mockito.ArgumentMatchers.eq;
@ -178,32 +179,17 @@ public class MessagingControllerTest extends K9RobolectricTest {
} }
@Test @Test
public void searchLocalMessagesSynchronous_shouldCallSearchForMessagesOnLocalStore() public void searchLocalMessages_shouldIgnoreExceptions()
throws Exception {
when(search.searchAllAccounts()).thenReturn(true);
when(search.getAccountUuids()).thenReturn(new String[0]);
controller.searchLocalMessagesSynchronous(search, listener);
verify(localStore).searchForMessages(nullable(MessageRetrievalListener.class), eq(search));
}
@Test
public void searchLocalMessagesSynchronous_shouldNotifyWhenStoreFinishesRetrievingAMessage()
throws Exception { throws Exception {
LocalMessage localMessage = mock(LocalMessage.class); LocalMessage localMessage = mock(LocalMessage.class);
when(localMessage.getFolder()).thenReturn(localFolder); when(localMessage.getFolder()).thenReturn(localFolder);
when(search.searchAllAccounts()).thenReturn(true); when(search.searchAllAccounts()).thenReturn(true);
when(search.getAccountUuids()).thenReturn(new String[0]); when(search.getAccountUuids()).thenReturn(new String[0]);
when(localStore.searchForMessages(nullable(MessageRetrievalListener.class), eq(search))) when(localStore.searchForMessages(search)).thenThrow(new MessagingException("Test"));
.thenThrow(new MessagingException("Test"));
controller.searchLocalMessagesSynchronous(search, listener); List<LocalMessage> messages = controller.searchLocalMessages(search);
verify(localStore).searchForMessages(messageRetrievalListenerCaptor.capture(), eq(search)); assertThat(messages).isEmpty();
messageRetrievalListenerCaptor.getValue().messageFinished(localMessage, 1, 1);
verify(listener).listLocalMessagesAddMessages(eq(account),
eq((String) null), eq(Collections.singletonList(localMessage)));
} }
private void setupRemoteSearch() throws Exception { private void setupRemoteSearch() throws Exception {
@ -419,7 +405,7 @@ public class MessagingControllerTest extends K9RobolectricTest {
when(localStore.getFolder(SENT_FOLDER_ID)).thenReturn(sentFolder); when(localStore.getFolder(SENT_FOLDER_ID)).thenReturn(sentFolder);
when(sentFolder.getDatabaseId()).thenReturn(SENT_FOLDER_ID); when(sentFolder.getDatabaseId()).thenReturn(SENT_FOLDER_ID);
when(localFolder.exists()).thenReturn(true); when(localFolder.exists()).thenReturn(true);
when(localFolder.getMessages(null)).thenReturn(Collections.singletonList(localMessageToSend1)); when(localFolder.getMessages()).thenReturn(Collections.singletonList(localMessageToSend1));
when(localMessageToSend1.getUid()).thenReturn("localMessageToSend1"); when(localMessageToSend1.getUid()).thenReturn("localMessageToSend1");
when(localMessageToSend1.getDatabaseId()).thenReturn(42L); when(localMessageToSend1.getDatabaseId()).thenReturn(42L);
when(localMessageToSend1.getHeader(K9.IDENTITY_HEADER)).thenReturn(new String[]{}); when(localMessageToSend1.getHeader(K9.IDENTITY_HEADER)).thenReturn(new String[]{});

View file

@ -478,16 +478,12 @@ public class MessageProvider extends ContentProvider {
} }
protected MatrixCursor getMessages(String[] projection) throws InterruptedException { protected MatrixCursor getMessages(String[] projection) throws InterruptedException {
BlockingQueue<List<MessageInfoHolder>> queue = new SynchronousQueue<>();
// new code for integrated inbox, only execute this once as it will be processed afterwards via the listener // new code for integrated inbox, only execute this once as it will be processed afterwards via the listener
SearchAccount integratedInboxAccount = SearchAccount.createUnifiedInboxAccount(); SearchAccount integratedInboxAccount = SearchAccount.createUnifiedInboxAccount();
MessagingController msgController = MessagingController.getInstance(getContext()); MessagingController msgController = MessagingController.getInstance(getContext());
msgController.searchLocalMessages(integratedInboxAccount.getRelatedSearch(), List<LocalMessage> messages = msgController.searchLocalMessages(integratedInboxAccount.getRelatedSearch());
new MessageInfoHolderRetrieverListener(queue)); List<MessageInfoHolder> holders = convertToMessageInfoHolder(messages);
List<MessageInfoHolder> holders = queue.take();
// TODO add sort order parameter // TODO add sort order parameter
Collections.sort(holders, new ReverseDateComparator()); Collections.sort(holders, new ReverseDateComparator());
@ -521,6 +517,20 @@ public class MessageProvider extends ContentProvider {
return cursor; return cursor;
} }
private List<MessageInfoHolder> convertToMessageInfoHolder(List<LocalMessage> messages) {
List<MessageInfoHolder> holders = new ArrayList<>();
Context context = getContext();
for (LocalMessage message : messages) {
Account messageAccount = message.getAccount();
MessageInfoHolder messageInfoHolder = MessageInfoHolder.create(context, message, messageAccount);
holders.add(messageInfoHolder);
}
return holders;
}
protected LinkedHashMap<String, FieldExtractor<MessageInfoHolder, ?>> resolveMessageExtractors( protected LinkedHashMap<String, FieldExtractor<MessageInfoHolder, ?>> resolveMessageExtractors(
String[] projection, int count) { String[] projection, int count) {
LinkedHashMap<String, FieldExtractor<MessageInfoHolder, ?>> extractors = new LinkedHashMap<>(); LinkedHashMap<String, FieldExtractor<MessageInfoHolder, ?>> extractors = new LinkedHashMap<>();
@ -1033,38 +1043,4 @@ public class MessageProvider extends ContentProvider {
return wrapped; return wrapped;
} }
} }
/**
* Synchronized listener used to retrieve {@link MessageInfoHolder}s using a given {@link BlockingQueue}.
*/
protected class MessageInfoHolderRetrieverListener extends SimpleMessagingListener {
private final BlockingQueue<List<MessageInfoHolder>> queue;
private List<MessageInfoHolder> holders = new ArrayList<>();
public MessageInfoHolderRetrieverListener(BlockingQueue<List<MessageInfoHolder>> queue) {
this.queue = queue;
}
@Override
public void listLocalMessagesAddMessages(Account account, String folderServerId, List<LocalMessage> messages) {
Context context = getContext();
for (LocalMessage message : messages) {
Account messageAccount = message.getAccount();
MessageInfoHolder messageInfoHolder = MessageInfoHolder.create(context, message, messageAccount);
holders.add(messageInfoHolder);
}
}
@Override
public void listLocalMessagesFinished() {
try {
queue.put(holders);
} catch (InterruptedException e) {
Timber.e(e, "Unable to return message list back to caller");
}
}
}
} }

View file

@ -483,7 +483,7 @@ internal class ImapSync(
unsyncedMessages, unsyncedMessages,
fetchProfile, fetchProfile,
object : MessageRetrievalListener<ImapMessage> { object : MessageRetrievalListener<ImapMessage> {
override fun messageFinished(message: ImapMessage, number: Int, ofTotal: Int) { override fun messageFinished(message: ImapMessage) {
try { try {
if (message.isSet(Flag.DELETED)) { if (message.isSet(Flag.DELETED)) {
Timber.v( Timber.v(
@ -509,9 +509,6 @@ internal class ImapSync(
Timber.e(e, "Error while storing downloaded message.") Timber.e(e, "Error while storing downloaded message.")
} }
} }
override fun messageStarted(uid: String, number: Int, ofTotal: Int) = Unit
override fun messagesFinished(total: Int) = Unit
}, },
syncConfig.maximumAutoDownloadMessageSize syncConfig.maximumAutoDownloadMessageSize
) )
@ -536,7 +533,7 @@ internal class ImapSync(
smallMessages, smallMessages,
fetchProfile, fetchProfile,
object : MessageRetrievalListener<ImapMessage> { object : MessageRetrievalListener<ImapMessage> {
override fun messageFinished(message: ImapMessage, number: Int, ofTotal: Int) { override fun messageFinished(message: ImapMessage) {
try { try {
// Store the updated message locally // Store the updated message locally
backendFolder.saveMessage(message, MessageDownloadState.FULL) backendFolder.saveMessage(message, MessageDownloadState.FULL)
@ -562,9 +559,6 @@ internal class ImapSync(
Timber.e(e, "SYNC: fetch small messages") Timber.e(e, "SYNC: fetch small messages")
} }
} }
override fun messageStarted(uid: String, number: Int, ofTotal: Int) = Unit
override fun messagesFinished(total: Int) = Unit
}, },
-1 -1
) )

View file

@ -97,7 +97,7 @@ class TestImapFolder(override val serverId: String) : ImapFolder {
) { ) {
if (messages.isEmpty()) return if (messages.isEmpty()) return
messages.forEachIndexed { index, imapMessage -> for (imapMessage in messages) {
val uid = imapMessage.uid.toLong() val uid = imapMessage.uid.toLong()
val flags = messageFlags[uid].orEmpty().toSet() val flags = messageFlags[uid].orEmpty().toSet()
@ -109,7 +109,7 @@ class TestImapFolder(override val serverId: String) : ImapFolder {
} }
imapMessage.body = storedMessage.body imapMessage.body = storedMessage.body
listener?.messageFinished(imapMessage, index, messages.size) listener?.messageFinished(imapMessage)
} }
} }

View file

@ -419,7 +419,7 @@ class Pop3Sync {
remoteFolder.fetch(unsyncedMessages, fp, remoteFolder.fetch(unsyncedMessages, fp,
new MessageRetrievalListener<Pop3Message>() { new MessageRetrievalListener<Pop3Message>() {
@Override @Override
public void messageFinished(Pop3Message message, int number, int ofTotal) { public void messageFinished(Pop3Message message) {
try { try {
if (message.isSet(Flag.DELETED) || message.olderThan(earliestDate)) { if (message.isSet(Flag.DELETED) || message.olderThan(earliestDate)) {
if (message.isSet(Flag.DELETED)) { if (message.isSet(Flag.DELETED)) {
@ -447,16 +447,6 @@ class Pop3Sync {
Timber.e(e, "Error while storing downloaded message."); Timber.e(e, "Error while storing downloaded message.");
} }
} }
@Override
public void messageStarted(String uid, int number, int ofTotal) {
}
@Override
public void messagesFinished(int total) {
// FIXME this method is almost never invoked by various Stores! Don't rely on it unless fixed!!
}
}, },
syncConfig.getMaximumAutoDownloadMessageSize()); syncConfig.getMaximumAutoDownloadMessageSize());
} }
@ -477,7 +467,7 @@ class Pop3Sync {
remoteFolder.fetch(smallMessages, remoteFolder.fetch(smallMessages,
fp, new MessageRetrievalListener<Pop3Message>() { fp, new MessageRetrievalListener<Pop3Message>() {
@Override @Override
public void messageFinished(final Pop3Message message, int number, int ofTotal) { public void messageFinished(final Pop3Message message) {
try { try {
// Store the updated message locally // Store the updated message locally
@ -503,14 +493,6 @@ class Pop3Sync {
Timber.e(e, "SYNC: fetch small messages"); Timber.e(e, "SYNC: fetch small messages");
} }
} }
@Override
public void messageStarted(String uid, int number, int ofTotal) {
}
@Override
public void messagesFinished(int total) {
}
}, },
-1); -1);

View file

@ -405,7 +405,7 @@ class WebDavSync {
remoteFolder.fetch(unsyncedMessages, fp, remoteFolder.fetch(unsyncedMessages, fp,
new MessageRetrievalListener<WebDavMessage>() { new MessageRetrievalListener<WebDavMessage>() {
@Override @Override
public void messageFinished(WebDavMessage message, int number, int ofTotal) { public void messageFinished(WebDavMessage message) {
try { try {
if (message.isSet(Flag.DELETED) || message.olderThan(earliestDate)) { if (message.isSet(Flag.DELETED) || message.olderThan(earliestDate)) {
if (message.isSet(Flag.DELETED)) { if (message.isSet(Flag.DELETED)) {
@ -433,16 +433,6 @@ class WebDavSync {
Timber.e(e, "Error while storing downloaded message."); Timber.e(e, "Error while storing downloaded message.");
} }
} }
@Override
public void messageStarted(String uid, int number, int ofTotal) {
}
@Override
public void messagesFinished(int total) {
// FIXME this method is almost never invoked by various Stores! Don't rely on it unless fixed!!
}
}, },
syncConfig.getMaximumAutoDownloadMessageSize()); syncConfig.getMaximumAutoDownloadMessageSize());
} }
@ -463,7 +453,7 @@ class WebDavSync {
remoteFolder.fetch(smallMessages, remoteFolder.fetch(smallMessages,
fp, new MessageRetrievalListener<WebDavMessage>() { fp, new MessageRetrievalListener<WebDavMessage>() {
@Override @Override
public void messageFinished(final WebDavMessage message, int number, int ofTotal) { public void messageFinished(final WebDavMessage message) {
try { try {
// Store the updated message locally // Store the updated message locally
@ -488,14 +478,6 @@ class WebDavSync {
Timber.e(e, "SYNC: fetch small messages"); Timber.e(e, "SYNC: fetch small messages");
} }
} }
@Override
public void messageStarted(String uid, int number, int ofTotal) {
}
@Override
public void messagesFinished(int total) {
}
}, },
-1); -1);

View file

@ -3,12 +3,5 @@ package com.fsck.k9.mail;
public interface MessageRetrievalListener<T extends Message> { public interface MessageRetrievalListener<T extends Message> {
void messageStarted(String uid, int number, int ofTotal); void messageFinished(T message);
void messageFinished(T message, int number, int ofTotal);
/**
* FIXME <strong>this method is almost never invoked by various Stores! Don't rely on it unless fixed!!</strong>
*/
void messagesFinished(int total);
} }

View file

@ -502,12 +502,10 @@ internal class RealImapFolder(
// crazy adding stuff at the top. // crazy adding stuff at the top.
val uids = searchResponse.numbers.sortedDescending() val uids = searchResponse.numbers.sortedDescending()
val count = uids.size return uids.map { uidLong ->
return uids.mapIndexed { index, uidLong ->
val uid = uidLong.toString() val uid = uidLong.toString()
listener?.messageStarted(uid, index, count)
val message = ImapMessage(uid) val message = ImapMessage(uid)
listener?.messageFinished(message, index, count) listener?.messageFinished(message)
message message
} }
@ -572,7 +570,6 @@ internal class RealImapFolder(
val command = String.format("UID FETCH %s (%s)", commaSeparatedUids, spaceSeparatedFetchFields) val command = String.format("UID FETCH %s (%s)", commaSeparatedUids, spaceSeparatedFetchFields)
connection!!.sendCommand(command, false) connection!!.sendCommand(command, false)
var messageNumber = 0
var callback: ImapResponseCallback? = null var callback: ImapResponseCallback? = null
if (fetchProfile.contains(FetchProfile.Item.BODY) || if (fetchProfile.contains(FetchProfile.Item.BODY) ||
fetchProfile.contains(FetchProfile.Item.BODY_SANE) fetchProfile.contains(FetchProfile.Item.BODY_SANE)
@ -596,8 +593,6 @@ internal class RealImapFolder(
continue continue
} }
listener?.messageStarted(uid, messageNumber++, messageMap.size)
val literal = handleFetchResponse(message, fetchList) val literal = handleFetchResponse(message, fetchList)
if (literal != null) { if (literal != null) {
when (literal) { when (literal) {
@ -615,7 +610,7 @@ internal class RealImapFolder(
} }
} }
listener?.messageFinished(message, messageNumber, messageMap.size) listener?.messageFinished(message)
} else { } else {
handleUntaggedResponse(response) handleUntaggedResponse(response)
} }
@ -650,7 +645,6 @@ internal class RealImapFolder(
val command = String.format("UID FETCH %s (UID %s)", message.uid, fetch) val command = String.format("UID FETCH %s (UID %s)", message.uid, fetch)
connection!!.sendCommand(command, false) connection!!.sendCommand(command, false)
var messageNumber = 0
val callback: ImapResponseCallback = FetchPartCallback(part, bodyFactory) val callback: ImapResponseCallback = FetchPartCallback(part, bodyFactory)
var response: ImapResponse var response: ImapResponse
@ -668,8 +662,6 @@ internal class RealImapFolder(
continue continue
} }
listener?.messageStarted(uid, messageNumber++, 1)
val literal = handleFetchResponse(message, fetchList) val literal = handleFetchResponse(message, fetchList)
if (literal != null) { if (literal != null) {
when (literal) { when (literal) {
@ -692,7 +684,7 @@ internal class RealImapFolder(
} }
} }
listener?.messageFinished(message, messageNumber, 1) listener?.messageFinished(message)
} else { } else {
handleUntaggedResponse(response) handleUntaggedResponse(response)
} }

View file

@ -475,8 +475,7 @@ class RealImapFolderTest {
val messages = folder.getMessages(1, 10, null, listener) val messages = folder.getMessages(1, 10, null, listener)
verify(listener).messageStarted("99", 0, 1) verify(listener).messageFinished(messages[0])
verify(listener).messageFinished(messages[0], 0, 1)
verifyNoMoreInteractions(listener) verifyNoMoreInteractions(listener)
} }
@ -563,8 +562,7 @@ class RealImapFolderTest {
val messages = folder.getMessages(setOf(1L), true, listener) val messages = folder.getMessages(setOf(1L), true, listener)
verify(listener).messageStarted("99", 0, 1) verify(listener).messageFinished(messages[0])
verify(listener).messageFinished(messages[0], 0, 1)
verifyNoMoreInteractions(listener) verifyNoMoreInteractions(listener)
} }

View file

@ -120,7 +120,6 @@ public class Pop3Folder {
throw new MessagingException("getMessages", ioe); throw new MessagingException("getMessages", ioe);
} }
List<Pop3Message> messages = new ArrayList<>(); List<Pop3Message> messages = new ArrayList<>();
int i = 0;
for (int msgNum = start; msgNum <= end; msgNum++) { for (int msgNum = start; msgNum <= end; msgNum++) {
Pop3Message message = msgNumToMsgMap.get(msgNum); Pop3Message message = msgNumToMsgMap.get(msgNum);
if (message == null) { if (message == null) {
@ -133,12 +132,9 @@ public class Pop3Folder {
continue; continue;
} }
if (listener != null) {
listener.messageStarted(message.getUid(), i++, (end - start) + 1);
}
messages.add(message); messages.add(message);
if (listener != null) { if (listener != null) {
listener.messageFinished(message, i++, (end - start) + 1); listener.messageFinished(message);
} }
} }
return messages; return messages;
@ -317,12 +313,8 @@ public class Pop3Folder {
} catch (IOException ioe) { } catch (IOException ioe) {
throw new MessagingException("fetch", ioe); throw new MessagingException("fetch", ioe);
} }
for (int i = 0, count = messages.size(); i < count; i++) { for (Pop3Message pop3Message : messages) {
Pop3Message pop3Message = messages.get(i);
try { try {
if (listener != null && !fp.contains(FetchProfile.Item.ENVELOPE)) {
listener.messageStarted(pop3Message.getUid(), i, count);
}
if (fp.contains(FetchProfile.Item.BODY)) { if (fp.contains(FetchProfile.Item.BODY)) {
fetchBody(pop3Message, -1); fetchBody(pop3Message, -1);
} else if (fp.contains(FetchProfile.Item.BODY_SANE)) { } else if (fp.contains(FetchProfile.Item.BODY_SANE)) {
@ -343,7 +335,7 @@ public class Pop3Folder {
pop3Message.setBody(null); pop3Message.setBody(null);
} }
if (listener != null && !(fp.contains(FetchProfile.Item.ENVELOPE) && fp.size() == 1)) { if (listener != null && !(fp.contains(FetchProfile.Item.ENVELOPE) && fp.size() == 1)) {
listener.messageFinished(pop3Message, i, count); listener.messageFinished(pop3Message);
} }
} catch (IOException ioe) { } catch (IOException ioe) {
throw new MessagingException("Unable to fetch message", ioe); throw new MessagingException("Unable to fetch message", ioe);
@ -367,11 +359,7 @@ public class Pop3Folder {
* In extreme cases we'll do a command per message instead of a bulk request * In extreme cases we'll do a command per message instead of a bulk request
* to hopefully save some time and bandwidth. * to hopefully save some time and bandwidth.
*/ */
for (int i = 0, count = messages.size(); i < count; i++) { for (Pop3Message message : messages) {
Pop3Message message = messages.get(i);
if (listener != null) {
listener.messageStarted(message.getUid(), i, count);
}
String response = connection.executeSimpleCommand( String response = connection.executeSimpleCommand(
String.format(Locale.US, LIST_COMMAND + " %d", String.format(Locale.US, LIST_COMMAND + " %d",
uidToMsgNumMap.get(message.getUid()))); uidToMsgNumMap.get(message.getUid())));
@ -380,7 +368,7 @@ public class Pop3Folder {
int msgSize = Integer.parseInt(listParts[2]); int msgSize = Integer.parseInt(listParts[2]);
message.setSize(msgSize); message.setSize(msgSize);
if (listener != null) { if (listener != null) {
listener.messageFinished(message, i, count); listener.messageFinished(message);
} }
} }
} else { } else {
@ -400,12 +388,9 @@ public class Pop3Folder {
int msgSize = Integer.parseInt(listParts[1]); int msgSize = Integer.parseInt(listParts[1]);
Pop3Message pop3Message = msgNumToMsgMap.get(msgNum); Pop3Message pop3Message = msgNumToMsgMap.get(msgNum);
if (pop3Message != null && msgUidIndex.contains(pop3Message.getUid())) { if (pop3Message != null && msgUidIndex.contains(pop3Message.getUid())) {
if (listener != null) {
listener.messageStarted(pop3Message.getUid(), i, count);
}
pop3Message.setSize(msgSize); pop3Message.setSize(msgSize);
if (listener != null) { if (listener != null) {
listener.messageFinished(pop3Message, i, count); listener.messageFinished(pop3Message);
} }
i++; i++;
} }

View file

@ -207,7 +207,6 @@ public class WebDavFolder {
List<WebDavMessage> messages = new ArrayList<>(); List<WebDavMessage> messages = new ArrayList<>();
String[] uids; String[] uids;
Map<String, String> headers = new HashMap<>(); Map<String, String> headers = new HashMap<>();
int uidsLength;
String messageBody; String messageBody;
int prevStart = start; int prevStart = start;
@ -232,18 +231,14 @@ public class WebDavFolder {
DataSet dataset = store.processRequest(this.mFolderUrl, "SEARCH", messageBody, headers); DataSet dataset = store.processRequest(this.mFolderUrl, "SEARCH", messageBody, headers);
uids = dataset.getUids(); uids = dataset.getUids();
Map<String, String> uidToUrl = dataset.getUidToUrl(); Map<String, String> uidToUrl = dataset.getUidToUrl();
uidsLength = uids.length;
for (int i = 0; i < uidsLength; i++) { for (String uid : uids) {
if (listener != null) { WebDavMessage message = new WebDavMessage(uid, this);
listener.messageStarted(uids[i], i, uidsLength); message.setUrl(uidToUrl.get(uid));
}
WebDavMessage message = new WebDavMessage(uids[i], this);
message.setUrl(uidToUrl.get(uids[i]));
messages.add(message); messages.add(message);
if (listener != null) { if (listener != null) {
listener.messageFinished(message, i, uidsLength); listener.messageFinished(message);
} }
} }
@ -309,14 +304,9 @@ public class WebDavFolder {
/** /**
* We can't hand off to processRequest() since we need the stream to parse. * We can't hand off to processRequest() since we need the stream to parse.
*/ */
for (int i = 0, count = messages.size(); i < count; i++) { for (WebDavMessage wdMessage : messages) {
WebDavMessage wdMessage = messages.get(i);
int statusCode = 0; int statusCode = 0;
if (listener != null) {
listener.messageStarted(wdMessage.getUid(), i, count);
}
/** /**
* If fetch is called outside of the initial list (ie, a locally stored message), it may not have a URL * If fetch is called outside of the initial list (ie, a locally stored message), it may not have a URL
* associated. Verify and fix that * associated. Verify and fix that
@ -405,7 +395,7 @@ public class WebDavFolder {
} }
if (listener != null) { if (listener != null) {
listener.messageFinished(wdMessage, i, count); listener.messageFinished(wdMessage);
} }
} }
} }
@ -511,13 +501,7 @@ public class WebDavFolder {
Map<String, ParsedMessageEnvelope> envelopes = dataset.getMessageEnvelopes(); Map<String, ParsedMessageEnvelope> envelopes = dataset.getMessageEnvelopes();
int count = messages.size(); for (WebDavMessage message : messages) {
for (int i = messages.size() - 1; i >= 0; i--) {
WebDavMessage message = messages.get(i);
if (listener != null) {
listener.messageStarted(messages.get(i).getUid(), i, count);
}
ParsedMessageEnvelope envelope = envelopes.get(message.getUid()); ParsedMessageEnvelope envelope = envelopes.get(message.getUid());
if (envelope != null) { if (envelope != null) {
message.setNewHeaders(envelope); message.setNewHeaders(envelope);
@ -527,7 +511,7 @@ public class WebDavFolder {
} }
if (listener != null) { if (listener != null) {
listener.messageFinished(messages.get(i), i, count); listener.messageFinished(message);
} }
} }
} }

View file

@ -221,8 +221,7 @@ public class WebDavFolderTest {
FetchProfile profile = new FetchProfile(); FetchProfile profile = new FetchProfile();
profile.add(FetchProfile.Item.BODY_SANE); profile.add(FetchProfile.Item.BODY_SANE);
folder.fetch(messages, profile, listener, MAX_DOWNLOAD_SIZE); folder.fetch(messages, profile, listener, MAX_DOWNLOAD_SIZE);
verify(listener, times(25)).messageStarted(any(String.class), anyInt(), eq(25)); verify(listener, times(25)).messageFinished(any(WebDavMessage.class));
verify(listener, times(25)).messageFinished(any(WebDavMessage.class), anyInt(), eq(25));
} }
@Test @Test
@ -252,8 +251,7 @@ public class WebDavFolderTest {
profile.add(FetchProfile.Item.FLAGS); profile.add(FetchProfile.Item.FLAGS);
profile.add(FetchProfile.Item.BODY); profile.add(FetchProfile.Item.BODY);
folder.fetch(messages, profile, listener, MAX_DOWNLOAD_SIZE); folder.fetch(messages, profile, listener, MAX_DOWNLOAD_SIZE);
verify(listener, times(25)).messageStarted(any(String.class), anyInt(), anyInt()); verify(listener, times(25)).messageFinished(any(WebDavMessage.class));
verify(listener, times(25)).messageFinished(any(WebDavMessage.class), anyInt(), anyInt());
} }
private void setupStoreForMessageFetching() { private void setupStoreForMessageFetching() {
@ -293,8 +291,7 @@ public class WebDavFolderTest {
FetchProfile profile = new FetchProfile(); FetchProfile profile = new FetchProfile();
profile.add(FetchProfile.Item.BODY_SANE); profile.add(FetchProfile.Item.BODY_SANE);
folder.fetch(messages, profile, listener, MAX_DOWNLOAD_SIZE); folder.fetch(messages, profile, listener, MAX_DOWNLOAD_SIZE);
verify(listener, times(25)).messageStarted(any(String.class), anyInt(), eq(25)); verify(listener, times(25)).messageFinished(any(WebDavMessage.class));
verify(listener, times(25)).messageFinished(any(WebDavMessage.class), anyInt(), eq(25));
} }
@Test @Test
@ -324,8 +321,7 @@ public class WebDavFolderTest {
FetchProfile profile = new FetchProfile(); FetchProfile profile = new FetchProfile();
profile.add(FetchProfile.Item.BODY_SANE); profile.add(FetchProfile.Item.BODY_SANE);
folder.fetch(messages, profile, listener, MAX_DOWNLOAD_SIZE); folder.fetch(messages, profile, listener, MAX_DOWNLOAD_SIZE);
verify(listener, times(25)).messageStarted(any(String.class), anyInt(), eq(25)); verify(listener, times(25)).messageFinished(any(WebDavMessage.class));
verify(listener, times(25)).messageFinished(any(WebDavMessage.class), anyInt(), eq(25));
} }
@Test @Test
@ -391,8 +387,7 @@ public class WebDavFolderTest {
folder.getMessages(messageStart, messageEnd, listener); folder.getMessages(messageStart, messageEnd, listener);
verify(listener, times(5)).messageStarted(anyString(), anyInt(), eq(5)); verify(listener, times(5)).messageFinished(any(WebDavMessage.class));
verify(listener, times(5)).messageFinished(any(WebDavMessage.class), anyInt(), eq(5));
} }
@Test @Test