From 608e3c714ef53fc74d84a6e8ecc1e2c15d8a2d41 Mon Sep 17 00:00:00 2001 From: Tomas Bjerre Date: Mon, 6 Apr 2015 13:15:21 +0200 Subject: [PATCH] Listening for each specific event, instead of all pull request events * To avoid handling same event twice --- CHANGELOG.md | 3 + .../PrnfsPullRequestEventListener.java | 94 +++++++++++-------- .../PrnfsPullRequestEventListenerTest.java | 13 --- .../prnfs/admin/utils/PrnfsTestBuilder.java | 2 +- 4 files changed, 59 insertions(+), 53 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4a550dce..74b8434d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ Changelog of Pull Request Notifier for Stash. +## 1.4 +* Bugfix: Avoiding multiple notifications being sent from same event. + ## 1.3 Same as version 1.2 but with different version number. When version 1.2 was initially rejected I fixed the issue and created a new 1.2. But a new version number was needed for a resubmission to Atlassian Marketplace. diff --git a/src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java b/src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java index ea872805..25c56ef8 100644 --- a/src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java +++ b/src/main/java/se/bjurr/prnfs/listener/PrnfsPullRequestEventListener.java @@ -1,40 +1,37 @@ package se.bjurr.prnfs.listener; -import static com.google.common.cache.CacheBuilder.newBuilder; -import static com.google.common.collect.Lists.newArrayList; -import static java.lang.Boolean.FALSE; -import static java.lang.Boolean.TRUE; -import static java.util.Collections.sort; -import static java.util.concurrent.TimeUnit.MILLISECONDS; import static java.util.regex.Pattern.compile; import static se.bjurr.prnfs.settings.SettingsStorage.getPrnfsSettings; -import java.util.ArrayList; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import se.bjurr.prnfs.listener.PrnfsRenderer.PrnfsVariable; import se.bjurr.prnfs.settings.PrnfsNotification; import se.bjurr.prnfs.settings.PrnfsSettings; import se.bjurr.prnfs.settings.ValidationException; import com.atlassian.event.api.EventListener; import com.atlassian.sal.api.pluginsettings.PluginSettingsFactory; +import com.atlassian.stash.event.pull.PullRequestApprovedEvent; +import com.atlassian.stash.event.pull.PullRequestCommentAddedEvent; +import com.atlassian.stash.event.pull.PullRequestDeclinedEvent; import com.atlassian.stash.event.pull.PullRequestEvent; +import com.atlassian.stash.event.pull.PullRequestMergedEvent; +import com.atlassian.stash.event.pull.PullRequestOpenedEvent; +import com.atlassian.stash.event.pull.PullRequestReopenedEvent; +import com.atlassian.stash.event.pull.PullRequestRescopedEvent; +import com.atlassian.stash.event.pull.PullRequestUnapprovedEvent; +import com.atlassian.stash.event.pull.PullRequestUpdatedEvent; import com.google.common.annotations.VisibleForTesting; -import com.google.common.cache.Cache; public class PrnfsPullRequestEventListener { private UrlInvoker urlInvoker = new UrlInvoker(); private final PluginSettingsFactory pluginSettingsFactory; private static final Logger logger = LoggerFactory.getLogger(PrnfsPullRequestEventListener.class); - private static Cache duplicateEventCache; public PrnfsPullRequestEventListener(PluginSettingsFactory pluginSettingsFactory) { this.pluginSettingsFactory = pluginSettingsFactory; - duplicateEventCache = newBuilder().maximumSize(1000).expireAfterWrite(50, MILLISECONDS).build(); } @VisibleForTesting @@ -43,12 +40,53 @@ public void setUrlInvoker(UrlInvoker urlInvoker) { } @EventListener - public void anEvent(PullRequestEvent o) { - if (dublicateEventBug(o)) { - return; - } + public void onEvent(PullRequestApprovedEvent e) { + handleEvent(e); + } + + @EventListener + public void onEvent(PullRequestCommentAddedEvent e) { + handleEvent(e); + } + + @EventListener + public void onEvent(PullRequestDeclinedEvent e) { + handleEvent(e); + } + + @EventListener + public void onEvent(PullRequestMergedEvent e) { + handleEvent(e); + } + + @EventListener + public void onEvent(PullRequestOpenedEvent e) { + handleEvent(e); + } + + @EventListener + public void onEvent(PullRequestReopenedEvent e) { + handleEvent(e); + } + + @EventListener + public void onEvent(PullRequestRescopedEvent e) { + handleEvent(e); + } + + @EventListener + public void onEvent(PullRequestUnapprovedEvent e) { + handleEvent(e); + } + + @EventListener + public void onEvent(PullRequestUpdatedEvent e) { + handleEvent(e); + } + + @VisibleForTesting + public void handleEvent(PullRequestEvent o) { final PrnfsRenderer renderer = new PrnfsRenderer(o); - logEvent(renderer, o); try { final PrnfsSettings settings = getPrnfsSettings(pluginSettingsFactory.createGlobalSettings()); for (final PrnfsNotification n : settings.getNotifications()) { @@ -64,26 +102,4 @@ public void anEvent(PullRequestEvent o) { logger.error("", e); } } - - private void logEvent(PrnfsRenderer renderer, PullRequestEvent o) { - final StringBuilder renderString = new StringBuilder(); - final ArrayList variables = newArrayList(PrnfsRenderer.PrnfsVariable.values()); - sort(variables); - for (final PrnfsRenderer.PrnfsVariable variable : variables) { - renderString.append(" " + variable.name() + ": ${" + variable.name() + "}"); - } - logger.info(renderer.render(renderString.toString().trim())); - } - - /** - * Looks like there is a bug in Stash that causes events to be fired twice. - */ - public static boolean dublicateEventBug(PullRequestEvent o) { - final String footprint = o.getPullRequest().getId() + "_" + o.getAction().name(); - if (duplicateEventCache.asMap().containsKey(footprint)) { - return TRUE; - } - duplicateEventCache.put(footprint, TRUE); - return FALSE; - } } \ No newline at end of file diff --git a/src/test/java/se/bjurr/prnfs/admin/PrnfsPullRequestEventListenerTest.java b/src/test/java/se/bjurr/prnfs/admin/PrnfsPullRequestEventListenerTest.java index 761c2b55..12a7015f 100644 --- a/src/test/java/se/bjurr/prnfs/admin/PrnfsPullRequestEventListenerTest.java +++ b/src/test/java/se/bjurr/prnfs/admin/PrnfsPullRequestEventListenerTest.java @@ -1,15 +1,11 @@ package se.bjurr.prnfs.admin; -import static com.atlassian.stash.pull.PullRequestAction.APPROVED; import static com.atlassian.stash.pull.PullRequestAction.MERGED; import static com.atlassian.stash.pull.PullRequestAction.OPENED; import static com.google.common.base.Charsets.UTF_8; import static com.google.common.base.Joiner.on; import static com.google.common.collect.Lists.newArrayList; import static com.google.common.io.Resources.getResource; -import static java.lang.Boolean.FALSE; -import static java.lang.Boolean.TRUE; -import static java.lang.Thread.sleep; import static java.util.Collections.sort; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; @@ -17,7 +13,6 @@ import static se.bjurr.prnfs.admin.utils.PrnfsTestBuilder.prnfsTestBuilder; import static se.bjurr.prnfs.admin.utils.PullRequestEventBuilder.pullRequestEventBuilder; import static se.bjurr.prnfs.admin.utils.PullRequestRefBuilder.pullRequestRefBuilder; -import static se.bjurr.prnfs.listener.PrnfsPullRequestEventListener.dublicateEventBug; import java.io.IOException; import java.net.URL; @@ -140,14 +135,6 @@ public void testThatBasicAuthenticationHeaderIsSentIfThereIsAUser() { .invokedUser("theuser").invokedPassword("thepassword"); } - @Test - public void testThatDuplicateEventsFiredInStashAreIgnored() throws InterruptedException { - assertEquals(FALSE, dublicateEventBug(pullRequestEventBuilder().withId(100L).withPullRequestAction(APPROVED).build())); - assertEquals(TRUE, dublicateEventBug(pullRequestEventBuilder().withId(100L).withPullRequestAction(APPROVED).build())); - sleep(100); - assertEquals(FALSE, dublicateEventBug(pullRequestEventBuilder().withId(100L).withPullRequestAction(APPROVED).build())); - } - @Test public void testThatFieldsUsedInAdminGUIArePresentInAdminFormFields() throws IOException { final URL resource = getResource("admin.vm"); diff --git a/src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java b/src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java index 12c04fa1..516ec114 100644 --- a/src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java +++ b/src/test/java/se/bjurr/prnfs/admin/utils/PrnfsTestBuilder.java @@ -238,7 +238,7 @@ public void ivoke(String url, Optional userParam, Optional passw usedPassword.add(passwordParam.or("")); } }); - listener.anEvent(event); + listener.handleEvent(event); return this; }