JAMES-2342 Moving a Spam mail to Trash should not alter SpamAssassin state The mail should still be considered as Spam.
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/5c80ce22 Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/5c80ce22 Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/5c80ce22 Branch: refs/heads/master Commit: 5c80ce22d0b01dcfbd3678eba8abe8c230849962 Parents: a6b9123 Author: benwa <[email protected]> Authored: Wed Mar 7 14:21:44 2018 +0700 Committer: Antoine Duprat <[email protected]> Committed: Thu Mar 8 11:02:27 2018 +0100 ---------------------------------------------------------------------- .../spamassassin/SpamAssassinListener.java | 19 ++++-- .../spamassassin/SpamAssassinListenerTest.java | 17 +++++ .../integration/SpamAssassinContract.java | 69 ++++++++++++++++++++ 3 files changed, 100 insertions(+), 5 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/5c80ce22/mailbox/plugin/spamassassin/src/main/java/org/apache/james/mailbox/spamassassin/SpamAssassinListener.java ---------------------------------------------------------------------- diff --git a/mailbox/plugin/spamassassin/src/main/java/org/apache/james/mailbox/spamassassin/SpamAssassinListener.java b/mailbox/plugin/spamassassin/src/main/java/org/apache/james/mailbox/spamassassin/SpamAssassinListener.java index 63a8edf..9572051 100644 --- a/mailbox/plugin/spamassassin/src/main/java/org/apache/james/mailbox/spamassassin/SpamAssassinListener.java +++ b/mailbox/plugin/spamassassin/src/main/java/org/apache/james/mailbox/spamassassin/SpamAssassinListener.java @@ -89,8 +89,7 @@ public class SpamAssassinListener implements SpamEventListener { @VisibleForTesting boolean isMessageMovedToSpamMailbox(MessageMoveEvent event) { try { - MailboxPath spamMailboxPath = MailboxPath.forUser(event.getSession().getUser().getUserName(), Role.SPAM.getDefaultMailbox()); - MailboxId spamMailboxId = mapperFactory.getMailboxMapper(event.getSession()).findMailboxByPath(spamMailboxPath).getMailboxId(); + MailboxId spamMailboxId = getMailboxId(event, Role.SPAM); return event.getMessageMoves().addedMailboxIds().contains(spamMailboxId); } catch (MailboxException e) { @@ -102,13 +101,23 @@ public class SpamAssassinListener implements SpamEventListener { @VisibleForTesting boolean isMessageMovedOutOfSpamMailbox(MessageMoveEvent event) { try { - MailboxPath spamMailboxPath = MailboxPath.forUser(event.getSession().getUser().getUserName(), Role.SPAM.getDefaultMailbox()); - MailboxId spamMailboxId = mapperFactory.getMailboxMapper(event.getSession()).findMailboxByPath(spamMailboxPath).getMailboxId(); + MailboxId spamMailboxId = getMailboxId(event, Role.SPAM); + MailboxId trashMailboxId = getMailboxId(event, Role.TRASH); - return event.getMessageMoves().removedMailboxIds().contains(spamMailboxId); + return event.getMessageMoves().removedMailboxIds().contains(spamMailboxId) + && !event.getMessageMoves().addedMailboxIds().contains(trashMailboxId); } catch (MailboxException e) { LOGGER.warn("Could not resolve Spam mailbox", e); return false; } } + + private MailboxId getMailboxId(MessageMoveEvent event, Role role) throws MailboxException { + String userName = event.getSession().getUser().getUserName(); + MailboxPath mailboxPath = MailboxPath.forUser(userName, role.getDefaultMailbox()); + + return mapperFactory.getMailboxMapper(event.getSession()) + .findMailboxByPath(mailboxPath) + .getMailboxId(); + } } http://git-wip-us.apache.org/repos/asf/james-project/blob/5c80ce22/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java ---------------------------------------------------------------------- diff --git a/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java b/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java index ab0f78b..4ec7213 100644 --- a/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java +++ b/mailbox/plugin/spamassassin/src/test/java/org/apache/james/mailbox/spamassassin/SpamAssassinListenerTest.java @@ -60,6 +60,7 @@ public class SpamAssassinListenerTest { private MailboxId mailboxId2; private MailboxId spamMailboxId; private MailboxId spamCapitalMailboxId; + private MailboxId trashMailboxId; private MailboxMapper mailboxMapper; @Before @@ -71,6 +72,7 @@ public class SpamAssassinListenerTest { mailboxId2 = mailboxMapper.save(new SimpleMailbox(MailboxPath.forUser(USER, "mailbox2"), UID_VALIDITY)); spamMailboxId = mailboxMapper.save(new SimpleMailbox(MailboxPath.forUser(USER, "Spam"), UID_VALIDITY)); spamCapitalMailboxId = mailboxMapper.save(new SimpleMailbox(MailboxPath.forUser(USER, "SPAM"), UID_VALIDITY)); + trashMailboxId = mailboxMapper.save(new SimpleMailbox(MailboxPath.forUser(USER, "Trash"), UID_VALIDITY)); listener = new SpamAssassinListener(spamAssassin, mapperFactory); } @@ -189,6 +191,21 @@ public class SpamAssassinListenerTest { } @Test + public void isMessageMovedOutOfSpamMailboxShouldReturnFalseWhenMessageMovedToTrash() { + MessageMoveEvent messageMoveEvent = MessageMoveEvent.builder() + .session(MAILBOX_SESSION) + .messageMoves(MessageMoves.builder() + .previousMailboxIds(spamMailboxId) + .targetMailboxIds(trashMailboxId) + .build()) + .messages(ImmutableMap.of(MessageUid.of(45), + createMessage(mailboxId2))) + .build(); + + assertThat(listener.isMessageMovedOutOfSpamMailbox(messageMoveEvent)).isFalse(); + } + + @Test public void eventShouldCallSpamAssassinHamLearningWhenTheEventMatches() { MessageMoveEvent messageMoveEvent = MessageMoveEvent.builder() .session(MAILBOX_SESSION) http://git-wip-us.apache.org/repos/asf/james-project/blob/5c80ce22/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SpamAssassinContract.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SpamAssassinContract.java b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SpamAssassinContract.java index 0968d40..6a57aca 100644 --- a/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SpamAssassinContract.java +++ b/server/protocols/jmap-integration-testing/jmap-integration-testing-common/src/test/java/org/apache/james/jmap/methods/integration/SpamAssassinContract.java @@ -320,6 +320,72 @@ public interface SpamAssassinContract { } @Test + default void movingAMailToTrashShouldNotImpactSpamassassinLearning(JamesWithSpamAssassin james) throws Exception { + Duration slowPacedPollInterval = Duration.FIVE_HUNDRED_MILLISECONDS; + ConditionFactory calmlyAwait = Awaitility.with().pollInterval(slowPacedPollInterval).and().with().pollDelay(slowPacedPollInterval).await(); + + james.getSpamAssassinExtension().getSpamAssassin().train(ALICE); + AccessToken aliceAccessToken = accessTokenFor(james.getJmapServer(), ALICE, ALICE_PASSWORD); + AccessToken bobAccessToken = accessTokenFor(james.getJmapServer(), BOB, BOB_PASSWORD); + + // Bob is sending a message to Alice + given() + .header("Authorization", bobAccessToken.serialize()) + .body(setMessageCreate(bobAccessToken)) + .when() + .post("/jmap"); + calmlyAwait.atMost(30, TimeUnit.SECONDS).until(() -> areMessagesFoundInMailbox(aliceAccessToken, getInboxId(aliceAccessToken), 1)); + + // Alice is moving this message to Spam -> learning in SpamAssassin + List<String> messageIds = with() + .header("Authorization", aliceAccessToken.serialize()) + .body("[[\"getMessageList\", {\"filter\":{\"inMailboxes\":[\"" + getInboxId(aliceAccessToken) + "\"]}}, \"#0\"]]") + .when() + .post("/jmap") + .then() + .statusCode(200) + .body(NAME, equalTo("messageList")) + .body(ARGUMENTS + ".messageIds", hasSize(1)) + .extract() + .path(ARGUMENTS + ".messageIds"); + + messageIds + .forEach(messageId -> given() + .header("Authorization", aliceAccessToken.serialize()) + .body(String.format("[[\"setMessages\", {\"update\": {\"%s\" : { \"mailboxIds\": [\"" + getSpamId(aliceAccessToken) + "\"] } } }, \"#0\"]]", messageId)) + .when() + .post("/jmap") + .then() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(ARGUMENTS + ".updated", hasSize(1))); + calmlyAwait.atMost(30, TimeUnit.SECONDS).until(() -> areMessagesFoundInMailbox(aliceAccessToken, getSpamId(aliceAccessToken), 1)); + + // Alice is moving this message to trash + messageIds + .forEach(messageId -> given() + .header("Authorization", aliceAccessToken.serialize()) + .body(String.format("[[\"setMessages\", {\"update\": {\"%s\" : { \"mailboxIds\": [\"" + getTrashId(aliceAccessToken) + "\"] } } }, \"#0\"]]", messageId)) + .when() + .post("/jmap") + .then() + .statusCode(200) + .body(NAME, equalTo("messagesSet")) + .body(ARGUMENTS + ".updated", hasSize(1))); + calmlyAwait.atMost(30, TimeUnit.SECONDS).until(() -> areMessagesFoundInMailbox(aliceAccessToken, getTrashId(aliceAccessToken), 1)); + + // Bob is sending again the same message to Alice + given() + .header("Authorization", bobAccessToken.serialize()) + .body(setMessageCreate(bobAccessToken)) + .when() + .post("/jmap"); + + // This message is delivered in Alice Spam mailbox (she now must have 1 messages in her Spam mailbox) + calmlyAwait.atMost(10, TimeUnit.SECONDS).until(() -> areMessagesFoundInMailbox(aliceAccessToken, getSpamId(aliceAccessToken), 1)); + } + + @Test default void spamAssassinShouldForgetMessagesMovedOutOfSpamFolderUsingIMAP(JamesWithSpamAssassin james) throws Exception { Duration slowPacedPollInterval = Duration.FIVE_HUNDRED_MILLISECONDS; ConditionFactory calmlyAwait = Awaitility.with().pollInterval(slowPacedPollInterval).and().with().pollDelay(slowPacedPollInterval).await(); @@ -577,4 +643,7 @@ public interface SpamAssassinContract { default String getSpamId(AccessToken accessToken) { return getMailboxId(accessToken, Role.SPAM); } + default String getTrashId(AccessToken accessToken) { + return getMailboxId(accessToken, Role.TRASH); + } } --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
