This is an automated email from the ASF dual-hosted git repository. btellier pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/james-project.git
commit 062cecdf20a975b9dd04851a8444b7fb606c2bd5 Author: Benoit Tellier <[email protected]> AuthorDate: Fri Feb 22 15:58:23 2019 +0700 MAILBOX-379 Run preDeletion hooks only for messages marked as deleted upon expunge Also avoid running the hooks when nothing to delete --- .../apache/james/mailbox/MailboxManagerTest.java | 57 ++++++++++++++++++++-- .../james/mailbox/store/StoreMessageManager.java | 5 ++ 2 files changed, 57 insertions(+), 5 deletions(-) diff --git a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java index ae6e3c9..87272e8 100644 --- a/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java +++ b/mailbox/api/src/test/java/org/apache/james/mailbox/MailboxManagerTest.java @@ -27,6 +27,7 @@ import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; import static org.mockito.Mockito.when; import java.nio.charset.StandardCharsets; @@ -1362,6 +1363,7 @@ public abstract class MailboxManagerTest<T extends MailboxManager> { @Test void expungeShouldCallAllPreDeletionHooks() throws Exception { ComposedMessageId composeId = inboxManager.appendMessage(AppendCommand.builder() + .withFlags(new Flags(Flags.Flag.DELETED)) .build(message), session); inboxManager.expunge(MessageRange.one(composeId.getUid()), session); @@ -1381,8 +1383,12 @@ public abstract class MailboxManagerTest<T extends MailboxManager> { @Test void expungeShouldCallAllPreDeletionHooksOnEachMessageDeletionCall() throws Exception { - ComposedMessageId composeId1 = inboxManager.appendMessage(AppendCommand.builder().build(message), session); - ComposedMessageId composeId2 = inboxManager.appendMessage(AppendCommand.builder().build(message), session); + ComposedMessageId composeId1 = inboxManager.appendMessage(AppendCommand.builder() + .withFlags(new Flags(Flags.Flag.DELETED)) + .build(message), session); + ComposedMessageId composeId2 = inboxManager.appendMessage(AppendCommand.builder() + .withFlags(new Flags(Flags.Flag.DELETED)) + .build(message), session); inboxManager.expunge(MessageRange.one(composeId1.getUid()), session); inboxManager.expunge(MessageRange.one(composeId2.getUid()), session); @@ -1402,9 +1408,48 @@ public abstract class MailboxManagerTest<T extends MailboxManager> { } @Test + void expungeShouldCallAllPreDeletionHooksOnlyOnMessagesMarkedAsDeleted() throws Exception { + ComposedMessageId composeId1 = inboxManager.appendMessage(AppendCommand.builder() + .withFlags(new Flags(Flags.Flag.DELETED)) + .build(message), session); + inboxManager.appendMessage(AppendCommand.builder() + .build(message), session); + + inboxManager.expunge(MessageRange.all(), session); + + ArgumentCaptor<PreDeletionHook.DeleteOperation> preDeleteCaptor1 = ArgumentCaptor.forClass(PreDeletionHook.DeleteOperation.class); + ArgumentCaptor<PreDeletionHook.DeleteOperation> preDeleteCaptor2 = ArgumentCaptor.forClass(PreDeletionHook.DeleteOperation.class); + verify(preDeletionHook1, times(1)).notifyDelete(preDeleteCaptor1.capture()); + verify(preDeletionHook2, times(1)).notifyDelete(preDeleteCaptor2.capture()); + + assertThat(preDeleteCaptor1.getValue().getDeletionMetadataList()) + .hasSize(1) + .hasSameElementsAs(preDeleteCaptor2.getValue().getDeletionMetadataList()) + .allSatisfy(deleteMetadata -> SoftAssertions.assertSoftly(softy -> { + softy.assertThat(deleteMetadata.getMailboxId()).isEqualTo(inboxId); + softy.assertThat(deleteMetadata.getMessageMetaData().getMessageId()).isEqualTo(composeId1.getMessageId()); + })); + } + + @Test + void expungeShouldNotCallPredeletionHooksWhenNoMessagesMarkedAsDeleted() throws Exception { + inboxManager.appendMessage(AppendCommand.builder() + .build(message), session); + + inboxManager.expunge(MessageRange.all(), session); + + verifyZeroInteractions(preDeletionHook1); + verifyZeroInteractions(preDeletionHook2); + } + + @Test void expungeShouldCallAllPreDeletionHooksOnEachMessageDeletionOnDifferentMailboxes() throws Exception { - ComposedMessageId composeId1 = inboxManager.appendMessage(AppendCommand.builder().build(message), session); - ComposedMessageId composeId2 = anotherMailboxManager.appendMessage(AppendCommand.builder().build(message), session); + ComposedMessageId composeId1 = inboxManager.appendMessage(AppendCommand.builder() + .withFlags(new Flags(Flags.Flag.DELETED)) + .build(message), session); + ComposedMessageId composeId2 = anotherMailboxManager.appendMessage(AppendCommand.builder() + .withFlags(new Flags(Flags.Flag.DELETED)) + .build(message), session); inboxManager.expunge(MessageRange.one(composeId1.getUid()), session); anotherMailboxManager.expunge(MessageRange.one(composeId2.getUid()), session); @@ -1432,7 +1477,9 @@ public abstract class MailboxManagerTest<T extends MailboxManager> { when(preDeletionHook1.notifyDelete(any(PreDeletionHook.DeleteOperation.class))) .thenThrow(new RuntimeException("throw at hook 1")); - ComposedMessageId composeId1 = inboxManager.appendMessage(AppendCommand.builder().build(message), session); + ComposedMessageId composeId1 = inboxManager.appendMessage(AppendCommand.builder() + .withFlags(new Flags(Flags.Flag.DELETED)) + .build(message), session); assertThatThrownBy(() -> inboxManager.expunge(MessageRange.one(composeId1.getUid()), session)) .isInstanceOf(RuntimeException.class); diff --git a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java index 0a18a65..f0d6ddd 100644 --- a/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java +++ b/mailbox/store/src/main/java/org/apache/james/mailbox/store/StoreMessageManager.java @@ -101,6 +101,7 @@ import org.slf4j.LoggerFactory; import com.github.steveash.guavate.Guavate; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSortedMap; import reactor.core.publisher.Flux; @@ -291,6 +292,10 @@ public class StoreMessageManager implements MessageManager { } private Map<MessageUid, MessageMetaData> deleteMessages(List<MessageUid> messageUids, MailboxSession session) throws MailboxException { + if (messageUids.isEmpty()) { + return ImmutableMap.of(); + } + MessageMapper messageMapper = mapperFactory.getMessageMapper(session); runPredeletionHooks(messageUids, session); --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
