JENKINS-17156 - Remove issue from carry over list after successful co…

…mment submission Merge remote-tracking branch 'origin/pr/50' Conflicts: src/main/java/hudson/plugins/jira/Updater.java src/test/java/hudson/plugins/jira/UpdaterTest.java
jenkinsci · Sep 8, 2015 · 0322951 · 0322951
2 parents 286a532 + 28f559b
commit 0322951
Show file tree

Hide file tree

Showing 4 changed files with 105 additions and 15 deletions.
diff --git a/src/main/java/hudson/plugins/jira/Updater.java b/src/main/java/hudson/plugins/jira/Updater.java
@@ -1,5 +1,7 @@
 package hudson.plugins.jira;
 
+import com.atlassian.jira.rest.client.api.RestClientException;
+import com.google.common.base.Joiner;
 import hudson.Util;
 import hudson.model.AbstractBuild;
 import hudson.model.AbstractBuild.DependencyChange;
@@ -17,6 +19,7 @@
 import java.io.PrintStream;
 import java.lang.reflect.Method;
 import java.net.URL;
+import java.rmi.Remote;
 import java.rmi.RemoteException;
 import java.util.ArrayList;
 import java.util.HashSet;
@@ -107,41 +110,59 @@ static boolean perform(AbstractBuild<?, ?> build, BuildListener listener) {
 
     /**
      * Submits comments for the given issues.
-     * Removes from <code>issues</code> the ones which appear to be invalid.
+     * Remvoes from <code>issues</code> issues which have been successfully updated or are invalid
      *
      * @param build
      * @param logger
      * @param jenkinsRootUrl
-     * @param issues
      * @param session
      * @param useWikiStyleComments
      * @param recordScmChanges
      * @param groupVisibility
-     * @throws RemoteException
+     * @throws RestClientException
      */
     static void submitComments(
             AbstractBuild<?, ?> build, PrintStream logger, String jenkinsRootUrl,
             List<JiraIssue> issues, JiraSession session,
-            boolean useWikiStyleComments, boolean recordScmChanges, String groupVisibility, String roleVisibility) throws RemoteException {
+            boolean useWikiStyleComments, boolean recordScmChanges, String groupVisibility, String roleVisibility) throws RestClientException {
+
         // copy to prevent ConcurrentModificationException
         List<JiraIssue> copy = new ArrayList<JiraIssue>(issues);
+
         for (JiraIssue issue : copy) {
+            logger.println(Messages.Updater_Updating(issue.id));
+
             try {
-                logger.println(Messages.Updater_Updating(issue.id));
                 session.addComment(
                         issue.id,
                         createComment(build, useWikiStyleComments, jenkinsRootUrl, recordScmChanges, issue),
-                        groupVisibility, roleVisibility);
-            } catch (Exception e) {
-                // Seems like RemotePermissionException can mean 'no permission' as well as
-                // 'issue doesn't exist'.
-                // To prevent carrying forward invalid issues forever, we have to drop them
-                // even if the cause of the exception was different.
-                logger.println("Looks like " + issue.id + " is no valid JIRA issue or you don't have permission to update the issue.\n" +
-                        "Issue will not be updated.\n" + e);
-                issues.remove(issue);
+                        groupVisibility, roleVisibility
+                );
+
+            } catch (RestClientException e) {
+
+                if (e.getStatusCode().or(0).equals(404)) {
+                    logger.println(issue.id + " - Issue not found. Dropping comment from update queue.");
+                }
+
+                if (e.getStatusCode().or(0).equals(403)) {
+                    logger.println(issue.id + " - Jenkins JIRA user does not have permissions to comment on this issue. Preserving comment for future update.");
+                    continue;
+                }
+
+                if (e.getStatusCode().or(0).equals(401)) {
+                    logger.println(issue.id + " - Jenkins JIRA authentication problem. Preserving comment for future update.");
+                    continue;
+                }
+
+                logger.println(Messages.Updater_FailedToCommentOnIssue(issue.id));
+                logger.println(e.getLocalizedMessage());
             }
+
+            // if no exception is thrown during update, remove from the list as succesfully updated
+            issues.remove(issue);
         }
+
     }
 
     private static List<JiraIssue> getJiraIssues(

diff --git a/src/main/resources/hudson/plugins/jira/Messages.properties b/src/main/resources/hudson/plugins/jira/Messages.properties
@@ -9,6 +9,8 @@ Updater.NoJenkinsUrl=Jenkins URL is not configured yet. Go to system configurati
 Updater.NoJiraSite=No jira site is configured for this project. This must be a project configuration error
 Updater.NoRemoteAccess=The system configuration does not allow remote JIRA access
 Updater.Updating=Updating {0}
+Updater.FailedToCommentOnIssue=Commenting failed on {0}. Carrying over to next build.
+Updater.ErrorCommentingIssues=Could not comment on some issues: {0}
 JiraReleaseVersionBuilder.DisplayName=Mark a JIRA Version as Released
 JiraReleaseVersionMigrator.DisplayName=Move issues matching JQL to the specified version
 JiraIssueUpdateBuilder.DisplayName=Progress JIRA issues by workflow action

diff --git a/src/main/resources/hudson/plugins/jira/Messages_de.properties b/src/main/resources/hudson/plugins/jira/Messages_de.properties
@@ -8,3 +8,5 @@ Updater.NoJenkinsUrl=Jenkins URL ist noch nicht konfiguriert. Bitte geben Sie si
 Updater.NoJiraSite=Keine JIRA Instanz konfiguriert für dieses Projekt.
 Updater.NoRemoteAccess=Die JIRA Konfiguration erlaubt keinen Remote-Zugriff auf JIRA.
 Updater.Updating=Aktualisiere {0}
+Updater.FailedToCommentOnIssue=Kommentierung an {0} fehlgeschlagen. Der Versuch wird beim nächsten Build wiederholt.
+Updater.ErrorCommentingIssues=Konnte einige Vorgänge nicht kommentieren: {0}
diff --git a/src/test/java/hudson/plugins/jira/UpdaterTest.java b/src/test/java/hudson/plugins/jira/UpdaterTest.java
@@ -1,5 +1,7 @@
 package hudson.plugins.jira;
 
+import com.atlassian.jira.rest.client.api.RestClientException;
+import com.atlassian.jira.rest.client.api.domain.Comment;
 import com.atlassian.jira.rest.client.api.domain.Issue;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Sets;
@@ -14,10 +16,12 @@
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
 
+import java.rmi.RemoteException;
 import java.util.*;
 import java.util.regex.Pattern;
-
 import static org.mockito.Mockito.*;
+import static org.hamcrest.CoreMatchers.*;
+
 
 /**
  * Test case for the JIRA {@link Updater}.
@@ -343,5 +347,66 @@ public void testUserPatternMatchTwoIssuesInOneComment() {
         Assert.assertTrue(ids.contains("FOOBAR-4711"));
         Assert.assertTrue(ids.contains("FOOBAR-21"));
     }
+
+    /**
+     * Checks if issues are correctly removed from the carry over list.
+     * @throws RemoteException
+     */
+    @Test
+    @Bug(17156)
+    public void testIssueIsRemovedFromCarryOverListAfterSubmission() throws RestClientException {
+
+        final JiraIssue firstIssue = new JiraIssue("FOOBAR-1", "Title");
+        final JiraIssue secondIssue = new JiraIssue("ALIBA-1", "Title");
+        final JiraIssue thirdIssue = new JiraIssue("MOONA-1", "Title");
+        final JiraIssue deletedIssue = new JiraIssue("FOOBAR-2", "Title");
+        final JiraIssue forbiddenIssue = new JiraIssue("LASSO-17", "Title");
+
+        // assume that there is a following list of jira issues from scm commit messages out of hudson.plugins.jira.JiraCarryOverAction
+        List<JiraIssue> issues = Lists.newArrayList(firstIssue, secondIssue, forbiddenIssue, thirdIssue);
+
+        // mock JIRA session:
+        JiraSession session = mock(JiraSession.class);
+        when(session.existsIssue(Mockito.anyString())).thenReturn(Boolean.TRUE);
+//        when(session.getIssue(Mockito.anyString())).thenReturn( new Issue());
+//        when(session.getGroup(Mockito.anyString())).thenReturn(new Group("Software Development", null));
+
+        final List<Comment> comments = new ArrayList<Comment>();
+
+        Answer answer = new Answer<Object>() {
+            public Object answer(InvocationOnMock invocation) throws Throwable {
+                Comment c = Comment.createWithGroupLevel((String) invocation.getArguments()[0], (String) invocation.getArguments()[1]);
+                comments.add(c);
+                return null;
+            }
+        };
+
+        doAnswer(answer).when(session).addComment(eq(firstIssue.id), Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
+        doAnswer(answer).when(session).addComment(eq(secondIssue.id), Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
+        doAnswer(answer).when(session).addComment(eq(thirdIssue.id), Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
 
+        // issue for the caught exception
+        doThrow(new RestClientException(new Throwable(), 404)).when(session).addComment(eq(deletedIssue.id), Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
+        doThrow(new RestClientException(new Throwable(), 403)).when(session).addComment(eq(forbiddenIssue.id), Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
+
+        // mock build:
+        FreeStyleBuild build = mock(FreeStyleBuild.class);
+        FreeStyleProject project = mock(FreeStyleProject.class);
+        when(build.getProject()).thenReturn(project);
+        ChangeLogSet changeLogSet = ChangeLogSet.createEmpty(build);
+        when(build.getChangeSet()).thenReturn(changeLogSet);
+        when(build.getResult()).thenReturn(Result.SUCCESS);
+
+        final String groupVisibility = "";
+        final String roleVisibility = "";
+
+        Updater.submitComments(
+                build, System.out, "http://jenkins", issues, session, false, false, groupVisibility, roleVisibility
+        );
+
+        // expected issue list
+        final List<JiraIssue> expectedIssuesToCarryOver = new ArrayList<JiraIssue>();
+        expectedIssuesToCarryOver.add(forbiddenIssue);
+        Assert.assertThat(issues, is(expectedIssuesToCarryOver));
+    }
 }