Changing ignored commiters list from being space delimited to be comma delimited

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;
+    }
+
 }

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}

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));
+    }
+
 }