SONAR-4865 Keep issue locations on existing issues

6 files changed, 83 insertions, 19 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueLifecycle.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueLifecycle.java
index 93cbffd32cd..a7f71a02c03 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueLifecycle.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/IssueLifecycle.java
@@ -88,6 +88,7 @@ public class IssueLifecycle {
 
     // fields coming from raw
     updater.setPastLine(raw, base.getLine());
+    updater.setPastLocations(raw, base.getLocations());
     updater.setPastMessage(raw, base.getMessage(), changeContext);
     updater.setPastEffortToFix(raw, base.effortToFix(), changeContext);
     updater.setPastTechnicalDebt(raw, base.debt(), changeContext);
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerRawInputFactory.java b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerRawInputFactory.java
index 2d670a756a4..fca45414d4c 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerRawInputFactory.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/issue/TrackerRawInputFactory.java
@@ -19,9 +19,6 @@
  */
 package org.sonar.server.computation.issue;
 
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.List;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rule.RuleKey;
 import org.sonar.api.utils.log.Loggers;
@@ -40,6 +37,10 @@ import org.sonar.server.computation.issue.commonrule.CommonRuleEngine;
 import org.sonar.server.computation.source.SourceLinesRepository;
 import org.sonar.server.rule.CommonRuleKeys;
 
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
 import static com.google.common.collect.Lists.newArrayList;
 
 public class TrackerRawInputFactory {
@@ -172,7 +173,7 @@ public class TrackerRawInputFactory {
     }
   }
 
-  private DbCommons.TextRange.Builder convertTextRange(BatchReport.TextRange sourceRange) {
+  private static DbCommons.TextRange.Builder convertTextRange(BatchReport.TextRange sourceRange) {
     DbCommons.TextRange.Builder targetRange = DbCommons.TextRange.newBuilder();
     if (sourceRange.hasStartLine()) {
       targetRange.setStartLine(sourceRange.getStartLine());
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueLifecycleTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueLifecycleTest.java
index 80fbd779a46..ee8ec5f039a 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueLifecycleTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/issue/IssueLifecycleTest.java
@@ -28,6 +28,8 @@ import org.sonar.core.issue.DefaultIssue;
 import org.sonar.core.issue.IssueChangeContext;
 import org.sonar.core.issue.IssueUpdater;
 import org.sonar.core.issue.workflow.IssueWorkflow;
+import org.sonar.db.protobuf.DbCommons;
+import org.sonar.db.protobuf.DbIssues;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -97,6 +99,13 @@ public class IssueLifecycleTest {
       .setCreationDate(parseDate("2015-10-01"))
       .setUpdateDate(parseDate("2015-10-02"))
       .setCloseDate(parseDate("2015-10-03"));
+
+    DbIssues.Locations issueLocations = DbIssues.Locations.newBuilder()
+      .setTextRange(DbCommons.TextRange.newBuilder()
+        .setStartLine(10)
+        .setEndLine(12)
+        .build())
+      .build();
     DefaultIssue base = new DefaultIssue()
       .setKey("BASE_KEY")
       .setCreationDate(parseDate("2015-01-01"))
@@ -115,7 +124,8 @@ public class IssueLifecycleTest {
       .setMessage("message")
       .setEffortToFix(15d)
       .setDebt(Duration.create(15L))
-      .setManualSeverity(false);
+      .setManualSeverity(false)
+      .setLocations(issueLocations);
 
     when(debtCalculator.calculate(raw)).thenReturn(DEFAULT_DURATION);
 
@@ -140,6 +150,7 @@ public class IssueLifecycleTest {
     verify(updater).setPastLine(raw, 10);
     verify(updater).setPastMessage(raw, "message", issueChangeContext);
     verify(updater).setPastTechnicalDebt(raw, Duration.create(15L), issueChangeContext);
+    verify(updater).setPastLocations(raw, issueLocations);
   }
 
   @Test
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java
index 46ac2fbad12..f9a4f4da016 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java
@@ -26,18 +26,6 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Maps;
-import java.io.Serializable;
-import java.util.ArrayList;
-import java.util.Calendar;
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Date;
-import java.util.LinkedHashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.lang.builder.ToStringBuilder;
 import org.apache.commons.lang.builder.ToStringStyle;
@@ -49,6 +37,20 @@ import org.sonar.api.rule.Severity;
 import org.sonar.api.utils.Duration;
 import org.sonar.core.issue.tracking.Trackable;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.Calendar;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Date;
+import java.util.LinkedHashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
 import static java.lang.String.format;
 
 public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure.Issue {
@@ -553,12 +555,14 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure.
     return this;
   }
 
+  @CheckForNull
   public <T> T getLocations() {
     return (T)locations;
   }
 
-  public void setLocations(Object locations) {
+  public DefaultIssue setLocations(@Nullable Object locations) {
     this.locations = locations;
+    return this;
   }
 
   @Override
diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java
index 851523cb5be..a7d0edb1803 100644
--- a/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java
+++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueUpdater.java
@@ -123,6 +123,22 @@ public class IssueUpdater {
     return setLine(issue, currentLine);
   }
 
+  public boolean setLocations(DefaultIssue issue, @Nullable Object locations) {
+    if (!Objects.equal(locations, issue.getLocations())) {
+      issue.setLocations(locations);
+      issue.setChanged(true);
+      return true;
+    }
+    return false;
+  }
+
+  public boolean setPastLocations(DefaultIssue issue, @Nullable Object previousLocations) {
+    Object currentLocations = issue.getLocations();
+    issue.setLocations(previousLocations);
+    return setLocations(issue, currentLocations);
+
+  }
+
   public boolean setResolution(DefaultIssue issue, @Nullable String resolution, IssueChangeContext context) {
     if (!Objects.equal(resolution, issue.resolution())) {
       issue.setFieldChange(context, RESOLUTION, issue.resolution(), resolution);
@@ -302,4 +318,5 @@ public class IssueUpdater {
     return false;
   }
 
+
 }
diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java
index ab5954ac145..86f103885c0 100644
--- a/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueUpdaterTest.java
@@ -202,7 +202,7 @@ public class IssueUpdaterTest {
   }
 
   @Test
-  public void not_change_line() {
+  public void line_is_not_changed() {
     issue.setLine(123);
     boolean updated = updater.setLine(issue, 123);
     assertThat(updated).isFalse();
@@ -212,6 +212,36 @@ public class IssueUpdaterTest {
   }
 
   @Test
+  public void change_locations() {
+    issue.setLocations("[1-3]");
+    boolean updated = updater.setLocations(issue, "[1-4]");
+    assertThat(updated).isTrue();
+    assertThat(issue.getLocations()).isEqualTo("[1-4]");
+    assertThat(issue.currentChange()).isNull();
+    assertThat(issue.mustSendNotifications()).isFalse();
+  }
+
+  @Test
+  public void do_not_change_locations() {
+    issue.setLocations("[1-3]");
+    boolean updated = updater.setLocations(issue, "[1-3]");
+    assertThat(updated).isFalse();
+    assertThat(issue.getLocations()).isEqualTo("[1-3]");
+    assertThat(issue.currentChange()).isNull();
+    assertThat(issue.mustSendNotifications()).isFalse();
+  }
+
+  @Test
+  public void set_locations_for_the_first_time() {
+    issue.setLocations(null);
+    boolean updated = updater.setLocations(issue, "[1-4]");
+    assertThat(updated).isTrue();
+    assertThat(issue.getLocations()).isEqualTo("[1-4]");
+    assertThat(issue.currentChange()).isNull();
+    assertThat(issue.mustSendNotifications()).isFalse();
+  }
+
+  @Test
   public void set_resolution() {
     boolean updated = updater.setResolution(issue, "OPEN", context);
     assertThat(updated).isTrue();