JAMES-2641 AbstractMessageIdManagerSideEffectTest should not rely on mock assertions
This makes change harder to test and depends heavily on the underlying implementation Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/b4d9035a Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/b4d9035a Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/b4d9035a Branch: refs/heads/master Commit: b4d9035a67a06592c2ecd57d89c8f34146a4f998 Parents: 8a21cb5 Author: Benoit Tellier <btell...@linagora.com> Authored: Mon Jan 7 11:24:41 2019 +0700 Committer: Benoit Tellier <btell...@linagora.com> Committed: Tue Jan 8 14:46:01 2019 +0700 ---------------------------------------------------------------------- .../AbstractMessageIdManagerSideEffectTest.java | 146 +++++++++++-------- 1 file changed, 86 insertions(+), 60 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/b4d9035a/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerSideEffectTest.java ---------------------------------------------------------------------- diff --git a/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerSideEffectTest.java b/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerSideEffectTest.java index e98bd59..f1f2c2f 100644 --- a/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerSideEffectTest.java +++ b/mailbox/store/src/test/java/org/apache/james/mailbox/store/AbstractMessageIdManagerSideEffectTest.java @@ -22,12 +22,7 @@ package org.apache.james.mailbox.store; import static org.apache.james.mailbox.fixture.MailboxFixture.ALICE; import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.reset; -import static org.mockito.Mockito.times; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import java.util.List; @@ -36,11 +31,13 @@ import javax.mail.Flags; import org.apache.james.core.quota.QuotaCount; import org.apache.james.core.quota.QuotaSize; +import org.apache.james.mailbox.MailboxListener; import org.apache.james.mailbox.MailboxSession; import org.apache.james.mailbox.MailboxSessionUtil; import org.apache.james.mailbox.MessageIdManager; import org.apache.james.mailbox.MessageManager; import org.apache.james.mailbox.MessageManager.FlagsUpdateMode; +import org.apache.james.mailbox.MessageMoveEvent; import org.apache.james.mailbox.MessageUid; import org.apache.james.mailbox.exception.MailboxException; import org.apache.james.mailbox.exception.OverQuotaException; @@ -53,14 +50,18 @@ import org.apache.james.mailbox.model.Quota; import org.apache.james.mailbox.model.QuotaRoot; import org.apache.james.mailbox.model.UpdatedFlags; import org.apache.james.mailbox.quota.QuotaManager; +import org.apache.james.mailbox.store.event.DefaultDelegatingMailboxListener; import org.apache.james.mailbox.store.event.MailboxEventDispatcher; import org.apache.james.mailbox.store.mail.model.Mailbox; -import org.apache.james.mailbox.store.mail.model.MailboxMessage; +import org.apache.james.mailbox.util.EventCollector; +import org.assertj.core.api.AbstractListAssert; +import org.assertj.core.api.ObjectAssert; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSortedMap; public abstract class AbstractMessageIdManagerSideEffectTest { private static final Quota<QuotaCount> OVER_QUOTA = Quota.<QuotaCount>builder() @@ -70,28 +71,31 @@ public abstract class AbstractMessageIdManagerSideEffectTest { private static final MessageUid messageUid1 = MessageUid.of(111); private static final MessageUid messageUid2 = MessageUid.of(113); - public static final Flags FLAGS = new Flags(); + private static final Flags FLAGS = new Flags(); + private static final MailboxSession SESSION = MailboxSessionUtil.create("any"); @Rule public ExpectedException expectedException = ExpectedException.none(); private MessageIdManager messageIdManager; - private MailboxEventDispatcher dispatcher; private MailboxSession session; private Mailbox mailbox1; private Mailbox mailbox2; private Mailbox mailbox3; private QuotaManager quotaManager; private MessageIdManagerTestSystem testingData; + private EventCollector eventCollector; + private DefaultDelegatingMailboxListener delegatingMailboxListener; protected abstract MessageIdManagerTestSystem createTestSystem(QuotaManager quotaManager, MailboxEventDispatcher dispatcher) throws Exception; public void setUp() throws Exception { - dispatcher = mock(MailboxEventDispatcher.class); + delegatingMailboxListener = new DefaultDelegatingMailboxListener(); + eventCollector = new EventCollector(); quotaManager = mock(QuotaManager.class); session = MailboxSessionUtil.create(ALICE); - testingData = createTestSystem(quotaManager, dispatcher); + testingData = createTestSystem(quotaManager, new MailboxEventDispatcher(delegatingMailboxListener)); messageIdManager = testingData.getMessageIdManager(); mailbox1 = testingData.createMailbox(MailboxFixture.INBOX_ALICE, session); @@ -103,15 +107,22 @@ public abstract class AbstractMessageIdManagerSideEffectTest { public void deleteShouldCallEventDispatcher() throws Exception { givenUnlimitedQuota(); MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); MessageResult messageResult = messageIdManager.getMessages(ImmutableList.of(messageId), FetchGroupImpl.MINIMAL, session).get(0); MessageMetaData simpleMessageMetaData = messageResult.messageMetaData(); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.delete(messageId, ImmutableList.of(mailbox1.getMailboxId()), session); - verify(dispatcher).expunged(session, simpleMessageMetaData, mailbox1); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()) + .hasSize(1).first() + .isInstanceOf(MailboxListener.Expunged.class) + .satisfies(e -> { + MailboxListener.Expunged event = (MailboxListener.Expunged) e; + assertThat(event.getMailboxId()).isEqualTo(mailbox1.getMailboxId()); + assertThat(event.getMailboxPath()).isEqualTo(mailbox1.generateAssociatedPath()); + assertThat(event.getExpunged().values()).containsOnly(simpleMessageMetaData); + }); } @Test @@ -119,75 +130,84 @@ public abstract class AbstractMessageIdManagerSideEffectTest { givenUnlimitedQuota(); MessageId messageId1 = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session); MessageId messageId2 = testingData.persist(mailbox1.getMailboxId(), messageUid2, FLAGS, session); - reset(dispatcher); MessageResult messageResult1 = messageIdManager.getMessages(ImmutableList.of(messageId1), FetchGroupImpl.MINIMAL, session).get(0); MessageMetaData simpleMessageMetaData1 = messageResult1.messageMetaData(); MessageResult messageResult2 = messageIdManager.getMessages(ImmutableList.of(messageId2), FetchGroupImpl.MINIMAL, session).get(0); MessageMetaData simpleMessageMetaData2 = messageResult2.messageMetaData(); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.delete(ImmutableList.of(messageId1, messageId2), session); - verify(dispatcher).expunged(session, simpleMessageMetaData1, mailbox1); - verify(dispatcher).expunged(session, simpleMessageMetaData2, mailbox1); - verifyNoMoreInteractions(dispatcher); + AbstractListAssert<?, List<? extends MailboxListener.Expunged>, MailboxListener.Expunged, ObjectAssert<MailboxListener.Expunged>> events = + assertThat(eventCollector.getEvents()) + .hasSize(2) + .allSatisfy(event -> assertThat(event).isInstanceOf(MailboxListener.Expunged.class)) + .extracting(event -> (MailboxListener.Expunged) event); + events.extracting(MailboxListener.MailboxEvent::getMailboxId).containsOnly(mailbox1.getMailboxId(), mailbox1.getMailboxId()); + events.extracting(MailboxListener.Expunged::getExpunged) + .containsOnly(ImmutableSortedMap.of(simpleMessageMetaData1.getUid(), simpleMessageMetaData1), + ImmutableSortedMap.of(simpleMessageMetaData2.getUid(), simpleMessageMetaData2)); } @Test public void deleteShouldNotCallEventDispatcherWhenMessageIsInWrongMailbox() throws Exception { givenUnlimitedQuota(); MessageId messageId = testingData.persist(mailbox2.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.delete(messageId, ImmutableList.of(mailbox1.getMailboxId()), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test public void setInMailboxesShouldNotCallDispatcherWhenMessageAlreadyInMailbox() throws Exception { givenUnlimitedQuota(); MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setInMailboxes(messageId, ImmutableList.of(mailbox1.getMailboxId()), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test public void setInMailboxesShouldCallDispatcher() throws Exception { givenUnlimitedQuota(); MessageId messageId = testingData.persist(mailbox2.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setInMailboxes(messageId, ImmutableList.of(mailbox1.getMailboxId(), mailbox2.getMailboxId()), session); - verify(dispatcher).added(eq(session), eq(mailbox1), any(MailboxMessage.class)); - verify(dispatcher).moved(eq(session), any(), any()); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).hasSize(2); + assertThat(eventCollector.getEvents()).filteredOn(event -> event instanceof MessageMoveEvent).hasSize(1); + assertThat(eventCollector.getEvents()).filteredOn(event -> event instanceof MailboxListener.Added).hasSize(1) + .extracting(event -> (MailboxListener.Added) event).extracting(MailboxListener.Added::getMailboxId) + .containsOnly(mailbox1.getMailboxId()); } @Test public void setInMailboxesShouldCallDispatcherWithMultipleMailboxes() throws Exception { givenUnlimitedQuota(); MessageId messageId = testingData.persist(mailbox2.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setInMailboxes(messageId, ImmutableList.of(mailbox1.getMailboxId(), mailbox2.getMailboxId(), mailbox3.getMailboxId()), session); messageIdManager.getMessages(ImmutableList.of(messageId), FetchGroupImpl.MINIMAL, session); - verify(dispatcher).added(eq(session), eq(mailbox1), any(MailboxMessage.class)); - verify(dispatcher).added(eq(session), eq(mailbox3), any(MailboxMessage.class)); - verify(dispatcher).moved(eq(session), any(), any()); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).hasSize(3); + assertThat(eventCollector.getEvents()).filteredOn(event -> event instanceof MessageMoveEvent).hasSize(1); + assertThat(eventCollector.getEvents()).filteredOn(event -> event instanceof MailboxListener.Added).hasSize(2) + .extracting(event -> (MailboxListener.Added) event).extracting(MailboxListener.Added::getMailboxId) + .containsOnly(mailbox1.getMailboxId(), mailbox3.getMailboxId()); } @Test public void setInMailboxesShouldThrowExceptionWhenOverQuota() throws Exception { MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); + when(quotaManager.getStorageQuota(any(QuotaRoot.class))).thenReturn( Quota.<QuotaSize>builder().used(QuotaSize.size(2)).computedLimit(QuotaSize.unlimited()).build()); when(quotaManager.getMessageQuota(any(QuotaRoot.class))).thenReturn(OVER_QUOTA); @@ -204,16 +224,21 @@ public abstract class AbstractMessageIdManagerSideEffectTest { givenUnlimitedQuota(); MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session); messageIdManager.setInMailboxes(messageId, ImmutableList.of(mailbox1.getMailboxId(), mailbox2.getMailboxId()), session); - reset(dispatcher); List<MessageResult> messageResults = messageIdManager.getMessages(ImmutableList.of(messageId), FetchGroupImpl.MINIMAL, session); assertThat(messageResults).hasSize(2); + + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setInMailboxes(messageId, ImmutableList.of(mailbox1.getMailboxId(), mailbox3.getMailboxId()), session); - verify(dispatcher).expunged(eq(session), any(MessageMetaData.class), eq(mailbox2)); - verify(dispatcher).added(eq(session), eq(mailbox3), any(MailboxMessage.class)); - verify(dispatcher).moved(eq(session), any(), any()); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).hasSize(3); + assertThat(eventCollector.getEvents()).filteredOn(event -> event instanceof MessageMoveEvent).hasSize(1); + assertThat(eventCollector.getEvents()).filteredOn(event -> event instanceof MailboxListener.Added).hasSize(1) + .extracting(event -> (MailboxListener.Added) event).extracting(MailboxListener.Added::getMailboxId) + .containsOnly(mailbox3.getMailboxId()); + assertThat(eventCollector.getEvents()).filteredOn(event -> event instanceof MailboxListener.Expunged).hasSize(1) + .extracting(event -> (MailboxListener.Expunged) event).extracting(MailboxListener.Expunged::getMailboxId) + .containsOnly(mailbox2.getMailboxId()); } @Test @@ -221,11 +246,11 @@ public abstract class AbstractMessageIdManagerSideEffectTest { givenUnlimitedQuota(); Flags newFlags = new Flags(Flags.Flag.SEEN); MessageId messageId = testingData.persist(mailbox2.getMailboxId(), messageUid1, newFlags, session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setFlags(newFlags, MessageManager.FlagsUpdateMode.ADD, messageId, ImmutableList.of(mailbox2.getMailboxId()), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test @@ -234,11 +259,11 @@ public abstract class AbstractMessageIdManagerSideEffectTest { Flags newFlags = new Flags(Flags.Flag.SEEN); MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, newFlags, session); messageIdManager.setInMailboxes(messageId, ImmutableList.of(mailbox2.getMailboxId()), session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setFlags(newFlags, MessageManager.FlagsUpdateMode.ADD, messageId, ImmutableList.of(mailbox1.getMailboxId(), mailbox2.getMailboxId()), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test @@ -246,11 +271,11 @@ public abstract class AbstractMessageIdManagerSideEffectTest { givenUnlimitedQuota(); Flags newFlags = new Flags(Flags.Flag.SEEN); MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setFlags(newFlags, MessageManager.FlagsUpdateMode.ADD, messageId, ImmutableList.of(mailbox2.getMailboxId()), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test @@ -258,11 +283,11 @@ public abstract class AbstractMessageIdManagerSideEffectTest { givenUnlimitedQuota(); Flags newFlags = new Flags(Flags.Flag.SEEN); MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setFlags(newFlags, MessageManager.FlagsUpdateMode.ADD, messageId, ImmutableList.of(), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test @@ -271,20 +296,19 @@ public abstract class AbstractMessageIdManagerSideEffectTest { Flags newFlags = new Flags(Flags.Flag.SEEN); MessageId messageId = testingData.persist(mailbox1.getMailboxId(), messageUid1, FLAGS, session); messageIdManager.setInMailboxes(messageId, ImmutableList.of(mailbox1.getMailboxId(), mailbox2.getMailboxId()), session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setFlags(newFlags, MessageManager.FlagsUpdateMode.ADD, messageId, ImmutableList.of(mailbox1.getMailboxId(), mailbox2.getMailboxId()), session); - verify(dispatcher, times(2)).flagsUpdated(eq(session), any(Mailbox.class), any(UpdatedFlags.class)); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).hasSize(2).allSatisfy(event -> assertThat(event).isInstanceOf(MailboxListener.FlagsUpdated.class)); } @Test public void setFlagsShouldDispatchWhenMessageBelongsToTheMailboxes() throws Exception { givenUnlimitedQuota(); MessageId messageId = testingData.persist(mailbox2.getMailboxId(), messageUid1, FLAGS, session); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); Flags newFlags = new Flags(Flags.Flag.SEEN); messageIdManager.setFlags(newFlags, MessageManager.FlagsUpdateMode.ADD, messageId, ImmutableList.of(mailbox1.getMailboxId(), mailbox2.getMailboxId()), session); @@ -300,8 +324,12 @@ public abstract class AbstractMessageIdManagerSideEffectTest { .newFlags(newFlags) .build(); - verify(dispatcher, times(1)).flagsUpdated(session, mailbox2, updatedFlags); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).hasSize(1).first().isInstanceOf(MailboxListener.FlagsUpdated.class) + .satisfies(e -> { + MailboxListener.FlagsUpdated event = (MailboxListener.FlagsUpdated) e; + assertThat(event.getUpdatedFlags()).containsOnly(updatedFlags); + assertThat(event.getMailboxId()).isEqualTo(mailbox2.getMailboxId()); + }); } @Test @@ -309,11 +337,10 @@ public abstract class AbstractMessageIdManagerSideEffectTest { givenUnlimitedQuota(); MessageId messageId = testingData.createNotUsedMessageId(); - reset(dispatcher); - + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.delete(messageId, ImmutableList.of(mailbox1.getMailboxId()), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test @@ -321,33 +348,32 @@ public abstract class AbstractMessageIdManagerSideEffectTest { givenUnlimitedQuota(); MessageId messageId = testingData.createNotUsedMessageId(); - reset(dispatcher); - + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.delete(ImmutableList.of(messageId), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test public void setFlagsShouldNotDispatchEventWhenMessageDoesNotExist() throws Exception { givenUnlimitedQuota(); MessageId messageId = testingData.createNotUsedMessageId(); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setFlags(FLAGS, FlagsUpdateMode.ADD, messageId, ImmutableList.of(mailbox2.getMailboxId()), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } @Test public void setInMailboxesShouldNotDispatchEventWhenMessageDoesNotExist() throws Exception { givenUnlimitedQuota(); MessageId messageId = testingData.createNotUsedMessageId(); - reset(dispatcher); + delegatingMailboxListener.addGlobalListener(eventCollector, SESSION); messageIdManager.setInMailboxes(messageId, ImmutableList.of(mailbox1.getMailboxId()), session); - verifyNoMoreInteractions(dispatcher); + assertThat(eventCollector.getEvents()).isEmpty(); } private void givenUnlimitedQuota() throws MailboxException { --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org