From 09b8bbcb742e94caa7198d6b4d913ae6502f03bd Mon Sep 17 00:00:00 2001 From: Duarte Meneses Date: Thu, 11 Jan 2018 14:31:01 +0100 Subject: [PATCH] SONAR-10257 Allow null revision in changesets --- .../task/projectanalysis/scm/Changeset.java | 20 +++++-------- .../projectanalysis/source/ScmLineReader.java | 28 ++++++++++++++++++- .../step/PersistFileSourcesStep.java | 12 ++++---- .../projectanalysis/scm/ChangesetTest.java | 27 +++--------------- 4 files changed, 44 insertions(+), 43 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/Changeset.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/Changeset.java index 8e15ba2f3c7..c376349a748 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/Changeset.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/scm/Changeset.java @@ -28,14 +28,14 @@ import static java.util.Objects.requireNonNull; @Immutable public final class Changeset { - + @CheckForNull private final String revision; private final long date; @CheckForNull private final String author; private Changeset(Builder builder) { - this.revision = builder.revision.intern(); + this.revision = builder.revision == null ? null : builder.revision.intern(); this.author = builder.author == null ? null : builder.author.intern(); this.date = builder.date; } @@ -54,8 +54,8 @@ public final class Changeset { // prevents direct instantiation } - public Builder setRevision(String revision) { - this.revision = checkRevision(revision); + public Builder setRevision(@Nullable String revision) { + this.revision = revision; return this; } @@ -70,21 +70,16 @@ public final class Changeset { } public Changeset build() { - checkRevision(revision); checkDate(date); return new Changeset(this); } - private static String checkRevision(String revision) { - return requireNonNull(revision, "Revision cannot be null"); - } - private static long checkDate(Long date) { return requireNonNull(date, "Date cannot be null"); } - } + @CheckForNull public String getRevision() { return revision; } @@ -108,13 +103,12 @@ public final class Changeset { } Changeset changeset = (Changeset) o; - - return revision.equals(changeset.revision); + return Objects.equals(revision, changeset.revision) && Objects.equals(author, changeset.author) && Objects.equals(date, changeset.date); } @Override public int hashCode() { - return Objects.hash(revision); + return Objects.hash(revision, author, date); } @Override diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/ScmLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/ScmLineReader.java index 04909d741d3..a120a4882a3 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/ScmLineReader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/source/ScmLineReader.java @@ -29,6 +29,8 @@ public class ScmLineReader implements LineReader { private final ScmInfo scmReport; @CheckForNull private Changeset latestChange; + @CheckForNull + private Changeset latestChangeWithRevision; public ScmLineReader(ScmInfo scmReport) { this.scmReport = scmReport; @@ -41,9 +43,16 @@ public class ScmLineReader implements LineReader { if (author != null) { lineBuilder.setScmAuthor(author); } - lineBuilder.setScmRevision(changeset.getRevision()); + String revision = changeset.getRevision(); + if (revision != null) { + lineBuilder.setScmRevision(revision); + } lineBuilder.setScmDate(changeset.getDate()); updateLatestChange(changeset); + + if (revision != null) { + updateLatestChangeWithRevision(changeset); + } } private void updateLatestChange(Changeset newChangeSet) { @@ -58,6 +67,23 @@ public class ScmLineReader implements LineReader { } } + private void updateLatestChangeWithRevision(Changeset newChangeSet) { + if (latestChangeWithRevision == null) { + latestChangeWithRevision = newChangeSet; + } else { + long newChangesetDate = newChangeSet.getDate(); + long latestChangeDate = latestChange.getDate(); + if (newChangesetDate > latestChangeDate) { + latestChange = newChangeSet; + } + } + } + + @CheckForNull + public Changeset getLatestChangeWithRevision() { + return latestChangeWithRevision; + } + @CheckForNull public Changeset getLatestChange() { return latestChange; diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistFileSourcesStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistFileSourcesStep.java index 4a237a4ed07..9279a10fe6f 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistFileSourcesStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/task/projectanalysis/step/PersistFileSourcesStep.java @@ -118,13 +118,13 @@ public class PersistFileSourcesStep implements ComputationStep { LineReaders lineReaders = new LineReaders(reportReader, scmInfoRepository, duplicationRepository, file)) { ComputeFileSourceData computeFileSourceData = new ComputeFileSourceData(linesIterator, lineReaders.readers(), file.getFileAttributes().getLines()); ComputeFileSourceData.Data fileSourceData = computeFileSourceData.compute(); - persistSource(fileSourceData, file.getUuid(), lineReaders.getLatestChange()); + persistSource(fileSourceData, file.getUuid(), lineReaders.getLatestChangeWithRevision()); } catch (Exception e) { throw new IllegalStateException(String.format("Cannot persist sources of %s", file.getKey()), e); } } - private void persistSource(ComputeFileSourceData.Data fileSourceData, String componentUuid, @Nullable Changeset latestChange) { + private void persistSource(ComputeFileSourceData.Data fileSourceData, String componentUuid, @Nullable Changeset latestChangeWithRevision) { DbFileSources.Data fileData = fileSourceData.getFileSourceData(); byte[] data = FileSourceDto.encodeSourceData(fileData); @@ -144,14 +144,14 @@ public class PersistFileSourcesStep implements ComputationStep { .setLineHashes(lineHashes) .setCreatedAt(system2.now()) .setUpdatedAt(system2.now()) - .setRevision(computeRevision(latestChange)); + .setRevision(computeRevision(latestChangeWithRevision)); dbClient.fileSourceDao().insert(session, dto); session.commit(); } else { // Update only if data_hash has changed or if src_hash is missing or revision is missing (progressive migration) boolean binaryDataUpdated = !dataHash.equals(previousDto.getDataHash()); boolean srcHashUpdated = !srcHash.equals(previousDto.getSrcHash()); - String revision = computeRevision(latestChange); + String revision = computeRevision(latestChangeWithRevision); boolean revisionUpdated = !ObjectUtils.equals(revision, previousDto.getRevision()); if (binaryDataUpdated || srcHashUpdated || revisionUpdated) { previousDto @@ -219,11 +219,11 @@ public class PersistFileSourcesStep implements ComputationStep { } @CheckForNull - public Changeset getLatestChange() { + public Changeset getLatestChangeWithRevision() { if (scmLineReader == null) { return null; } - return scmLineReader.getLatestChange(); + return scmLineReader.getLatestChangeWithRevision(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ChangesetTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ChangesetTest.java index e8ca509370a..8d96ca4605b 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ChangesetTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/task/projectanalysis/scm/ChangesetTest.java @@ -56,26 +56,7 @@ public class ChangesetTest { } @Test - public void fail_with_NPE_when_setting_null_revision() { - thrown.expect(NullPointerException.class); - thrown.expectMessage("Revision cannot be null"); - - Changeset.newChangesetBuilder().setRevision(null); - } - - @Test - public void fail_with_NPE_when_building_without_revision() { - thrown.expect(NullPointerException.class); - thrown.expectMessage("Revision cannot be null"); - - Changeset.newChangesetBuilder() - .setAuthor("john") - .setDate(123456789L) - .build(); - } - - @Test - public void fail_with_NPE_when_setting_null_date() { + public void fail_with_NPE_when_setting_null_date() throws Exception { thrown.expect(NullPointerException.class); thrown.expectMessage("Date cannot be null"); @@ -105,7 +86,7 @@ public class ChangesetTest { } @Test - public void equals_and_hashcode_are_based_on_revision_alone() { + public void equals_and_hashcode_are_based_on_all_fields() throws Exception { Changeset.Builder changesetBuilder = Changeset.newChangesetBuilder() .setAuthor("john") .setDate(123456789L) @@ -128,12 +109,12 @@ public class ChangesetTest { assertThat(changeset).isEqualTo(changeset); assertThat(changeset).isEqualTo(sameChangeset); - assertThat(changeset).isEqualTo(anotherChangesetWithSameRevision); + assertThat(changeset).isNotEqualTo(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(anotherChangesetWithSameRevision.hashCode()); assertThat(changeset.hashCode()).isNotEqualTo(anotherChangeset.hashCode()); } } -- 2.39.5