diff options
author | Julien Lancelot <julien.lancelot@sonarsource.com> | 2015-10-20 12:47:24 +0200 |
---|---|---|
committer | Julien Lancelot <julien.lancelot@sonarsource.com> | 2015-10-21 18:45:43 +0200 |
commit | c69b3c6c5c3cfe819dd2f3eb4b3c23254647c2f8 (patch) | |
tree | 8d8cb06853852343ab22828768643ecbe803c881 /server/sonar-server | |
parent | 146b8393422e64b7a8a4be3a894e0ef0f8caca5c (diff) | |
download | sonarqube-c69b3c6c5c3cfe819dd2f3eb4b3c23254647c2f8.tar.gz sonarqube-c69b3c6c5c3cfe819dd2f3eb4b3c23254647c2f8.zip |
SONAR-6318 Don't fail when bad highlighting/symbols
If highlighting data doesn't match source code data (for instance in the case of a bad encoding), the compute engine will not fail anymore but will instead display a warning and then ignore the remaining highlighting
Diffstat (limited to 'server/sonar-server')
8 files changed, 490 insertions, 450 deletions
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java index 3286c6d3c0b..60dc74a1cc1 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java @@ -21,23 +21,27 @@ package org.sonar.server.computation.source; import com.google.common.collect.ImmutableMap; -import org.sonar.batch.protocol.Constants; -import org.sonar.batch.protocol.output.BatchReport; -import org.sonar.db.protobuf.DbFileSources; - -import javax.annotation.CheckForNull; - import java.util.Iterator; import java.util.List; import java.util.Map; +import javax.annotation.CheckForNull; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; +import org.sonar.batch.protocol.Constants; +import org.sonar.batch.protocol.output.BatchReport; +import org.sonar.db.protobuf.DbFileSources; +import org.sonar.server.computation.source.RangeOffsetConverter.RangeOffsetConverterException; import static com.google.common.collect.Lists.newArrayList; -import static org.sonar.server.computation.source.RangeOffsetHelper.OFFSET_SEPARATOR; -import static org.sonar.server.computation.source.RangeOffsetHelper.SYMBOLS_SEPARATOR; -import static org.sonar.server.computation.source.RangeOffsetHelper.offsetToString; +import static org.sonar.server.computation.source.RangeOffsetConverter.OFFSET_SEPARATOR; +import static org.sonar.server.computation.source.RangeOffsetConverter.SYMBOLS_SEPARATOR; public class HighlightingLineReader implements LineReader { + private static final Logger LOG = Loggers.get(HighlightingLineReader.class); + + private boolean isHighlightingValid = true; + private static final Map<Constants.HighlightingType, String> cssClassByType = ImmutableMap.<Constants.HighlightingType, String>builder() .put(Constants.HighlightingType.ANNOTATION, "a") .put(Constants.HighlightingType.CONSTANT, "c") @@ -51,17 +55,31 @@ public class HighlightingLineReader implements LineReader { .build(); private final Iterator<BatchReport.SyntaxHighlighting> lineHighlightingIterator; + private final RangeOffsetConverter rangeOffsetConverter; + private final List<BatchReport.SyntaxHighlighting> highlightingList; private BatchReport.SyntaxHighlighting currentItem; - private List<BatchReport.SyntaxHighlighting> highlightingList; - public HighlightingLineReader(Iterator<BatchReport.SyntaxHighlighting> lineHighlightingIterator) { + public HighlightingLineReader(Iterator<BatchReport.SyntaxHighlighting> lineHighlightingIterator, RangeOffsetConverter rangeOffsetConverter) { this.lineHighlightingIterator = lineHighlightingIterator; + this.rangeOffsetConverter = rangeOffsetConverter; this.highlightingList = newArrayList(); } @Override public void read(DbFileSources.Line.Builder lineBuilder) { + if (!isHighlightingValid) { + return; + } + try { + processHighlightings(lineBuilder); + } catch (RangeOffsetConverterException e) { + isHighlightingValid = false; + LOG.warn("Inconsistency detected in Highlighting data. Highlighting will be ignored", e); + } + } + + private void processHighlightings(DbFileSources.Line.Builder lineBuilder) { int line = lineBuilder.getLine(); StringBuilder highlighting = new StringBuilder(); @@ -74,14 +92,16 @@ public class HighlightingLineReader implements LineReader { } } - private static void processHighlighting(Iterator<BatchReport.SyntaxHighlighting> syntaxHighlightingIterator, StringBuilder highlighting, + private void processHighlighting(Iterator<BatchReport.SyntaxHighlighting> syntaxHighlightingIterator, StringBuilder highlighting, DbFileSources.Line.Builder lineBuilder) { BatchReport.SyntaxHighlighting syntaxHighlighting = syntaxHighlightingIterator.next(); int line = lineBuilder.getLine(); BatchReport.TextRange range = syntaxHighlighting.getRange(); if (range.getStartLine() <= line) { - String offsets = offsetToString(syntaxHighlighting.getRange(), line, lineBuilder.getSource().length()); - if (!offsets.isEmpty()) { + String offsets = rangeOffsetConverter.offsetToString(syntaxHighlighting.getRange(), line, lineBuilder.getSource().length()); + if (offsets.isEmpty()) { + syntaxHighlightingIterator.remove(); + } else { if (highlighting.length() > 0) { highlighting.append(SYMBOLS_SEPARATOR); } @@ -91,8 +111,6 @@ public class HighlightingLineReader implements LineReader { if (range.getEndLine() == line) { syntaxHighlightingIterator.remove(); } - } else { - syntaxHighlightingIterator.remove(); } } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetConverter.java b/server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetConverter.java index 368320b6932..da4dfa7fbce 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetConverter.java +++ b/server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetConverter.java @@ -29,11 +29,7 @@ public class RangeOffsetConverter { static final String OFFSET_SEPARATOR = ","; static final String SYMBOLS_SEPARATOR = ";"; - private RangeOffsetConverter() { - // Only static methods - } - - public static String offsetToString(BatchReport.TextRange range, int lineIndex, int lineLength) { + public String offsetToString(BatchReport.TextRange range, int lineIndex, int lineLength) { StringBuilder element = new StringBuilder(); validateOffsetOrder(range, lineIndex); @@ -52,20 +48,29 @@ public class RangeOffsetConverter { } private static void validateOffsetOrder(BatchReport.TextRange range, int line) { - if (range.getStartLine() == range.getEndLine() && range.getStartOffset() > range.getEndOffset()) { - throw new IllegalArgumentException(format("End offset %s cannot be defined before start offset %s on line %s", range.getEndOffset(), range.getStartOffset(), line)); - } + checkExpression(range.getStartLine() != range.getEndLine() || range.getStartOffset() <= range.getEndOffset(), + "End offset %s cannot be defined before start offset %s on line %s", range.getEndOffset(), range.getStartOffset(), line); } private static void validateStartOffsetNotGreaterThanLineLength(BatchReport.TextRange range, int lineLength, int line) { - if (range.getStartLine() == line && range.getStartOffset() > lineLength) { - throw new IllegalArgumentException(format("Start offset %s is defined outside the length (%s) of the line %s", range.getStartOffset(), lineLength, line)); - } + checkExpression(range.getStartLine() != line || range.getStartOffset() <= lineLength, + "Start offset %s is defined outside the length (%s) of the line %s", range.getStartOffset(), lineLength, line); } private static void validateEndOffsetNotGreaterThanLineLength(BatchReport.TextRange range, int lineLength, int line) { - if (range.getEndLine() == line && range.getEndOffset() > lineLength) { - throw new IllegalArgumentException(format("End offset %s is defined outside the length (%s) of the line %s", range.getEndOffset(), lineLength, line)); + checkExpression(range.getEndLine() != line || range.getEndOffset() <= lineLength, + "End offset %s is defined outside the length (%s) of the line %s", range.getEndOffset(), lineLength, line); + } + + private static void checkExpression(boolean expression, String errorMessageTemplate, Object... errorMessageArgs) { + if (!expression) { + throw new RangeOffsetConverterException(format(errorMessageTemplate, errorMessageArgs)); + } + } + + public static class RangeOffsetConverterException extends RuntimeException { + public RangeOffsetConverterException(String message) { + super(message); } } 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 83586978b91..78ae951f56e 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 @@ -21,9 +21,6 @@ package org.sonar.server.computation.source; import com.google.common.collect.Lists; -import org.sonar.batch.protocol.output.BatchReport; -import org.sonar.db.protobuf.DbFileSources; - import java.io.Serializable; import java.util.ArrayList; import java.util.Collections; @@ -34,17 +31,26 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; +import org.sonar.api.utils.log.Logger; +import org.sonar.api.utils.log.Loggers; +import org.sonar.batch.protocol.output.BatchReport; +import org.sonar.db.protobuf.DbFileSources; -import static org.sonar.server.computation.source.RangeOffsetHelper.OFFSET_SEPARATOR; -import static org.sonar.server.computation.source.RangeOffsetHelper.SYMBOLS_SEPARATOR; -import static org.sonar.server.computation.source.RangeOffsetHelper.offsetToString; +import static org.sonar.server.computation.source.RangeOffsetConverter.OFFSET_SEPARATOR; +import static org.sonar.server.computation.source.RangeOffsetConverter.SYMBOLS_SEPARATOR; public class SymbolsLineReader implements LineReader { + private static final Logger LOG = Loggers.get(HighlightingLineReader.class); + + private final RangeOffsetConverter rangeOffsetConverter; private final List<BatchReport.Symbol> symbols; private final Map<BatchReport.Symbol, Integer> idsBySymbol; - public SymbolsLineReader(Iterator<BatchReport.Symbol> symbols) { + private boolean areSymbolsValid = true; + + public SymbolsLineReader(Iterator<BatchReport.Symbol> symbols, RangeOffsetConverter rangeOffsetConverter) { + this.rangeOffsetConverter = rangeOffsetConverter; this.symbols = Lists.newArrayList(symbols); // Sort symbols to have deterministic results and avoid false variation that would lead to an unnecessary update of the source files // data @@ -55,6 +61,18 @@ public class SymbolsLineReader implements LineReader { @Override public void read(DbFileSources.Line.Builder lineBuilder) { + if (!areSymbolsValid) { + return; + } + try { + processSymbols(lineBuilder); + } catch (RangeOffsetConverter.RangeOffsetConverterException e) { + areSymbolsValid = false; + LOG.warn("Inconsistency detected in Symbols data. Symbols will be ignored", e); + } + } + + private void processSymbols(DbFileSources.Line.Builder lineBuilder) { int line = lineBuilder.getLine(); List<BatchReport.Symbol> lineSymbols = findSymbolsMatchingLine(line); for (BatchReport.Symbol lineSymbol : lineSymbols) { @@ -72,9 +90,9 @@ public class SymbolsLineReader implements LineReader { } } - private static void appendSymbol(StringBuilder lineSymbol, BatchReport.TextRange range, int line, int symbolId, String sourceLine) { + private void appendSymbol(StringBuilder lineSymbol, BatchReport.TextRange range, int line, int symbolId, String sourceLine) { if (matchLine(range, line)) { - String offsets = offsetToString(range, line, sourceLine.length()); + String offsets = rangeOffsetConverter.offsetToString(range, line, sourceLine.length()); if (!offsets.isEmpty()) { if (lineSymbol.length() > 0) { lineSymbol.append(SYMBOLS_SEPARATOR); 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 79cca8b1676..93865e41336 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 @@ -55,6 +55,7 @@ import org.sonar.server.computation.source.CoverageLineReader; import org.sonar.server.computation.source.DuplicationLineReader; import org.sonar.server.computation.source.HighlightingLineReader; import org.sonar.server.computation.source.LineReader; +import org.sonar.server.computation.source.RangeOffsetConverter; import org.sonar.server.computation.source.ScmLineReader; import org.sonar.server.computation.source.SourceLinesRepository; import org.sonar.server.computation.source.SymbolsLineReader; @@ -215,13 +216,14 @@ public class PersistFileSourcesStep implements ComputationStep { this.scmLineReader = null; } + RangeOffsetConverter rangeOffsetConverter = new RangeOffsetConverter(); CloseableIterator<BatchReport.SyntaxHighlighting> highlightingIt = reportReader.readComponentSyntaxHighlighting(componentRef); closeables.add(highlightingIt); - readers.add(new HighlightingLineReader(highlightingIt)); + readers.add(new HighlightingLineReader(highlightingIt, rangeOffsetConverter)); CloseableIterator<BatchReport.Symbol> symbolsIt = reportReader.readComponentSymbols(componentRef); closeables.add(symbolsIt); - readers.add(new SymbolsLineReader(symbolsIt)); + readers.add(new SymbolsLineReader(symbolsIt, rangeOffsetConverter)); CloseableIterator<BatchReport.Duplication> duplicationsIt = reportReader.readComponentDuplications(componentRef); closeables.add(duplicationsIt); diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java index dcbec7dfebc..2f5485f0a8d 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java @@ -20,20 +20,52 @@ package org.sonar.server.computation.source; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; +import java.util.Map; +import java.util.Random; +import org.junit.Rule; import org.junit.Test; +import org.sonar.api.utils.log.LogTester; import org.sonar.batch.protocol.Constants; import org.sonar.batch.protocol.output.BatchReport; +import org.sonar.batch.protocol.output.BatchReport.TextRange; import org.sonar.db.protobuf.DbFileSources; +import org.sonar.server.computation.source.RangeOffsetConverter.RangeOffsetConverterException; -import java.util.Collections; - -import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.ImmutableMap.of; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.sonar.api.utils.log.LoggerLevel.WARN; +import static org.sonar.batch.protocol.Constants.HighlightingType.ANNOTATION; +import static org.sonar.batch.protocol.Constants.HighlightingType.COMMENT; +import static org.sonar.batch.protocol.Constants.HighlightingType.CONSTANT; +import static org.sonar.batch.protocol.Constants.HighlightingType.HIGHLIGHTING_STRING; import static org.sonar.db.protobuf.DbFileSources.Data.newBuilder; public class HighlightingLineReaderTest { + @Rule + public LogTester logTester = new LogTester(); + + static final int DEFAULT_LINE_LENGTH = 5; + + static final int LINE_1 = 1; + static final int LINE_2 = 2; + static final int LINE_3 = 3; + static final int LINE_4 = 4; + + static final String RANGE_LABEL_1 = "1,2"; + static final String RANGE_LABEL_2 = "2,3"; + static final String RANGE_LABEL_3 = "3,4"; + static final String RANGE_LABEL_4 = "0,2"; + static final String RANGE_LABEL_5 = "0,3"; + + RangeOffsetConverter rangeOffsetConverter = mock(RangeOffsetConverter.class); + DbFileSources.Data.Builder sourceData = newBuilder(); DbFileSources.Line.Builder line1 = sourceData.addLinesBuilder().setSource("line1").setLine(1); DbFileSources.Line.Builder line2 = sourceData.addLinesBuilder().setSource("line2").setLine(2); @@ -42,7 +74,7 @@ public class HighlightingLineReaderTest { @Test public void nothing_to_read() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(Collections.<BatchReport.SyntaxHighlighting>emptyList().iterator()); + HighlightingLineReader highlightingLineReader = newReader(Collections.<TextRange, Constants.HighlightingType>emptyMap()); DbFileSources.Line.Builder lineBuilder = newBuilder().addLinesBuilder().setLine(1); highlightingLineReader.read(lineBuilder); @@ -52,236 +84,180 @@ public class HighlightingLineReaderTest { @Test public void read_one_line() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(2).setEndOffset(4) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build()).iterator()); + HighlightingLineReader highlightingLineReader = newReader(of( + newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_1), + ANNOTATION)); highlightingLineReader.read(line1); - assertThat(line1.getHighlighting()).isEqualTo("2,4,a"); + assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); } @Test public void read_many_lines() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(0).setEndOffset(4) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build(), - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(2).setEndLine(2) - .setStartOffset(0).setEndOffset(1) - .build()) - .setType(Constants.HighlightingType.COMMENT) - .build(), - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(4).setEndLine(4) - .setStartOffset(1).setEndOffset(2) - .build()) - .setType(Constants.HighlightingType.CONSTANT) - .build()).iterator()); + HighlightingLineReader highlightingLineReader = newReader(of( + newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_1), ANNOTATION, + newSingleLineTextRangeWithExpectingLabel(LINE_2, RANGE_LABEL_2), COMMENT, + newSingleLineTextRangeWithExpectingLabel(LINE_4, RANGE_LABEL_3), CONSTANT + )); highlightingLineReader.read(line1); highlightingLineReader.read(line2); highlightingLineReader.read(line3); highlightingLineReader.read(line4); - assertThat(line1.getHighlighting()).isEqualTo("0,4,a"); - assertThat(line2.getHighlighting()).isEqualTo("0,1,cd"); - assertThat(line4.getHighlighting()).isEqualTo("1,2,c"); + assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); + assertThat(line2.getHighlighting()).isEqualTo(RANGE_LABEL_2 + ",cd"); + assertThat(line4.getHighlighting()).isEqualTo(RANGE_LABEL_3 + ",c"); } @Test public void read_many_syntax_highlighting_on_same_line() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(2).setEndOffset(3) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build(), - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(4).setEndOffset(5) - .build()) - .setType(Constants.HighlightingType.COMMENT) - .build()).iterator()); + HighlightingLineReader highlightingLineReader = newReader(of( + newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_1), ANNOTATION, + newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_2), COMMENT + )); highlightingLineReader.read(line1); - assertThat(line1.getHighlighting()).isEqualTo("2,3,a;4,5,cd"); - } - - @Test - public void read_nested_syntax_highlighting_on_same_line() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(0).setEndOffset(4) - .build()) - .setType(Constants.HighlightingType.CONSTANT) - .build(), - // This highlighting is nested in previous one - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(2).setEndOffset(3) - .build()) - .setType(Constants.HighlightingType.KEYWORD) - .build()).iterator()); - - highlightingLineReader.read(line1); - - assertThat(line1.getHighlighting()).isEqualTo("0,4,c;2,3,k"); + assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a;" + RANGE_LABEL_2 + ",cd"); } @Test public void read_one_syntax_highlighting_on_many_lines() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - // This highlighting begin on line 1 and finish on line 3 - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(3) - .setStartOffset(3).setEndOffset(2) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build()).iterator()); + // This highlighting begin on line 1 and finish on line 3 + TextRange textRange = newTextRange(LINE_1, LINE_3); + when(rangeOffsetConverter.offsetToString(textRange, LINE_1, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_1); + when(rangeOffsetConverter.offsetToString(textRange, LINE_2, 6)).thenReturn(RANGE_LABEL_2); + when(rangeOffsetConverter.offsetToString(textRange, LINE_3, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_3); + + HighlightingLineReader highlightingLineReader = newReader(of(textRange, ANNOTATION)); highlightingLineReader.read(line1); DbFileSources.Line.Builder line2 = sourceData.addLinesBuilder().setSource("line 2").setLine(2); highlightingLineReader.read(line2); highlightingLineReader.read(line3); - assertThat(line1.getHighlighting()).isEqualTo("3,5,a"); - assertThat(line2.getHighlighting()).isEqualTo("0,6,a"); - assertThat(line3.getHighlighting()).isEqualTo("0,2,a"); + assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); + assertThat(line2.getHighlighting()).isEqualTo(RANGE_LABEL_2 + ",a"); + assertThat(line3.getHighlighting()).isEqualTo(RANGE_LABEL_3 + ",a"); } @Test public void read_many_syntax_highlighting_on_many_lines() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(3) - .setStartOffset(3).setEndOffset(2) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build(), - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(2).setEndLine(4) - .setStartOffset(0).setEndOffset(3) - .build()) - .setType(Constants.HighlightingType.HIGHLIGHTING_STRING) - .build(), - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(2).setEndLine(2) - .setStartOffset(1).setEndOffset(2) - .build()) - .setType(Constants.HighlightingType.COMMENT) - .build()).iterator()); + TextRange textRange1 = newTextRange(LINE_1, LINE_3); + when(rangeOffsetConverter.offsetToString(textRange1, LINE_1, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_1); + when(rangeOffsetConverter.offsetToString(textRange1, LINE_2, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_2); + when(rangeOffsetConverter.offsetToString(textRange1, LINE_3, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_3); + + TextRange textRange2 = newTextRange(LINE_2, LINE_4); + when(rangeOffsetConverter.offsetToString(textRange2, LINE_2, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_2); + when(rangeOffsetConverter.offsetToString(textRange2, LINE_3, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_2); + when(rangeOffsetConverter.offsetToString(textRange2, LINE_4, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_4); + + TextRange textRange3 = newTextRange(LINE_2, LINE_2); + when(rangeOffsetConverter.offsetToString(textRange3, LINE_2, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_5); + + HighlightingLineReader highlightingLineReader = newReader(of( + textRange1, ANNOTATION, + textRange2, HIGHLIGHTING_STRING, + textRange3, COMMENT + )); highlightingLineReader.read(line1); highlightingLineReader.read(line2); highlightingLineReader.read(line3); highlightingLineReader.read(line4); - assertThat(line1.getHighlighting()).isEqualTo("3,5,a"); - assertThat(line2.getHighlighting()).isEqualTo("0,5,a;0,5,s;1,2,cd"); - assertThat(line3.getHighlighting()).isEqualTo("0,2,a;0,5,s"); - assertThat(line4.getHighlighting()).isEqualTo("0,3,s"); + assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); + assertThat(line2.getHighlighting()).isEqualTo(RANGE_LABEL_2 + ",a;" + RANGE_LABEL_2 + ",s;" + RANGE_LABEL_5 + ",cd"); + assertThat(line3.getHighlighting()).isEqualTo(RANGE_LABEL_3 + ",a;" + RANGE_LABEL_2 + ",s"); + assertThat(line4.getHighlighting()).isEqualTo(RANGE_LABEL_4 + ",s"); } @Test public void read_highlighting_declared_on_a_whole_line() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(2) - .setStartOffset(0).setEndOffset(0) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build()).iterator()); + TextRange textRange = newTextRange(LINE_1, LINE_2); + when(rangeOffsetConverter.offsetToString(textRange, LINE_1, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_1); + when(rangeOffsetConverter.offsetToString(textRange, LINE_2, DEFAULT_LINE_LENGTH)).thenReturn(""); + + HighlightingLineReader highlightingLineReader = newReader(of(textRange, ANNOTATION)); highlightingLineReader.read(line1); highlightingLineReader.read(line2); highlightingLineReader.read(line3); - assertThat(line1.getHighlighting()).isEqualTo("0,5,a"); + assertThat(line1.getHighlighting()).isEqualTo(RANGE_LABEL_1 + ",a"); // Nothing should be set on line 2 assertThat(line2.getHighlighting()).isEmpty(); assertThat(line3.getHighlighting()).isEmpty(); } @Test - public void fail_when_end_offset_is_before_start_offset() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(4).setEndOffset(2) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build()).iterator()); - - try { - highlightingLineReader.read(line1); - failBecauseExceptionWasNotThrown(IllegalArgumentException.class); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("End offset 2 cannot be defined before start offset 4 on line 1"); - } + public void not_fail_and_stop_processing_when_range_offset_converter_throw_RangeOffsetConverterException() { + TextRange textRange1 = newTextRange(LINE_1, LINE_1); + doThrow(RangeOffsetConverterException.class).when(rangeOffsetConverter).offsetToString(textRange1, LINE_1, DEFAULT_LINE_LENGTH); + + HighlightingLineReader highlightingLineReader = newReader(of( + textRange1, ANNOTATION, + newSingleLineTextRangeWithExpectingLabel(LINE_2, RANGE_LABEL_1), HIGHLIGHTING_STRING)); + + highlightingLineReader.read(line1); + highlightingLineReader.read(line2); + + assertNoHighlighting(); + assertThat(logTester.logs(WARN)).isNotEmpty(); } @Test - public void fail_when_end_offset_is_higher_than_line_length() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(2).setEndOffset(10) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build()).iterator()); - - try { - highlightingLineReader.read(line1); - failBecauseExceptionWasNotThrown(IllegalArgumentException.class); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("End offset 10 is defined outside the length (5) of the line 1"); - } + public void keep_existing_processed_highlighting_when_range_offset_converter_throw_RangeOffsetConverterException() { + TextRange textRange2 = newTextRange(LINE_2, LINE_2); + doThrow(RangeOffsetConverterException.class).when(rangeOffsetConverter).offsetToString(textRange2, LINE_2, DEFAULT_LINE_LENGTH); + + HighlightingLineReader highlightingLineReader = newReader(of( + newSingleLineTextRangeWithExpectingLabel(LINE_1, RANGE_LABEL_1), ANNOTATION, + textRange2, HIGHLIGHTING_STRING + )); + + highlightingLineReader.read(line1); + highlightingLineReader.read(line2); + + assertThat(line1.hasHighlighting()).isTrue(); + assertThat(line2.hasHighlighting()).isFalse(); + assertThat(logTester.logs(WARN)).isNotEmpty(); } - @Test - public void fail_when_start_offset_is_higher_than_line_length() { - HighlightingLineReader highlightingLineReader = new HighlightingLineReader(newArrayList( - BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(10).setEndOffset(11) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build()).iterator()); - - try { - highlightingLineReader.read(line1); - failBecauseExceptionWasNotThrown(IllegalArgumentException.class); - } catch (IllegalArgumentException e) { - assertThat(e).hasMessage("Start offset 10 is defined outside the length (5) of the line 1"); + private HighlightingLineReader newReader(Map<TextRange, Constants.HighlightingType> textRangeByType) { + List<BatchReport.SyntaxHighlighting> syntaxHighlightingList = new ArrayList<>(); + for (Map.Entry<TextRange, Constants.HighlightingType> entry : textRangeByType.entrySet()) { + syntaxHighlightingList.add(BatchReport.SyntaxHighlighting.newBuilder() + .setRange(entry.getKey()) + .setType(entry.getValue()) + .build()); } + return new HighlightingLineReader(syntaxHighlightingList.iterator(), rangeOffsetConverter); + } + + private static TextRange newTextRange(int startLine, int enLine) { + Random random = new Random(); + return TextRange.newBuilder() + .setStartLine(startLine).setEndLine(enLine) + // Offsets are not used by the reader + .setStartOffset(random.nextInt()).setEndOffset(random.nextInt()) + .build(); + } + + private TextRange newSingleLineTextRangeWithExpectingLabel(int line, String rangeLabel) { + TextRange textRange = newTextRange(line, line); + when(rangeOffsetConverter.offsetToString(textRange, line, DEFAULT_LINE_LENGTH)).thenReturn(rangeLabel); + return textRange; + } + + private void assertNoHighlighting() { + assertThat(line1.hasHighlighting()).isFalse(); + assertThat(line2.hasHighlighting()).isFalse(); + assertThat(line3.hasHighlighting()).isFalse(); + assertThat(line4.hasHighlighting()).isFalse(); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetConverterTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetConverterTest.java index 0a7171be0c7..43a2aa46ea3 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetConverterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetConverterTest.java @@ -24,88 +24,103 @@ import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; import org.sonar.batch.protocol.output.BatchReport; +import org.sonar.server.computation.source.RangeOffsetConverter.RangeOffsetConverterException; import static org.assertj.core.api.Assertions.assertThat; -import static org.sonar.server.computation.source.RangeOffsetConverter.offsetToString; public class RangeOffsetConverterTest { @Rule public ExpectedException thrown = ExpectedException.none(); + static final int LINE_1 = 1; + static final int LINE_2 = 2; + static final int LINE_3 = 3; + + static final int OFFSET_0 = 0; + static final int OFFSET_2 = 2; + static final int OFFSET_3 = 3; + static final int OFFSET_4 = 4; + static final int BIG_OFFSET = 10; + + static final int DEFAULT_LINE_LENGTH = 5; + + RangeOffsetConverter underTest = new RangeOffsetConverter(); + + @Test + public void return_range() { + assertThat(underTest.offsetToString(createTextRange(LINE_1, LINE_1, OFFSET_2, OFFSET_3), + LINE_1, DEFAULT_LINE_LENGTH)) + .isEqualTo(OFFSET_2 + "," + OFFSET_3); + } + @Test - public void append_range() { - assertThat(offsetToString(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(2).setEndOffset(3) - .build(), 1, 5)).isEqualTo("2,3"); + public void return_range_not_finishing_in_current_line() { + assertThat(underTest.offsetToString(createTextRange(LINE_1, LINE_3, OFFSET_2, OFFSET_3), + LINE_1, DEFAULT_LINE_LENGTH)) + .isEqualTo(OFFSET_2 + "," + DEFAULT_LINE_LENGTH); } @Test - public void append_range_not_finishing_in_current_line() { - assertThat(offsetToString(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(3) - .setStartOffset(2).setEndOffset(3) - .build(), 1, 5)).isEqualTo("2,5"); + public void return_range_that_began_in_previous_line_and_finish_in_current_line() { + assertThat(underTest.offsetToString(createTextRange(LINE_1, LINE_3, OFFSET_2, OFFSET_3), + LINE_3, DEFAULT_LINE_LENGTH)) + .isEqualTo(OFFSET_0 + "," + OFFSET_3); } @Test - public void append_range_that_began_in_previous_line_and_finish_in_current_line() { - assertThat(offsetToString(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(3) - .setStartOffset(2).setEndOffset(3) - .build(), 3, 5)).isEqualTo("0,3"); + public void return_range_that_began_in_previous_line_and_not_finishing_in_current_line() { + assertThat(underTest.offsetToString(createTextRange(LINE_1, LINE_1, OFFSET_2, OFFSET_3), + LINE_2, DEFAULT_LINE_LENGTH)) + .isEqualTo(OFFSET_0 + "," + DEFAULT_LINE_LENGTH); } @Test - public void append_range_that_began_in_previous_line_and_not_finishing_in_current_line() { - assertThat(offsetToString(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(3) - .setStartOffset(2).setEndOffset(3) - .build(), 2, 5)).isEqualTo("0,5"); + public void return_empty_string_when_offset_is_empty() { + assertThat(underTest.offsetToString(createTextRange(LINE_1, LINE_1, OFFSET_0, OFFSET_0), + LINE_1, DEFAULT_LINE_LENGTH)) + .isEmpty(); } @Test - public void do_nothing_if_offset_is_empty() { - assertThat(offsetToString(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(0).setEndOffset(0) - .build(), 1, 5)).isEmpty(); + public void return_whole_line_offset_when_range_begin_at_first_character_and_ends_at_first_character_of_next_line() { + assertThat(underTest.offsetToString(createTextRange(LINE_1, LINE_2, OFFSET_0, OFFSET_0), + LINE_1, DEFAULT_LINE_LENGTH)) + .isEqualTo(OFFSET_0 + "," + DEFAULT_LINE_LENGTH); } @Test public void fail_when_end_offset_is_before_start_offset() { - thrown.expect(IllegalArgumentException.class); + thrown.expect(RangeOffsetConverterException.class); thrown.expectMessage("End offset 2 cannot be defined before start offset 4 on line 1"); - offsetToString(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(4).setEndOffset(2) - .build(), - 1, 5); + underTest.offsetToString(createTextRange(LINE_1, LINE_1, OFFSET_4, OFFSET_2), + LINE_1, DEFAULT_LINE_LENGTH); } @Test public void fail_when_end_offset_is_higher_than_line_length() { - thrown.expect(IllegalArgumentException.class); + thrown.expect(RangeOffsetConverterException.class); thrown.expectMessage("End offset 10 is defined outside the length (5) of the line 1"); - offsetToString(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(4).setEndOffset(10) - .build(), - 1, 5); + underTest.offsetToString(createTextRange(LINE_1, LINE_1, OFFSET_4, BIG_OFFSET), + LINE_1, DEFAULT_LINE_LENGTH); } @Test public void fail_when_start_offset_is_higher_than_line_length() { - thrown.expect(IllegalArgumentException.class); + thrown.expect(RangeOffsetConverterException.class); thrown.expectMessage("Start offset 10 is defined outside the length (5) of the line 1"); - offsetToString(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - .setStartOffset(10).setEndOffset(11) - .build(), - 1, 5); + underTest.offsetToString(createTextRange(LINE_1, LINE_1, BIG_OFFSET, BIG_OFFSET + 1), + LINE_1, DEFAULT_LINE_LENGTH); } + + private static BatchReport.TextRange createTextRange(int startLine, int enLine, int startOffset, int endOffset) { + return BatchReport.TextRange.newBuilder() + .setStartLine(startLine).setEndLine(enLine) + .setStartOffset(startOffset).setEndOffset(endOffset) + .build(); + } + } diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/source/SymbolsLineReaderTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/source/SymbolsLineReaderTest.java index 4087635d607..d98c2ed6b21 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/computation/source/SymbolsLineReaderTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/computation/source/SymbolsLineReaderTest.java @@ -20,17 +20,45 @@ package org.sonar.server.computation.source; -import java.util.List; +import java.util.Arrays; +import org.junit.Rule; import org.junit.Test; +import org.sonar.api.utils.log.LogTester; import org.sonar.batch.protocol.output.BatchReport; -import org.sonar.core.util.CloseableIterator; +import org.sonar.batch.protocol.output.BatchReport.TextRange; import org.sonar.db.protobuf.DbFileSources; -import static com.google.common.collect.Lists.newArrayList; import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.sonar.api.utils.log.LoggerLevel.WARN; public class SymbolsLineReaderTest { + @Rule + public LogTester logTester = new LogTester(); + + static final int DEFAULT_LINE_LENGTH = 5; + + static final int LINE_1 = 1; + static final int LINE_2 = 2; + static final int LINE_3 = 3; + static final int LINE_4 = 4; + + static final int OFFSET_0 = 0; + static final int OFFSET_1 = 1; + static final int OFFSET_2 = 2; + static final int OFFSET_3 = 3; + static final int OFFSET_4 = 4; + + static final String RANGE_LABEL_1 = "1,2"; + static final String RANGE_LABEL_2 = "2,3"; + static final String RANGE_LABEL_3 = "3,4"; + static final String RANGE_LABEL_4 = "0,2"; + + RangeOffsetConverter rangeOffsetConverter = mock(RangeOffsetConverter.class); + DbFileSources.Data.Builder sourceData = DbFileSources.Data.newBuilder(); DbFileSources.Line.Builder line1 = sourceData.addLinesBuilder().setSource("line1").setLine(1); DbFileSources.Line.Builder line2 = sourceData.addLinesBuilder().setSource("line2").setLine(2); @@ -39,7 +67,7 @@ public class SymbolsLineReaderTest { @Test public void read_nothing() { - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(CloseableIterator.<BatchReport.Symbol>emptyCloseableIterator()); + SymbolsLineReader symbolsLineReader = newReader(); symbolsLineReader.read(line1); @@ -48,250 +76,246 @@ public class SymbolsLineReaderTest { @Test public void read_symbols() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(2).setEndOffset(4) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(1).setEndOffset(3) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + SymbolsLineReader symbolsLineReader = newReader(newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_2, OFFSET_4, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_1, OFFSET_3, RANGE_LABEL_2) + )); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); symbolsLineReader.read(line3); - assertThat(line1.getSymbols()).isEqualTo("2,4,1"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); assertThat(line2.getSymbols()).isEmpty(); - assertThat(line3.getSymbols()).isEqualTo("1,3,1"); + assertThat(line3.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1"); } @Test public void read_symbols_with_reference_on_same_line() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(0).setEndOffset(1) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(2).setEndOffset(3) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + SymbolsLineReader symbolsLineReader = newReader(newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_0, OFFSET_1, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_2, OFFSET_3, RANGE_LABEL_2) + )); + symbolsLineReader.read(line1); - assertThat(line1.getSymbols()).isEqualTo("0,1,1;2,3,1"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1;" + RANGE_LABEL_2 + ",1"); } @Test public void read_symbols_with_two_references() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(2).setEndOffset(4) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(1).setEndOffset(3) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(2).setEndLine(2).setStartOffset(0).setEndOffset(2) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + SymbolsLineReader symbolsLineReader = newReader(newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_2, OFFSET_4, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_1, OFFSET_3, RANGE_LABEL_2), + newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_0, OFFSET_2, RANGE_LABEL_3) + )); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); symbolsLineReader.read(line3); - assertThat(line1.getSymbols()).isEqualTo("2,4,1"); - assertThat(line2.getSymbols()).isEqualTo("0,2,1"); - assertThat(line3.getSymbols()).isEqualTo("1,3,1"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); + assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_3 + ",1"); + assertThat(line3.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1"); } @Test public void read_symbols_with_two_references_on_the_same_line() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(2).setEndOffset(3) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(2).setEndLine(2).setStartOffset(0).setEndOffset(1) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(2).setEndLine(2).setStartOffset(2).setEndOffset(3) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + SymbolsLineReader symbolsLineReader = newReader(newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_2, OFFSET_3, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_0, OFFSET_1, RANGE_LABEL_2), + newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_2, OFFSET_3, RANGE_LABEL_3) + )); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); - assertThat(line1.getSymbols()).isEqualTo("2,3,1"); - assertThat(line2.getSymbols()).isEqualTo("0,1,1;2,3,1"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); + assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1;" + RANGE_LABEL_3 + ",1"); } @Test public void read_symbols_when_reference_line_is_before_declaration_line() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(2).setEndLine(2).setStartOffset(3).setEndOffset(4) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(1).setEndOffset(2) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + SymbolsLineReader symbolsLineReader = newReader(newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_3, OFFSET_4, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_1, OFFSET_2, RANGE_LABEL_2) + )); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); - assertThat(line1.getSymbols()).isEqualTo("1,2,1"); - assertThat(line2.getSymbols()).isEqualTo("3,4,1"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1"); + assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); } @Test public void read_many_symbols_on_lines() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(1).setEndOffset(2) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(2).setEndOffset(3) - .build()) - .build(), - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(3).setEndOffset(4) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(0).setEndOffset(1) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + SymbolsLineReader symbolsLineReader = newReader( + newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_1, OFFSET_2, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_2, OFFSET_3, RANGE_LABEL_2)), + newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_3, OFFSET_4, RANGE_LABEL_3), + newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_0, OFFSET_1, RANGE_LABEL_4) + )); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); symbolsLineReader.read(line3); - assertThat(line1.getSymbols()).isEqualTo("1,2,1;3,4,2"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1;" + RANGE_LABEL_3 + ",2"); assertThat(line2.getSymbols()).isEmpty(); - assertThat(line3.getSymbols()).isEqualTo("2,3,1;0,1,2"); + assertThat(line3.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1;" + RANGE_LABEL_4 + ",2"); } @Test public void symbol_declaration_should_be_sorted_by_offset() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - // This symbol begins after the second symbol, it should appear in second place - .setStartLine(1).setEndLine(1).setStartOffset(2).setEndOffset(3) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(2).setEndOffset(3) - .build()) - .build(), - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(0).setEndOffset(1) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(0).setEndOffset(1) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + SymbolsLineReader symbolsLineReader = newReader( + newSymbol( + // This symbol begins after the second symbol, it should appear in second place + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_2, OFFSET_3, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_2, OFFSET_3, RANGE_LABEL_1)), + newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_0, OFFSET_1, RANGE_LABEL_2), + newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_0, OFFSET_1, RANGE_LABEL_2) + )); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); symbolsLineReader.read(line3); - assertThat(line1.getSymbols()).isEqualTo("0,1,1;2,3,2"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1;" + RANGE_LABEL_1 + ",2"); assertThat(line2.getSymbols()).isEmpty(); - assertThat(line3.getSymbols()).isEqualTo("0,1,1;2,3,2"); + assertThat(line3.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1;" + RANGE_LABEL_1 + ",2"); } @Test public void symbol_declaration_should_be_sorted_by_line() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - // This symbol begins after the second symbol, it should appear in second place - .setStartLine(2).setEndLine(2).setStartOffset(0).setEndOffset(1) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(2).setEndOffset(3) - .build()) - .build(), - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1).setStartOffset(0).setEndOffset(1) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(0).setEndOffset(1) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + SymbolsLineReader symbolsLineReader = newReader( + newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_0, OFFSET_1, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_2, OFFSET_3, RANGE_LABEL_2)), + newSymbol( + newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_0, OFFSET_1, RANGE_LABEL_1), + newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_0, OFFSET_1, RANGE_LABEL_1) + )); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); symbolsLineReader.read(line3); - assertThat(line1.getSymbols()).isEqualTo("0,1,1"); - assertThat(line2.getSymbols()).isEqualTo("0,1,2"); - assertThat(line3.getSymbols()).isEqualTo("0,1,1;2,3,2"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); + assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",2"); + assertThat(line3.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1;" + RANGE_LABEL_2 + ",2"); } @Test public void read_symbols_defined_on_many_lines() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(2).setStartOffset(1).setEndOffset(3) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(4).setStartOffset(1).setEndOffset(3) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + TextRange declaration = newTextRange(LINE_1, LINE_2, OFFSET_1, OFFSET_3); + when(rangeOffsetConverter.offsetToString(declaration, LINE_1, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_1); + when(rangeOffsetConverter.offsetToString(declaration, LINE_2, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_2); + + TextRange reference = newTextRange(LINE_3, LINE_4, OFFSET_1, OFFSET_3); + when(rangeOffsetConverter.offsetToString(reference, LINE_3, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_1); + when(rangeOffsetConverter.offsetToString(reference, LINE_4, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_2); + + SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, reference)); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); symbolsLineReader.read(line3); symbolsLineReader.read(line4); - assertThat(line1.getSymbols()).isEqualTo("1,5,1"); - assertThat(line2.getSymbols()).isEqualTo("0,3,1"); - assertThat(line3.getSymbols()).isEqualTo("1,5,1"); - assertThat(line4.getSymbols()).isEqualTo("0,3,1"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); + assertThat(line2.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1"); + assertThat(line3.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); + assertThat(line4.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1"); } @Test public void read_symbols_declared_on_a_whole_line() { - List<BatchReport.Symbol> symbols = newArrayList( - BatchReport.Symbol.newBuilder() - .setDeclaration(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(2).setStartOffset(0).setEndOffset(0) - .build()) - .addReference(BatchReport.TextRange.newBuilder() - .setStartLine(3).setEndLine(3).setStartOffset(1).setEndOffset(3) - .build()) - .build()); - - SymbolsLineReader symbolsLineReader = new SymbolsLineReader(symbols.iterator()); + TextRange declaration = newTextRange(LINE_1, LINE_2, OFFSET_0, OFFSET_0); + when(rangeOffsetConverter.offsetToString(declaration, LINE_1, DEFAULT_LINE_LENGTH)).thenReturn(RANGE_LABEL_1); + when(rangeOffsetConverter.offsetToString(declaration, LINE_2, DEFAULT_LINE_LENGTH)).thenReturn(""); + TextRange reference = newSingleLineTextRangeWithExpectedLabel(LINE_3, OFFSET_1, OFFSET_3, RANGE_LABEL_2); + + SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, reference)); + symbolsLineReader.read(line1); symbolsLineReader.read(line2); symbolsLineReader.read(line3); symbolsLineReader.read(line4); - assertThat(line1.getSymbols()).isEqualTo("0,5,1"); + assertThat(line1.getSymbols()).isEqualTo(RANGE_LABEL_1 + ",1"); assertThat(line2.getSymbols()).isEmpty(); - assertThat(line3.getSymbols()).isEqualTo("1,3,1"); + assertThat(line3.getSymbols()).isEqualTo(RANGE_LABEL_2 + ",1"); assertThat(line4.getSymbols()).isEmpty(); } + @Test + public void not_fail_and_stop_processing_when_range_offset_converter_throw_RangeOffsetConverterException() { + TextRange declaration = newTextRange(LINE_1, LINE_1, OFFSET_1, OFFSET_3); + doThrow(RangeOffsetConverter.RangeOffsetConverterException.class).when(rangeOffsetConverter).offsetToString(declaration, LINE_1, DEFAULT_LINE_LENGTH); + + TextRange reference = newSingleLineTextRangeWithExpectedLabel(LINE_2, OFFSET_1, OFFSET_3, RANGE_LABEL_2); + + SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, reference)); + + symbolsLineReader.read(line1); + symbolsLineReader.read(line2); + + assertNoSymbol(); + assertThat(logTester.logs(WARN)).isNotEmpty(); + } + + @Test + public void keep_existing_processed_symbols_when_range_offset_converter_throw_RangeOffsetConverterException() { + TextRange declaration = newSingleLineTextRangeWithExpectedLabel(LINE_1, OFFSET_1, OFFSET_3, RANGE_LABEL_2); + + TextRange reference = newTextRange(LINE_2, LINE_2, OFFSET_1, OFFSET_3); + doThrow(RangeOffsetConverter.RangeOffsetConverterException.class).when(rangeOffsetConverter).offsetToString(reference, LINE_2, DEFAULT_LINE_LENGTH); + + SymbolsLineReader symbolsLineReader = newReader(newSymbol(declaration, reference)); + + symbolsLineReader.read(line1); + symbolsLineReader.read(line2); + + assertThat(line1.hasSymbols()).isTrue(); + assertThat(line2.hasSymbols()).isFalse(); + assertThat(logTester.logs(WARN)).isNotEmpty(); + } + + private BatchReport.Symbol newSymbol(TextRange declaration, TextRange... references) { + BatchReport.Symbol.Builder builder = BatchReport.Symbol.newBuilder() + .setDeclaration(declaration); + for (TextRange reference : references) { + builder.addReference(reference); + } + return builder.build(); + } + + private SymbolsLineReader newReader(BatchReport.Symbol... symbols) { + return new SymbolsLineReader(Arrays.asList(symbols).iterator(), rangeOffsetConverter); + } + + private TextRange newSingleLineTextRangeWithExpectedLabel(int line, int startOffset, int endOffset, String rangeLabel) { + TextRange textRange = newTextRange(line, line, startOffset, endOffset); + when(rangeOffsetConverter.offsetToString(textRange, line, DEFAULT_LINE_LENGTH)).thenReturn(rangeLabel); + return textRange; + } + + private static TextRange newTextRange(int startLine, int endLine, int startOffset, int endOffset) { + return TextRange.newBuilder() + .setStartLine(startLine).setEndLine(endLine) + .setStartOffset(startOffset).setEndOffset(endOffset) + .build(); + } + + private void assertNoSymbol() { + assertThat(line1.hasSymbols()).isFalse(); + assertThat(line2.hasSymbols()).isFalse(); + assertThat(line3.hasSymbols()).isFalse(); + assertThat(line4.hasSymbols()).isFalse(); + } + } 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 db6a0748f47..93831680e8d 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 @@ -24,6 +24,7 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.experimental.categories.Category; +import org.junit.rules.ExpectedException; import org.sonar.api.utils.System2; import org.sonar.batch.protocol.Constants; import org.sonar.batch.protocol.output.BatchReport; @@ -44,7 +45,6 @@ import org.sonar.test.DbTests; import static com.google.common.collect.Lists.newArrayList; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.failBecauseExceptionWasNotThrown; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -60,6 +60,9 @@ public class PersistFileSourcesStepTest extends BaseStepTest { private System2 system2 = mock(System2.class); @Rule + public ExpectedException thrown = ExpectedException.none(); + + @Rule public DbTester dbTester = DbTester.create(system2); @Rule public TreeRootHolderRule treeRootHolder = new TreeRootHolderRule(); @@ -370,7 +373,7 @@ public class PersistFileSourcesStepTest extends BaseStepTest { .setProjectUuid(PROJECT_UUID) .setFileUuid(FILE_UUID) .setDataType(Type.SOURCE) - // Source hash is missing, update will be made + // Source hash is missing, update will be made .setLineHashes("137f72c3708c6bd0de00a0e5a69c699b") .setDataHash("29f25900140c94db38035128cb6de6a2") .setSourceData(DbFileSources.Data.newBuilder() @@ -433,27 +436,6 @@ public class PersistFileSourcesStepTest extends BaseStepTest { assertThat(fileSourceDto.getRevision()).isEqualTo("rev-1"); } - @Test - public void display_file_path_when_exception_is_generated() { - initBasicReport(1); - - reportReader.putSyntaxHighlighting(FILE_REF, newArrayList(BatchReport.SyntaxHighlighting.newBuilder() - .setRange(BatchReport.TextRange.newBuilder() - .setStartLine(1).setEndLine(1) - // Wrong offset -> fail - .setStartOffset(4).setEndOffset(2) - .build()) - .setType(Constants.HighlightingType.ANNOTATION) - .build())); - - try { - underTest.execute(); - failBecauseExceptionWasNotThrown(IllegalStateException.class); - } catch (IllegalStateException e) { - assertThat(e).hasMessage("Cannot persist sources of MODULE_KEY:src/Foo.java").hasCauseInstanceOf(IllegalArgumentException.class); - } - } - private void initBasicReport(int numberOfLines) { treeRootHolder.setRoot(ReportComponent.builder(Component.Type.PROJECT, 1).setUuid(PROJECT_UUID).setKey(PROJECT_KEY).addChildren( ReportComponent.builder(Component.Type.MODULE, 2).setUuid("MODULE").setKey("MODULE_KEY").addChildren( |