From 8ca02569a601d30b0e0a3e395ea3a106bcc2383d Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Thu, 19 Sep 2013 13:15:49 +0200 Subject: [PATCH] SONAR-3387 Set manual issues as CLOSED/REMOVED when not relocatable --- .../core/issue/IssueTrackingDecorator.java | 14 ++++- .../issue/IssueTrackingDecoratorTest.java | 56 +++++++++++++++++-- 2 files changed, 64 insertions(+), 6 deletions(-) diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java index 75e4531d249..42d1013dc1c 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java @@ -177,7 +177,7 @@ public class IssueTrackingDecorator implements Decorator { private void addUnmatched(Collection unmatchedIssues, SourceHashHolder sourceHashHolder, Collection issues) { for (IssueDto unmatchedDto : unmatchedIssues) { DefaultIssue unmatched = unmatchedDto.toDefaultIssue(); - if (StringUtils.isNotBlank(unmatchedDto.getReporter())) { + if (StringUtils.isNotBlank(unmatchedDto.getReporter()) && !Issue.STATUS_CLOSED.equals(unmatchedDto.getStatus())) { relocateManualIssue(unmatched, unmatchedDto, sourceHashHolder); } updateUnmatchedIssue(unmatched, false /* manual issues can be kept open */); @@ -217,7 +217,17 @@ public class IssueTrackingDecorator implements Decorator { Collection newLinesWithSameHash = sourceHashHolder.getNewLinesMatching(oldIssue.getLine()); logger.debug("Found the following lines with same hash: {}", newLinesWithSameHash); - if (newLinesWithSameHash.size() == 1) { + if (newLinesWithSameHash.size() == 0) { + if (oldIssue.getLine() > sourceHashHolder.getHashedSource().length()) { + logger.debug("Old issue line {} is out of new source, closing and removing line number", oldIssue.getLine()); + newIssue.setLine(null); + updater.setStatus(newIssue, Issue.STATUS_CLOSED, changeContext); + updater.setResolution(newIssue, Issue.RESOLUTION_REMOVED, changeContext); + updater.setPastLine(newIssue, oldIssue.getLine()); + updater.setPastMessage(newIssue, oldIssue.getMessage(), changeContext); + updater.setPastEffortToFix(newIssue, oldIssue.getEffortToFix(), changeContext); + } + } else if (newLinesWithSameHash.size() == 1) { Integer newLine = newLinesWithSameHash.iterator().next(); logger.debug("Relocating issue to line {}", newLine); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java index ecf13a44d90..8e75b331c92 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java @@ -19,15 +19,16 @@ */ package org.sonar.plugins.core.issue; -import org.sonar.api.batch.SonarIndex; +import org.mockito.Matchers; -import org.sonar.batch.scan.LastSnapshots; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.ArgumentMatcher; import org.sonar.api.batch.DecoratorContext; +import org.sonar.api.batch.SonarIndex; import org.sonar.api.component.ResourcePerspectives; +import org.sonar.api.issue.Issue; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; import org.sonar.api.profiles.RulesProfile; @@ -38,6 +39,7 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; import org.sonar.batch.issue.IssueCache; +import org.sonar.batch.scan.LastSnapshots; import org.sonar.core.issue.IssueUpdater; import org.sonar.core.issue.db.IssueDto; import org.sonar.core.issue.workflow.IssueWorkflow; @@ -50,7 +52,17 @@ import java.util.Collections; import java.util.List; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.Mockito.*; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyCollection; +import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.eq; +import static org.mockito.Matchers.isA; +import static org.mockito.Mockito.RETURNS_MOCKS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { @@ -151,7 +163,6 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { @Test public void manual_issues_should_be_moved_if_matching_line_found() throws Exception { - // "Unmatched" issues existed in previous scan but not in current one -> they have to be closed Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); // INPUT : one issue existing during previous scan @@ -196,6 +207,40 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { assertThat(issue.isOnDisabledRule()).isFalse(); } + @Test + public void manual_issues_should_be_untouched_if_already_closed() throws Exception { + Resource file = new File("Action.java").setEffectiveKey("struts:Action.java").setId(123); + + // INPUT : one issue existing during previous scan + IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setReporter("freddy").setLine(1).setStatus("CLOSED").setRuleKey_unit_test_only("manual", "Performance"); + when(ruleFinder.findByKey(RuleKey.of("manual", "Performance"))).thenReturn(new Rule("manual", "Performance")); + + IssueTrackingResult trackingResult = new IssueTrackingResult(); + trackingResult.addUnmatched(unmatchedIssue); + + String originalSource = "public interface Action {}"; + when(index.getSource(file)).thenReturn(originalSource); + when(lastSnapshots.getSource(file)).thenReturn(originalSource); + + when(tracking.track(isA(SourceHashHolder.class), anyCollection(), anyCollection())).thenReturn(trackingResult); + + decorator.doDecorate(file); + + verify(workflow, times(1)).doAutomaticTransition(any(DefaultIssue.class), any(IssueChangeContext.class)); + verify(handlers, times(1)).execute(any(DefaultIssue.class), any(IssueChangeContext.class)); + + ArgumentCaptor argument = ArgumentCaptor.forClass(DefaultIssue.class); + verify(issueCache).put(argument.capture()); + + DefaultIssue issue = argument.getValue(); + assertThat(issue.line()).isEqualTo(1); + assertThat(issue.key()).isEqualTo("ABCDE"); + assertThat(issue.isNew()).isFalse(); + assertThat(issue.isEndOfLife()).isFalse(); + assertThat(issue.isOnDisabledRule()).isFalse(); + assertThat(issue.status()).isEqualTo("CLOSED"); + } + @Test public void manual_issues_should_be_kept_if_matching_line_not_found() throws Exception { // "Unmatched" issues existed in previous scan but not in current one -> they have to be closed @@ -396,6 +441,9 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { verify(issueCache).put(argument.capture()); DefaultIssue issue = argument.getValue(); + verify(updater).setResolution(eq(issue), eq(Issue.RESOLUTION_REMOVED), any(IssueChangeContext.class)); + verify(updater).setStatus(eq(issue), eq(Issue.STATUS_CLOSED), any(IssueChangeContext.class)); + assertThat(issue.key()).isEqualTo("ABCDE"); assertThat(issue.isNew()).isFalse(); assertThat(issue.isEndOfLife()).isTrue(); -- 2.39.5