]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-6318 Don't fail when bad highlighting/symbols
authorJulien Lancelot <julien.lancelot@sonarsource.com>
Tue, 20 Oct 2015 10:47:24 +0000 (12:47 +0200)
committerJulien Lancelot <julien.lancelot@sonarsource.com>
Wed, 21 Oct 2015 16:45:43 +0000 (18:45 +0200)
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

server/sonar-server/src/main/java/org/sonar/server/computation/source/HighlightingLineReader.java
server/sonar-server/src/main/java/org/sonar/server/computation/source/RangeOffsetConverter.java
server/sonar-server/src/main/java/org/sonar/server/computation/source/SymbolsLineReader.java
server/sonar-server/src/main/java/org/sonar/server/computation/step/PersistFileSourcesStep.java
server/sonar-server/src/test/java/org/sonar/server/computation/source/HighlightingLineReaderTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/source/RangeOffsetConverterTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/source/SymbolsLineReaderTest.java
server/sonar-server/src/test/java/org/sonar/server/computation/step/PersistFileSourcesStepTest.java

index 3286c6d3c0b07a3bf53bc87acefc8bf9fcbbf4cd..60dc74a1cc134f277d76c10e419355a3eccf45ee 100644 (file)
 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();
       }
     }
   }
index 368320b6932df1b52d6865c4d5bb271407aa3f92..da4dfa7fbce57b8e2fe10b3fd3129bfb98ba4065 100644 (file)
@@ -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);
     }
   }
 
index 83586978b91b58fdde6f82140f13df960ed4e1f1..78ae951f56e0d78a9cda5f275460d5cdcc78daa3 100644 (file)
@@ -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);
index 79cca8b167623c1dc8db7080c9ec8b3dd5469c3b..93865e41336ff6ccd5be4c3aa32a29fa42c7ce2d 100644 (file)
@@ -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);
index dcbec7dfebcc55c4ac4cbeb7a0991b6f141efa10..2f5485f0a8d41d88c69df9472f455ce05868977a 100644 (file)
 
 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();
   }
 
 }
index 0a7171be0c7a22c52346c287041f567da454e868..43a2aa46ea39bb8fe4ea3e49af1838d054210d3f 100644 (file)
@@ -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();
+  }
+
 }
index 4087635d607a672d2561565991b92bd185ade3d2..d98c2ed6b21115f2ca3adb22220eb3b9d50cc85e 100644 (file)
 
 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();
+  }
+
 }
index db6a0748f470f9439e15f4b8797459e0d0a9b33f..93831680e8da3dd1005abbef866da898ca5521fb 100644 (file)
@@ -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;
 
@@ -59,6 +59,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
@@ -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(