From 4b7f9388c6bf2474e3c4137331f8d71056d68682 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Mon, 28 Sep 2015 17:00:39 +0200 Subject: [PATCH] SONAR-6397 persist latest revision of file to DB --- .../computation/source/ScmLineReader.java | 23 +++- .../step/PersistFileSourcesStep.java | 51 ++++++-- .../computation/source/ScmLineReaderTest.java | 118 +++++++++++++++++- .../step/PersistFileSourcesStepTest.java | 17 ++- .../org/sonar/db/source/FileSourceDto.java | 2 +- 5 files changed, 192 insertions(+), 19 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/ScmLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/ScmLineReader.java index 4dd9d3adf0e..35bacf16ffb 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/ScmLineReader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/ScmLineReader.java @@ -20,12 +20,17 @@ package org.sonar.server.computation.source; +import javax.annotation.CheckForNull; import org.sonar.batch.protocol.output.BatchReport; import org.sonar.db.protobuf.DbFileSources; +import static com.google.common.base.Preconditions.checkArgument; + public class ScmLineReader implements LineReader { private final BatchReport.Changesets scmReport; + @CheckForNull + private BatchReport.Changesets.Changeset latestChange; public ScmLineReader(BatchReport.Changesets scmReport) { this.scmReport = scmReport; @@ -48,9 +53,23 @@ public class ScmLineReader implements LineReader { lineBuilder.setScmDate(changeset.getDate()); } - if (!hasAuthor && !hasRevision && !hasDate) { - throw new IllegalArgumentException("A changeset must contains at least one of : author, revision or date"); + checkArgument( + hasAuthor || hasRevision || hasDate, + "A changeset must contain at least one of : author, revision or date"); + updateLatestChange(changeset); + } + + private void updateLatestChange(BatchReport.Changesets.Changeset newChangeSet) { + if (this.latestChange == null) { + this.latestChange = newChangeSet; + } else if (newChangeSet.hasDate() && this.latestChange.hasDate() + && newChangeSet.getDate() > this.latestChange.getDate()) { + this.latestChange = newChangeSet; } } + @CheckForNull + public BatchReport.Changesets.Changeset getLatestChange() { + return latestChange; + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistFileSourcesStep.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistFileSourcesStep.java index 824364110a6..49f9d86eac1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistFileSourcesStep.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistFileSourcesStep.java @@ -21,6 +21,12 @@ package org.sonar.server.computation.step; import com.google.common.collect.ImmutableMap; +import java.util.ArrayList; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; import org.apache.commons.codec.digest.DigestUtils; import org.apache.ibatis.session.ResultContext; import org.apache.ibatis.session.ResultHandler; @@ -48,11 +54,6 @@ import org.sonar.server.computation.source.ScmLineReader; import org.sonar.server.computation.source.SourceLinesRepository; import org.sonar.server.computation.source.SymbolsLineReader; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; -import java.util.Map; - import static org.sonar.server.computation.component.ComponentVisitor.Order.PRE_ORDER; public class PersistFileSourcesStep implements ComputationStep { @@ -117,7 +118,7 @@ public class PersistFileSourcesStep implements ComputationStep { try { ComputeFileSourceData computeFileSourceData = new ComputeFileSourceData(linesIterator, lineReaders.readers(), component.getLines()); ComputeFileSourceData.Data fileSourceData = computeFileSourceData.compute(); - persistSource(fileSourceData, file.getUuid()); + persistSource(fileSourceData, file.getUuid(), lineReaders.getLatestChange()); } catch (Exception e) { throw new IllegalStateException(String.format("Cannot persist sources of %s", file.getKey()), e); } finally { @@ -126,7 +127,8 @@ public class PersistFileSourcesStep implements ComputationStep { } } - private void persistSource(ComputeFileSourceData.Data fileSourceData, String componentUuid) { + private void persistSource(ComputeFileSourceData.Data fileSourceData, String componentUuid, + @Nullable BatchReport.Changesets.Changeset latestChange) { DbFileSources.Data fileData = fileSourceData.getFileSourceData(); byte[] data = FileSourceDto.encodeSourceData(fileData); @@ -145,7 +147,8 @@ public class PersistFileSourcesStep implements ComputationStep { .setDataHash(dataHash) .setLineHashes(lineHashes) .setCreatedAt(system2.now()) - .setUpdatedAt(system2.now()); + .setUpdatedAt(system2.now()) + .setRevision(computeRevision(latestChange)); dbClient.fileSourceDao().insert(session, dto); session.commit(); } else { @@ -158,6 +161,7 @@ public class PersistFileSourcesStep implements ComputationStep { .setDataHash(dataHash) .setSrcHash(srcHash) .setLineHashes(lineHashes) + .setRevision(computeRevision(previousDto, latestChange)) .setUpdatedAt(system2.now()); dbClient.fileSourceDao().update(previousDto); session.commit(); @@ -166,9 +170,27 @@ public class PersistFileSourcesStep implements ComputationStep { } } + @CheckForNull + private static String computeRevision(FileSourceDto previousDto, @Nullable BatchReport.Changesets.Changeset latestChange) { + if (latestChange == null) { + return previousDto.getRevision(); + } + return latestChange.getRevision(); + } + + @CheckForNull + private static String computeRevision(@Nullable BatchReport.Changesets.Changeset latestChange) { + if (latestChange == null) { + return null; + } + return latestChange.getRevision(); + } + private static class LineReaders { private final List readers = new ArrayList<>(); private final List> closeables = new ArrayList<>(); + @CheckForNull + private final ScmLineReader scmLineReader; LineReaders(BatchReportReader reportReader, int componentRef) { CloseableIterator coverageIt = reportReader.readComponentCoverage(componentRef); @@ -177,7 +199,10 @@ public class PersistFileSourcesStep implements ComputationStep { BatchReport.Changesets scmReport = reportReader.readChangesets(componentRef); if (scmReport != null) { - readers.add(new ScmLineReader(scmReport)); + this.scmLineReader = new ScmLineReader(scmReport); + readers.add(scmLineReader); + } else { + this.scmLineReader = null; } CloseableIterator highlightingIt = reportReader.readComponentSyntaxHighlighting(componentRef); @@ -202,6 +227,14 @@ public class PersistFileSourcesStep implements ComputationStep { reportIterator.close(); } } + + @CheckForNull + public BatchReport.Changesets.Changeset getLatestChange() { + if (scmLineReader == null) { + return null; + } + return scmLineReader.getLatestChange(); + } } @Override diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/ScmLineReaderTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/ScmLineReaderTest.java index f0ca2c26f7e..30ce8f560ff 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/ScmLineReaderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/ScmLineReaderTest.java @@ -32,7 +32,7 @@ public class ScmLineReaderTest { @Test public void set_scm() { BatchReport.Changesets scmReport = BatchReport.Changesets.newBuilder() - .addChangeset(BatchReport.Changesets.Changeset.newBuilder() + .addChangeset(newChangeSetBuilder() .setAuthor("john") .setDate(123456789L) .setRevision("rev-1") @@ -53,7 +53,7 @@ public class ScmLineReaderTest { @Test public void set_only_author() { BatchReport.Changesets scmReport = BatchReport.Changesets.newBuilder() - .addChangeset(BatchReport.Changesets.Changeset.newBuilder() + .addChangeset(newChangeSetBuilder() .setAuthor("john") .build()) .addChangesetIndexByLine(0) @@ -72,7 +72,7 @@ public class ScmLineReaderTest { @Test public void set_only_date() { BatchReport.Changesets scmReport = BatchReport.Changesets.newBuilder() - .addChangeset(BatchReport.Changesets.Changeset.newBuilder() + .addChangeset(newChangeSetBuilder() .setDate(123456789L) .build()) .addChangesetIndexByLine(0) @@ -91,7 +91,7 @@ public class ScmLineReaderTest { @Test public void set_only_revision() { BatchReport.Changesets scmReport = BatchReport.Changesets.newBuilder() - .addChangeset(BatchReport.Changesets.Changeset.newBuilder() + .addChangeset(newChangeSetBuilder() .setRevision("rev-1") .build()) .addChangesetIndexByLine(0) @@ -110,7 +110,7 @@ public class ScmLineReaderTest { @Test public void fail_when_changeset_is_empty() { BatchReport.Changesets scmReport = BatchReport.Changesets.newBuilder() - .addChangeset(BatchReport.Changesets.Changeset.newBuilder() + .addChangeset(newChangeSetBuilder() .build()) .addChangesetIndexByLine(0) .build(); @@ -122,8 +122,114 @@ public class ScmLineReaderTest { lineScm.read(lineBuilder); failBecauseExceptionWasNotThrown(IllegalArgumentException.class); } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("A changeset must contains at least one of : author, revision or date"); + assertThat(e).hasMessage("A changeset must contain at least one of : author, revision or date"); } } + @Test + public void getLatestChange_returns_changeset_with_highest_date_of_read_lines() { + long refDate = 123456789L; + BatchReport.Changesets.Changeset changeset0 = newChangeSetBuilder().setDate(refDate - 636).build(); + BatchReport.Changesets.Changeset changeset1 = newChangeSetBuilder().setDate(refDate + 1).build(); + BatchReport.Changesets.Changeset changeset2 = newChangeSetBuilder().setDate(refDate + 2).build(); + BatchReport.Changesets scmReport = setup8LinesChangeset(changeset0, changeset1, changeset2); + + ScmLineReader lineScm = new ScmLineReader(scmReport); + + // before any line is read, the latest change is null + assertThat(lineScm.getLatestChange()).isNull(); + + // read line 1, only one changeset => 0 + readLineAndAssertLatestChangeDate(lineScm, 1, changeset0); + + // read line 2, latest changeset is 1 + readLineAndAssertLatestChangeDate(lineScm, 2, changeset1); + + // read line 3, latest changeset is still 1 + readLineAndAssertLatestChangeDate(lineScm, 3, changeset1); + + // read line 4, latest changeset is now 2 + readLineAndAssertLatestChangeDate(lineScm, 4, changeset2); + + // read line 5 to 8, there will never be any changeset more recent than 2 + readLineAndAssertLatestChangeDate(lineScm, 5, changeset2); + readLineAndAssertLatestChangeDate(lineScm, 6, changeset2); + readLineAndAssertLatestChangeDate(lineScm, 7, changeset2); + readLineAndAssertLatestChangeDate(lineScm, 8, changeset2); + } + + @Test + public void getLatestChange_returns_first_changeset_when_none_have_dates() { + BatchReport.Changesets.Changeset changeset0 = newChangeSetBuilder().setRevision("0").build(); + BatchReport.Changesets.Changeset changeset1 = newChangeSetBuilder().setRevision("1").build(); + BatchReport.Changesets.Changeset changeset2 = newChangeSetBuilder().setRevision("2").build(); + BatchReport.Changesets scmReport = setup8LinesChangeset(changeset0, changeset1, changeset2); + + ScmLineReader lineScm = new ScmLineReader(scmReport); + + // before any line is read, the latest change is null + assertThat(lineScm.getLatestChange()).isNull(); + + // read lines 1 to 8, no date => changeset 0 + readLineAndAssertLatestChangeDate(lineScm, 1, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 2, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 3, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 4, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 5, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 6, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 7, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 8, changeset0); + } + + @Test + public void getLatestChange_returns_first_changeset_when_the_first_one_has_no_date() { + BatchReport.Changesets.Changeset changeset0 = newChangeSetBuilder().setRevision("0").build(); + BatchReport.Changesets.Changeset changeset1 = newChangeSetBuilder().setRevision("1").setDate(95454154L).build(); + BatchReport.Changesets.Changeset changeset2 = newChangeSetBuilder().setRevision("2").setDate(9654545444L).build(); + BatchReport.Changesets scmReport = setup8LinesChangeset(changeset0, changeset1, changeset2); + + ScmLineReader lineScm = new ScmLineReader(scmReport); + + // before any line is read, the latest change is null + assertThat(lineScm.getLatestChange()).isNull(); + + // read lines 1 to 8, first encountered changeset has no date => changeset 0 + readLineAndAssertLatestChangeDate(lineScm, 1, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 2, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 3, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 4, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 5, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 6, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 7, changeset0); + readLineAndAssertLatestChangeDate(lineScm, 8, changeset0); + } + + private static BatchReport.Changesets setup8LinesChangeset(BatchReport.Changesets.Changeset changeset0, + BatchReport.Changesets.Changeset changeset1, + BatchReport.Changesets.Changeset changeset2) { + return BatchReport.Changesets.newBuilder() + .addChangeset(changeset0) + .addChangeset(changeset1) + .addChangeset(changeset2) + .addChangesetIndexByLine(0) + .addChangesetIndexByLine(1) + .addChangesetIndexByLine(1) + .addChangesetIndexByLine(2) + .addChangesetIndexByLine(0) + .addChangesetIndexByLine(1) + .addChangesetIndexByLine(0) + .addChangesetIndexByLine(0) + .build(); + } + + private void readLineAndAssertLatestChangeDate(ScmLineReader lineScm, int line, BatchReport.Changesets.Changeset expectedChangeset) { + DbFileSources.Line.Builder lineBuilder = DbFileSources.Data.newBuilder().addLinesBuilder().setLine(line); + lineScm.read(lineBuilder); + assertThat(lineScm.getLatestChange()).isSameAs(expectedChangeset); + } + + private static BatchReport.Changesets.Changeset.Builder newChangeSetBuilder() { + return BatchReport.Changesets.Changeset.newBuilder(); + } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java index 86195aa39be..54af795b529 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java @@ -204,6 +204,9 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(dbTester.countRowsOfTable("file_sources")).isEqualTo(1); FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE_UUID); + + assertThat(fileSourceDto.getRevision()).isEqualTo("rev-1"); + DbFileSources.Data data = FileSourceDto.decodeSourceData(fileSourceDto.getBinaryData()); assertThat(data.getLinesList()).hasSize(1); @@ -348,17 +351,29 @@ public class PersistFileSourcesStepTest extends BaseStepTest { .build()) .build()) .setCreatedAt(past) - .setUpdatedAt(past)); + .setUpdatedAt(past) + .setRevision("rev-0")); dbTester.getSession().commit(); initBasicReport(1); + reportReader.putChangesets(BatchReport.Changesets.newBuilder() + .setComponentRef(FILE_REF) + .addChangeset(BatchReport.Changesets.Changeset.newBuilder() + .setAuthor("john") + .setDate(123456789L) + .setRevision("rev-1") + .build()) + .addChangesetIndexByLine(0) + .build()); + underTest.execute(); assertThat(dbTester.countRowsOfTable("file_sources")).isEqualTo(1); FileSourceDto fileSourceDto = dbClient.fileSourceDao().selectSourceByFileUuid(session, FILE_UUID); assertThat(fileSourceDto.getCreatedAt()).isEqualTo(past); assertThat(fileSourceDto.getUpdatedAt()).isEqualTo(now); + assertThat(fileSourceDto.getRevision()).isEqualTo("rev-1"); } @Test diff --git a/sonar-db/src/main/java/org/sonar/db/source/FileSourceDto.java b/sonar-db/src/main/java/org/sonar/db/source/FileSourceDto.java index 686635e0142..6e43439665e 100644 --- a/sonar-db/src/main/java/org/sonar/db/source/FileSourceDto.java +++ b/sonar-db/src/main/java/org/sonar/db/source/FileSourceDto.java @@ -270,7 +270,7 @@ public class FileSourceDto { return revision; } - public FileSourceDto setRevision(String revision) { + public FileSourceDto setRevision(@Nullable String revision) { this.revision = revision; return this; } -- 2.39.5