Skip to content

Commit 5589daa

Browse files
authored
GH-9546: IMAP: Flag messages after messages have been copied
Fixes: #9546 Issue link: #9546 Sometimes the IMAP connection breaks just after the message flags have been set, but the message has not been copied yet. This then leads to the message never being received by (as it has been flagged and the next search will not return it). * Flag and maybe delete messages after messages have been copied **Auto-cherry-pick to `6.3.x` & `6.2.x`**
1 parent 6b42758 commit 5589daa

File tree

2 files changed

+70
-7
lines changed

2 files changed

+70
-7
lines changed

Diff for: spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java

+16-6
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@
6868
* @author Dominik Simmen
6969
* @author Yuxin Wang
7070
* @author Ngoc Nhan
71+
* @author Filip Hrisafov
7172
*/
7273
public abstract class AbstractMailReceiver extends IntegrationObjectSupport implements MailReceiver, DisposableBean {
7374

@@ -503,18 +504,27 @@ private Object byteArrayToContent(Map<String, Object> headers, ByteArrayOutputSt
503504
}
504505

505506
private void postProcessFilteredMessages(Message[] filteredMessages) throws MessagingException {
506-
setMessageFlags(filteredMessages);
507-
508-
if (shouldDeleteMessages()) {
509-
deleteMessages(filteredMessages);
510-
}
511507
// Copy messages to cause an eager fetch
508+
Message[] messages = filteredMessages;
512509
if (this.headerMapper == null && (this.autoCloseFolder || this.simpleContent)) {
510+
messages = new Message[filteredMessages.length];
513511
for (int i = 0; i < filteredMessages.length; i++) {
514-
MimeMessage mimeMessage = new IntegrationMimeMessage((MimeMessage) filteredMessages[i]);
512+
Message originalMessage = filteredMessages[i];
513+
messages[i] = originalMessage;
514+
MimeMessage mimeMessage = new IntegrationMimeMessage((MimeMessage) originalMessage);
515515
filteredMessages[i] = mimeMessage;
516516
}
517517
}
518+
519+
setMessageFlagsAndMaybeDeleteMessages(messages);
520+
}
521+
522+
private void setMessageFlagsAndMaybeDeleteMessages(Message[] messages) throws MessagingException {
523+
setMessageFlags(messages);
524+
525+
if (shouldDeleteMessages()) {
526+
deleteMessages(messages);
527+
}
518528
}
519529

520530
private void setMessageFlags(Message[] filteredMessages) throws MessagingException {

Diff for: spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java

+54-1
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,15 @@
1717
package org.springframework.integration.mail;
1818

1919
import java.io.IOException;
20+
import java.io.OutputStream;
2021
import java.lang.reflect.Field;
2122
import java.util.ArrayList;
2223
import java.util.Arrays;
2324
import java.util.List;
2425
import java.util.Properties;
2526
import java.util.concurrent.CountDownLatch;
2627
import java.util.concurrent.TimeUnit;
28+
import java.util.concurrent.atomic.AtomicBoolean;
2729
import java.util.concurrent.atomic.AtomicInteger;
2830
import java.util.concurrent.atomic.AtomicReference;
2931
import java.util.logging.Handler;
@@ -88,6 +90,7 @@
8890
import org.springframework.util.MimeTypeUtils;
8991

9092
import static org.assertj.core.api.Assertions.assertThat;
93+
import static org.assertj.core.api.Assertions.assertThatThrownBy;
9194
import static org.mockito.ArgumentMatchers.any;
9295
import static org.mockito.ArgumentMatchers.anyString;
9396
import static org.mockito.BDDMockito.given;
@@ -107,6 +110,7 @@
107110
* @author Artem Bilan
108111
* @author Alexander Pinske
109112
* @author Dominik Simmen
113+
* @author Filip Hrisafov
110114
*/
111115
@SpringJUnitConfig
112116
@ContextConfiguration(
@@ -299,6 +303,11 @@ public void receiveAndMarkAsReadDontDelete() throws Exception {
299303

300304
private AbstractMailReceiver receiveAndMarkAsReadDontDeleteGuts(AbstractMailReceiver receiver, Message msg1,
301305
Message msg2) throws NoSuchFieldException, IllegalAccessException, MessagingException {
306+
return receiveAndMarkAsReadDontDeleteGuts(receiver, msg1, msg2, true);
307+
}
308+
309+
private AbstractMailReceiver receiveAndMarkAsReadDontDeleteGuts(AbstractMailReceiver receiver, Message msg1,
310+
Message msg2, boolean receive) throws NoSuchFieldException, IllegalAccessException, MessagingException {
302311

303312
((ImapMailReceiver) receiver).setShouldMarkMessagesAsRead(true);
304313
receiver = spy(receiver);
@@ -326,7 +335,9 @@ private AbstractMailReceiver receiveAndMarkAsReadDontDeleteGuts(AbstractMailRece
326335
willAnswer(invocation -> messages).given(folder).search(any(SearchTerm.class));
327336

328337
willAnswer(invocation -> null).given(receiver).fetchMessages(messages);
329-
receiver.receive();
338+
if (receive) {
339+
receiver.receive();
340+
}
330341
return receiver;
331342
}
332343

@@ -980,6 +991,31 @@ private void setUpScheduler(ImapMailReceiver mailReceiver, ThreadPoolTaskSchedul
980991
mailReceiver.setBeanFactory(bf);
981992
}
982993

994+
@Test
995+
public void receiveAndMarkAsReadDontDeleteWithThrowingWhenCopying() throws Exception {
996+
AbstractMailReceiver receiver = new ImapMailReceiver();
997+
MimeMessage msg1 = spy(GreenMailUtil.newMimeMessage("test1"));
998+
MimeMessage greenMailMsg2 = GreenMailUtil.newMimeMessage("test2");
999+
TestThrowingMimeMessage msg2 = new TestThrowingMimeMessage(greenMailMsg2);
1000+
receiver = receiveAndMarkAsReadDontDeleteGuts(receiver, msg1, msg2, false);
1001+
assertThatThrownBy(receiver::receive)
1002+
.isInstanceOf(MessagingException.class)
1003+
.hasMessage("IOException while copying message")
1004+
.cause()
1005+
.isInstanceOf(IOException.class)
1006+
.hasMessage("Simulated exception");
1007+
assertThat(msg1.getFlags().contains(Flag.SEEN)).isFalse();
1008+
assertThat(msg2.getFlags().contains(Flag.SEEN)).isFalse();
1009+
verify(msg1, times(0)).setFlags(Mockito.any(), Mockito.anyBoolean());
1010+
1011+
receiver.receive();
1012+
assertThat(msg1.getFlags().contains(Flag.SEEN)).isTrue();
1013+
assertThat(msg2.getFlags().contains(Flag.SEEN)).isTrue();
1014+
// msg2 is marked with the user and seen flags
1015+
verify(msg1, times(2)).setFlags(Mockito.any(), Mockito.anyBoolean());
1016+
verify(receiver, times(0)).deleteMessages(Mockito.any());
1017+
}
1018+
9831019
private static class ImapSearchLoggingHandler extends Handler {
9841020

9851021
private final List<String> searches = new ArrayList<>();
@@ -1015,4 +1051,21 @@ public void close() throws SecurityException {
10151051

10161052
}
10171053

1054+
private static class TestThrowingMimeMessage extends MimeMessage {
1055+
1056+
protected final AtomicBoolean throwExceptionBeforeWrite = new AtomicBoolean(true);
1057+
1058+
private TestThrowingMimeMessage(MimeMessage source) throws MessagingException {
1059+
super(source);
1060+
}
1061+
1062+
@Override
1063+
public void writeTo(OutputStream os) throws IOException, MessagingException {
1064+
if (this.throwExceptionBeforeWrite.getAndSet(false)) {
1065+
throw new IOException("Simulated exception");
1066+
}
1067+
super.writeTo(os);
1068+
}
1069+
}
1070+
10181071
}

0 commit comments

Comments
 (0)