SONAR-4716 Fix issue when setting technical debt on existing issues

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 d8c9bd6936a1018c97927ecdf1cf10930ea65132..0d668bc8d546c9b1d2c0e6e06592f612c3f7866a 100644 (file)
--- 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
@@ -66,13 +66,13 @@ public class IssueTrackingDecorator implements Decorator {
   private final RuleFinder ruleFinder;
 
   public IssueTrackingDecorator(IssueCache issueCache, InitialOpenIssuesStack initialOpenIssues, IssueTracking tracking,
-      LastSnapshots lastSnapshots, SonarIndex index,
-      IssueHandlers handlers, IssueWorkflow workflow,
-      IssueUpdater updater,
-      Project project,
-      ResourcePerspectives perspectives,
-      RulesProfile rulesProfile,
-      RuleFinder ruleFinder) {
+                                LastSnapshots lastSnapshots, SonarIndex index,
+                                IssueHandlers handlers, IssueWorkflow workflow,
+                                IssueUpdater updater,
+                                Project project,
+                                ResourcePerspectives perspectives,
+                                RulesProfile rulesProfile,
+                                RuleFinder ruleFinder) {
     this.issueCache = issueCache;
     this.initialOpenIssues = initialOpenIssues;
     this.tracking = tracking;
@@ -131,7 +131,8 @@ public class IssueTrackingDecorator implements Decorator {
     }
   }
 
-  private void mergeMatched(IssueTrackingResult result) {
+  @VisibleForTesting
+  protected void mergeMatched(IssueTrackingResult result) {
     for (DefaultIssue issue : result.matched()) {
       IssueDto ref = result.matching(issue);
 
@@ -168,9 +169,8 @@ public class IssueTrackingDecorator implements Decorator {
       updater.setPastMessage(issue, ref.getMessage(), changeContext);
       updater.setPastEffortToFix(issue, ref.getEffortToFix(), changeContext);
       Long technicalDebt = ref.getTechnicalDebt();
-      if (technicalDebt != null) {
-        updater.setPastTechnicalDebt(issue, TechnicalDebt.fromLong(technicalDebt), changeContext);
-      }
+      TechnicalDebt previousTechnicalDebt = technicalDebt != null ? TechnicalDebt.fromLong(technicalDebt) : null;
+      updater.setPastTechnicalDebt(issue, previousTechnicalDebt, changeContext);
     }
   }
 

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 5b1d0b0bbc4c0388beecb7b7e26103745b676d60..c01199f3c616571a4a0fcad8b054060a3bde2158 100644 (file)
--- 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
@@ -36,6 +36,7 @@ import org.sonar.api.resources.Resource;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RuleFinder;
+import org.sonar.api.technicaldebt.TechnicalDebt;
 import org.sonar.batch.issue.IssueCache;
 import org.sonar.batch.scan.LastSnapshots;
 import org.sonar.core.issue.IssueUpdater;
@@ -49,6 +50,7 @@ import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 
+import static com.google.common.collect.Lists.newArrayList;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyCollection;
@@ -56,6 +58,7 @@ import static org.mockito.Matchers.argThat;
 import static org.mockito.Matchers.eq;
 import static org.mockito.Matchers.isA;
 import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.anyString;
 
 public class IssueTrackingDecoratorTest extends AbstractDaoTestCase {
 
@@ -500,4 +503,38 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase {
       }
     }));
   }
+
+  @Test
+  public void merge_matched_issue() throws Exception {
+    IssueDto previousIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle")
+      .setLine(10).setSeverity("MAJOR").setMessage("Message").setEffortToFix(1.5).setTechnicalDebt(1L);
+    DefaultIssue issue = new DefaultIssue();
+
+    IssueTrackingResult trackingResult = mock(IssueTrackingResult.class);
+    when(trackingResult.matched()).thenReturn(newArrayList(issue));
+    when(trackingResult.matching(eq(issue))).thenReturn(previousIssue);
+    decorator.mergeMatched(trackingResult);
+
+    verify(updater).setPastSeverity(eq(issue), eq("MAJOR"), any(IssueChangeContext.class));
+    verify(updater).setPastLine(eq(issue), eq(10));
+    verify(updater).setPastMessage(eq(issue), eq("Message"), any(IssueChangeContext.class));
+    verify(updater).setPastEffortToFix(eq(issue), eq(1.5), any(IssueChangeContext.class));
+    verify(updater).setPastTechnicalDebt(eq(issue), eq(TechnicalDebt.of(1, 0, 0)), any(IssueChangeContext.class));
+  }
+
+  @Test
+  public void merge_matched_issue_on_manual_severity() throws Exception {
+    IssueDto previousIssue = new IssueDto().setKee("ABCDE").setResolution(null).setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle")
+      .setLine(10).setManualSeverity(true).setSeverity("MAJOR").setMessage("Message").setEffortToFix(1.5).setTechnicalDebt(1L);
+    DefaultIssue issue = new DefaultIssue();
+
+    IssueTrackingResult trackingResult = mock(IssueTrackingResult.class);
+    when(trackingResult.matched()).thenReturn(newArrayList(issue));
+    when(trackingResult.matching(eq(issue))).thenReturn(previousIssue);
+    decorator.mergeMatched(trackingResult);
+
+    assertThat(issue.manualSeverity()).isTrue();
+    assertThat(issue.severity()).isEqualTo("MAJOR");
+    verify(updater, never()).setPastSeverity(eq(issue), anyString(), any(IssueChangeContext.class));
+  }
 }