Add Jira issue selector to collect issues since last successful build (#453)

src/main/java/hudson/plugins/jira/selector/AbstractIssueSelector.java

@@ -3,10 +3,20 @@
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.ExtensionPoint;
 import hudson.model.AbstractDescribableImpl;
+import hudson.model.ParameterValue;
+import hudson.model.ParametersAction;
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import hudson.plugins.jira.JiraSite;
+import hudson.plugins.jira.RunScmChangeExtractor;
+import hudson.plugins.jira.listissuesparameter.JiraIssueParameterValue;
+import hudson.scm.ChangeLogSet;
+import hudson.scm.ChangeLogSet.Entry;
 import java.util.Set;
+import java.util.logging.Logger;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.commons.lang.StringUtils;
 
 /**
  * Strategy of finding issues which should be updated after completed run.
@@ -16,6 +26,8 @@
 public abstract class AbstractIssueSelector extends AbstractDescribableImpl<AbstractIssueSelector>
         implements ExtensionPoint {
 
+    private static final Logger BASE_LOGGER = Logger.getLogger(AbstractIssueSelector.class.getName());
+
     /**
      * Finds the strings that match Jira issue ID patterns.
      *
@@ -29,4 +41,72 @@ public abstract class AbstractIssueSelector extends AbstractDescribableImpl<Abst
      */
     public abstract Set<String> findIssueIds(
             @NonNull Run<?, ?> run, @NonNull JiraSite site, @NonNull TaskListener listener);
+
+    /**
+     * Adds issues to issueIds from parameters
+     */
+    protected void addIssuesFromParameters(
+            Run<?, ?> build, JiraSite site, TaskListener listener, Set<String> issueIds) {
+        // Now look for any JiraIssueParameterValue's set in the build
+        // Implements JENKINS-12312
+        ParametersAction parameters = build.getAction(ParametersAction.class);
+
+        if (parameters != null) {
+            for (ParameterValue val : parameters.getParameters()) {
+                if (val instanceof JiraIssueParameterValue) {
+                    String issueId = ((JiraIssueParameterValue) val).getValue().toString();
+                    if (issueIds.add(issueId)) {
+                        BASE_LOGGER.finer("Added perforce issue " + issueId + " from build " + build);
+                    }
+                }
+            }
+        }
+    }
+
+    /**
+     * Calls {@link #findIssues(Run, Set, Pattern, TaskListener)} with
+     * {@link JiraSite#getIssuePattern()} as pattern
+     */
+    protected void addIssuesFromChangeLog(Run<?, ?> build, JiraSite site, TaskListener listener, Set<String> issueIds) {
+        Pattern pattern = site.getIssuePattern();
+        findIssues(build, issueIds, pattern, listener);
+    }
+
+    /**
+     * Adds issues to issueIds from the current build. Issues from parameters
+     * are added as well as issues matching pattern
+     * {@link #addIssuesFromChangeLog(Run, JiraSite, TaskListener, Set)}
+     * {@link #addIssuesFromParameters(Run, JiraSite, TaskListener, Set)}
+     */
+    protected void addIssuesFromCurrentBuild(
+            Run<?, ?> build, JiraSite site, TaskListener listener, Set<String> issueIds) {
+        addIssuesFromChangeLog(build, site, listener, issueIds);
+        addIssuesFromParameters(build, site, listener, issueIds);
+    }
+
+    /**
+     * Finds the strings that match Jira issue ID patterns. This method returns
+     * all likely candidates and doesn't check if such ID actually exists or
+     * not. We don't want to use {@link JiraSite#existsIssue(String)} here so
+     * that new projects in Jira can be detected.
+     *
+     */
+    protected static void findIssues(Run<?, ?> build, Set<String> issueIds, Pattern pattern, TaskListener listener) {
+        for (ChangeLogSet<? extends Entry> set : RunScmChangeExtractor.getChanges(build)) {
+            for (Entry change : set) {
+                BASE_LOGGER.fine("Looking for Jira ID in " + change.getMsg());
+                Matcher m = pattern.matcher(change.getMsg());
+
+                while (m.find()) {
+                    if (m.groupCount() >= 1) {
+                        String content = StringUtils.upperCase(m.group(1));
+                        issueIds.add(content);
+                    } else {
+                        listener.getLogger()
+                                .println("Warning: The Jira pattern " + pattern + " doesn't define a capturing group!");
+                    }
+                }
+            }
+        }
+    }
 }

src/main/java/hudson/plugins/jira/selector/DefaultIssueSelector.java

@@ -5,26 +5,19 @@
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractBuild.DependencyChange;
 import hudson.model.Descriptor;
-import hudson.model.ParameterValue;
-import hudson.model.ParametersAction;
 import hudson.model.Run;
 import hudson.model.TaskListener;
 import hudson.plugins.jira.JiraCarryOverAction;
 import hudson.plugins.jira.JiraSite;
 import hudson.plugins.jira.Messages;
 import hudson.plugins.jira.RunScmChangeExtractor;
-import hudson.plugins.jira.listissuesparameter.JiraIssueParameterValue;
-import hudson.scm.ChangeLogSet;
-import hudson.scm.ChangeLogSet.Entry;
 import java.util.Collection;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.Set;
 import java.util.logging.Level;
 import java.util.logging.Logger;
-import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import org.apache.commons.lang.StringUtils;
 import org.kohsuke.stapler.DataBoundConstructor;
 
 public class DefaultIssueSelector extends AbstractIssueSelector {
@@ -58,41 +51,6 @@ protected Logger getLogger() {
         return LOGGER;
     }
 
-    /**
-     * Finds the strings that match Jira issue ID patterns. This method returns
-     * all likely candidates and doesn't check if such ID actually exists or
-     * not. We don't want to use {@link JiraSite#existsIssue(String)} here so
-     * that new projects in Jira can be detected.
-     *
-     */
-    protected static void findIssues(Run<?, ?> build, Set<String> issueIds, Pattern pattern, TaskListener listener) {
-        for (ChangeLogSet<? extends Entry> set : RunScmChangeExtractor.getChanges(build)) {
-            for (Entry change : set) {
-                LOGGER.fine("Looking for Jira ID in " + change.getMsg());
-                Matcher m = pattern.matcher(change.getMsg());
-
-                while (m.find()) {
-                    if (m.groupCount() >= 1) {
-                        String content = StringUtils.upperCase(m.group(1));
-                        issueIds.add(content);
-                    } else {
-                        listener.getLogger()
-                                .println("Warning: The Jira pattern " + pattern + " doesn't define a capturing group!");
-                    }
-                }
-            }
-        }
-    }
-
-    /**
-     * Calls {@link #findIssues(Run, Set, Pattern, TaskListener)} with
-     * {@link JiraSite#getIssuePattern()} as pattern
-     */
-    protected void addIssuesFromChangeLog(Run<?, ?> build, JiraSite site, TaskListener listener, Set<String> issueIds) {
-        Pattern pattern = site.getIssuePattern();
-        findIssues(build, issueIds, pattern, listener);
-    }
-
     /**
      * Adds issues to issueIds. Adds issues carried over from previous build,
      * issues from current build and from dependent builds
@@ -106,18 +64,6 @@ protected void addIssuesRecursive(Run<?, ?> build, JiraSite site, TaskListener l
         addIssuesFromDependentBuilds(build, site, listener, issuesIds);
     }
 
-    /**
-     * Adds issues to issueIds from the current build. Issues from parameters
-     * are added as well as issues matching pattern
-     * {@link #addIssuesFromChangeLog(Run, JiraSite, TaskListener, Set)}
-     * {@link #addIssuesFromParameters(Run, JiraSite, TaskListener, Set)}
-     */
-    protected void addIssuesFromCurrentBuild(
-            Run<?, ?> build, JiraSite site, TaskListener listener, Set<String> issueIds) {
-        addIssuesFromChangeLog(build, site, listener, issueIds);
-        addIssuesFromParameters(build, site, listener, issueIds);
-    }
-
     /**
      * Adds issues to issueIds by examining dependency changes from last build.
      * For each dependency change
@@ -139,27 +85,6 @@ protected void addIssuesFromDependentBuilds(
         }
     }
 
-    /**
-     * Adds issues to issueIds from parameters
-     */
-    protected void addIssuesFromParameters(
-            Run<?, ?> build, JiraSite site, TaskListener listener, Set<String> issueIds) {
-        // Now look for any JiraIssueParameterValue's set in the build
-        // Implements JENKINS-12312
-        ParametersAction parameters = build.getAction(ParametersAction.class);
-
-        if (parameters != null) {
-            for (ParameterValue val : parameters.getParameters()) {
-                if (val instanceof JiraIssueParameterValue) {
-                    String issueId = ((JiraIssueParameterValue) val).getValue().toString();
-                    if (issueIds.add(issueId)) {
-                        getLogger().finer("Added perforce issue " + issueId + " from build " + build);
-                    }
-                }
-            }
-        }
-    }
-
     /**
      * Adds issues that were carried over from previous build to issueIds
      */

src/main/java/hudson/plugins/jira/selector/SinceLastSuccessfulBuildIssueSelector.java

@@ -0,0 +1,63 @@
+package hudson.plugins.jira.selector;
+
+import edu.umd.cs.findbugs.annotations.NonNull;
+import hudson.Extension;
+import hudson.model.Descriptor;
+import hudson.model.Run;
+import hudson.model.TaskListener;
+import hudson.plugins.jira.JiraSite;
+import hudson.plugins.jira.Messages;
+import java.util.LinkedHashSet;
+import java.util.Set;
+import org.kohsuke.stapler.DataBoundConstructor;
+
+/**
+ * Selects JIRA issues from the current build and all builds
+ * since the last successful one.
+ */
+public class SinceLastSuccessfulBuildIssueSelector extends AbstractIssueSelector {
+
+    @DataBoundConstructor
+    public SinceLastSuccessfulBuildIssueSelector() {}
+
+    @Override
+    public Set<String> findIssueIds(
+            @NonNull final Run<?, ?> run, @NonNull final JiraSite site, @NonNull final TaskListener listener) {
+        Set<String> issueIds = new LinkedHashSet<>();
+
+        Run<?, ?> lastSuccessfulBuild = run.getPreviousSuccessfulBuild();
+
+        if (lastSuccessfulBuild == null) {
+            listener.getLogger().println("No previous successful build found. Searching only in current build.");
+            addIssuesFromCurrentBuild(run, site, listener, issueIds);
+            return issueIds;
+        }
+
+        listener.getLogger()
+                .println("Collecting JIRA issues since last successful build #" + lastSuccessfulBuild.getNumber());
+
+        Run<?, ?> build = run;
+        int buildsProcessed = 0;
+
+        while (build != null && build != lastSuccessfulBuild) {
+            addIssuesFromCurrentBuild(build, site, listener, issueIds);
+
+            buildsProcessed++;
+            build = build.getPreviousBuild();
+        }
+
+        listener.getLogger()
+                .println("Found " + issueIds.size() + " JIRA issue(s) across " + buildsProcessed + " build(s)");
+
+        return issueIds;
+    }
+
+    @Extension
+    public static final class DescriptorImpl extends Descriptor<AbstractIssueSelector> {
+
+        @Override
+        public String getDisplayName() {
+            return Messages.SinceLastSuccessfulBuildIssueSelector_DisplayName();
+        }
+    }
+}

src/main/resources/hudson/plugins/jira/Messages.properties

@@ -47,3 +47,4 @@ ErrorCommentingIssues=[Jira] Could not comment on some issues: {0}
 JiraSite.threadExecutorMinimunSize = Thread Executor Size must be at least {0} (higher values are recommended)
 JiraSite.timeoutMinimunValue = Connection timeout must be at least {0}
 JiraSite.readTimeoutMinimunValue = Read timeout must be at least {0}
+SinceLastSuccessfulBuildIssueSelector.DisplayName = Select issues since the last successful build

src/test/java/hudson/plugins/jira/selector/DefaultIssueSelectorTest.java

@@ -3,6 +3,7 @@
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 import hudson.model.BuildListener;
@@ -233,4 +234,91 @@ void defaultPatternNotToMatchMavenRelease() {
         DefaultIssueSelector.findIssues(build, ids, JiraSite.DEFAULT_ISSUE_PATTERN, null);
         assertEquals(0, ids.size());
     }
+
+    /**
+     * Tests that non-JiraIssueParameterValue parameters are ignored.
+     * when ParameterValue is NOT instanceof JiraIssueParameterValue
+     */
+    @Test
+    void testNonJiraIssueParameterValueIgnored() {
+        FreeStyleBuild build = mock(FreeStyleBuild.class);
+        ChangeLogSet changeLogSet = mock(ChangeLogSet.class);
+        BuildListener listener = mock(BuildListener.class);
+        JiraSite site = mock(JiraSite.class);
+
+        when(site.getIssuePattern()).thenReturn(JiraSite.DEFAULT_ISSUE_PATTERN);
+        when(changeLogSet.iterator()).thenReturn(Collections.EMPTY_LIST.iterator());
+        when(build.getChangeSet()).thenReturn(changeLogSet);
+
+        // Create ParametersAction with non-JiraIssueParameterValue
+        ParametersAction action = mock(ParametersAction.class);
+        List<ParameterValue> parameters = new ArrayList<>();
+        ParameterValue nonJiraParam = mock(ParameterValue.class);
+        parameters.add(nonJiraParam);
+
+        when(build.getAction(ParametersAction.class)).thenReturn(action);
+        when(action.getParameters()).thenReturn(parameters);
+
+        Set<String> ids = new DefaultIssueSelector().findIssueIds(build, site, listener);
+        assertTrue(ids.isEmpty());
+    }
+
+    @Test
+    void testDuplicateIssueIdsNotAddedTwice() {
+        FreeStyleBuild build = mock(FreeStyleBuild.class);
+        ChangeLogSet changeLogSet = mock(ChangeLogSet.class);
+        BuildListener listener = mock(BuildListener.class);
+        JiraSite site = mock(JiraSite.class);
+
+        when(site.getIssuePattern()).thenReturn(JiraSite.DEFAULT_ISSUE_PATTERN);
+        when(changeLogSet.iterator()).thenReturn(Collections.EMPTY_LIST.iterator());
+        when(build.getChangeSet()).thenReturn(changeLogSet);
+
+        // Create ParametersAction with duplicate JiraIssueParameterValue
+        ParametersAction action = mock(ParametersAction.class);
+        List<ParameterValue> parameters = new ArrayList<>();
+        JiraIssueParameterValue parameter1 = mock(JiraIssueParameterValue.class);
+        JiraIssueParameterValue parameter2 = mock(JiraIssueParameterValue.class);
+
+        when(parameter1.getValue()).thenReturn("JIRA-123");
+        when(parameter2.getValue()).thenReturn("JIRA-123"); // Same issue ID
+
+        parameters.add(parameter1);
+        parameters.add(parameter2);
+
+        when(build.getAction(ParametersAction.class)).thenReturn(action);
+        when(action.getParameters()).thenReturn(parameters);
+
+        Set<String> ids = new DefaultIssueSelector().findIssueIds(build, site, listener);
+        assertEquals(1, ids.size());
+        assertEquals("JIRA-123", ids.iterator().next());
+    }
+
+    @Test
+    void testPatternWithoutCapturingGroupTriggersWarning() {
+        FreeStyleBuild build = mock(FreeStyleBuild.class);
+        ChangeLogSet changeLogSet = mock(ChangeLogSet.class);
+        BuildListener listener = mock(BuildListener.class);
+        java.io.PrintStream printStream = mock(java.io.PrintStream.class);
+
+        when(build.getChangeSet()).thenReturn(changeLogSet);
+        when(listener.getLogger()).thenReturn(printStream);
+
+        // Create entry with text that matches pattern but pattern has no capturing group
+        Set<? extends Entry> entries = new HashSet(Arrays.asList(new MockEntry("Fixed ABC-123")));
+        when(changeLogSet.iterator()).thenReturn(entries.iterator());
+
+        List<ChangeLogSet<? extends ChangeLogSet.Entry>> changeSets = new ArrayList<>();
+        changeSets.add(changeLogSet);
+        when(build.getChangeSets()).thenReturn(changeSets);
+
+        Set<String> ids = new LinkedHashSet<>();
+        // Pattern without capturing group - just matches but doesn't capture
+        Pattern patternWithoutGroup = Pattern.compile("ABC-\\d+");
+        DefaultIssueSelector.findIssues(build, ids, patternWithoutGroup, listener);
+
+        assertEquals(0, ids.size());
+        verify(printStream)
+                .println("Warning: The Jira pattern " + patternWithoutGroup + " doesn't define a capturing group!");
+    }
 }