From bc9b7030d7d250c61b7bf8b624c6fa84c0363dd3 Mon Sep 17 00:00:00 2001 From: Apoorv Khatreja Date: Tue, 28 Jun 2011 16:50:48 +0530 Subject: [PATCH] COPYUID implementation now in place and working, restructured appendMessages, copyMessages and moveMessages globally to return a Map of srcUids -> destUids rather than returning nothing. This is now used to bring local and remote UIDs upto speed without the need for additional requests. --- .../k9/controller/MessagingController.java | 94 ++++++++++++++++--- src/com/fsck/k9/mail/Folder.java | 11 ++- src/com/fsck/k9/mail/store/ImapStore.java | 83 ++++++++-------- src/com/fsck/k9/mail/store/LocalStore.java | 30 ++++-- src/com/fsck/k9/mail/store/Pop3Store.java | 4 +- src/com/fsck/k9/mail/store/WebDavStore.java | 10 +- 6 files changed, 163 insertions(+), 69 deletions(-) diff --git a/src/com/fsck/k9/controller/MessagingController.java b/src/com/fsck/k9/controller/MessagingController.java index 079b72812..198f4b9a0 100644 --- a/src/com/fsck/k9/controller/MessagingController.java +++ b/src/com/fsck/k9/controller/MessagingController.java @@ -2088,9 +2088,32 @@ public class MessagingController implements Runnable { command.arguments[0] = srcFolder; command.arguments[1] = destFolder; command.arguments[2] = Boolean.toString(isCopy); - System.arraycopy(uids, 0, command.arguments, 3, uids.length); + command.arguments[3] = Boolean.toString(false); + System.arraycopy(uids, 0, command.arguments, 4, uids.length); queuePendingCommand(account, command); } + + private void queueMoveOrCopy(Account account, String srcFolder, String destFolder, boolean isCopy, String uids[], Map uidMap) { + if (uidMap == null || uidMap.isEmpty()) { + queueMoveOrCopy(account, srcFolder, destFolder, isCopy, uids); + } else { + if (account.getErrorFolderName().equals(srcFolder)) { + return; + } + PendingCommand command = new PendingCommand(); + command.command = PENDING_COMMAND_MOVE_OR_COPY_BULK; + + int length = 4 + uidMap.keySet().size() + uidMap.values().size(); + command.arguments = new String[length]; + command.arguments[0] = srcFolder; + command.arguments[1] = destFolder; + command.arguments[2] = Boolean.toString(isCopy); + command.arguments[3] = Boolean.toString(true); + System.arraycopy(uidMap.keySet().toArray(), 0, command.arguments, 4, uidMap.keySet().size()); + System.arraycopy(uidMap.values().toArray(), 0, command.arguments, 4 + uidMap.keySet().size(), uidMap.values().size()); + queuePendingCommand(account, command); + } + } /** * Process a pending trash message command. * @@ -2102,6 +2125,7 @@ public class MessagingController implements Runnable { throws MessagingException { Folder remoteSrcFolder = null; Folder remoteDestFolder = null; + Folder localDestFolder = null; try { String srcFolder = command.arguments[0]; if (account.getErrorFolderName().equals(srcFolder)) { @@ -2109,14 +2133,42 @@ public class MessagingController implements Runnable { } String destFolder = command.arguments[1]; String isCopyS = command.arguments[2]; + String hasNewUidsS = command.arguments[3]; + + boolean hasNewUids = false; + if (hasNewUidsS != null) { + hasNewUids = Boolean.parseBoolean(hasNewUidsS); + } + Store remoteStore = account.getRemoteStore(); remoteSrcFolder = remoteStore.getFolder(srcFolder); + Store localStore = account.getLocalStore(); + localDestFolder = localStore.getFolder(destFolder); List messages = new ArrayList(); - for (int i = 3; i < command.arguments.length; i++) { - String uid = command.arguments[i]; - if (!uid.startsWith(K9.LOCAL_UID_PREFIX)) { - messages.add(remoteSrcFolder.getMessage(uid)); + + /* + * We split up the localUidMap into two parts while sending the command, here we assemble it back. + */ + Map localUidMap = new HashMap(); + if (hasNewUids) { + int offset = (command.arguments.length - 4) / 2; + + for (int i = 4; i < 4 + offset; i++) { + localUidMap.put(command.arguments[i], command.arguments[i + offset]); + + String uid = command.arguments[i]; + if (!uid.startsWith(K9.LOCAL_UID_PREFIX)) { + messages.add(remoteSrcFolder.getMessage(uid)); + } + } + + } else { + for (int i = 4; i < command.arguments.length; i++) { + String uid = command.arguments[i]; + if (!uid.startsWith(K9.LOCAL_UID_PREFIX)) { + messages.add(remoteSrcFolder.getMessage(uid)); + } } } @@ -2137,6 +2189,8 @@ public class MessagingController implements Runnable { Log.d(K9.LOG_TAG, "processingPendingMoveOrCopy: source folder = " + srcFolder + ", " + messages.size() + " messages, destination folder = " + destFolder + ", isCopy = " + isCopy); + Map remoteUidMap = new HashMap(); + if (!isCopy && destFolder.equals(account.getTrashFolderName())) { if (K9.DEBUG) Log.d(K9.LOG_TAG, "processingPendingMoveOrCopy doing special case for deleting message"); @@ -2150,9 +2204,9 @@ public class MessagingController implements Runnable { remoteDestFolder = remoteStore.getFolder(destFolder); if (isCopy) { - remoteSrcFolder.copyMessages(messages.toArray(EMPTY_MESSAGE_ARRAY), remoteDestFolder); + remoteUidMap = remoteSrcFolder.copyMessages(messages.toArray(EMPTY_MESSAGE_ARRAY), remoteDestFolder); } else { - remoteSrcFolder.moveMessages(messages.toArray(EMPTY_MESSAGE_ARRAY), remoteDestFolder); + remoteUidMap = remoteSrcFolder.moveMessages(messages.toArray(EMPTY_MESSAGE_ARRAY), remoteDestFolder); } } if (!isCopy && Account.EXPUNGE_IMMEDIATELY.equals(account.getExpungePolicy())) { @@ -2161,12 +2215,27 @@ public class MessagingController implements Runnable { remoteSrcFolder.expunge(); } + + /* + * This next part is used to bring the local UIDs of the local destination folder + * upto speed with the remote UIDs of remote destionation folder. + */ + if (!localUidMap.isEmpty() && !remoteUidMap.isEmpty()) { + Set remoteSrcUids = remoteUidMap.keySet(); + Iterator remoteSrcUidsIterator = remoteSrcUids.iterator(); + + while (remoteSrcUidsIterator.hasNext()) { + String remoteSrcUid = remoteSrcUidsIterator.next(); + String localDestUid = localUidMap.get(remoteSrcUid); + + Message localDestMessage = localDestFolder.getMessage(localDestUid); + localDestMessage.setUid(remoteUidMap.get(remoteSrcUid)); + } + } } finally { closeFolder(remoteSrcFolder); closeFolder(remoteDestFolder); } - - } private void queueSetFlag(final Account account, final String folderName, final String newState, final String flag, final String[] uids) { @@ -3248,6 +3317,7 @@ public class MessagingController implements Runnable { private void moveOrCopyMessageSynchronous(final Account account, final String srcFolder, final Message[] inMessages, final String destFolder, final boolean isCopy, MessagingListener listener) { try { + Map uidMap = new HashMap(); Store localStore = account.getLocalStore(); Store remoteStore = account.getRemoteStore(); if (!isCopy && (!remoteStore.isMoveCapable() || !localStore.isMoveCapable())) { @@ -3285,9 +3355,9 @@ public class MessagingController implements Runnable { fp.add(FetchProfile.Item.ENVELOPE); fp.add(FetchProfile.Item.BODY); localSrcFolder.fetch(messages, fp, null); - localSrcFolder.copyMessages(messages, localDestFolder); + uidMap = localSrcFolder.copyMessages(messages, localDestFolder); } else { - localSrcFolder.moveMessages(messages, localDestFolder); + uidMap = localSrcFolder.moveMessages(messages, localDestFolder); for (String origUid : origUidMap.keySet()) { for (MessagingListener l : getListeners()) { l.messageUidChanged(account, srcFolder, origUid, origUidMap.get(origUid).getUid()); @@ -3296,7 +3366,7 @@ public class MessagingController implements Runnable { } } - queueMoveOrCopy(account, srcFolder, destFolder, isCopy, origUidMap.keySet().toArray(EMPTY_STRING_ARRAY)); + queueMoveOrCopy(account, srcFolder, destFolder, isCopy, origUidMap.keySet().toArray(EMPTY_STRING_ARRAY), uidMap); } processPendingCommands(account); diff --git a/src/com/fsck/k9/mail/Folder.java b/src/com/fsck/k9/mail/Folder.java index 0f5b96060..e3dafc3e5 100644 --- a/src/com/fsck/k9/mail/Folder.java +++ b/src/com/fsck/k9/mail/Folder.java @@ -1,6 +1,7 @@ package com.fsck.k9.mail; import java.util.Date; +import java.util.Map; import android.util.Log; import com.fsck.k9.Account; @@ -102,11 +103,15 @@ public abstract class Folder { public abstract Message[] getMessages(String[] uids, MessageRetrievalListener listener) throws MessagingException; - public abstract void appendMessages(Message[] messages) throws MessagingException; + public abstract Map appendMessages(Message[] messages) throws MessagingException; - public void copyMessages(Message[] msgs, Folder folder) throws MessagingException {} + public Map copyMessages(Message[] msgs, Folder folder) throws MessagingException { + return null; + } - public void moveMessages(Message[] msgs, Folder folder) throws MessagingException {} + public Map moveMessages(Message[] msgs, Folder folder) throws MessagingException { + return null; + } public void delete(Message[] msgs, String trashFolderName) throws MessagingException { for (Message message : msgs) { diff --git a/src/com/fsck/k9/mail/store/ImapStore.java b/src/com/fsck/k9/mail/store/ImapStore.java index 8824e0988..b4da67248 100644 --- a/src/com/fsck/k9/mail/store/ImapStore.java +++ b/src/com/fsck/k9/mail/store/ImapStore.java @@ -29,7 +29,6 @@ import java.text.SimpleDateFormat; import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; -import java.util.Comparator; import java.util.Date; import java.util.HashMap; import java.util.HashSet; @@ -40,9 +39,7 @@ import java.util.List; import java.util.Locale; import java.util.Map; import java.util.Set; -import java.util.SortedSet; import java.util.StringTokenizer; -import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; @@ -806,32 +803,19 @@ public class ImapStore extends Store { } @Override - public void copyMessages(Message[] messages, Folder folder) throws MessagingException { + public Map copyMessages(Message[] messages, Folder folder) throws MessagingException { if (!(folder instanceof ImapFolder)) { throw new MessagingException("ImapFolder.copyMessages passed non-ImapFolder"); } if (messages.length == 0) - return; + return null; ImapFolder iFolder = (ImapFolder)folder; checkOpen(); - SortedSet messageSet = new TreeSet(new Comparator() { - public int compare(Message m1, Message m2) { - int uid1 = Integer.parseInt(m1.getUid()), uid2 = Integer.parseInt(m2.getUid()); - if (uid1 < uid2) { - return -1; - } else if (uid1 == uid2) { - return 0; - } else { - return 1; - } - } - }); String[] uids = new String[messages.length]; for (int i = 0, count = messages.length; i < count; i++) { - messageSet.add(messages[i]); // Not bothering to sort the UIDs in ascending order while sending the command for convenience, and because it does not make a difference. uids[i] = messages[i].getUid(); @@ -875,31 +859,36 @@ public class ImapStore extends Store { Object responseList = response.get(1); + Map uidMap = null; + if (responseList instanceof ImapList) { final ImapList copyList = (ImapList) responseList; if ((copyList.size() >= 4) && copyList.getString(0).equals("COPYUID")) { - List oldUids = parseSequenceSet(copyList.getString(2)); - List newUids = parseSequenceSet(copyList.getString(3)); - if (oldUids.size() == newUids.size()) { - Iterator messageIterator = messageSet.iterator(); - for (int i = 0; i < messages.length && messageIterator.hasNext(); i++) { - Message nextMessage = messageIterator.next(); - if (oldUids.get(i).equals(nextMessage.getUid())) { - /* - * Here, we need to *create* new messages in the localstore, same as the older messages, the only changes are that old UIDs need to be swapped with new UIDs, - * and old folder swapped with new folder. - */ -// nextMessage.setUid(newUids.get(i)); - } + List srcUids = parseSequenceSet(copyList.getString(2)); + List destUids = parseSequenceSet(copyList.getString(3)); + if (srcUids.size() == destUids.size()) { + Iterator srcUidsIterator = srcUids.iterator(); + Iterator destUidsIterator = destUids.iterator(); + uidMap = new HashMap(); + while (srcUidsIterator.hasNext() && destUidsIterator.hasNext()) { + uidMap.put(srcUidsIterator.next(), destUidsIterator.next()); } } } } + return uidMap; } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } } + /** + * Can be used to parse sequence sets or UID sets appearing is responses such as COPYUID. + * e.g. [COPYUID 38505 304,319:320 3956:3958] + * + * @param set + * @return List sequenceSet + */ private List parseSequenceSet(String set) { int index = 0; List sequenceList = new ArrayList(); @@ -947,11 +936,12 @@ public class ImapStore extends Store { } @Override - public void moveMessages(Message[] messages, Folder folder) throws MessagingException { + public Map moveMessages(Message[] messages, Folder folder) throws MessagingException { if (messages.length == 0) - return; - copyMessages(messages, folder); + return null; + Map uidMap = copyMessages(messages, folder); setFlags(messages, new Flag[] { Flag.DELETED }, true); + return uidMap; } @Override @@ -1698,9 +1688,10 @@ public class ImapStore extends Store { * new server UID. */ @Override - public void appendMessages(Message[] messages) throws MessagingException { + public Map appendMessages(Message[] messages) throws MessagingException { checkOpen(); try { + Map uidMap = null; for (Message message : messages) { mConnection.sendCommand( String.format("APPEND %s (%s) {%d}", @@ -1734,9 +1725,18 @@ public class ImapStore extends Store { if (responseList instanceof ImapList) { final ImapList appendList = (ImapList) responseList; if ((appendList.size() >= 3) && appendList.getString(0).equals("APPENDUID")) { - String serverUid = appendList.getString(2); - if (!TextUtils.isEmpty(serverUid)) { - message.setUid(serverUid); + String newUid = appendList.getString(2); + + /* + * We need uidMap to be null initially to maintain consistency with the behavior of other similar methods (copyMessages, moveMessages) which + * return null if new UIDs are not available. Therefore, we initialize uidMap over here. + */ + if (uidMap == null) { + uidMap = new HashMap(); + } + uidMap.put(message.getUid(), newUid); + if (!TextUtils.isEmpty(newUid)) { + message.setUid(newUid); continue; } } @@ -1751,11 +1751,14 @@ public class ImapStore extends Store { Log.d(K9.LOG_TAG, "Got UID " + newUid + " for message for " + getLogId()); if (newUid != null) { + if (uidMap == null) { + uidMap = new HashMap(); + } + uidMap.put(message.getUid(), newUid); message.setUid(newUid); } - - } + return uidMap; } catch (IOException ioe) { throw ioExceptionHandler(mConnection, ioe); } diff --git a/src/com/fsck/k9/mail/store/LocalStore.java b/src/com/fsck/k9/mail/store/LocalStore.java index fba282164..0c3647034 100644 --- a/src/com/fsck/k9/mail/store/LocalStore.java +++ b/src/com/fsck/k9/mail/store/LocalStore.java @@ -1885,21 +1885,23 @@ public class LocalStore extends Store implements Serializable { } @Override - public void copyMessages(Message[] msgs, Folder folder) throws MessagingException { + public Map copyMessages(Message[] msgs, Folder folder) throws MessagingException { if (!(folder instanceof LocalFolder)) { throw new MessagingException("copyMessages called with incorrect Folder"); } - ((LocalFolder) folder).appendMessages(msgs, true); + return ((LocalFolder) folder).appendMessages(msgs, true); } @Override - public void moveMessages(final Message[] msgs, final Folder destFolder) throws MessagingException { + public Map moveMessages(final Message[] msgs, final Folder destFolder) throws MessagingException { if (!(destFolder instanceof LocalFolder)) { throw new MessagingException("moveMessages called with non-LocalFolder"); } final LocalFolder lDestFolder = (LocalFolder)destFolder; + final Map uidMap = new HashMap(); + try { database.execute(false, new DbCallback() { @Override @@ -1936,7 +1938,7 @@ public class LocalStore extends Store implements Serializable { LocalMessage placeHolder = new LocalMessage(oldUID, LocalFolder.this); placeHolder.setFlagInternal(Flag.DELETED, true); placeHolder.setFlagInternal(Flag.SEEN, true); - appendMessages(new Message[] { placeHolder }); + uidMap.putAll(appendMessages(new Message[] { placeHolder })); } } catch (MessagingException e) { throw new WrappedException(e); @@ -1944,6 +1946,7 @@ public class LocalStore extends Store implements Serializable { return null; } }); + return uidMap; } catch (WrappedException e) { throw(MessagingException) e.getCause(); } @@ -1989,8 +1992,8 @@ public class LocalStore extends Store implements Serializable { * message, retrieve the appropriate local message instance first (if it already exists). */ @Override - public void appendMessages(Message[] messages) throws MessagingException { - appendMessages(messages, false); + public Map appendMessages(Message[] messages) throws MessagingException { + return appendMessages(messages, false); } public void destroyMessages(final Message[] messages) throws MessagingException { @@ -2026,10 +2029,12 @@ public class LocalStore extends Store implements Serializable { * message, retrieve the appropriate local message instance first (if it already exists). * @param messages * @param copy + * @return Map uidMap of srcUids -> destUids */ - private void appendMessages(final Message[] messages, final boolean copy) throws MessagingException { + private Map appendMessages(final Message[] messages, final boolean copy) throws MessagingException { open(OpenMode.READ_WRITE); try { + final Map uidMap = new HashMap(); database.execute(true, new DbCallback() { @Override public Void doDbWork(final SQLiteDatabase db) throws WrappedException, UnavailableStorageException { @@ -2040,11 +2045,15 @@ public class LocalStore extends Store implements Serializable { } String uid = message.getUid(); - if (uid == null || copy) { + if (uid == null && !copy) { uid = K9.LOCAL_UID_PREFIX + UUID.randomUUID().toString(); - if (!copy) { - message.setUid(uid); + message.setUid(uid); + } else if (copy) { + String temp = K9.LOCAL_UID_PREFIX + UUID.randomUUID().toString(); + if (uid != null) { + uidMap.put(uid, temp); } + uid = temp; } else { Message oldMessage = getMessage(uid); if (oldMessage != null && !oldMessage.isSet(Flag.SEEN)) { @@ -2150,6 +2159,7 @@ public class LocalStore extends Store implements Serializable { return null; } }); + return uidMap; } catch (WrappedException e) { throw(MessagingException) e.getCause(); } diff --git a/src/com/fsck/k9/mail/store/Pop3Store.java b/src/com/fsck/k9/mail/store/Pop3Store.java index a307c8484..e0c57591e 100644 --- a/src/com/fsck/k9/mail/store/Pop3Store.java +++ b/src/com/fsck/k9/mail/store/Pop3Store.java @@ -24,6 +24,7 @@ import java.util.LinkedList; import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; public class Pop3Store extends Store { public static final int CONNECTION_SECURITY_NONE = 0; @@ -736,7 +737,8 @@ public class Pop3Store extends Store { } @Override - public void appendMessages(Message[] messages) throws MessagingException { + public Map appendMessages(Message[] messages) throws MessagingException { + return null; } @Override diff --git a/src/com/fsck/k9/mail/store/WebDavStore.java b/src/com/fsck/k9/mail/store/WebDavStore.java index 81a498f25..7720b9c18 100644 --- a/src/com/fsck/k9/mail/store/WebDavStore.java +++ b/src/com/fsck/k9/mail/store/WebDavStore.java @@ -51,6 +51,7 @@ import java.util.HashMap; import java.util.LinkedList; import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Stack; import java.util.zip.GZIPInputStream; @@ -1161,13 +1162,15 @@ public class WebDavStore extends Store { } @Override - public void copyMessages(Message[] messages, Folder folder) throws MessagingException { + public Map copyMessages(Message[] messages, Folder folder) throws MessagingException { moveOrCopyMessages(messages, folder.getName(), false); + return null; } @Override - public void moveMessages(Message[] messages, Folder folder) throws MessagingException { + public Map moveMessages(Message[] messages, Folder folder) throws MessagingException { moveOrCopyMessages(messages, folder.getName(), true); + return null; } @Override @@ -1728,8 +1731,9 @@ public class WebDavStore extends Store { } @Override - public void appendMessages(Message[] messages) throws MessagingException { + public Map appendMessages(Message[] messages) throws MessagingException { appendWebDavMessages(messages); + return null; } public Message[] appendWebDavMessages(Message[] messages) throws MessagingException {