From 6c5e5b1481a2d20068be2de2fe306b82649e46d6 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Tue, 14 Apr 2015 17:20:00 +0200 Subject: [PATCH] SONAR-6258 Persist duplications into file sources --- .../source/DuplicationLineReader.java | 112 ++++++++ .../computation/source/SymbolsLineReader.java | 3 +- .../step/PersistFileSourcesStep.java | 4 + .../source/DuplicationLineReaderTest.java | 255 ++++++++++++++++++ .../step/PersistFileSourcesStepTest.java | 52 +++- 5 files changed, 414 insertions(+), 12 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/computation/source/DuplicationLineReaderTest.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java new file mode 100644 index 00000000000..bdd1bbea633 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/DuplicationLineReader.java @@ -0,0 +1,112 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.computation.source; + +import org.sonar.batch.protocol.output.BatchReport; +import org.sonar.server.source.db.FileSourceDb; + +import java.io.Serializable; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Map; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; + +public class DuplicationLineReader implements LineReader { + + private final List duplications; + private final Map duplicationIdsByRange; + + public DuplicationLineReader(List duplications) { + this.duplications = newArrayList(duplications); + // Sort duplication to have deterministic results and avoid false variation that would lead to an unnecessary update of the source files + // data + Collections.sort(this.duplications, new DuplicationComparator()); + + this.duplicationIdsByRange = createDuplicationIdsByRange(this.duplications); + } + + @Override + public void read(FileSourceDb.Line.Builder lineBuilder) { + int line = lineBuilder.getLine(); + List blocks = findDuplicationBlockMatchingLine(line); + for (BatchReport.Range block : blocks) { + lineBuilder.addDuplication(duplicationIdsByRange.get(block)); + } + } + + private List findDuplicationBlockMatchingLine(int line) { + List blocks = newArrayList(); + for (BatchReport.Duplication duplication : duplications) { + if (matchLine(duplication.getOriginPosition(), line)) { + blocks.add(duplication.getOriginPosition()); + } + for (BatchReport.Duplicate duplicate : duplication.getDuplicateList()) { + if (isDuplicationOnSameFile(duplicate) && matchLine(duplicate.getRange(), line)) { + blocks.add(duplicate.getRange()); + } + } + } + return blocks; + } + + private static boolean isDuplicationOnSameFile(BatchReport.Duplicate duplicate) { + return !duplicate.hasOtherFileKey() && !duplicate.hasOtherFileRef(); + } + + private static boolean matchLine(BatchReport.Range range, int line) { + return range.getStartLine() <= line && line <= range.getEndLine(); + } + + private static int length(BatchReport.Range range) { + return range.getEndLine() - range.getStartLine() + 1; + } + + private Map createDuplicationIdsByRange(List duplications) { + Map map = newHashMap(); + int blockId = 1; + for (BatchReport.Duplication duplication : this.duplications) { + map.put(duplication.getOriginPosition(), blockId); + blockId++; + for (BatchReport.Duplicate duplicate : duplication.getDuplicateList()) { + if (isDuplicationOnSameFile(duplicate)) { + map.put(duplicate.getRange(), blockId); + blockId++; + } + } + } + return map; + } + + private static class DuplicationComparator implements Comparator, Serializable { + @Override + public int compare(BatchReport.Duplication d1, BatchReport.Duplication d2) { + if (d1.getOriginPosition().getStartLine() == d2.getOriginPosition().getStartLine()) { + return Integer.compare(length(d1.getOriginPosition()), length(d2.getOriginPosition())); + } else { + return Integer.compare(d1.getOriginPosition().getStartLine(), d2.getOriginPosition().getStartLine()); + } + } + } + +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java index fecebb58c81..5efa84f0c1c 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java @@ -23,6 +23,7 @@ package org.sonar.server.computation.source; import org.sonar.batch.protocol.output.BatchReport; import org.sonar.server.source.db.FileSourceDb; +import java.io.Serializable; import java.util.Collections; import java.util.Comparator; import java.util.List; @@ -101,7 +102,7 @@ public class SymbolsLineReader implements LineReader { return map; } - private static class SymbolsDuplication implements Comparator { + private static class SymbolsDuplication implements Comparator, Serializable { @Override public int compare(BatchReport.Symbols.Symbol o1, BatchReport.Symbols.Symbol o2) { if (o1.getDeclaration().getStartLine() == o2.getDeclaration().getStartLine()) { 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 d6732477793..97dc0d599e7 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 @@ -177,6 +177,7 @@ public class PersistFileSourcesStep implements ComputationStep { BatchReport.Scm scmReport = reportReader.readComponentScm(componentRef); File highlightingFile = reportReader.readComponentSyntaxHighlighting(componentRef); List symbols = reportReader.readComponentSymbols(componentRef); + List duplications = reportReader.readComponentDuplications(componentRef); if (coverageFile != null) { ReportIterator coverageReportIterator = new ReportIterator<>(coverageFile, BatchReport.Coverage.PARSER); @@ -191,6 +192,9 @@ public class PersistFileSourcesStep implements ComputationStep { reportIterators.add(syntaxHighlightingReportIterator); lineReaders.add(new HighlightingLineReader(syntaxHighlightingReportIterator)); } + if (!duplications.isEmpty()) { + lineReaders.add(new DuplicationLineReader(duplications)); + } if (!symbols.isEmpty()) { lineReaders.add(new SymbolsLineReader(newArrayList(symbols))); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/DuplicationLineReaderTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/DuplicationLineReaderTest.java new file mode 100644 index 00000000000..8a462856e44 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/DuplicationLineReaderTest.java @@ -0,0 +1,255 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.computation.source; + +import org.junit.Test; +import org.sonar.batch.protocol.output.BatchReport; +import org.sonar.server.source.db.FileSourceDb; + +import java.util.Collections; + +import static com.google.common.collect.Lists.newArrayList; +import static org.assertj.core.api.Assertions.assertThat; + +public class DuplicationLineReaderTest { + + FileSourceDb.Data.Builder sourceData = FileSourceDb.Data.newBuilder(); + FileSourceDb.Line.Builder line1 = sourceData.addLinesBuilder().setSource("line1").setLine(1); + FileSourceDb.Line.Builder line2 = sourceData.addLinesBuilder().setSource("line2").setLine(2); + FileSourceDb.Line.Builder line3 = sourceData.addLinesBuilder().setSource("line3").setLine(3); + FileSourceDb.Line.Builder line4 = sourceData.addLinesBuilder().setSource("line4").setLine(4); + + @Test + public void read_nothing() throws Exception { + DuplicationLineReader reader = new DuplicationLineReader(Collections.emptyList()); + + reader.read(line1); + + assertThat(line1.getDuplicationList()).isEmpty(); + } + + @Test + public void read_duplication_with_duplicates_on_same_file() throws Exception { + DuplicationLineReader reader = new DuplicationLineReader(newArrayList( + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(2) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setRange(BatchReport.Range.newBuilder() + .setStartLine(3) + .setEndLine(4) + .build()) + .build()) + .build() + )); + + reader.read(line1); + reader.read(line2); + reader.read(line3); + reader.read(line4); + + assertThat(line1.getDuplicationList()).containsExactly(1); + assertThat(line2.getDuplicationList()).containsExactly(1); + assertThat(line3.getDuplicationList()).containsExactly(2); + assertThat(line4.getDuplicationList()).containsExactly(2); + } + + @Test + public void read_duplication_with_duplicates_on_other_file() throws Exception { + DuplicationLineReader reader = new DuplicationLineReader(newArrayList( + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(2) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setOtherFileRef(2) + .setRange(BatchReport.Range.newBuilder() + .setStartLine(3) + .setEndLine(4) + .build()) + .build()) + .build() + )); + + reader.read(line1); + reader.read(line2); + reader.read(line3); + reader.read(line4); + + assertThat(line1.getDuplicationList()).containsExactly(1); + assertThat(line2.getDuplicationList()).containsExactly(1); + assertThat(line3.getDuplicationList()).isEmpty(); + assertThat(line4.getDuplicationList()).isEmpty(); + } + + @Test + public void read_duplication_with_duplicates_on_other_file_from_other_project() throws Exception { + DuplicationLineReader reader = new DuplicationLineReader(newArrayList( + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(2) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setOtherFileKey("other-component-key-from-another-project") + .setRange(BatchReport.Range.newBuilder() + .setStartLine(3) + .setEndLine(4) + .build()) + .build()) + .build() + )); + + reader.read(line1); + reader.read(line2); + reader.read(line3); + reader.read(line4); + + assertThat(line1.getDuplicationList()).containsExactly(1); + assertThat(line2.getDuplicationList()).containsExactly(1); + assertThat(line3.getDuplicationList()).isEmpty(); + assertThat(line4.getDuplicationList()).isEmpty(); + } + + @Test + public void read_many_duplications() throws Exception { + DuplicationLineReader reader = new DuplicationLineReader(newArrayList( + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(1) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setRange(BatchReport.Range.newBuilder() + .setStartLine(2) + .setEndLine(2) + .build()) + .build()) + .build(), + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(2) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setRange(BatchReport.Range.newBuilder() + .setStartLine(3) + .setEndLine(4) + .build()) + .build()) + .build() + )); + + reader.read(line1); + reader.read(line2); + reader.read(line3); + reader.read(line4); + + assertThat(line1.getDuplicationList()).containsExactly(1, 3); + assertThat(line2.getDuplicationList()).containsExactly(2, 3); + assertThat(line3.getDuplicationList()).containsExactly(4); + assertThat(line4.getDuplicationList()).containsExactly(4); + } + + @Test + public void should_be_sorted_by_line_block() throws Exception { + DuplicationLineReader reader = new DuplicationLineReader(newArrayList( + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(2) + .setEndLine(2) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setRange(BatchReport.Range.newBuilder() + .setStartLine(4) + .setEndLine(4) + .build()) + .build()) + .build(), + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(1) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setRange(BatchReport.Range.newBuilder() + .setStartLine(3) + .setEndLine(3) + .build()) + .build()) + .build() + )); + + reader.read(line1); + reader.read(line2); + reader.read(line3); + reader.read(line4); + + assertThat(line1.getDuplicationList()).containsExactly(1); + assertThat(line2.getDuplicationList()).containsExactly(3); + assertThat(line3.getDuplicationList()).containsExactly(2); + assertThat(line4.getDuplicationList()).containsExactly(4); + } + + @Test + public void should_be_sorted_by_line_length() throws Exception { + DuplicationLineReader reader = new DuplicationLineReader(newArrayList( + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(2) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setRange(BatchReport.Range.newBuilder() + .setStartLine(3) + .setEndLine(4) + .build()) + .build()) + .build(), + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(1) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setRange(BatchReport.Range.newBuilder() + .setStartLine(4) + .setEndLine(4) + .build()) + .build()) + .build() + )); + + reader.read(line1); + reader.read(line2); + reader.read(line3); + reader.read(line4); + + assertThat(line1.getDuplicationList()).containsExactly(1, 3); + assertThat(line2.getDuplicationList()).containsExactly(3); + assertThat(line3.getDuplicationList()).containsExactly(4); + assertThat(line4.getDuplicationList()).containsExactly(2, 4); + } + +} 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 b75846f1a1e..03d156daca1 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 @@ -141,7 +141,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { .setRef(FILE_REF) .setType(Constants.ComponentType.FILE) .setUuid(FILE_UUID) - // Lines is set to 3 but only 2 lines are read from the file -> the last lines should be added + // Lines is set to 3 but only 2 lines are read from the file -> the last lines should be added .setLines(3) .build()); @@ -232,13 +232,13 @@ public class PersistFileSourcesStepTest extends BaseStepTest { BatchReportWriter writer = initBasicReport(1); writer.writeComponentSyntaxHighlighting(FILE_REF, newArrayList(BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.Range.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(2).setEndOffset(4) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build() - )); + .setRange(BatchReport.Range.newBuilder() + .setStartLine(1).setEndLine(1) + .setStartOffset(2).setEndOffset(4) + .build()) + .setType(Constants.HighlightingType.ANNOTATION) + .build() + )); sut.execute(new ComputationContext(new BatchReportReader(reportDir), ComponentTesting.newProjectDto(PROJECT_UUID))); @@ -261,10 +261,10 @@ public class PersistFileSourcesStepTest extends BaseStepTest { .setStartLine(1).setEndLine(1).setStartOffset(2).setEndOffset(4) .build()) .addReference(BatchReport.Range.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(1).setEndOffset(3) - .build() + .setStartLine(3).setEndLine(3).setStartOffset(1).setEndOffset(3) + .build() ).build() - )); + )); sut.execute(new ComputationContext(new BatchReportReader(reportDir), ComponentTesting.newProjectDto(PROJECT_UUID))); @@ -279,6 +279,36 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(data.getLines(2).getSymbols()).isEqualTo("1,3,1"); } + @Test + public void persist_duplication() throws Exception { + BatchReportWriter writer = initBasicReport(1); + + writer.writeComponentDuplications(FILE_REF, newArrayList( + BatchReport.Duplication.newBuilder() + .setOriginPosition(BatchReport.Range.newBuilder() + .setStartLine(1) + .setEndLine(2) + .build()) + .addDuplicate(BatchReport.Duplicate.newBuilder() + .setRange(BatchReport.Range.newBuilder() + .setStartLine(3) + .setEndLine(4) + .build()) + .build()) + .build() + )); + + sut.execute(new ComputationContext(new BatchReportReader(reportDir), ComponentTesting.newProjectDto(PROJECT_UUID))); + + assertThat(dbTester.countRowsOfTable("file_sources")).isEqualTo(1); + FileSourceDto fileSourceDto = dbClient.fileSourceDao().select(FILE_UUID); + FileSourceDb.Data data = FileSourceDto.decodeData(fileSourceDto.getBinaryData()); + + assertThat(data.getLinesList()).hasSize(1); + + assertThat(data.getLines(0).getDuplicationList()).hasSize(1); + } + @Test public void not_update_sources_when_nothing_has_changed() throws Exception { // Existing sources -- 2.39.5