Skip to content

Commit

Permalink
SONAR-7316 DbScmInfo should use a single ChangeSet object per revision
Browse files Browse the repository at this point in the history
  • Loading branch information
@sns-seb
sns-seb committed Feb 24, 2016
1 parent e0452cc commit f28e7a1
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 28 deletions.
22 changes: 8 additions & 14 deletions server/sonar-server/src/main/java/org/sonar/server/computation/scm/Changeset.java
View file
Expand Up @@ -75,11 +75,11 @@ public Changeset build() {
return new Changeset(this); return new Changeset(this);
} }


private static String checkRevision(String revision){ private static String checkRevision(String revision) {
return requireNonNull(revision, "Revision cannot be null"); 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"); return requireNonNull(date, "Date cannot be null");
} }


Expand Down Expand Up @@ -109,26 +109,20 @@ public boolean equals(@Nullable Object o) {


Changeset changeset = (Changeset) o; Changeset changeset = (Changeset) o;


if (date != changeset.date) { return revision.equals(changeset.revision);
return false;
}
if (!revision.equals(changeset.revision)) {
return false;
}
return Objects.equals(author, changeset.author);
} }


@Override @Override
public int hashCode() { public int hashCode() {
return Objects.hash(revision, author, date); return Objects.hash(revision);
} }


@Override @Override
public String toString() { public String toString() {
return "Changeset{" + return "Changeset{" +
"revision='" + revision + '\'' + "revision='" + revision + '\'' +
", author='" + author + '\'' + ", author='" + author + '\'' +
", date=" + date + ", date=" + date +
'}'; '}';
} }
} }
28 changes: 19 additions & 9 deletions server/sonar-server/src/main/java/org/sonar/server/computation/scm/DbScmInfo.java
View file
Expand Up @@ -21,6 +21,9 @@


import com.google.common.base.Function; import com.google.common.base.Function;
import com.google.common.base.Optional; 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.Nonnull;
import javax.annotation.Nullable; import javax.annotation.Nullable;
import javax.annotation.concurrent.Immutable; import javax.annotation.concurrent.Immutable;
Expand All @@ -30,7 +33,6 @@
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import static com.google.common.base.Predicates.notNull; import static com.google.common.base.Predicates.notNull;
import static com.google.common.collect.FluentIterable.from; import static com.google.common.collect.FluentIterable.from;
import static com.google.common.collect.Iterables.isEmpty;
import static java.lang.String.format; import static java.lang.String.format;


/** /**
Expand All @@ -47,11 +49,11 @@ private DbScmInfo(ScmInfo delegate) {


static Optional<ScmInfo> create(Component component, Iterable<DbFileSources.Line> lines) { static Optional<ScmInfo> create(Component component, Iterable<DbFileSources.Line> lines) {
LineToChangeset lineToChangeset = new LineToChangeset(); LineToChangeset lineToChangeset = new LineToChangeset();
Iterable<Changeset> lineChangesets = from(lines) List<Changeset> lineChangesets = from(lines)
.transform(lineToChangeset) .transform(lineToChangeset)
.filter(notNull()) .filter(notNull())
.toList(); .toList();
if (isEmpty(lineChangesets)) { if (lineChangesets.isEmpty()) {
return Optional.absent(); return Optional.absent();
} }
checkState(!lineToChangeset.isEncounteredLineWithoutScmInfo(), checkState(!lineToChangeset.isEncounteredLineWithoutScmInfo(),
Expand Down Expand Up @@ -85,16 +87,24 @@ public Iterable<Changeset> getAllChangesets() {
*/ */
private static class LineToChangeset implements Function<DbFileSources.Line, Changeset> { private static class LineToChangeset implements Function<DbFileSources.Line, Changeset> {
private boolean encounteredLineWithoutScmInfo = false; private boolean encounteredLineWithoutScmInfo = false;
private final Map<String, Changeset> cache = new HashMap<>();
private final Changeset.Builder builder = Changeset.newChangesetBuilder();


@Override @Override
@Nullable @Nullable
public Changeset apply(@Nonnull DbFileSources.Line input) { public Changeset apply(@Nonnull DbFileSources.Line input) {
if (input.hasScmRevision() || input.hasScmAuthor() || input.hasScmDate()) { if (input.hasScmRevision() && input.hasScmDate()) {
return Changeset.newChangesetBuilder() String revision = input.getScmRevision();
.setRevision(input.getScmRevision()) Changeset changeset = cache.get(revision);
.setAuthor(input.getScmAuthor()) if (changeset == null) {
.setDate(input.getScmDate()) changeset = builder
.build(); .setRevision(revision)
.setAuthor(input.hasScmAuthor() ? input.getScmAuthor() : null)
.setDate(input.getScmDate())
.build();
cache.put(revision, changeset);
}
return changeset;
} }


this.encounteredLineWithoutScmInfo = true; this.encounteredLineWithoutScmInfo = true;
Expand Down
18 changes: 15 additions & 3 deletions server/sonar-server/src/test/java/org/sonar/server/computation/scm/ChangesetTest.java
View file
Expand Up @@ -105,7 +105,7 @@ public void test_to_string() throws Exception {
} }


@Test @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() Changeset.Builder changesetBuilder = Changeset.newChangesetBuilder()
.setAuthor("john") .setAuthor("john")
.setDate(123456789L) .setDate(123456789L)
Expand All @@ -114,14 +114,26 @@ public void test_equals_and_hash_code() throws Exception {
Changeset changeset = changesetBuilder.build(); Changeset changeset = changesetBuilder.build();
Changeset sameChangeset = changesetBuilder.build(); Changeset sameChangeset = changesetBuilder.build();


Changeset anotherChangeset = Changeset.newChangesetBuilder() Changeset anotherChangesetWithSameRevision = Changeset.newChangesetBuilder()
.setAuthor("henry") .setAuthor("henry")
.setDate(1234567810L) .setDate(1234567810L)
.setRevision("rev-1")
.build();

Changeset anotherChangeset = Changeset.newChangesetBuilder()
.setAuthor("henry")
.setDate(996L)
.setRevision("rev-2") .setRevision("rev-2")
.build(); .build();


assertThat(changeset).isEqualTo(sameChangeset);
assertThat(changeset).isEqualTo(changeset); assertThat(changeset).isEqualTo(changeset);
assertThat(changeset).isEqualTo(sameChangeset);
assertThat(changeset).isEqualTo(anotherChangesetWithSameRevision);
assertThat(changeset).isNotEqualTo(anotherChangeset); 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());
} }
} }
58 changes: 56 additions & 2 deletions server/sonar-server/src/test/java/org/sonar/server/computation/scm/DbScmInfoTest.java
View file
Expand Up @@ -64,6 +64,21 @@ public void return_changeset_for_a_given_line() throws Exception {
assertThat(changeset.getRevision()).isEqualTo("rev-1"); 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 @Test
public void return_latest_changeset() throws Exception { public void return_latest_changeset() throws Exception {
DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder(); DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder();
Expand All @@ -89,15 +104,54 @@ public void return_absent_dsm_info_when_no_changeset() throws Exception {
assertThat(DbScmInfo.create(FILE, fileDataBuilder.getLinesList())).isAbsent(); 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 @Test
public void fail_with_ISE_when_changeset_has_no_field() throws Exception { public void fail_with_ISE_when_changeset_has_no_field() throws Exception {
thrown.expect(IllegalStateException.class); thrown.expect(IllegalStateException.class);
thrown.expectMessage("Partial scm information stored in DB for component 'ReportComponent{ref=1, key='FILE_KEY', type=FILE}'. " + 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"); "Not all lines have SCM info. Can not proceed");


DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder(); DbFileSources.Data.Builder fileDataBuilder = DbFileSources.Data.newBuilder();
fileDataBuilder.addLinesBuilder().setLine(1); fileDataBuilder.addLinesBuilder().setScmRevision("rev").setScmDate(543L).setLine(1);
fileDataBuilder.addLinesBuilder().setScmAuthor("John").setLine(2); 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(); fileDataBuilder.build();


DbScmInfo.create(FILE, fileDataBuilder.getLinesList()).get().getAllChangesets(); DbScmInfo.create(FILE, fileDataBuilder.getLinesList()).get().getAllChangesets();
Expand Down

0 comments on commit f28e7a1

Please sign in to comment.