SONAR-7316 DbScmInfo should use a single ChangeSet object per revision

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/scm/Changeset.java b/server/sonar-server/src/main/java/org/sonar/server/computation/scm/Changeset.java
index 06a81167ae48d1f6044d0d59ff83391cc0920aca..522d1d3f9908575cf43db770ad92b6141f3ad1dc 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/scm/Changeset.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/scm/Changeset.java
@@ -75,11 +75,11 @@ public final class Changeset {
       return new Changeset(this);
     }
 
-    private static String checkRevision(String revision){
+    private static String checkRevision(String revision) {
       return requireNonNull(revision, "Revision cannot be null");
     }
 
-    private static long checkDate(Long date){
+    private static long checkDate(Long date) {
       return requireNonNull(date, "Date cannot be null");
     }
 
@@ -109,26 +109,20 @@ public final class Changeset {
 
     Changeset changeset = (Changeset) o;
 
-    if (date != changeset.date) {
-      return false;
-    }
-    if (!revision.equals(changeset.revision)) {
-      return false;
-    }
-    return Objects.equals(author, changeset.author);
+    return revision.equals(changeset.revision);
   }
 
   @Override
   public int hashCode() {
-    return Objects.hash(revision, author, date);
+    return Objects.hash(revision);
   }
 
   @Override
   public String toString() {
     return "Changeset{" +
-        "revision='" + revision + '\'' +
-        ", author='" + author + '\'' +
-        ", date=" + date +
-        '}';
+      "revision='" + revision + '\'' +
+      ", author='" + author + '\'' +
+      ", date=" + date +
+      '}';
   }
 }

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/scm/DbScmInfo.java b/server/sonar-server/src/main/java/org/sonar/server/computation/scm/DbScmInfo.java
index 3e67e039f4a456e821889a6b56bc86cdd71792fb..0f2cb035ad3cc017388ebf716dfa12e099ea2ace 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/scm/DbScmInfo.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/scm/DbScmInfo.java
@@ -21,6 +21,9 @@ package org.sonar.server.computation.scm;
 
 import com.google.common.base.Function;
 import com.google.common.base.Optional;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.Immutable;
@@ -30,7 +33,6 @@ import org.sonar.server.computation.component.Component;
 import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.base.Predicates.notNull;
 import static com.google.common.collect.FluentIterable.from;
-import static com.google.common.collect.Iterables.isEmpty;
 import static java.lang.String.format;
 
 /**
@@ -47,11 +49,11 @@ class DbScmInfo implements ScmInfo {
 
   static Optional<ScmInfo> create(Component component, Iterable<DbFileSources.Line> lines) {
     LineToChangeset lineToChangeset = new LineToChangeset();
-    Iterable<Changeset> lineChangesets = from(lines)
+    List<Changeset> lineChangesets = from(lines)
       .transform(lineToChangeset)
       .filter(notNull())
       .toList();
-    if (isEmpty(lineChangesets)) {
+    if (lineChangesets.isEmpty()) {
       return Optional.absent();
     }
     checkState(!lineToChangeset.isEncounteredLineWithoutScmInfo(),
@@ -85,16 +87,24 @@ class DbScmInfo implements ScmInfo {
    */
   private static class LineToChangeset implements Function<DbFileSources.Line, Changeset> {
     private boolean encounteredLineWithoutScmInfo = false;
+    private final Map<String, Changeset> cache = new HashMap<>();
+    private final Changeset.Builder builder = Changeset.newChangesetBuilder();
 
     @Override
     @Nullable
     public Changeset apply(@Nonnull DbFileSources.Line input) {
-      if (input.hasScmRevision() || input.hasScmAuthor() || input.hasScmDate()) {
-        return Changeset.newChangesetBuilder()
-          .setRevision(input.getScmRevision())
-          .setAuthor(input.getScmAuthor())
-          .setDate(input.getScmDate())
-          .build();
+      if (input.hasScmRevision() && input.hasScmDate()) {
+        String revision = input.getScmRevision();
+        Changeset changeset = cache.get(revision);
+        if (changeset == null) {
+          changeset = builder
+              .setRevision(revision)
+              .setAuthor(input.hasScmAuthor() ? input.getScmAuthor() : null)
+              .setDate(input.getScmDate())
+              .build();
+          cache.put(revision, changeset);
+        }
+        return changeset;
       }
 
       this.encounteredLineWithoutScmInfo = true;

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/scm/ChangesetTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/scm/ChangesetTest.java
index 1396b32147ad701edd2eb946d0ec8991f2135d88..016a00ac7a7291e98faf76ec17cccfb76450bc99 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/scm/ChangesetTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/scm/ChangesetTest.java
@@ -105,7 +105,7 @@ public class ChangesetTest {
   }
 
   @Test
-  public void test_equals_and_hash_code() throws Exception {
+  public void equals_and_hashcode_are_based_on_revision_alone() throws Exception {
     Changeset.Builder changesetBuilder = Changeset.newChangesetBuilder()
       .setAuthor("john")
       .setDate(123456789L)
@@ -114,14 +114,26 @@ public class ChangesetTest {
     Changeset changeset = changesetBuilder.build();
     Changeset sameChangeset = changesetBuilder.build();
 
-    Changeset anotherChangeset = Changeset.newChangesetBuilder()
+    Changeset anotherChangesetWithSameRevision = Changeset.newChangesetBuilder()
       .setAuthor("henry")
       .setDate(1234567810L)
+      .setRevision("rev-1")
+      .build();
+
+    Changeset anotherChangeset = Changeset.newChangesetBuilder()
+      .setAuthor("henry")
+      .setDate(996L)
       .setRevision("rev-2")
       .build();
 
-    assertThat(changeset).isEqualTo(sameChangeset);
     assertThat(changeset).isEqualTo(changeset);
+    assertThat(changeset).isEqualTo(sameChangeset);
+    assertThat(changeset).isEqualTo(anotherChangesetWithSameRevision);
     assertThat(changeset).isNotEqualTo(anotherChangeset);
+
+    assertThat(changeset.hashCode()).isEqualTo(changeset.hashCode());
+    assertThat(changeset.hashCode()).isEqualTo(sameChangeset.hashCode());
+    assertThat(changeset.hashCode()).isEqualTo(anotherChangesetWithSameRevision.hashCode());
+    assertThat(changeset.hashCode()).isNotEqualTo(anotherChangeset.hashCode());
   }
 }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/scm/DbScmInfoTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/scm/DbScmInfoTest.java
index 5b9e60bac9e279b4f2691ca03bfa9cdb1cd1c82c..2f2a8deaf080670d094e9a9d664eabafc7a05641 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/scm/DbScmInfoTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/scm/DbScmInfoTest.java
@@ -64,6 +64,21 @@ public class DbScmInfoTest {
     assertThat(changeset.getRevision()).isEqualTo("rev-1");
   }
 
+  @Test
+  public void return_same_changeset_objects_for_lines_with_same_revision() throws Exception {
+    DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder();
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev").setScmDate(65L).setLine(1);
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev2").setScmDate(6541L).setLine(2);
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev1").setScmDate(6541L).setLine(3);
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev").setScmDate(6542L).setLine(4);
+
+    ScmInfo scmInfo = DbScmInfo.create(FILE, fileDataBuilder.getLinesList()).get();
+
+    assertThat(scmInfo.getAllChangesets()).hasSize(4);
+
+    assertThat(scmInfo.getChangesetForLine(1)).isSameAs(scmInfo.getChangesetForLine(4));
+  }
+
   @Test
   public void return_latest_changeset() throws Exception {
     DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder();
@@ -89,6 +104,17 @@ public class DbScmInfoTest {
     assertThat(DbScmInfo.create(FILE, fileDataBuilder.getLinesList())).isAbsent();
   }
 
+  @Test
+  public void return_absent_dsm_info_when_changeset_line_has_both_revision_and_date() throws Exception {
+    DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder();
+    fileDataBuilder.addLinesBuilder().setLine(1);
+    fileDataBuilder.addLinesBuilder().setScmDate(6541L).setLine(2);
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev").setLine(3);
+    fileDataBuilder.addLinesBuilder().setScmAuthor("author").setLine(4);
+
+    assertThat(DbScmInfo.create(FILE, fileDataBuilder.getLinesList())).isAbsent();
+  }
+
   @Test
   public void fail_with_ISE_when_changeset_has_no_field() throws Exception {
     thrown.expect(IllegalStateException.class);
@@ -96,8 +122,36 @@ public class DbScmInfoTest {
       "Not all lines have SCM info. Can not proceed");
 
     DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder();
-    fileDataBuilder.addLinesBuilder().setLine(1);
-    fileDataBuilder.addLinesBuilder().setScmAuthor("John").setLine(2);
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev").setScmDate(543L).setLine(1);
+    fileDataBuilder.addLinesBuilder().setLine(2);
+    fileDataBuilder.build();
+
+    DbScmInfo.create(FILE, fileDataBuilder.getLinesList()).get().getAllChangesets();
+  }
+
+  @Test
+  public void fail_with_ISE_when_changeset_has_only_revision_field() throws Exception {
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Partial scm information stored in DB for component 'ReportComponent{ref=1, key='FILE_KEY', type=FILE}'. " +
+      "Not all lines have SCM info. Can not proceed");
+
+    DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder();
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev").setScmDate(555L).setLine(1);
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev-1").setLine(2);
+    fileDataBuilder.build();
+
+    DbScmInfo.create(FILE, fileDataBuilder.getLinesList()).get().getAllChangesets();
+  }
+
+  @Test
+  public void fail_with_ISE_when_changeset_has_only_author_field() throws Exception {
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Partial scm information stored in DB for component 'ReportComponent{ref=1, key='FILE_KEY', type=FILE}'. " +
+      "Not all lines have SCM info. Can not proceed");
+
+    DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder();
+    fileDataBuilder.addLinesBuilder().setScmAuthor("John").setLine(1);
+    fileDataBuilder.addLinesBuilder().setScmRevision("rev").setScmDate(555L).setLine(2);
     fileDataBuilder.build();
 
     DbScmInfo.create(FILE, fileDataBuilder.getLinesList()).get().getAllChangesets();