JENKINS-17156: remove issue from carry over list after submitting the…

… comment
jenkinsci · Apr 2, 2015 · 28f559b · 28f559b
1 parent ba862ed
commit 28f559b
Show file tree

Hide file tree

Showing 4 changed files with 102 additions and 11 deletions.
diff --git a/src/main/java/hudson/plugins/jira/Updater.java b/src/main/java/hudson/plugins/jira/Updater.java
@@ -127,22 +127,38 @@ static void submitComments(
             boolean useWikiStyleComments, boolean recordScmChanges, String groupVisibility, String roleVisibility) throws RemoteException {
         // copy to prevent ConcurrentModificationException
         List<JiraIssue> copy = new ArrayList<JiraIssue>(issues);
+        StringBuilder issuesWithExceptions = new StringBuilder();
         for (JiraIssue issue : copy) {
-            try {
                 logger.println(Messages.Updater_Updating(issue.id));
-                session.addComment(
+
+                try {
+                    session.addComment(
                         issue.id,
                         createComment(build, useWikiStyleComments, jenkinsRootUrl, recordScmChanges, issue),
                         groupVisibility, roleVisibility);
-            } catch (RemotePermissionException 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);
+                } catch (RemotePermissionException 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);
+                } catch (RemoteException e) {
+                    if( issuesWithExceptions.length() > 0) {
+                        issuesWithExceptions.append(", ");
+                    }
+                    issuesWithExceptions.append(issue.id);
+                    logger.println(e.getLocalizedMessage());
+                    logger.println(Messages.Updater_FailedToCommentOnIssue(issue.id));
+                    continue;
+                }
+
                 issues.remove(issue);
-            }
+        }
+
+        //    throw RemoteException if problems occured
+        if( issuesWithExceptions.length() > 0) {
+            throw new RemoteException(Messages.Updater_ErrorCommentingIssues(issuesWithExceptions.toString()));
         }
     }
 

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
@@ -7,6 +7,7 @@
 import hudson.plugins.jira.soap.RemoteComment;
 import hudson.plugins.jira.soap.RemoteGroup;
 import hudson.plugins.jira.soap.RemoteIssue;
+import hudson.plugins.jira.soap.RemotePermissionException;
 import hudson.scm.ChangeLogSet;
 import hudson.scm.ChangeLogSet.Entry;
 import org.junit.Assert;
@@ -18,10 +19,12 @@
 
 import javax.xml.rpc.ServiceException;
 import java.io.IOException;
+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}.
@@ -352,5 +355,73 @@ 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 RemoteException {
+
+        final JiraIssue brokenIssue = new JiraIssue("FOOBAR-2", "Title");
+        final JiraIssue forbiddenIssue = new JiraIssue("LASSO-17", "Title");
+        final JiraIssue firstIssue = new JiraIssue("FOOBAR-1", "Title");
+        final JiraIssue secondIssue = new JiraIssue("ALIBA-1", "Title");
+        final JiraIssue thirdIssue = new JiraIssue("MOONA-1", "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, brokenIssue, 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 RemoteIssue());
+        when(session.getGroup(Mockito.anyString())).thenReturn(new RemoteGroup("Software Development", null));
+
+        final List<RemoteComment> comments = new ArrayList<RemoteComment>();
 
+        Answer answer = new Answer<Object>() {
+            public Object answer(InvocationOnMock invocation) throws Throwable {
+                RemoteComment rc = new RemoteComment();
+                rc.setId((String) invocation.getArguments()[0]);
+                rc.setBody((String) invocation.getArguments()[1]);
+                rc.setGroupLevel((String) invocation.getArguments()[2]);
+                comments.add(rc);
+                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 catched exception
+        doThrow(new RemotePermissionException()).when(session).addComment(eq(forbiddenIssue.id), Mockito.anyString(), Mockito.anyString(), Mockito.anyString());
+        // issue for the exception that has not been catched
+        doThrow(new RemoteException("an error occured")).when(session).addComment(eq(brokenIssue.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 = "";
+
+        try {
+            Updater.submitComments(build,
+                System.out, "http://jenkins", issues, session, false, false, groupVisibility, roleVisibility);
+
+            Assert.fail("Did not throw expected exception");
+        } catch (RemoteException e)
+        {
+            // expected issue list
+            final List<JiraIssue> expectedIssuesToCarryOver = Lists.newArrayList(brokenIssue);
+
+            Assert.assertThat(issues, is(expectedIssuesToCarryOver));
+        }
+    }
 }