From ee593d4b874d558144e37655cb1e33cd71da1059 Mon Sep 17 00:00:00 2001 From: Matteo Mara Date: Wed, 30 Aug 2023 12:37:47 +0200 Subject: [PATCH] SONAR-19372 apply anticipated transitions only to eligible issues --- ...itionIssuesToAnticipatedStatesVisitor.java | 47 ++++++++++-- ...nIssuesToAnticipatedStatesVisitorTest.java | 73 ++++++++++++++++++- 2 files changed, 110 insertions(+), 10 deletions(-) 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 8cfb9e4e46f..0c18fba6afc 100644 --- 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 anticipatedTransitions; private final AnticipatedTransitionTracker 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 tracking = tracker.track(List.of(issue), anticipatedTransitions); Map 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 0517c680c4a..f7eace23e23 100644 --- 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); -- 2.39.5