From f28e7a1b934a246f44b0d327d087db3aeb632173 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Tue, 23 Feb 2016 17:51:49 +0100 Subject: [PATCH] SONAR-7316 DbScmInfo should use a single ChangeSet object per revision --- .../server/computation/scm/Changeset.java | 22 +++---- .../server/computation/scm/DbScmInfo.java | 28 ++++++--- .../server/computation/scm/ChangesetTest.java | 18 +++++- .../server/computation/scm/DbScmInfoTest.java | 58 ++++++++++++++++++- 4 files changed, 98 insertions(+), 28 deletions(-) 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 06a81167ae4..522d1d3f990 100644 --- 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 3e67e039f4a..0f2cb035ad3 100644 --- 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 create(Component component, Iterable lines) { LineToChangeset lineToChangeset = new LineToChangeset(); - Iterable lineChangesets = from(lines) + List 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 { private boolean encounteredLineWithoutScmInfo = false; + private final Map 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 1396b32147a..016a00ac7a7 100644 --- 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 5b9e60bac9e..2f2a8deaf08 100644 --- 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(); -- 2.39.5