]> source.dussan.org Git - sonarqube.git/commitdiff
SONAR-5672 Fix regression with measure duplication_lines_data
authorJulien HENRY <julien.henry@sonarsource.com>
Thu, 2 Oct 2014 15:14:05 +0000 (17:14 +0200)
committerJulien HENRY <julien.henry@sonarsource.com>
Thu, 2 Oct 2014 15:14:52 +0000 (17:14 +0200)
plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/DefaultCpdEngine.java
plugins/sonar-cpd-plugin/src/main/java/org/sonar/plugins/cpd/JavaCpdEngine.java
plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/CpdSensorTest.java
plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/DefaultCpdEngineTest.java
plugins/sonar-cpd-plugin/src/test/java/org/sonar/plugins/cpd/JavaCpdEngineTest.java
sonar-batch/src/main/java/org/sonar/batch/scan2/DefaultFileLinesContext.java
sonar-batch/src/main/java/org/sonar/batch/scan2/DefaultFileLinesContextFactory.java
sonar-plugin-api/src/main/java/org/sonar/api/measures/FileLinesContext.java

index 4c198581b8ac70ec86356e18cc34b14c16bb3f2e..ba472cbc2b0f6e64b14ee09a2f7955dfcbc82bfb 100644 (file)
@@ -33,7 +33,6 @@ import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.DeprecatedDefaultInputFile;
 import org.sonar.api.batch.sensor.SensorContext;
 import org.sonar.api.config.Settings;
-import org.sonar.api.measures.FileLinesContextFactory;
 import org.sonar.api.resources.Project;
 import org.sonar.api.utils.SonarException;
 import org.sonar.batch.duplication.BlockCache;
@@ -70,21 +69,18 @@ public class DefaultCpdEngine extends CpdEngine {
   private final Settings settings;
   private final BlockCache duplicationCache;
   private final Project project;
-  private final FileLinesContextFactory contextFactory;
 
-  public DefaultCpdEngine(@Nullable Project project, IndexFactory indexFactory, CpdMappings mappings, FileSystem fs, Settings settings, BlockCache duplicationCache,
-    FileLinesContextFactory contextFactory) {
+  public DefaultCpdEngine(@Nullable Project project, IndexFactory indexFactory, CpdMappings mappings, FileSystem fs, Settings settings, BlockCache duplicationCache) {
     this.project = project;
     this.indexFactory = indexFactory;
     this.mappings = mappings;
     this.fs = fs;
     this.settings = settings;
     this.duplicationCache = duplicationCache;
-    this.contextFactory = contextFactory;
   }
 
-  public DefaultCpdEngine(IndexFactory indexFactory, CpdMappings mappings, FileSystem fs, Settings settings, BlockCache duplicationCache, FileLinesContextFactory contextFactory) {
-    this(null, indexFactory, mappings, fs, settings, duplicationCache, contextFactory);
+  public DefaultCpdEngine(IndexFactory indexFactory, CpdMappings mappings, FileSystem fs, Settings settings, BlockCache duplicationCache) {
+    this(null, indexFactory, mappings, fs, settings, duplicationCache);
   }
 
   @Override
@@ -135,7 +131,7 @@ public class DefaultCpdEngine extends CpdEngine {
           throw new SonarException("Fail during detection of duplication for " + inputFile, e);
         }
 
-        JavaCpdEngine.save(context, inputFile, filtered, contextFactory);
+        JavaCpdEngine.save(context, inputFile, filtered);
       }
     } finally {
       executorService.shutdown();
index 5dfff7e3dcd0bfb53c415ab6f957bce089426666..5632e4956810f20a8668905943e0f82da7330931 100644 (file)
@@ -35,9 +35,8 @@ import org.sonar.api.batch.sensor.duplication.DuplicationBuilder;
 import org.sonar.api.batch.sensor.duplication.internal.DefaultDuplicationBuilder;
 import org.sonar.api.config.Settings;
 import org.sonar.api.measures.CoreMetrics;
-import org.sonar.api.measures.FileLinesContext;
-import org.sonar.api.measures.FileLinesContextFactory;
 import org.sonar.api.resources.Project;
+import org.sonar.api.utils.KeyValueFormat;
 import org.sonar.api.utils.SonarException;
 import org.sonar.duplications.block.Block;
 import org.sonar.duplications.block.BlockChunker;
@@ -60,8 +59,10 @@ import java.io.FileNotFoundException;
 import java.io.InputStreamReader;
 import java.io.Reader;
 import java.util.Collection;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.Callable;
 import java.util.concurrent.ExecutionException;
@@ -85,18 +86,16 @@ public class JavaCpdEngine extends CpdEngine {
   private final FileSystem fs;
   private final Settings settings;
   private final Project project;
-  private final FileLinesContextFactory contextFactory;
 
-  public JavaCpdEngine(@Nullable Project project, IndexFactory indexFactory, FileSystem fs, Settings settings, FileLinesContextFactory contextFactory) {
+  public JavaCpdEngine(@Nullable Project project, IndexFactory indexFactory, FileSystem fs, Settings settings) {
     this.project = project;
     this.indexFactory = indexFactory;
     this.fs = fs;
     this.settings = settings;
-    this.contextFactory = contextFactory;
   }
 
-  public JavaCpdEngine(IndexFactory indexFactory, FileSystem fs, Settings settings, FileLinesContextFactory contextFactory) {
-    this(null, indexFactory, fs, settings, contextFactory);
+  public JavaCpdEngine(IndexFactory indexFactory, FileSystem fs, Settings settings) {
+    this(null, indexFactory, fs, settings);
   }
 
   @Override
@@ -172,7 +171,7 @@ public class JavaCpdEngine extends CpdEngine {
           throw new SonarException("Fail during detection of duplication for " + inputFile, e);
         }
 
-        save(context, inputFile, clones, contextFactory);
+        save(context, inputFile, clones);
       }
     } finally {
       executorService.shutdown();
@@ -193,18 +192,22 @@ public class JavaCpdEngine extends CpdEngine {
     }
   }
 
-  static void save(org.sonar.api.batch.sensor.SensorContext context, InputFile inputFile, @Nullable Iterable<CloneGroup> duplications,
-    FileLinesContextFactory contextFactory) {
+  static void save(org.sonar.api.batch.sensor.SensorContext context, InputFile inputFile, @Nullable Iterable<CloneGroup> duplications) {
     if (duplications == null || Iterables.isEmpty(duplications)) {
       return;
     }
     Set<Integer> duplicatedLines = new HashSet<Integer>();
     int duplicatedBlocks = computeBlockAndLineCount(duplications, duplicatedLines);
-    FileLinesContext linesContext = contextFactory.createFor(inputFile);
+    Map<Integer, Integer> duplicationByLine = new HashMap<Integer, Integer>();
     for (int i = 1; i <= inputFile.lines(); i++) {
-      linesContext.setIntValue(CoreMetrics.DUPLICATION_LINES_DATA_KEY, i, duplicatedLines.contains(i) ? 1 : 0);
+      duplicationByLine.put(i, duplicatedLines.contains(i) ? 1 : 0);
     }
-    linesContext.save();
+    ((DefaultMeasure<String>) context.<String>newMeasure()
+      .forMetric(CoreMetrics.DUPLICATION_LINES_DATA)
+      .onFile(inputFile)
+      .withValue(KeyValueFormat.format(duplicationByLine)))
+      .setFromCore()
+      .save();
     // Save
     context.<Integer>newMeasure()
       .forMetric(CoreMetrics.DUPLICATED_FILES)
index a307f859edf30f945529a58a5c3a858d7b23510e..7724fa6f725943fc3cce05edc7e86678e13676cd 100644 (file)
@@ -41,8 +41,8 @@ public class CpdSensorTest {
   @Before
   public void setUp() {
     IndexFactory indexFactory = mock(IndexFactory.class);
-    sonarEngine = new JavaCpdEngine(indexFactory, null, null, null);
-    sonarBridgeEngine = new DefaultCpdEngine(indexFactory, new CpdMappings(), null, null, mock(BlockCache.class), null);
+    sonarEngine = new JavaCpdEngine(indexFactory, null, null);
+    sonarBridgeEngine = new DefaultCpdEngine(indexFactory, new CpdMappings(), null, null, mock(BlockCache.class));
     settings = new Settings(new PropertyDefinitions(CpdPlugin.class));
 
     DefaultFileSystem fs = new DefaultFileSystem();
index 4c65940417a76ed4c2ee67459fad7879a4caefd4..6dc28f4e64d445925b35f8b5666eaa9d6de678e3 100644 (file)
@@ -22,7 +22,6 @@ package org.sonar.plugins.cpd;
 import org.junit.Before;
 import org.junit.Test;
 import org.slf4j.Logger;
-import org.sonar.api.CoreProperties;
 import org.sonar.api.config.Settings;
 import org.sonar.api.resources.Project;
 import org.sonar.batch.duplication.BlockCache;
@@ -42,7 +41,7 @@ public class DefaultCpdEngineTest {
   @Before
   public void init() {
     settings = new Settings();
-    engine = new DefaultCpdEngine(null, null, null, settings, mock(BlockCache.class), null);
+    engine = new DefaultCpdEngine(null, null, null, settings, mock(BlockCache.class));
   }
 
   @Test
index 1b565ff56b39538e7d7cc882532b63fdea356779..aa87b90dc13ff64e63526ba7e6f7db0cdf530eeb 100644 (file)
@@ -36,8 +36,6 @@ import org.sonar.api.batch.sensor.duplication.internal.DefaultDuplicationBuilder
 import org.sonar.api.batch.sensor.measure.Measure;
 import org.sonar.api.batch.sensor.measure.internal.DefaultMeasure;
 import org.sonar.api.measures.CoreMetrics;
-import org.sonar.api.measures.FileLinesContext;
-import org.sonar.api.measures.FileLinesContextFactory;
 import org.sonar.duplications.index.CloneGroup;
 import org.sonar.duplications.index.ClonePart;
 
@@ -61,8 +59,6 @@ public class JavaCpdEngineTest {
   SensorContext context = mock(SensorContext.class);
   DeprecatedDefaultInputFile inputFile;
   private DefaultDuplicationBuilder duplicationBuilder;
-  private FileLinesContextFactory contextFactory;
-  private FileLinesContext linesContext;
   private SensorStorage storage = mock(SensorStorage.class);
 
   @Before
@@ -77,46 +73,38 @@ public class JavaCpdEngineTest {
     duplicationBuilder = spy(new DefaultDuplicationBuilder(inputFile));
     when(context.duplicationBuilder(any(InputFile.class))).thenReturn(duplicationBuilder);
     inputFile.setFile(temp.newFile("Foo.java"));
-    contextFactory = mock(FileLinesContextFactory.class);
-    linesContext = mock(FileLinesContext.class);
-    when(contextFactory.createFor(inputFile)).thenReturn(linesContext);
   }
 
   @SuppressWarnings("unchecked")
   @Test
   public void testNothingToSave() {
-    JavaCpdEngine.save(context, inputFile, null, contextFactory);
-    JavaCpdEngine.save(context, inputFile, Collections.EMPTY_LIST, contextFactory);
+    JavaCpdEngine.save(context, inputFile, null);
+    JavaCpdEngine.save(context, inputFile, Collections.EMPTY_LIST);
 
     verifyZeroInteractions(context);
   }
 
   @Test
   public void testOneSimpleDuplicationBetweenTwoFiles() {
-    inputFile.setLines(300);
-    List<CloneGroup> groups = Arrays.asList(newCloneGroup(new ClonePart("key1", 0, 5, 204), new ClonePart("key2", 0, 15, 214)));
-    JavaCpdEngine.save(context, inputFile, groups, contextFactory);
+    inputFile.setLines(5);
+    List<CloneGroup> groups = Arrays.asList(newCloneGroup(new ClonePart("key1", 0, 2, 4), new ClonePart("key2", 0, 15, 17)));
+    JavaCpdEngine.save(context, inputFile, groups);
 
     verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_FILES).onFile(inputFile).withValue(1));
     verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_BLOCKS).onFile(inputFile).withValue(1));
-    verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_LINES).onFile(inputFile).withValue(200));
+    verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_LINES).onFile(inputFile).withValue(3));
+    verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATION_LINES_DATA).onFile(inputFile).withValue("1=0;2=1;3=1;4=1;5=0"));
 
     InOrder inOrder = Mockito.inOrder(duplicationBuilder);
-    inOrder.verify(duplicationBuilder).originBlock(5, 204);
-    inOrder.verify(duplicationBuilder).isDuplicatedBy("key2", 15, 214);
+    inOrder.verify(duplicationBuilder).originBlock(2, 4);
+    inOrder.verify(duplicationBuilder).isDuplicatedBy("key2", 15, 17);
     inOrder.verify(duplicationBuilder).build();
-
-    verify(linesContext).setIntValue(CoreMetrics.DUPLICATION_LINES_DATA_KEY, 1, 0);
-    verify(linesContext).setIntValue(CoreMetrics.DUPLICATION_LINES_DATA_KEY, 4, 0);
-    verify(linesContext).setIntValue(CoreMetrics.DUPLICATION_LINES_DATA_KEY, 5, 1);
-    verify(linesContext).setIntValue(CoreMetrics.DUPLICATION_LINES_DATA_KEY, 204, 1);
-    verify(linesContext).setIntValue(CoreMetrics.DUPLICATION_LINES_DATA_KEY, 205, 0);
   }
 
   @Test
   public void testDuplicationOnSameFile() throws Exception {
     List<CloneGroup> groups = Arrays.asList(newCloneGroup(new ClonePart("key1", 0, 5, 204), new ClonePart("key1", 0, 215, 414)));
-    JavaCpdEngine.save(context, inputFile, groups, contextFactory);
+    JavaCpdEngine.save(context, inputFile, groups);
 
     verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_FILES).onFile(inputFile).withValue(1));
     verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_BLOCKS).onFile(inputFile).withValue(2));
@@ -131,7 +119,7 @@ public class JavaCpdEngineTest {
   @Test
   public void testOneDuplicatedGroupInvolvingMoreThanTwoFiles() throws Exception {
     List<CloneGroup> groups = Arrays.asList(newCloneGroup(new ClonePart("key1", 0, 5, 204), new ClonePart("key2", 0, 15, 214), new ClonePart("key3", 0, 25, 224)));
-    JavaCpdEngine.save(context, inputFile, groups, contextFactory);
+    JavaCpdEngine.save(context, inputFile, groups);
 
     verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_FILES).onFile(inputFile).withValue(1));
     verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_BLOCKS).onFile(inputFile).withValue(1));
@@ -155,7 +143,7 @@ public class JavaCpdEngineTest {
     List<CloneGroup> groups = Arrays.asList(
       newCloneGroup(new ClonePart("key1", 0, 5, 204), new ClonePart("key2", 0, 15, 214)),
       newCloneGroup(new ClonePart("key1", 0, 15, 214), new ClonePart("key3", 0, 15, 214)));
-    JavaCpdEngine.save(context, inputFile, groups, contextFactory);
+    JavaCpdEngine.save(context, inputFile, groups);
 
     verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_FILES).onFile(inputFile).withValue(1));
     verify(storage).store(new DefaultMeasure().forMetric(CoreMetrics.DUPLICATED_BLOCKS).onFile(inputFile).withValue(2));
index 545a8d176268d8b9883f0de52a3bd5208e7d0d8b..c4aa0550914f4375602dbc5667f9b09bbafcedd9 100644 (file)
@@ -25,32 +25,27 @@ import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.Maps;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.measure.MetricFinder;
-import org.sonar.api.batch.sensor.measure.Measure;
+import org.sonar.api.batch.sensor.SensorStorage;
 import org.sonar.api.batch.sensor.measure.internal.DefaultMeasure;
 import org.sonar.api.measures.FileLinesContext;
 import org.sonar.api.utils.KeyValueFormat;
-import org.sonar.api.utils.KeyValueFormat.Converter;
-import org.sonar.core.component.ComponentKeys;
 
 import java.util.Map;
 
 public class DefaultFileLinesContext implements FileLinesContext {
 
-  private final MeasureCache measureCache;
+  private final SensorStorage sensorStorage;
   private final InputFile inputFile;
 
   /**
    * metric key -> line -> value
    */
   private final Map<String, Map<Integer, Object>> map = Maps.newHashMap();
-  private String projectKey;
   private MetricFinder metricFinder;
 
-  public DefaultFileLinesContext(MetricFinder metricFinder, MeasureCache measureCache, String projectKey, InputFile inputFile) {
+  public DefaultFileLinesContext(MetricFinder metricFinder, SensorStorage sensorStorage, InputFile inputFile) {
     this.metricFinder = metricFinder;
-    this.projectKey = projectKey;
-    Preconditions.checkNotNull(measureCache);
-    this.measureCache = measureCache;
+    this.sensorStorage = sensorStorage;
     this.inputFile = inputFile;
   }
 
@@ -62,16 +57,7 @@ public class DefaultFileLinesContext implements FileLinesContext {
   }
 
   public Integer getIntValue(String metricKey, int line) {
-    Preconditions.checkNotNull(metricKey);
-    Preconditions.checkArgument(line > 0);
-
-    Map lines = map.get(metricKey);
-    if (lines == null) {
-      // not in memory, so load
-      lines = loadData(metricKey, KeyValueFormat.newIntegerConverter());
-      map.put(metricKey, lines);
-    }
-    return (Integer) lines.get(line);
+    throw new UnsupportedOperationException();
   }
 
   public void setStringValue(String metricKey, int line, String value) {
@@ -83,16 +69,7 @@ public class DefaultFileLinesContext implements FileLinesContext {
   }
 
   public String getStringValue(String metricKey, int line) {
-    Preconditions.checkNotNull(metricKey);
-    Preconditions.checkArgument(line > 0);
-
-    Map lines = map.get(metricKey);
-    if (lines == null) {
-      // not in memory, so load
-      lines = loadData(metricKey, KeyValueFormat.newStringConverter());
-      map.put(metricKey, lines);
-    }
-    return (String) lines.get(line);
+    throw new UnsupportedOperationException();
   }
 
   private Map<Integer, Object> getOrCreateLines(String metricKey) {
@@ -116,34 +93,14 @@ public class DefaultFileLinesContext implements FileLinesContext {
         throw new IllegalStateException("Unable to find metric with key: " + metricKey);
       }
       Map<Integer, Object> lines = entry.getValue();
-      if (shouldSave(lines)) {
-        String data = KeyValueFormat.format(lines);
-        measureCache.put(projectKey, ComponentKeys.createEffectiveKey(projectKey, inputFile), new DefaultMeasure<String>()
-          .forMetric(metric)
-          .onFile(inputFile)
-          .withValue(data));
-        entry.setValue(ImmutableMap.copyOf(lines));
-      }
-    }
-  }
-
-  private Map loadData(String metricKey, Converter converter) {
-    Measure measure = measureCache.byMetric(projectKey, ComponentKeys.createEffectiveKey(projectKey, inputFile), metricKey);
-    if (measure == null) {
-      // no such measure
-      return ImmutableMap.of();
+      String data = KeyValueFormat.format(lines);
+      new DefaultMeasure<String>(sensorStorage)
+        .forMetric(metric)
+        .onFile(inputFile)
+        .withValue(data)
+        .save();
+      entry.setValue(ImmutableMap.copyOf(lines));
     }
-    return ImmutableMap.copyOf(KeyValueFormat.parse((String) measure.value(), KeyValueFormat.newIntegerConverter(), converter));
-  }
-
-  /**
-   * Checks that measure was not saved.
-   *
-   * @see #loadData(String, Converter)
-   * @see #save()
-   */
-  private boolean shouldSave(Map<Integer, Object> lines) {
-    return !(lines instanceof ImmutableMap);
   }
 
   @Override
index c93fb84312f74e594697a5465ecd90a8462ccf77..8b268b14be94558cbeeedf1941f503bfb60115f1 100644 (file)
@@ -23,6 +23,7 @@ import org.sonar.api.batch.bootstrap.ProjectDefinition;
 import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.measure.MetricFinder;
+import org.sonar.api.batch.sensor.SensorStorage;
 import org.sonar.api.measures.FileLinesContext;
 import org.sonar.api.measures.FileLinesContextFactory;
 import org.sonar.api.resources.Resource;
@@ -30,16 +31,16 @@ import org.sonar.batch.scan.filesystem.InputPathCache;
 
 public class DefaultFileLinesContextFactory implements FileLinesContextFactory {
 
-  private final MeasureCache measureCache;
+  private final SensorStorage sensorStorage;
   private final MetricFinder metricFinder;
   private final ProjectDefinition def;
-  private InputPathCache fileCache;
+  private final InputPathCache fileCache;
 
-  public DefaultFileLinesContextFactory(InputPathCache fileCache, FileSystem fs, MetricFinder metricFinder, MeasureCache measureCache,
+  public DefaultFileLinesContextFactory(InputPathCache fileCache, FileSystem fs, MetricFinder metricFinder, SensorStorage sensorStorage,
     ProjectDefinition def) {
     this.fileCache = fileCache;
     this.metricFinder = metricFinder;
-    this.measureCache = measureCache;
+    this.sensorStorage = sensorStorage;
     this.def = def;
   }
 
@@ -53,7 +54,7 @@ public class DefaultFileLinesContextFactory implements FileLinesContextFactory {
     if (fileCache.getFile(def.getKey(), inputFile.relativePath()) == null) {
       throw new IllegalStateException("InputFile is not indexed: " + inputFile);
     }
-    return new DefaultFileLinesContext(metricFinder, measureCache, def.getKey(), inputFile);
+    return new DefaultFileLinesContext(metricFinder, sensorStorage, inputFile);
   }
 
 }
index f828a6f5660e69f4075f278dffbeb2fa6ac5a056..1f0e1ed1739f82617726a1b1d2c4050b85847a04 100644 (file)
@@ -19,9 +19,8 @@
  */
 package org.sonar.api.measures;
 
-
 /**
- * Provides access to measures for the lines of file.
+ * Provides facility to store measures for the lines of file.
  * Examples:
  * <ul>
  * <li>line 1 is a line of code</li>
@@ -47,7 +46,9 @@ public interface FileLinesContext {
 
   /**
    * @return value, or null if no such metric for given line
+   * @deprecated since 5.0 sensors should not read data
    */
+  @Deprecated
   Integer getIntValue(String metricKey, int line);
 
   /**
@@ -57,7 +58,9 @@ public interface FileLinesContext {
 
   /**
    * @return value, or null if no such metric for given line
+   * @deprecated since 5.0 sensors should not read data
    */
+  @Deprecated
   String getStringValue(String metricKey, int line);
 
   /**