SONAR-19372 apply anticipated transitions only to eligible issues

diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TransitionIssuesToAnticipatedStatesVisitor.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TransitionIssuesToAnticipatedStatesVisitor.java
index 8cfb9e4e46f68b8029aa99a06a88ee28d25da408..0c18fba6afc2ea94684ddd77f6aa7504e7b2eba4 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TransitionIssuesToAnticipatedStatesVisitor.java
+++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TransitionIssuesToAnticipatedStatesVisitor.java
@@ -19,16 +19,22 @@
  */
 package org.sonar.ce.task.projectanalysis.issue;
 
+import java.time.Instant;
 import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 import org.apache.logging.log4j.util.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+import org.sonar.ce.task.log.CeTaskMessages;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.core.issue.AnticipatedTransition;
 import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.tracking.AnticipatedTransitionTracker;
 import org.sonar.core.issue.tracking.Tracking;
+import org.sonar.db.dismissmessage.MessageType;
 
+import static org.sonar.api.issue.Issue.STATUS_OPEN;
 import static org.sonar.ce.task.projectanalysis.component.Component.Type.FILE;
 
 /**
@@ -36,15 +42,21 @@ import static org.sonar.ce.task.projectanalysis.component.Component.Type.FILE;
  */
 public class TransitionIssuesToAnticipatedStatesVisitor extends IssueVisitor {
 
+  private static final Logger LOGGER = LoggerFactory.getLogger(TransitionIssuesToAnticipatedStatesVisitor.class);
+  public static final String TRANSITION_ERROR_TEMPLATE = "Cannot resolve issue at line {} of {} due to: {}";
+
   private Collection<AnticipatedTransition> anticipatedTransitions;
   private final AnticipatedTransitionTracker<DefaultIssue, AnticipatedTransition> tracker = new AnticipatedTransitionTracker<>();
   private final IssueLifecycle issueLifecycle;
 
+  private final CeTaskMessages ceTaskMessages;
+
   private final AnticipatedTransitionRepository anticipatedTransitionRepository;
 
-  public TransitionIssuesToAnticipatedStatesVisitor(AnticipatedTransitionRepository anticipatedTransitionRepository, IssueLifecycle issueLifecycle) {
+  public TransitionIssuesToAnticipatedStatesVisitor(AnticipatedTransitionRepository anticipatedTransitionRepository, IssueLifecycle issueLifecycle, CeTaskMessages ceTaskMessages) {
     this.anticipatedTransitionRepository = anticipatedTransitionRepository;
     this.issueLifecycle = issueLifecycle;
+    this.ceTaskMessages = ceTaskMessages;
   }
 
   @Override
@@ -56,7 +68,7 @@ public class TransitionIssuesToAnticipatedStatesVisitor extends IssueVisitor {
 
   @Override
   public void onIssue(Component component, DefaultIssue issue) {
-    if (issue.isNew()) {
+    if (isEligibleForAnticipatedTransitions(issue)) {
       Tracking<DefaultIssue, AnticipatedTransition> tracking = tracker.track(List.of(issue), anticipatedTransitions);
       Map<DefaultIssue, AnticipatedTransition> matchedRaws = tracking.getMatchedRaws();
       if (matchedRaws.containsKey(issue)) {
@@ -65,13 +77,32 @@ public class TransitionIssuesToAnticipatedStatesVisitor extends IssueVisitor {
     }
   }
 
+  private static boolean isEligibleForAnticipatedTransitions(DefaultIssue issue) {
+    return issue.isNew() && STATUS_OPEN.equals(issue.getStatus()) && null == issue.resolution();
+  }
+
   private void performAnticipatedTransition(DefaultIssue issue, AnticipatedTransition anticipatedTransition) {
-    issue.setBeingClosed(true);
-    issue.setAnticipatedTransitionUuid(anticipatedTransition.getUuid());
-    issueLifecycle.doManualTransition(issue, anticipatedTransition.getTransition(), anticipatedTransition.getUserUuid());
-    String transitionComment = anticipatedTransition.getComment();
-    String comment = Strings.isNotBlank(transitionComment) ? transitionComment : "Automatically transitioned from SonarLint";
-    issueLifecycle.addComment(issue, comment, anticipatedTransition.getUserUuid());
+    try {
+      issueLifecycle.doManualTransition(issue, anticipatedTransition.getTransition(), anticipatedTransition.getUserUuid());
+      String transitionComment = anticipatedTransition.getComment();
+      String comment = Strings.isNotBlank(transitionComment) ? transitionComment : "Automatically transitioned from SonarLint";
+      issueLifecycle.addComment(issue, comment, anticipatedTransition.getUserUuid());
+      issue.setBeingClosed(true);
+      issue.setAnticipatedTransitionUuid(anticipatedTransition.getUuid());
+    } catch (Exception e) {
+      LOGGER.warn(TRANSITION_ERROR_TEMPLATE, issue.getLine(), issue.componentKey(), e.getMessage());
+      ceTaskMessages.add(
+        new CeTaskMessages.Message(getMessage(issue, e),
+          Instant.now().toEpochMilli(),
+          MessageType.GENERIC));
+    }
+  }
+
+  private static String getMessage(DefaultIssue issue, Exception e) {
+    final int MAX_LENGTH = 50;
+    int componentKeyLength = issue.componentKey().length();
+    String componentKey = componentKeyLength > MAX_LENGTH ? ("..." + issue.componentKey().substring(componentKeyLength - MAX_LENGTH, componentKeyLength)) : issue.componentKey();
+    return String.format(TRANSITION_ERROR_TEMPLATE.replace("{}", "%s"), issue.getLine(), componentKey, e.getMessage());
   }
 
 }

diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TransitionIssuesToAnticipatedStatesVisitorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TransitionIssuesToAnticipatedStatesVisitorTest.java
index 0517c680c4a9f3a45db36e2e9e9d84a133a5a647..f7eace23e230a130a7b93c2d560d58d4496af0b8 100644 (file)
--- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TransitionIssuesToAnticipatedStatesVisitorTest.java
+++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TransitionIssuesToAnticipatedStatesVisitorTest.java
@@ -23,8 +23,12 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import org.junit.Rule;
 import org.junit.Test;
+import org.slf4j.event.Level;
 import org.sonar.api.rule.RuleKey;
+import org.sonar.api.testfixtures.log.LogTester;
+import org.sonar.ce.task.log.CeTaskMessages;
 import org.sonar.ce.task.projectanalysis.component.Component;
 import org.sonar.ce.task.projectanalysis.component.ComponentImpl;
 import org.sonar.ce.task.projectanalysis.component.ProjectAttributes;
@@ -33,19 +37,28 @@ import org.sonar.core.issue.AnticipatedTransition;
 import org.sonar.core.issue.DefaultIssue;
 
 import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.doThrow;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.times;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 import static org.mockito.Mockito.when;
+import static org.sonar.api.issue.Issue.STATUS_OPEN;
+import static org.sonar.api.issue.Issue.STATUS_RESOLVED;
 import static org.sonar.ce.task.projectanalysis.component.Component.Type.PROJECT;
 
 public class TransitionIssuesToAnticipatedStatesVisitorTest {
-
+  @Rule
+  public LogTester logTester = new LogTester();
   private final IssueLifecycle issueLifecycle = mock(IssueLifecycle.class);
 
   private final AnticipatedTransitionRepository anticipatedTransitionRepository = mock(AnticipatedTransitionRepository.class);
 
-  private final TransitionIssuesToAnticipatedStatesVisitor underTest = new TransitionIssuesToAnticipatedStatesVisitor(anticipatedTransitionRepository, issueLifecycle);
+  private final CeTaskMessages ceTaskMessages = mock(CeTaskMessages.class);
+
+  private final TransitionIssuesToAnticipatedStatesVisitor underTest = new TransitionIssuesToAnticipatedStatesVisitor(anticipatedTransitionRepository, issueLifecycle, ceTaskMessages);
 
   @Test
   public void givenMatchingAnticipatedTransitions_transitionsShouldBeAppliedToIssues() {
@@ -63,6 +76,60 @@ public class TransitionIssuesToAnticipatedStatesVisitorTest {
     verify(issueLifecycle).addComment(issue, "doing the transition in an anticipated way", "admin");
   }
 
+  @Test
+  public void givenMatchingAnticipatedTransitions_whenExceptionIsThrown_transitionsShouldNotBeAppliedAndWarningLogged() {
+    Component component = getComponent(Component.Type.FILE);
+    String exceptionMessage = "Cannot apply transition";
+
+    when(anticipatedTransitionRepository.getAnticipatedTransitionByComponent(component)).thenReturn(getAnticipatedTransitions("projectKey", "fileName"));
+    doThrow(new IllegalStateException(exceptionMessage)).when(issueLifecycle).doManualTransition(any(), any(), any());
+    DefaultIssue issue = getDefaultIssue(1, "abcdefghi", "issue message");
+    issue.setComponentKey(component.getKey());
+
+    underTest.beforeComponent(component);
+    underTest.onIssue(component, issue);
+
+    assertThat(issue.isBeingClosed()).isFalse();
+    assertThat(issue.getAnticipatedTransitionUuid()).isEmpty();
+    verify(issueLifecycle).doManualTransition(issue, "wontfix", "admin");
+    verifyNoMoreInteractions(issueLifecycle);
+    assertThat(logTester.logs(Level.WARN))
+      .contains(String.format("Cannot resolve issue at line %s of %s due to: %s", issue.getLine(), issue.componentKey(), exceptionMessage));
+    verify(ceTaskMessages, times(1)).add(any());
+  }
+
+  @Test
+  public void givenMatchingAnticipatedTransitionsOnResolvedIssue_transitionsShouldNotBeAppliedToIssues() {
+    Component component = getComponent(Component.Type.FILE);
+    when(anticipatedTransitionRepository.getAnticipatedTransitionByComponent(component)).thenReturn(getAnticipatedTransitions("projectKey", "fileName"));
+
+    DefaultIssue issue = getDefaultIssue(1, "abcdefghi", "issue message");
+    issue.setStatus(STATUS_RESOLVED);
+
+    underTest.beforeComponent(component);
+    underTest.onIssue(component, issue);
+
+    assertThat(issue.isBeingClosed()).isFalse();
+    assertThat(issue.getAnticipatedTransitionUuid()).isNotPresent();
+    verifyNoInteractions(issueLifecycle);
+  }
+
+  @Test
+  public void givenMatchingAnticipatedTransitions_whenIssueIsNotNew_transitionsShouldNotBeAppliedToIssues() {
+    Component component = getComponent(Component.Type.FILE);
+    when(anticipatedTransitionRepository.getAnticipatedTransitionByComponent(component)).thenReturn(getAnticipatedTransitions("projectKey", "fileName"));
+
+    DefaultIssue issue = getDefaultIssue(1, "abcdefghi", "issue message");
+    issue.setNew(false);
+
+    underTest.beforeComponent(component);
+    underTest.onIssue(component, issue);
+
+    assertThat(issue.isBeingClosed()).isFalse();
+    assertThat(issue.getAnticipatedTransitionUuid()).isNotPresent();
+    verifyNoInteractions(issueLifecycle);
+  }
+
   @Test
   public void givenNonMatchingAnticipatedTransitions_transitionsAreNotAppliedToIssues() {
     Component component = getComponent(Component.Type.FILE);
@@ -140,6 +207,8 @@ public class TransitionIssuesToAnticipatedStatesVisitorTest {
 
   private DefaultIssue getDefaultIssue(Integer line, String hash, String message) {
     DefaultIssue defaultIssue = new DefaultIssue();
+    defaultIssue.setStatus(STATUS_OPEN);
+    defaultIssue.setResolution(null);
     defaultIssue.setLine(line);
     defaultIssue.setChecksum(hash);
     defaultIssue.setMessage(message);