JAMES-1717 Vacation mailet should not send notification twice
Project: http://git-wip-us.apache.org/repos/asf/james-project/repo Commit: http://git-wip-us.apache.org/repos/asf/james-project/commit/0be6047b Tree: http://git-wip-us.apache.org/repos/asf/james-project/tree/0be6047b Diff: http://git-wip-us.apache.org/repos/asf/james-project/diff/0be6047b Branch: refs/heads/master Commit: 0be6047b1c6c49bbd16def26a8703b642e3277ed Parents: 5e01ded Author: Benoit Tellier <btell...@linagora.com> Authored: Wed Apr 20 12:11:37 2016 +0700 Committer: Benoit Tellier <btell...@linagora.com> Committed: Fri May 27 18:02:46 2016 +0700 ---------------------------------------------------------------------- .../james/jmap/MailetPreconditionTest.java | 4 +- .../james/jmap/mailet/VacationMailet.java | 25 ++++- .../james/jmap/mailet/VacationMailetTest.java | 102 ++++++++++++++++++- 3 files changed, 125 insertions(+), 6 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/james-project/blob/0be6047b/server/container/guice/guice-common/src/test/java/org/apache/james/jmap/MailetPreconditionTest.java ---------------------------------------------------------------------- diff --git a/server/container/guice/guice-common/src/test/java/org/apache/james/jmap/MailetPreconditionTest.java b/server/container/guice/guice-common/src/test/java/org/apache/james/jmap/MailetPreconditionTest.java index 5b70b2f..ecfa619 100644 --- a/server/container/guice/guice-common/src/test/java/org/apache/james/jmap/MailetPreconditionTest.java +++ b/server/container/guice/guice-common/src/test/java/org/apache/james/jmap/MailetPreconditionTest.java @@ -53,7 +53,7 @@ public class MailetPreconditionTest { @Test(expected = ConfigurationException.class) public void vacationMailetCheckShouldThrowOnWrongMatcher() throws Exception { - List<MatcherMailetPair> pairs = Lists.newArrayList(new MatcherMailetPair(new All(), new VacationMailet(null, null, null))); + List<MatcherMailetPair> pairs = Lists.newArrayList(new MatcherMailetPair(new All(), new VacationMailet(null, null, null, null))); new JMAPModule.VacationMailetCheck().check(pairs); } @@ -65,7 +65,7 @@ public class MailetPreconditionTest { @Test public void vacationMailetCheckShouldNotThrowIfValidPairPresent() throws Exception { - List<MatcherMailetPair> pairs = Lists.newArrayList(new MatcherMailetPair(new RecipientIsLocal(), new VacationMailet(null, null, null))); + List<MatcherMailetPair> pairs = Lists.newArrayList(new MatcherMailetPair(new RecipientIsLocal(), new VacationMailet(null, null, null, null))); new JMAPModule.VacationMailetCheck().check(pairs); } http://git-wip-us.apache.org/repos/asf/james-project/blob/0be6047b/server/protocols/jmap/src/main/java/org/apache/james/jmap/mailet/VacationMailet.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/main/java/org/apache/james/jmap/mailet/VacationMailet.java b/server/protocols/jmap/src/main/java/org/apache/james/jmap/mailet/VacationMailet.java index 410e0ed..374336b 100644 --- a/server/protocols/jmap/src/main/java/org/apache/james/jmap/mailet/VacationMailet.java +++ b/server/protocols/jmap/src/main/java/org/apache/james/jmap/mailet/VacationMailet.java @@ -26,6 +26,8 @@ import javax.inject.Inject; import javax.mail.MessagingException; import org.apache.james.jmap.api.vacation.AccountId; +import org.apache.james.jmap.api.vacation.NotificationRegistry; +import org.apache.james.jmap.api.vacation.RecipientId; import org.apache.james.jmap.api.vacation.Vacation; import org.apache.james.jmap.api.vacation.VacationRepository; import org.apache.james.util.date.ZonedDateTimeProvider; @@ -43,12 +45,15 @@ public class VacationMailet extends GenericMailet { private final VacationRepository vacationRepository; private final ZonedDateTimeProvider zonedDateTimeProvider; private final AutomaticallySentMailDetector automaticallySentMailDetector; + private final NotificationRegistry notificationRegistry; @Inject - public VacationMailet(VacationRepository vacationRepository, ZonedDateTimeProvider zonedDateTimeProvider, AutomaticallySentMailDetector automaticallySentMailDetector) { + public VacationMailet(VacationRepository vacationRepository, ZonedDateTimeProvider zonedDateTimeProvider, + AutomaticallySentMailDetector automaticallySentMailDetector, NotificationRegistry notificationRegistry) { this.vacationRepository = vacationRepository; this.zonedDateTimeProvider = zonedDateTimeProvider; this.automaticallySentMailDetector = automaticallySentMailDetector; + this.notificationRegistry = notificationRegistry; } @Override @@ -77,13 +82,26 @@ public class VacationMailet extends GenericMailet { private boolean shouldSendNotification(Vacation vacation, Mail processedMail, MailAddress recipient, ZonedDateTime processingDate) { try { return vacation.isActiveAtDate(processingDate) - && ! automaticallySentMailDetector.isAutomaticallySent(processedMail); + && ! automaticallySentMailDetector.isAutomaticallySent(processedMail) + && hasNotSentNotificationsYet(processedMail, recipient); } catch (MessagingException e) { LOGGER.warn("Failed detect automatic response in a mail from {} to {}", processedMail.getSender(), recipient, e); return false; } } + private boolean hasNotSentNotificationsYet(Mail processedMail, MailAddress recipient) { + CompletableFuture<Boolean> hasAlreadyBeenSent = notificationRegistry.isRegistered( + AccountId.fromString(recipient.toString()), + RecipientId.fromMailAddress(processedMail.getSender())); + try { + return !hasAlreadyBeenSent.join(); + } catch (Throwable t) { + LOGGER.warn("Error while checking registration state of vacation notification for user {} and sender {}", recipient, processedMail.getSender(), t); + return true; + } + } + private void sendNotification(MailAddress recipient, Mail processedMail, Vacation vacation) { try { VacationReply vacationReply = VacationReply.builder(processedMail) @@ -91,6 +109,9 @@ public class VacationMailet extends GenericMailet { .reason(vacation.getTextBody()) .build(); sendNotification(vacationReply); + notificationRegistry.register(AccountId.fromString(recipient.toString()), + RecipientId.fromMailAddress(processedMail.getSender()), + vacation.getToDate()); } catch (MessagingException e) { LOGGER.warn("Failed to send JMAP vacation notification from {} to {}", recipient, processedMail.getSender(), e); } http://git-wip-us.apache.org/repos/asf/james-project/blob/0be6047b/server/protocols/jmap/src/test/java/org/apache/james/jmap/mailet/VacationMailetTest.java ---------------------------------------------------------------------- diff --git a/server/protocols/jmap/src/test/java/org/apache/james/jmap/mailet/VacationMailetTest.java b/server/protocols/jmap/src/test/java/org/apache/james/jmap/mailet/VacationMailetTest.java index 105c1c3..1de142a 100644 --- a/server/protocols/jmap/src/test/java/org/apache/james/jmap/mailet/VacationMailetTest.java +++ b/server/protocols/jmap/src/test/java/org/apache/james/jmap/mailet/VacationMailetTest.java @@ -34,6 +34,8 @@ import java.util.concurrent.CompletableFuture; import javax.mail.MessagingException; import org.apache.james.jmap.api.vacation.AccountId; +import org.apache.james.jmap.api.vacation.NotificationRegistry; +import org.apache.james.jmap.api.vacation.RecipientId; import org.apache.james.jmap.api.vacation.Vacation; import org.apache.james.jmap.api.vacation.VacationRepository; import org.apache.james.util.date.ZonedDateTimeProvider; @@ -54,6 +56,7 @@ public class VacationMailetTest { public static final ZonedDateTime DATE_TIME_2018 = ZonedDateTime.parse("2018-10-09T08:07:06+07:00[Asia/Vientiane]"); public static final String USERNAME = "be...@apache.org"; + public static final AccountId ACCOUNT_ID = AccountId.fromString(USERNAME); private VacationMailet testee; private VacationRepository vacationRepository; private ZonedDateTimeProvider zonedDateTimeProvider; @@ -61,16 +64,20 @@ public class VacationMailetTest { private MailAddress originalSender; private MailAddress originalRecipient; private AutomaticallySentMailDetector automaticallySentMailDetector; + private NotificationRegistry notificationRegistry; + private RecipientId recipientId; @Before public void setUp() throws Exception { originalSender = new MailAddress("dist...@apache.org"); originalRecipient = new MailAddress(USERNAME); + recipientId = RecipientId.fromMailAddress(originalSender); vacationRepository = mock(VacationRepository.class); zonedDateTimeProvider = mock(ZonedDateTimeProvider.class); automaticallySentMailDetector = mock(AutomaticallySentMailDetector.class); - testee = new VacationMailet(vacationRepository, zonedDateTimeProvider, automaticallySentMailDetector); + notificationRegistry = mock(NotificationRegistry.class); + testee = new VacationMailet(vacationRepository, zonedDateTimeProvider, automaticallySentMailDetector, notificationRegistry); mailetContext = mock(MailetContext.class); testee.init(new FakeMailetConfig("vacation", mailetContext)); } @@ -109,10 +116,90 @@ public class VacationMailetTest { .build())); when(zonedDateTimeProvider.get()).thenReturn(DATE_TIME_2017); when(automaticallySentMailDetector.isAutomaticallySent(mail)).thenReturn(false); + when(notificationRegistry.isRegistered(ACCOUNT_ID, recipientId)) + .thenReturn(CompletableFuture.completedFuture(false)); testee.service(mail); verify(mailetContext).sendMail(eq(originalRecipient), eq(ImmutableList.of(originalSender)), any()); + verify(notificationRegistry).isRegistered(ACCOUNT_ID, recipientId); + verify(notificationRegistry).register(ACCOUNT_ID, recipientId, Optional.of(DATE_TIME_2018)); + verifyNoMoreInteractions(mailetContext, notificationRegistry); + } + + @Test + public void activateVacationShouldNotSendNotificationIfAlreadySent() throws Exception { + FakeMail mail = FakeMail.builder() + .fileName("spamMail.eml") + .recipient(originalRecipient) + .sender(originalSender) + .build(); + when(vacationRepository.retrieveVacation(AccountId.fromString(USERNAME))) + .thenReturn(CompletableFuture.completedFuture( + Vacation.builder() + .enabled(true) + .fromDate(Optional.of(DATE_TIME_2016)) + .toDate(Optional.of(DATE_TIME_2018)) + .textBody("Explaining my vacation") + .build())); + when(zonedDateTimeProvider.get()).thenReturn(DATE_TIME_2017); + when(notificationRegistry.isRegistered(ACCOUNT_ID, recipientId)) + .thenReturn(CompletableFuture.completedFuture(true)); + + testee.service(mail); + + verifyNoMoreInteractions(mailetContext); + } + + @Test + public void activateVacationShouldSendNotificationIfErrorUpdatingNotificationRepository() throws Exception { + FakeMail mail = FakeMail.builder() + .fileName("spamMail.eml") + .recipient(originalRecipient) + .sender(originalSender) + .build(); + when(vacationRepository.retrieveVacation(AccountId.fromString(USERNAME))) + .thenReturn(CompletableFuture.completedFuture( + Vacation.builder() + .enabled(true) + .fromDate(Optional.of(DATE_TIME_2016)) + .toDate(Optional.of(DATE_TIME_2018)) + .textBody("Explaining my vacation") + .build())); + when(zonedDateTimeProvider.get()).thenReturn(DATE_TIME_2017); + RecipientId recipientId = RecipientId.fromMailAddress(originalSender); + when(notificationRegistry.isRegistered(ACCOUNT_ID, recipientId)) + .thenReturn(CompletableFuture.completedFuture(false)); + when(notificationRegistry.register(ACCOUNT_ID, recipientId, Optional.of(DATE_TIME_2018))).thenThrow(new RuntimeException()); + + testee.service(mail); + + verify(mailetContext).sendMail(eq(originalRecipient), eq(ImmutableList.of(originalSender)), any()); + verifyNoMoreInteractions(mailetContext); + } + + @Test + public void activateVacationShouldSendNotificationIfErrorRetrievingNotificationRepository() throws Exception { + FakeMail mail = FakeMail.builder() + .fileName("spamMail.eml") + .recipient(originalRecipient) + .sender(originalSender) + .build(); + when(vacationRepository.retrieveVacation(AccountId.fromString(USERNAME))) + .thenReturn(CompletableFuture.completedFuture( + Vacation.builder() + .enabled(true) + .fromDate(Optional.of(DATE_TIME_2016)) + .toDate(Optional.of(DATE_TIME_2018)) + .textBody("Explaining my vacation") + .build())); + when(zonedDateTimeProvider.get()).thenReturn(DATE_TIME_2017); + RecipientId recipientId = RecipientId.fromMailAddress(originalSender); + when(notificationRegistry.isRegistered(ACCOUNT_ID, recipientId)) + .thenThrow(new RuntimeException()); + + testee.service(mail); + verifyNoMoreInteractions(mailetContext); } @@ -133,6 +220,8 @@ public class VacationMailetTest { .build())); when(zonedDateTimeProvider.get()).thenReturn(DATE_TIME_2017); when(automaticallySentMailDetector.isAutomaticallySent(mail)).thenReturn(true); + when(notificationRegistry.isRegistered(ACCOUNT_ID, RecipientId.fromMailAddress(originalSender))) + .thenReturn(CompletableFuture.completedFuture(false)); testee.service(mail); @@ -143,6 +232,8 @@ public class VacationMailetTest { public void multipleRecipientShouldGenerateNotifications() throws MessagingException { String secondUserName = "defa...@any.com"; MailAddress secondRecipient = new MailAddress(secondUserName); + AccountId secondAccountId = AccountId.fromString(secondUserName); + FakeMail mail = FakeMail.builder() .fileName("spamMail.eml") .recipients(ImmutableList.of(originalRecipient, secondRecipient)) @@ -156,7 +247,7 @@ public class VacationMailetTest { .toDate(Optional.of(DATE_TIME_2018)) .textBody("Explaining my vacation") .build())); - when(vacationRepository.retrieveVacation(AccountId.fromString(secondUserName))) + when(vacationRepository.retrieveVacation(secondAccountId)) .thenReturn(CompletableFuture.completedFuture( Vacation.builder() .enabled(true) @@ -166,6 +257,10 @@ public class VacationMailetTest { .build())); when(zonedDateTimeProvider.get()).thenReturn(DATE_TIME_2017); when(automaticallySentMailDetector.isAutomaticallySent(mail)).thenReturn(false); + when(notificationRegistry.isRegistered(ACCOUNT_ID, RecipientId.fromMailAddress(originalSender))) + .thenReturn(CompletableFuture.completedFuture(false)); + when(notificationRegistry.isRegistered(secondAccountId, RecipientId.fromMailAddress(originalSender))) + .thenReturn(CompletableFuture.completedFuture(false)); testee.service(mail); @@ -233,6 +328,9 @@ public class VacationMailetTest { .build())); when(zonedDateTimeProvider.get()).thenReturn(DATE_TIME_2017); when(automaticallySentMailDetector.isAutomaticallySent(mail)).thenReturn(false); + when(notificationRegistry.isRegistered(ACCOUNT_ID, RecipientId.fromMailAddress(originalSender))) + .thenReturn(CompletableFuture.completedFuture(false)); + doThrow(new MessagingException()).when(mailetContext).sendMail(eq(originalSender), eq(ImmutableList.of(originalRecipient)), any()); testee.service(mail); --------------------------------------------------------------------- To unsubscribe, e-mail: server-dev-unsubscr...@james.apache.org For additional commands, e-mail: server-dev-h...@james.apache.org
