From 90f45c23f6aaad98761d67d1f632260c273ef13f Mon Sep 17 00:00:00 2001 From: Slawek Puklo Date: Fri, 26 Jun 2015 19:13:32 +0100 Subject: [PATCH] Changing ignored commiters list from being space delimited to be comma delimited. --- .../IgnoreCommittersEligibilityFilter.java | 76 +++---- src/main/resources/static/jenkins.soy | 2 +- ...IgnoreCommittersEligibilityFilterTest.java | 202 ++++++++++++------ 3 files changed, 173 insertions(+), 107 deletions(-) diff --git a/src/main/java/com/nerdwin15/stash/webhook/service/eligibility/IgnoreCommittersEligibilityFilter.java b/src/main/java/com/nerdwin15/stash/webhook/service/eligibility/IgnoreCommittersEligibilityFilter.java index c56b37a..92f9cd8 100644 --- a/src/main/java/com/nerdwin15/stash/webhook/service/eligibility/IgnoreCommittersEligibilityFilter.java +++ b/src/main/java/com/nerdwin15/stash/webhook/service/eligibility/IgnoreCommittersEligibilityFilter.java @@ -8,46 +8,50 @@ import com.nerdwin15.stash.webhook.service.SettingsService; /** - * An EligibilityFilter that checks if the user that initiated the - * RepositoryRefsChangedEvent is a user that is in the ignores list for the + * An EligibilityFilter that checks if the user that initiated the + * RepositoryRefsChangedEvent is a user that is in the ignores list for the * hook configuration. - * + * * @author Michael Irwin (mikesir87) */ public class IgnoreCommittersEligibilityFilter implements EligibilityFilter { - private static final Logger logger = // CHECKSTYLE:logger - LoggerFactory.getLogger(IgnoreCommittersEligibilityFilter.class); - - private SettingsService settingsService; - - /** - * Constructs a new instance - * @param settingsService Service to get the webhook settings - */ - public IgnoreCommittersEligibilityFilter( - SettingsService settingsService) { - this.settingsService = settingsService; - } - - @Override - public boolean shouldDeliverNotification(EventContext event) { - String eventUserName = event.getUsername(); - - final Settings settings = settingsService.getSettings( - event.getRepository()); - String ignoreCommitters = settings.getString(Notifier.IGNORE_COMMITTERS); - if (ignoreCommitters == null || eventUserName == null) - return true; - - for (String committer : ignoreCommitters.split(" ")) { - if (committer.equalsIgnoreCase(eventUserName)) { - logger.debug("Ignoring push event due to ignore committer {}", - committer); - return false; - } + private static final Logger logger = // CHECKSTYLE:logger + LoggerFactory.getLogger(IgnoreCommittersEligibilityFilter.class); + + public static final String IGNORED_COMMITTERS_LIST_SPLITTING_CHARACTER = ","; + + private SettingsService settingsService; + + /** + * Constructs a new instance + * + * @param settingsService Service to get the webhook settings + */ + public IgnoreCommittersEligibilityFilter( + SettingsService settingsService) { + this.settingsService = settingsService; } - return true; - } - + + @Override + public boolean shouldDeliverNotification(final EventContext event) { + final String eventUserName = event.getUsername(); + + final Settings settings = settingsService.getSettings(event.getRepository()); + final String ignoreCommitters = settings.getString(Notifier.IGNORE_COMMITTERS); + + if (ignoreCommitters == null || eventUserName == null) { + return true; + } + + for (final String committer : ignoreCommitters.split(IGNORED_COMMITTERS_LIST_SPLITTING_CHARACTER)) { + if (committer.trim().equalsIgnoreCase(eventUserName)) { + logger.debug("Ignoring push event due to ignore committer {}", + committer); + return false; + } + } + return true; + } + } diff --git a/src/main/resources/static/jenkins.soy b/src/main/resources/static/jenkins.soy index e8615ee..07aab5d 100644 --- a/src/main/resources/static/jenkins.soy +++ b/src/main/resources/static/jenkins.soy @@ -92,7 +92,7 @@ {param labelContent} {stash_i18n('stash.webhook.ignoreCommitters.label', 'Committers to Ignore')} {/param} - {param descriptionText: stash_i18n('stash.webhook.ignoreCommitters.description', 'Stash usernames of committer(s) whose pushes/merges should NOT trigger a Jenkins notification. Space delimited') /} + {param descriptionText: stash_i18n('stash.webhook.ignoreCommitters.description', 'Stash usernames of committer(s) whose pushes/merges should NOT trigger a Jenkins notification. Comma delimited.') /} {param extraClasses: 'long' /} {param errorTexts: $errors ? $errors['ignoreCommitters'] : null /} {/call} diff --git a/src/test/java/com/nerdwin15/stash/webhook/service/eligibility/IgnoreCommittersEligibilityFilterTest.java b/src/test/java/com/nerdwin15/stash/webhook/service/eligibility/IgnoreCommittersEligibilityFilterTest.java index 7ca6d0b..112f53c 100644 --- a/src/test/java/com/nerdwin15/stash/webhook/service/eligibility/IgnoreCommittersEligibilityFilterTest.java +++ b/src/test/java/com/nerdwin15/stash/webhook/service/eligibility/IgnoreCommittersEligibilityFilterTest.java @@ -1,8 +1,8 @@ package com.nerdwin15.stash.webhook.service.eligibility; +import static com.nerdwin15.stash.webhook.service.eligibility.IgnoreCommittersEligibilityFilter.IGNORED_COMMITTERS_LIST_SPLITTING_CHARACTER; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; -import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import org.junit.Before; @@ -12,80 +12,142 @@ import com.atlassian.stash.setting.Settings; import com.nerdwin15.stash.webhook.Notifier; import com.nerdwin15.stash.webhook.service.SettingsService; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.runners.MockitoJUnitRunner; /** * Test case for the {@link IgnoreCommittersEligibilityFilter} class - * + * * @author Michael Irwin (mikesir87) */ +@RunWith(MockitoJUnitRunner.class) public class IgnoreCommittersEligibilityFilterTest { - private SettingsService settingsService; - private IgnoreCommittersEligibilityFilter filter; - private Settings settings; - private Repository repo; - private EventContext eventContext; - private String username = "pinky"; - - /** - * Setup tasks - */ - @Before - public void setUp() throws Exception { - settingsService = mock(SettingsService.class); - repo = mock(Repository.class); - filter = new IgnoreCommittersEligibilityFilter(settingsService); - settings = mock(Settings.class); - when(settingsService.getSettings(repo)).thenReturn(settings); - - eventContext = mock(EventContext.class); - when(eventContext.getEventSource()).thenReturn(null); - when(eventContext.getRepository()).thenReturn(repo); - when(eventContext.getUsername()).thenReturn(username); - } - - /** - * Validate that the filter should still allow delivery when no ignored - * committers settings have been set. - * @throws Exception - */ - @Test - public void shouldAllowWhenIgnoredCommittersNull() throws Exception { - when(settings.getString(Notifier.IGNORE_COMMITTERS)).thenReturn(null); - assertTrue(filter.shouldDeliverNotification(eventContext)); - } - - /** - * Validate that the filter should still allow delivery when the event user - * does not match any of the ignored committers - * @throws Exception - */ - @Test - public void shouldAllowWhenIgnoredCommittersDoesntMatch() throws Exception { - when(settings.getString(Notifier.IGNORE_COMMITTERS)) - .thenReturn(username + "-notmatching"); - assertTrue(filter.shouldDeliverNotification(eventContext)); - } - - /** - * Validate that the filter should cancel if an ignored committer matches - * @throws Exception - */ - @Test - public void shouldCancelWhenIgnoredCommittersMatches() throws Exception { - when(settings.getString(Notifier.IGNORE_COMMITTERS)).thenReturn(username); - assertFalse(filter.shouldDeliverNotification(eventContext)); - } - - /** - * Validate that the filter should cancel if an ignored committer matches - * @throws Exception - */ - @Test - public void shouldCancelWhenMatchesWithMultipleCommitters() throws Exception { - when(settings.getString(Notifier.IGNORE_COMMITTERS)).thenReturn(username - + " anotherUser"); - assertFalse(filter.shouldDeliverNotification(eventContext)); - } - + private final String username = "Pinky"; + private final String usernameWithSpace = "The Brain"; + private final String spaces = " "; + + @Mock private SettingsService settingsService; + + @Mock private Settings settings; + + @Mock private Repository repo; + + @Mock private EventContext eventContext; + + private IgnoreCommittersEligibilityFilter filter; + + /** + * Setup tasks + */ + @Before + public void setUp() throws Exception { + when(settingsService.getSettings(repo)).thenReturn(settings); + when(eventContext.getEventSource()).thenReturn(null); + when(eventContext.getRepository()).thenReturn(repo); + when(eventContext.getUsername()).thenReturn(username); + + filter = new IgnoreCommittersEligibilityFilter(settingsService); + } + + /** + * Validate that the filter should still allow delivery when no ignored + * committers settings have been set. + * + * @throws Exception + */ + @Test + public void shouldAllowNotificationWhenIgnoredCommittersListIsNull() throws Exception { + when(settings.getString(Notifier.IGNORE_COMMITTERS)) + .thenReturn(null); + + assertTrue(filter.shouldDeliverNotification(eventContext)); + } + + /** + * Validate that the filter should still allow delivery when ignored + * committers settings have been set to empty string. + * + * @throws Exception + */ + @Test + public void shouldAllowNotificationWhenIgnoredCommittersListIsEmpty() throws Exception { + when(settings.getString(Notifier.IGNORE_COMMITTERS)) + .thenReturn(""); + + assertTrue(filter.shouldDeliverNotification(eventContext)); + } + + /** + * Validate that the filter should still allow delivery when the event user + * does not match any of the ignored committers. + * + * @throws Exception + */ + @Test + public void shouldAllowNotificationWhenIgnoredCommittersDoesntMatch() throws Exception { + when(settings.getString(Notifier.IGNORE_COMMITTERS)) + .thenReturn(username + "-notmatching"); + + assertTrue(filter.shouldDeliverNotification(eventContext)); + } + + /** + * Validate that the filter should not allow notification if an ignored committer matches. + * + * @throws Exception + */ + @Test + public void shouldNotAllowNotificationWhenIgnoredCommittersMatches() throws Exception { + when(settings.getString(Notifier.IGNORE_COMMITTERS)) + .thenReturn(username); + + assertFalse(filter.shouldDeliverNotification(eventContext)); + } + + /** + * Validate that the filter should not allow notification if an ignored committer matches, + * even if name is prefixed and suffixed with spaces. + * + * @throws Exception + */ + @Test + public void shouldNotAllowNotificationWhenIgnoredCommittersMatchesDespiteOfSpaces() throws Exception { + final String ignoredCommiter = spaces + username + spaces; + when(settings.getString(Notifier.IGNORE_COMMITTERS)) + .thenReturn(ignoredCommiter); + + assertFalse(filter.shouldDeliverNotification(eventContext)); + } + + /** + * Validate that the filter should not allow notification if one of ignored committers matches. + * + * @throws Exception + */ + @Test + public void shouldNotAllowNotificationWhenMatchesWithMultipleCommitters() throws Exception { + final String ignoredCommiters = username + IGNORED_COMMITTERS_LIST_SPLITTING_CHARACTER + usernameWithSpace; + when(settings.getString(Notifier.IGNORE_COMMITTERS)) + .thenReturn(ignoredCommiters); + + assertFalse(filter.shouldDeliverNotification(eventContext)); + } + + /** + * Validate that the filter should not allow notification if ignored committer matches + * and username of committer contains space. + * + * @throws Exception + */ + @Test + public void shouldNotAllowNotificationWhenMatchesUsernameContainingSpace() throws Exception { + when(eventContext.getUsername()).thenReturn(usernameWithSpace); + when(settings.getString(Notifier.IGNORE_COMMITTERS)) + .thenReturn(usernameWithSpace); + + assertFalse(filter.shouldDeliverNotification(eventContext)); + } + }