From a4e0563f753bf04f6560f7014a974d2d4f248d84 Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Wed, 6 Jul 2016 16:56:56 +0200 Subject: [PATCH] SONAR-7783 Ensure Sensors don't save same data twice --- .../api/batch/sensor/cpd/NewCpdTokens.java | 2 +- .../sensor/highlighting/NewHighlighting.java | 2 +- .../internal/InMemorySensorStorage.java | 42 ++++++++--- .../sensor/internal/SensorContextTester.java | 27 +++---- .../batch/sensor/symbol/NewSymbolTable.java | 2 +- .../internal/SensorContextTesterTest.java | 72 +++++++++++++++++++ .../scanner/cpd/index/SonarCpdBlockIndex.java | 6 +- .../scanner/sensor/DefaultSensorStorage.java | 13 +++- .../sensor/DefaultSensorStorageTest.java | 34 ++++++++- 9 files changed, 163 insertions(+), 37 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/cpd/NewCpdTokens.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/cpd/NewCpdTokens.java index e28caa4a647..76b2ce5b021 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/cpd/NewCpdTokens.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/cpd/NewCpdTokens.java @@ -58,7 +58,7 @@ public interface NewCpdTokens { NewCpdTokens addToken(int startLine, int startLineOffset, int endLine, int endLineOffset, String image); /** - * Call this method only once when your are done with defining tokens of the file. + * Call this method only once when your are done with defining tokens of the file. It is not supported to save CPD tokens twice for the same file. */ void save(); } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/NewHighlighting.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/NewHighlighting.java index ae3dd8211d6..a959d62d78e 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/NewHighlighting.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/highlighting/NewHighlighting.java @@ -71,7 +71,7 @@ public interface NewHighlighting { NewHighlighting highlight(int startLine, int startLineOffset, int endLine, int endLineOffset, TypeOfText typeOfText); /** - * Call this method only once when your are done with defining highlighting of the file. + * Call this method only once when your are done with defining highlighting of the file. It is not supported to save highlighting twice for the same file. * @throws IllegalStateException if you have defined overlapping highlighting */ void save(); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/internal/InMemorySensorStorage.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/internal/InMemorySensorStorage.java index 32c91d18610..53a1364604b 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/internal/InMemorySensorStorage.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/internal/InMemorySensorStorage.java @@ -23,7 +23,6 @@ import com.google.common.collect.HashBasedTable; import com.google.common.collect.Table; import java.util.ArrayList; import java.util.Collection; -import java.util.EnumMap; import java.util.HashMap; import java.util.Map; import org.sonar.api.batch.sensor.coverage.CoverageType; @@ -33,6 +32,7 @@ import org.sonar.api.batch.sensor.highlighting.internal.DefaultHighlighting; import org.sonar.api.batch.sensor.issue.Issue; import org.sonar.api.batch.sensor.measure.Measure; import org.sonar.api.batch.sensor.symbol.internal.DefaultSymbolTable; +import org.sonar.api.utils.SonarException; class InMemorySensorStorage implements SensorStorage { @@ -42,12 +42,18 @@ class InMemorySensorStorage implements SensorStorage { Map highlightingByComponent = new HashMap<>(); Map cpdTokensByComponent = new HashMap<>(); - Map> coverageByComponent = new HashMap<>(); + Table coverageByComponentAndType = HashBasedTable.create(); Map symbolsPerComponent = new HashMap<>(); @Override public void store(Measure measure) { - measuresByComponentAndMetric.row(measure.inputComponent().key()).put(measure.metric().key(), measure); + // Emulate duplicate measure check + String componentKey = measure.inputComponent().key(); + String metricKey = measure.metric().key(); + if (measuresByComponentAndMetric.contains(componentKey, metricKey)) { + throw new SonarException("Can not add the same measure twice"); + } + measuresByComponentAndMetric.row(componentKey).put(metricKey, measure); } @Override @@ -57,26 +63,42 @@ class InMemorySensorStorage implements SensorStorage { @Override public void store(DefaultHighlighting highlighting) { - highlightingByComponent.put(highlighting.inputFile().key(), highlighting); + String fileKey = highlighting.inputFile().key(); + // Emulate duplicate storage check + if (highlightingByComponent.containsKey(fileKey)) { + throw new UnsupportedOperationException("Trying to save highlighting twice for the same file is not supported: " + highlighting.inputFile().relativePath()); + } + highlightingByComponent.put(fileKey, highlighting); } @Override public void store(DefaultCoverage defaultCoverage) { - String key = defaultCoverage.inputFile().key(); - if (!coverageByComponent.containsKey(key)) { - coverageByComponent.put(key, new EnumMap(CoverageType.class)); + String fileKey = defaultCoverage.inputFile().key(); + // Emulate duplicate storage check + if (coverageByComponentAndType.contains(fileKey, defaultCoverage.type())) { + throw new UnsupportedOperationException("Trying to save coverage twice for the same file is not supported: " + defaultCoverage.inputFile().relativePath()); } - coverageByComponent.get(key).put(defaultCoverage.type(), defaultCoverage); + coverageByComponentAndType.row(fileKey).put(defaultCoverage.type(), defaultCoverage); } @Override public void store(DefaultCpdTokens defaultCpdTokens) { - cpdTokensByComponent.put(defaultCpdTokens.inputFile().key(), defaultCpdTokens); + String fileKey = defaultCpdTokens.inputFile().key(); + // Emulate duplicate storage check + if (cpdTokensByComponent.containsKey(fileKey)) { + throw new UnsupportedOperationException("Trying to save CPD tokens twice for the same file is not supported: " + defaultCpdTokens.inputFile().relativePath()); + } + cpdTokensByComponent.put(fileKey, defaultCpdTokens); } @Override public void store(DefaultSymbolTable symbolTable) { - symbolsPerComponent.put(symbolTable.inputFile().key(), symbolTable); + String fileKey = symbolTable.inputFile().key(); + // Emulate duplicate storage check + if (symbolsPerComponent.containsKey(fileKey)) { + throw new UnsupportedOperationException("Trying to save symbol table twice for the same file is not supported: " + symbolTable.inputFile().relativePath()); + } + symbolsPerComponent.put(fileKey, symbolTable); } } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/internal/SensorContextTester.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/internal/SensorContextTester.java index 71a35021a28..12cd04842db 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/internal/SensorContextTester.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/internal/SensorContextTester.java @@ -183,38 +183,29 @@ public class SensorContextTester implements SensorContext { @CheckForNull public Integer lineHits(String fileKey, CoverageType type, int line) { - Map defaultCoverageByType = sensorStorage.coverageByComponent.get(fileKey); - if (defaultCoverageByType == null) { + DefaultCoverage defaultCoverage = sensorStorage.coverageByComponentAndType.get(fileKey, type); + if (defaultCoverage == null) { return null; } - if (defaultCoverageByType.containsKey(type)) { - return defaultCoverageByType.get(type).hitsByLine().get(line); - } - return null; + return defaultCoverage.hitsByLine().get(line); } @CheckForNull public Integer conditions(String fileKey, CoverageType type, int line) { - Map defaultCoverageByType = sensorStorage.coverageByComponent.get(fileKey); - if (defaultCoverageByType == null) { + DefaultCoverage defaultCoverage = sensorStorage.coverageByComponentAndType.get(fileKey, type); + if (defaultCoverage == null) { return null; } - if (defaultCoverageByType.containsKey(type)) { - return defaultCoverageByType.get(type).conditionsByLine().get(line); - } - return null; + return defaultCoverage.conditionsByLine().get(line); } @CheckForNull public Integer coveredConditions(String fileKey, CoverageType type, int line) { - Map defaultCoverageByType = sensorStorage.coverageByComponent.get(fileKey); - if (defaultCoverageByType == null) { + DefaultCoverage defaultCoverage = sensorStorage.coverageByComponentAndType.get(fileKey, type); + if (defaultCoverage == null) { return null; } - if (defaultCoverageByType.containsKey(type)) { - return defaultCoverageByType.get(type).coveredConditionsByLine().get(line); - } - return null; + return defaultCoverage.coveredConditionsByLine().get(line); } @CheckForNull diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/symbol/NewSymbolTable.java b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/symbol/NewSymbolTable.java index af2cab645e1..a7da9ec5c40 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/symbol/NewSymbolTable.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/batch/sensor/symbol/NewSymbolTable.java @@ -69,7 +69,7 @@ public interface NewSymbolTable { NewSymbol newSymbol(int startLine, int startLineOffset, int endLine, int endLineOffset); /** - * Call this method only once when your are done with defining all symbols of the file. + * Call this method only once when your are done with defining all symbols of the file. It is not permitted to save a symbol table twice for the same file. * @throws IllegalStateException if you have defined overlapping symbols */ void save(); diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/internal/SensorContextTesterTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/internal/SensorContextTesterTest.java index 75d49b3a7b3..712e1eb0500 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/internal/SensorContextTesterTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/batch/sensor/internal/SensorContextTesterTest.java @@ -40,6 +40,7 @@ import org.sonar.api.batch.sensor.symbol.NewSymbolTable; import org.sonar.api.config.Settings; import org.sonar.api.measures.CoreMetrics; import org.sonar.api.rule.RuleKey; +import org.sonar.api.utils.SonarException; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.tuple; @@ -127,6 +128,20 @@ public class SensorContextTesterTest { assertThat(tester.measure("foo", "directories")).isNotNull(); } + @Test(expected = SonarException.class) + public void duplicateMeasures() { + tester.newMeasure() + .on(new DefaultInputFile("foo", "src/Foo.java")) + .forMetric(CoreMetrics.NCLOC) + .withValue(2) + .save(); + tester.newMeasure() + .on(new DefaultInputFile("foo", "src/Foo.java")) + .forMetric(CoreMetrics.NCLOC) + .withValue(2) + .save(); + } + @Test public void testHighlighting() { assertThat(tester.highlightingTypeAt("foo:src/Foo.java", 1, 3)).isEmpty(); @@ -140,6 +155,18 @@ public class SensorContextTesterTest { assertThat(tester.highlightingTypeAt("foo:src/Foo.java", 1, 9)).containsExactly(TypeOfText.CONSTANT, TypeOfText.COMMENT); } + @Test(expected = UnsupportedOperationException.class) + public void duplicateHighlighting() { + tester.newHighlighting() + .onFile(new DefaultInputFile("foo", "src/Foo.java").initMetadata(new FileMetadata().readMetadata(new StringReader("annot dsf fds foo bar")))) + .highlight(1, 0, 1, 5, TypeOfText.ANNOTATION) + .save(); + tester.newHighlighting() + .onFile(new DefaultInputFile("foo", "src/Foo.java").initMetadata(new FileMetadata().readMetadata(new StringReader("annot dsf fds foo bar")))) + .highlight(1, 0, 1, 5, TypeOfText.ANNOTATION) + .save(); + } + @Test public void testSymbolReferences() { assertThat(tester.referencesForSymbolAt("foo:src/Foo.java", 1, 0)).isNull(); @@ -162,6 +189,23 @@ public class SensorContextTesterTest { tuple(1, 10, 1, 13)); } + @Test(expected = UnsupportedOperationException.class) + public void duplicateSymbolReferences() { + NewSymbolTable symbolTable = tester.newSymbolTable() + .onFile(new DefaultInputFile("foo", "src/Foo.java").initMetadata(new FileMetadata().readMetadata(new StringReader("annot dsf fds foo bar")))); + symbolTable + .newSymbol(1, 8, 1, 10); + + symbolTable.save(); + + symbolTable = tester.newSymbolTable() + .onFile(new DefaultInputFile("foo", "src/Foo.java").initMetadata(new FileMetadata().readMetadata(new StringReader("annot dsf fds foo bar")))); + symbolTable + .newSymbol(1, 8, 1, 10); + + symbolTable.save(); + } + @Test public void testCoverageAtLineZero() { assertThat(tester.lineHits("foo:src/Foo.java", CoverageType.UNIT, 1)).isNull(); @@ -201,6 +245,20 @@ public class SensorContextTesterTest { assertThat(tester.lineHits("foo:src/Foo.java", CoverageType.UNIT, 2)).isEqualTo(3); } + @Test(expected = UnsupportedOperationException.class) + public void duplicateCoverage() { + tester.newCoverage() + .onFile(new DefaultInputFile("foo", "src/Foo.java").initMetadata(new FileMetadata().readMetadata(new StringReader("annot dsf fds foo bar\nasdas")))) + .ofType(CoverageType.UNIT) + .lineHits(1, 2) + .save(); + tester.newCoverage() + .onFile(new DefaultInputFile("foo", "src/Foo.java").initMetadata(new FileMetadata().readMetadata(new StringReader("annot dsf fds foo bar\nasdas")))) + .ofType(CoverageType.UNIT) + .lineHits(1, 2) + .save(); + } + @Test public void testConditions() { assertThat(tester.conditions("foo:src/Foo.java", CoverageType.UNIT, 1)).isNull(); @@ -234,4 +292,18 @@ public class SensorContextTesterTest { tuple("publicclass$IDENTIFIER{", 1, 1, 4), tuple("}", 3, 5, 5)); } + + @Test(expected = UnsupportedOperationException.class) + public void duplicateCpdTokens() { + DefaultInputFile inputFile = new DefaultInputFile("foo", "src/Foo.java").initMetadata(new FileMetadata().readMetadata(new StringReader("public class Foo {\n\n}"))); + tester.newCpdTokens() + .onFile(inputFile) + .addToken(inputFile.newRange(0, 6), "public") + .save(); + + tester.newCpdTokens() + .onFile(inputFile) + .addToken(inputFile.newRange(0, 6), "public") + .save(); + } } diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/cpd/index/SonarCpdBlockIndex.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/cpd/index/SonarCpdBlockIndex.java index 93984a86168..b20e5f2c763 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/cpd/index/SonarCpdBlockIndex.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/cpd/index/SonarCpdBlockIndex.java @@ -36,6 +36,7 @@ import org.sonar.duplications.index.CloneIndex; import org.sonar.duplications.index.PackedMemoryCloneIndex; import org.sonar.duplications.index.PackedMemoryCloneIndex.ResourceBlocks; import org.sonar.scanner.index.BatchComponentCache; +import org.sonar.scanner.protocol.output.FileStructure; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.report.ReportPublisher; @@ -57,6 +58,9 @@ public class SonarCpdBlockIndex extends AbstractCloneIndex { public void insert(InputFile inputFile, Collection blocks) { if (isCrossProjectDuplicationEnabled(settings)) { int id = batchComponentCache.get(inputFile).batchId(); + if (publisher.getWriter().hasComponentData(FileStructure.Domain.CPD_TEXT_BLOCKS, id)) { + throw new UnsupportedOperationException("Trying to save CPD tokens twice for the same file is not supported: " + inputFile.absolutePath()); + } final ScannerReport.CpdTextBlock.Builder builder = ScannerReport.CpdTextBlock.newBuilder(); publisher.getWriter().writeCpdTextBlocks(id, Iterables.transform(blocks, new Function() { @Override @@ -110,7 +114,7 @@ public class SonarCpdBlockIndex extends AbstractCloneIndex { public Iterator iterator() { return mem.iterator(); } - + @Override public int noResources() { return mem.noResources(); diff --git a/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java b/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java index 67fe7d9bc4b..1024e1747cd 100644 --- a/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java +++ b/sonar-scanner-engine/src/main/java/org/sonar/scanner/sensor/DefaultSensorStorage.java @@ -58,6 +58,7 @@ import org.sonar.scanner.cpd.index.SonarCpdBlockIndex; import org.sonar.scanner.index.BatchComponent; import org.sonar.scanner.index.BatchComponentCache; import org.sonar.scanner.issue.ModuleIssues; +import org.sonar.scanner.protocol.output.FileStructure; import org.sonar.scanner.protocol.output.ScannerReport; import org.sonar.scanner.protocol.output.ScannerReportWriter; import org.sonar.scanner.report.ReportPublisher; @@ -191,14 +192,22 @@ public class DefaultSensorStorage implements SensorStorage { public void store(DefaultHighlighting highlighting) { ScannerReportWriter writer = reportPublisher.getWriter(); DefaultInputFile inputFile = (DefaultInputFile) highlighting.inputFile(); - writer.writeComponentSyntaxHighlighting(componentCache.get(inputFile).batchId(), + int componentRef = componentCache.get(inputFile).batchId(); + if (writer.hasComponentData(FileStructure.Domain.SYNTAX_HIGHLIGHTINGS, componentRef)) { + throw new UnsupportedOperationException("Trying to save highlighting twice for the same file is not supported: " + inputFile.absolutePath()); + } + writer.writeComponentSyntaxHighlighting(componentRef, Iterables.transform(highlighting.getSyntaxHighlightingRuleSet(), new BuildSyntaxHighlighting())); } @Override public void store(DefaultSymbolTable symbolTable) { ScannerReportWriter writer = reportPublisher.getWriter(); - writer.writeComponentSymbols(componentCache.get(symbolTable.inputFile()).batchId(), + int componentRef = componentCache.get(symbolTable.inputFile()).batchId(); + if (writer.hasComponentData(FileStructure.Domain.SYMBOLS, componentRef)) { + throw new UnsupportedOperationException("Trying to save symbol table twice for the same file is not supported: " + symbolTable.inputFile().absolutePath()); + } + writer.writeComponentSymbols(componentRef, Iterables.transform(symbolTable.getReferencesBySymbol().entrySet(), new Function>, ScannerReport.Symbol>() { private ScannerReport.Symbol.Builder builder = ScannerReport.Symbol.newBuilder(); private ScannerReport.TextRange.Builder rangeBuilder = ScannerReport.TextRange.newBuilder(); diff --git a/sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java b/sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java index 4f825e9743b..72bbfafac76 100644 --- a/sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java +++ b/sonar-scanner-engine/src/test/java/org/sonar/scanner/sensor/DefaultSensorStorageTest.java @@ -29,7 +29,9 @@ import org.sonar.api.batch.fs.InputFile; import org.sonar.api.batch.fs.internal.DefaultInputFile; import org.sonar.api.batch.fs.internal.DefaultInputModule; import org.sonar.api.batch.measure.MetricFinder; +import org.sonar.api.batch.sensor.highlighting.internal.DefaultHighlighting; import org.sonar.api.batch.sensor.measure.internal.DefaultMeasure; +import org.sonar.api.batch.sensor.symbol.internal.DefaultSymbolTable; import org.sonar.api.config.Settings; import org.sonar.api.measures.CoreMetrics; import org.sonar.api.measures.Measure; @@ -39,9 +41,9 @@ import org.sonar.api.resources.Resource; import org.sonar.scanner.cpd.index.SonarCpdBlockIndex; import org.sonar.scanner.index.BatchComponentCache; import org.sonar.scanner.issue.ModuleIssues; +import org.sonar.scanner.protocol.output.ScannerReportWriter; import org.sonar.scanner.report.ReportPublisher; import org.sonar.scanner.scan.measure.MeasureCache; -import org.sonar.scanner.sensor.DefaultSensorStorage; import org.sonar.scanner.sensor.coverage.CoverageExclusions; import static org.assertj.core.api.Assertions.assertThat; @@ -67,7 +69,7 @@ public class DefaultSensorStorageTest { private BatchComponentCache resourceCache; @Before - public void prepare() { + public void prepare() throws Exception { MetricFinder metricFinder = mock(MetricFinder.class); when(metricFinder.findByKey(CoreMetrics.NCLOC_KEY)).thenReturn(CoreMetrics.NCLOC); when(metricFinder.findByKey(CoreMetrics.FUNCTION_COMPLEXITY_DISTRIBUTION_KEY)).thenReturn(CoreMetrics.FUNCTION_COMPLEXITY_DISTRIBUTION); @@ -78,8 +80,10 @@ public class DefaultSensorStorageTest { CoverageExclusions coverageExclusions = mock(CoverageExclusions.class); when(coverageExclusions.accept(any(Resource.class), any(Measure.class))).thenReturn(true); resourceCache = new BatchComponentCache(); + ReportPublisher reportPublisher = mock(ReportPublisher.class); + when(reportPublisher.getWriter()).thenReturn(new ScannerReportWriter(temp.newFolder())); underTest = new DefaultSensorStorage(metricFinder, - moduleIssues, settings, coverageExclusions, resourceCache, mock(ReportPublisher.class), measureCache, mock(SonarCpdBlockIndex.class)); + moduleIssues, settings, coverageExclusions, resourceCache, reportPublisher, measureCache, mock(SonarCpdBlockIndex.class)); } @Test @@ -131,4 +135,28 @@ public class DefaultSensorStorageTest { assertThat(m.getMetric()).isEqualTo(CoreMetrics.NCLOC); } + @Test(expected = UnsupportedOperationException.class) + public void duplicateHighlighting() throws Exception { + Resource sonarFile = File.create("src/Foo.java").setEffectiveKey("foo:src/Foo.java"); + DefaultInputFile inputFile = new DefaultInputFile("foo", "src/Foo.java") + .setModuleBaseDir(temp.newFolder().toPath()); + resourceCache.add(sonarFile, null).setInputComponent(inputFile); + DefaultHighlighting h = new DefaultHighlighting(null) + .onFile(inputFile); + underTest.store(h); + underTest.store(h); + } + + @Test(expected = UnsupportedOperationException.class) + public void duplicateSymbolTable() throws Exception { + Resource sonarFile = File.create("src/Foo.java").setEffectiveKey("foo:src/Foo.java"); + DefaultInputFile inputFile = new DefaultInputFile("foo", "src/Foo.java") + .setModuleBaseDir(temp.newFolder().toPath()); + resourceCache.add(sonarFile, null).setInputComponent(inputFile); + DefaultSymbolTable st = new DefaultSymbolTable(null) + .onFile(inputFile); + underTest.store(st); + underTest.store(st); + } + } -- 2.39.5