From 891caed4a7eef0069d7dfe9851eb57347c61eaee Mon Sep 17 00:00:00 2001 From: Filip Hrisafov Date: Fri, 11 Oct 2024 19:50:24 +0200 Subject: [PATCH] GH-9546: IMAP: Flag messages after messages have been copied Fixes: #9546 Issue link: https://github.com/spring-projects/spring-integration/pull/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 # Conflicts: # spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java # Conflicts: # spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java --- .../mail/AbstractMailReceiver.java | 24 ++++++--- .../mail/ImapMailReceiverTests.java | 54 ++++++++++++++++++- 2 files changed, 70 insertions(+), 8 deletions(-) diff --git a/spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java b/spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java index c91fb69270..1ba644a2a1 100755 --- a/spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java +++ b/spring-integration-mail/src/main/java/org/springframework/integration/mail/AbstractMailReceiver.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2024 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -67,6 +67,7 @@ import org.springframework.util.ObjectUtils; * @author Artem Bilan * @author Dominik Simmen * @author Yuxin Wang + * @author Filip Hrisafov */ public abstract class AbstractMailReceiver extends IntegrationObjectSupport implements MailReceiver, DisposableBean { @@ -502,18 +503,27 @@ public abstract class AbstractMailReceiver extends IntegrationObjectSupport impl } private void postProcessFilteredMessages(Message[] filteredMessages) throws MessagingException { - setMessageFlags(filteredMessages); - - if (shouldDeleteMessages()) { - deleteMessages(filteredMessages); - } // Copy messages to cause an eager fetch + Message[] messages = filteredMessages; if (this.headerMapper == null && (this.autoCloseFolder || this.simpleContent)) { + messages = new Message[filteredMessages.length]; for (int i = 0; i < filteredMessages.length; i++) { - MimeMessage mimeMessage = new IntegrationMimeMessage((MimeMessage) filteredMessages[i]); + Message originalMessage = filteredMessages[i]; + messages[i] = originalMessage; + MimeMessage mimeMessage = new IntegrationMimeMessage((MimeMessage) originalMessage); filteredMessages[i] = mimeMessage; } } + + setMessageFlagsAndMaybeDeleteMessages(messages); + } + + private void setMessageFlagsAndMaybeDeleteMessages(Message[] messages) throws MessagingException { + setMessageFlags(messages); + + if (shouldDeleteMessages()) { + deleteMessages(messages); + } } private void setMessageFlags(Message[] filteredMessages) throws MessagingException { diff --git a/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java b/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java index 3769d06406..bb0f2bf2b0 100644 --- a/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java +++ b/spring-integration-mail/src/test/java/org/springframework/integration/mail/ImapMailReceiverTests.java @@ -17,6 +17,7 @@ package org.springframework.integration.mail; import java.io.IOException; +import java.io.OutputStream; import java.lang.reflect.Field; import java.util.ArrayList; import java.util.Arrays; @@ -24,6 +25,7 @@ import java.util.List; import java.util.Properties; import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicInteger; import java.util.concurrent.atomic.AtomicReference; import java.util.logging.Handler; @@ -88,6 +90,7 @@ import org.springframework.test.context.junit.jupiter.SpringJUnitConfig; import org.springframework.util.MimeTypeUtils; import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.BDDMockito.given; @@ -107,6 +110,7 @@ import static org.mockito.Mockito.when; * @author Artem Bilan * @author Alexander Pinske * @author Dominik Simmen + * @author Filip Hrisafov */ @SpringJUnitConfig @ContextConfiguration( @@ -299,6 +303,11 @@ public class ImapMailReceiverTests { private AbstractMailReceiver receiveAndMarkAsReadDontDeleteGuts(AbstractMailReceiver receiver, Message msg1, Message msg2) throws NoSuchFieldException, IllegalAccessException, MessagingException { + return receiveAndMarkAsReadDontDeleteGuts(receiver, msg1, msg2, true); + } + + private AbstractMailReceiver receiveAndMarkAsReadDontDeleteGuts(AbstractMailReceiver receiver, Message msg1, + Message msg2, boolean receive) throws NoSuchFieldException, IllegalAccessException, MessagingException { ((ImapMailReceiver) receiver).setShouldMarkMessagesAsRead(true); receiver = spy(receiver); @@ -326,7 +335,9 @@ public class ImapMailReceiverTests { willAnswer(invocation -> messages).given(folder).search(any(SearchTerm.class)); willAnswer(invocation -> null).given(receiver).fetchMessages(messages); - receiver.receive(); + if (receive) { + receiver.receive(); + } return receiver; } @@ -983,6 +994,30 @@ public class ImapMailReceiverTests { mailReceiver.setBeanFactory(bf); } + @Test + public void receiveAndMarkAsReadDontDeleteWithThrowingWhenCopying() throws Exception { + AbstractMailReceiver receiver = new ImapMailReceiver(); + MimeMessage msg1 = spy(GreenMailUtil.newMimeMessage("test1")); + MimeMessage greenMailMsg2 = GreenMailUtil.newMimeMessage("test2"); + TestThrowingMimeMessage msg2 = new TestThrowingMimeMessage(greenMailMsg2); + receiver = receiveAndMarkAsReadDontDeleteGuts(receiver, msg1, msg2, false); + assertThatThrownBy(receiver::receive) + .isInstanceOf(MessagingException.class) + .hasMessage("IOException while copying message") + .cause() + .isInstanceOf(IOException.class) + .hasMessage("Simulated exception"); + assertThat(msg1.getFlags().contains(Flag.SEEN)).isFalse(); + assertThat(msg2.getFlags().contains(Flag.SEEN)).isFalse(); + verify(msg1, times(0)).setFlags(Mockito.any(), Mockito.anyBoolean()); + + receiver.receive(); + assertThat(msg1.getFlags().contains(Flag.SEEN)).isTrue(); + assertThat(msg2.getFlags().contains(Flag.SEEN)).isTrue(); + // msg2 is marked with the user and seen flags + verify(msg1, times(2)).setFlags(Mockito.any(), Mockito.anyBoolean()); + verify(receiver, times(0)).deleteMessages(Mockito.any()); + } private static class ImapSearchLoggingHandler extends Handler { @@ -1019,4 +1054,21 @@ public class ImapMailReceiverTests { } + private static class TestThrowingMimeMessage extends MimeMessage { + + protected final AtomicBoolean throwExceptionBeforeWrite = new AtomicBoolean(true); + + private TestThrowingMimeMessage(MimeMessage source) throws MessagingException { + super(source); + } + + @Override + public void writeTo(OutputStream os) throws IOException, MessagingException { + if (this.throwExceptionBeforeWrite.getAndSet(false)) { + throw new IOException("Simulated exception"); + } + super.writeTo(os); + } + } + }