Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MULTIAPPEND #2710

Closed
wants to merge 3 commits into from
Closed

Add support for MULTIAPPEND #2710

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
bd51e01
Add support for MULTIAPPEND
Aug 28, 2017
f39fd1a
Fix and add tests
Aug 28, 2017
5425b7c
Append messages in batches
Aug 29, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/Capabilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,5 @@ class Capabilities {
public static final String COMPRESS_DEFLATE = "COMPRESS=DEFLATE";
public static final String STARTTLS = "STARTTLS";
public static final String SPECIAL_USE = "SPECIAL-USE";
public static final String MULTIAPPEND = "MULTIAPPEND";
}
4 changes: 4 additions & 0 deletions k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapConnection.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -691,6 +691,10 @@ protected boolean isIdleCapable() {
return capabilities.contains(Capabilities.IDLE);
}

boolean isMultiappendCapable() {
return capabilities.contains(Capabilities.MULTIAPPEND);
}

public void close() {
open = false;
stacktraceForClose = new Exception();
Expand Down
192 changes: 142 additions & 50 deletions k9mail-library/src/main/java/com/fsck/k9/mail/store/imap/ImapFolder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ protected SimpleDateFormat initialValue() {
};
private static final int MORE_MESSAGES_WINDOW_SIZE = 500;
private static final int FETCH_WINDOW_SIZE = 100;
private static final long APPEND_WINDOW_SIZE = 10000000;


protected volatile int messageCount = -1;
Expand Down Expand Up @@ -1143,64 +1144,164 @@ private void parseBodyStructure(ImapList bs, Part part, String id) throws Messag
*/
@Override
public Map<String, String> appendMessages(List<? extends Message> messages) throws MessagingException {
if (messages == null || messages.size() == 0) {
return null;
}

open(OPEN_MODE_RW);
checkOpen();

try {
Map<String, String> uidMap = new HashMap<>();
for (Message message : messages) {
long messageSize = message.calculateSize();

String encodeFolderName = folderNameCodec.encode(getPrefixedName());
String escapedFolderName = ImapUtility.encodeString(encodeFolderName);
String command = String.format(Locale.US, "APPEND %s (%s) {%d}", escapedFolderName,
combineFlags(message.getFlags()), messageSize);
connection.sendCommand(command, false);
if (connection.isMultiappendCapable() && messages.size() > 1) {
appendWithMultiappend(messages, uidMap);
} else {
appendWithoutMultiappend(messages, uidMap);
}

ImapResponse response;
do {
response = connection.readResponse();
/*
* We need uidMap to be null if new UIDs are not available to maintain consistency
* with the behavior of other similar methods (copyMessages, moveMessages) which
* return null.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should really change this. Returning null is bad and an empty map would suffice.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd leave this for another PR, since this touches other methods besides append

*/
return (uidMap.isEmpty()) ? null : uidMap;
} catch (IOException ioe) {
throw ioExceptionHandler(connection, ioe);
}
}

handleUntaggedResponse(response);
private void appendWithoutMultiappend(List<? extends Message> messages, Map<String, String> uidMap)
throws IOException, MessagingException {
String escapedFolderName = getEscapedFolderName();

for (Message message : messages) {
long messageSize = message.calculateSize();

String command = String.format(Locale.US, "APPEND %s (%s) {%d}", escapedFolderName,
combineFlags(message.getFlags()), messageSize);
connection.sendCommand(command, false);

ImapResponse response;
do {
response = connection.readResponse();

handleUntaggedResponse(response);

if (response.isContinuationRequested()) {
EOLConvertingOutputStream eolOut = new EOLConvertingOutputStream(connection.getOutputStream());
message.writeTo(eolOut);
eolOut.write('\r');
eolOut.write('\n');
eolOut.flush();
}
} while (response.getTag() == null);

findAndSaveNewUids(response, Collections.singletonList(message), uidMap);
}
}

private void appendWithMultiappend(List<? extends Message> messages, Map<String, String> uidMap)
throws IOException, MessagingException {
EOLConvertingOutputStream eolOut = new EOLConvertingOutputStream(connection.getOutputStream());
ImapResponse response;
int i = 0;

while (i < messages.size()) {
boolean firstInBatch = true;
Message firstMessageInBatch = messages.get(i);
long currentWindowSize = 0;
List<Message> appendedMessages = new ArrayList<>();

String command = String.format(Locale.US, "APPEND %s (%s) {%d}", getEscapedFolderName(),
combineFlags(firstMessageInBatch.getFlags()), firstMessageInBatch.calculateSize());
connection.sendCommand(command, false);

if (response.isContinuationRequested()) {
EOLConvertingOutputStream eolOut = new EOLConvertingOutputStream(connection.getOutputStream());
message.writeTo(eolOut);
eolOut.write('\r');
eolOut.write('\n');
eolOut.flush();
response = connection.readResponse();
handleUntaggedResponse(response);

if (response.isContinuationRequested()) {
while ((currentWindowSize <= APPEND_WINDOW_SIZE || appendedMessages.size() == 0)
&& i < messages.size()) {
Message messageToAppend = messages.get(i);
long messageSize = messageToAppend.calculateSize();

if (currentWindowSize + messageSize > APPEND_WINDOW_SIZE &&
appendedMessages.size() > 0) {
break;
}
} while (response.getTag() == null);

if (response.size() > 1) {
/*
* If the server supports UIDPLUS, then along with the APPEND response it
* will return an APPENDUID response code, e.g.
*
* 11 OK [APPENDUID 2 238268] APPEND completed
*
* We can use the UID included in this response to update our records.
*/
Object responseList = response.get(1);
if (!firstInBatch) {
String newMessageContinuation = String.format(Locale.US, "(%s) {%d}",
combineFlags(messageToAppend.getFlags()), messageSize);
connection.sendContinuation(newMessageContinuation);
}

if (responseList instanceof ImapList) {
ImapList appendList = (ImapList) responseList;
if (appendList.size() >= 3 && appendList.getString(0).equals("APPENDUID")) {
String newUid = appendList.getString(2);
messageToAppend.writeTo(eolOut);
eolOut.flush();

if (!TextUtils.isEmpty(newUid)) {
message.setUid(newUid);
uidMap.put(message.getUid(), newUid);
continue;
}
appendedMessages.add(messageToAppend);
currentWindowSize += messageSize;
firstInBatch = false;
i++;
}

eolOut.write('\r');
eolOut.write('\n');
eolOut.flush();
} else {
throw new IllegalStateException("Did not get a continuation response to APPEND");
}

while (response != null && response.getTag() == null) {
response = connection.readResponse();
handleUntaggedResponse(response);
}

findAndSaveNewUids(response, appendedMessages, uidMap);
}
}

private String getEscapedFolderName() throws MessagingException {
String encodeFolderName = folderNameCodec.encode(getPrefixedName());
return ImapUtility.encodeString(encodeFolderName);
}

private void findAndSaveNewUids(ImapResponse response, List<? extends Message> messages,
Map<String, String> uidMap) throws MessagingException {
boolean updatedUids = false;
if (response.size() > 1) {
/*
* If the server supports UIDPLUS, then along with the APPEND response it
* will return an APPENDUID response code, e.g.
*
* 11 OK [APPENDUID 2 238268] APPEND completed
*
* We can use the UID included in this response to update our records.
*/
Object responseList = response.get(1);

if (responseList instanceof ImapList) {
ImapList appendList = (ImapList) responseList;
if (appendList.size() >= 3 && appendList.getString(0).equals("APPENDUID")) {
String newUidString = appendList.getString(2);

if (!TextUtils.isEmpty(newUidString)) {
List<String> newUids = ImapUtility.getImapSequenceValues(newUidString);
for (int i = 0;i < messages.size();i++) {
Message message = messages.get(i);
String newUid = newUids.get(i);
uidMap.put(message.getUid(), newUid);
message.setUid(newUid);
}
updatedUids = true;
}
}
}
}

/*
* This part is executed in case the server does not support UIDPLUS or does
* not implement the APPENDUID response code.
*/
if (!updatedUids) {
for (Message message : messages) {
String newUid = getUidFromMessageId(message);
if (K9MailLib.isDebug()) {
Timber.d("Got UID %s for message for %s", newUid, getLogId());
Expand All @@ -1211,15 +1312,6 @@ public Map<String, String> appendMessages(List<? extends Message> messages) thro
message.setUid(newUid);
}
}

/*
* We need uidMap to be null if new UIDs are not available to maintain consistency
* with the behavior of other similar methods (copyMessages, moveMessages) which
* return null.
*/
return (uidMap.isEmpty()) ? null : uidMap;
} catch (IOException ioe) {
throw ioExceptionHandler(connection, ioe);
}
}

Expand Down
25 changes: 23 additions & 2 deletions k9mail-library/src/test/java/com/fsck/k9/mail/store/imap/ImapFolderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@


import java.io.IOException;
import java.io.OutputStream;
import java.util.ArrayList;
import java.util.Collections;
import java.util.Date;
Expand Down Expand Up @@ -74,6 +75,7 @@ public void setUp() throws Exception {
when(imapStore.getStoreConfig()).thenReturn(storeConfig);

imapConnection = mock(ImapConnection.class);
when(imapConnection.getOutputStream()).thenReturn(mock(OutputStream.class));
}

@Test
Expand Down Expand Up @@ -974,18 +976,37 @@ public void fetchPart_withTextSection_shouldProcessImapResponses() throws Except
}

@Test
public void appendMessages_shouldIssueRespectiveCommand() throws Exception {
public void appendMessages_withoutMultiappend_shouldIssueRespectiveCommand() throws Exception {
ImapFolder folder = createFolder("Folder");
prepareImapFolderForOpen(OPEN_MODE_RW);
folder.open(OPEN_MODE_RW);
List<ImapMessage> messages = createImapMessages("1");
when(imapConnection.readResponse()).thenReturn(createImapResponse("x OK [APPENDUID 1 23]"));
when(imapConnection.isMultiappendCapable()).thenReturn(false);
when(imapConnection.readResponse()).thenReturn(createImapResponse("+ OK"))
.thenReturn(createImapResponse("x OK [APPENDUID 1 23]"));

folder.appendMessages(messages);

verify(imapConnection).sendCommand("APPEND \"Folder\" () {0}", false);
}

@Test
public void appendMessages_withMultiappend_shouldIssueRespectiveCommandAndContinuation() throws Exception {
ImapFolder folder = createFolder("Folder");
prepareImapFolderForOpen(OPEN_MODE_RW);
folder.open(OPEN_MODE_RW);
List<ImapMessage> messages = createImapMessages("1", "2");
when(imapConnection.isMultiappendCapable()).thenReturn(true);
when(imapConnection.readResponse()).thenReturn(createImapResponse("+ OK"))
.thenReturn(createImapResponse("+ OK"))
.thenReturn(createImapResponse("x OK [APPENDUID 1 23:24]"));

folder.appendMessages(messages);

verify(imapConnection).sendCommand("APPEND \"Folder\" () {0}", false);
verify(imapConnection).sendContinuation("() {0}");
}

@Test
public void getUidFromMessageId_withoutMessageIdHeader_shouldReturnNull() throws Exception {
ImapFolder folder = createFolder("Folder");
Expand Down
Loading