SONAR-6620 add methods to get a Measure for a rule or a characteristic

3 files changed, 219 insertions, 33 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepository.java b/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepository.java
index 80341ef81ab..5b63d22060b 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepository.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepository.java
@@ -21,7 +21,9 @@ package org.sonar.server.computation.measure;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.SetMultimap;
+import org.sonar.core.rule.RuleDto;
 import org.sonar.server.computation.component.Component;
+import org.sonar.server.computation.debt.Characteristic;
 import org.sonar.server.computation.metric.Metric;
 import org.sonar.server.computation.metric.MetricImpl;
 
@@ -46,11 +48,25 @@ public interface MeasureRepository {
   /**
    * Retrieves the measure created during the current analysis for the specified {@link Component} for the specified
    * {@link Metric} if it exists (ie. one created by the Compute Engine or the Batch) and which is <strong>not</strong>
-   * associated to a rule or a characteristic.
+   * associated to a rule nor a characteristic.
    */
   Optional<Measure> getRawMeasure(Component component, Metric metric);
 
   /**
+   * Retrieves the measure created during the current analysis for the specified {@link Component} for the specified
+   * {@link Metric} if it exists (ie. one created by the Compute Engine) and which is associated to the
+   * specified rule.
+   */
+  Optional<Measure> getRawMeasure(Component component, Metric metric, RuleDto rule);
+
+  /**
+   * Retrieves the measure created during the current analysis for the specified {@link Component} for the specified
+   * {@link Metric} if it exists (ie. one created by the Compute Engine) and which is associated to the
+   * specified Characteristic.
+   */
+  Optional<Measure> getRawMeasure(Component component, Metric metric, Characteristic characteristic);
+
+  /**
    * Returns the {@link Measure}s for the specified {@link Component} mapped by their metric key.
    * <p>
    * Their can be multiple measures for the same Metric but only one which has no rule nor characteristic, one with a
diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepositoryImpl.java b/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepositoryImpl.java
index 592613174a7..720a018aa40 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepositoryImpl.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/measure/MeasureRepositoryImpl.java
@@ -29,18 +29,23 @@ import com.google.common.collect.Multimaps;
 import com.google.common.collect.SetMultimap;
 import java.util.HashMap;
 import java.util.Map;
+import java.util.Objects;
 import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
+import javax.annotation.concurrent.Immutable;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.core.measure.db.MeasureDto;
 import org.sonar.core.persistence.DbSession;
+import org.sonar.core.rule.RuleDto;
 import org.sonar.server.computation.batch.BatchReportReader;
 import org.sonar.server.computation.component.Component;
+import org.sonar.server.computation.debt.Characteristic;
 import org.sonar.server.computation.issue.RuleCache;
 import org.sonar.server.computation.metric.Metric;
 import org.sonar.server.computation.metric.MetricRepository;
 import org.sonar.server.db.DbClient;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.collect.FluentIterable.from;
 import static java.util.Objects.requireNonNull;
 
@@ -50,7 +55,7 @@ public class MeasureRepositoryImpl implements MeasureRepository {
   private final MeasureDtoToMeasure measureDtoToMeasure = new MeasureDtoToMeasure();
   private final BatchMeasureToMeasure batchMeasureToMeasure;
   private final Function<BatchReport.Measure, Measure> batchMeasureToMeasureFunction;
-  private final Map<Integer, Map<String, Measure>> measures = new HashMap<>();
+  private final Map<Integer, Map<MeasureKey, Measure>> measures = new HashMap<>();
 
   public MeasureRepositoryImpl(DbClient dbClient, BatchReportReader reportReader,
     final MetricRepository metricRepository, final RuleCache ruleCache) {
@@ -85,25 +90,31 @@ public class MeasureRepositoryImpl implements MeasureRepository {
     requireNonNull(component);
     requireNonNull(metric);
 
-    Optional<Measure> local = findLocal(component, metric);
+    Optional<Measure> local = findLocal(component, metric, null, null);
     if (local.isPresent()) {
       return local;
     }
     return findInBatch(component, metric);
   }
 
-  private Optional<Measure> findInBatch(Component component, final Metric metric) {
-    BatchReport.Measure batchMeasure = Iterables.find(
-      reportReader.readComponentMeasures(component.getRef()),
-      new Predicate<BatchReport.Measure>() {
-        @Override
-        public boolean apply(@Nonnull BatchReport.Measure input) {
-          return input.getMetricKey().equals(metric.getKey());
-        }
-      }
-      , null);
+  @Override
+  public Optional<Measure> getRawMeasure(Component component, Metric metric, RuleDto rule) {
+    // fail fast
+    requireNonNull(component);
+    requireNonNull(metric);
+    requireNonNull(rule);
 
-    return batchMeasureToMeasure.toMeasure(batchMeasure, metric);
+    return findLocal(component, metric, rule, null);
+  }
+
+  @Override
+  public Optional<Measure> getRawMeasure(Component component, Metric metric, Characteristic characteristic) {
+    // fail fast
+    requireNonNull(component);
+    requireNonNull(metric);
+    requireNonNull(characteristic);
+
+    return findLocal(component, metric, null, characteristic);
   }
 
   @Override
@@ -112,21 +123,22 @@ public class MeasureRepositoryImpl implements MeasureRepository {
     requireNonNull(metric);
     requireNonNull(measure);
 
-    Optional<Measure> existingMeasure = findLocal(component, metric);
+    Optional<Measure> existingMeasure = findLocal(component, metric, measure);
     if (existingMeasure.isPresent()) {
       throw new UnsupportedOperationException(
         String.format(
-          "a measure can be set only once for a specific Component (ref=%s) and Metric (key=%s)",
+          "a measure can be set only once for a specific Component (ref=%s) and Metric (key=%s). Use update method",
           component.getRef(),
           metric.getKey()
           ));
     }
+    // assuming the measure comes from batch
     addLocal(component, metric, measure);
   }
 
   @Override
   public SetMultimap<String, Measure> getRawMeasures(Component component) {
-    Map<String, Measure> rawMeasures = measures.get(component.getRef());
+    Map<MeasureKey, Measure> rawMeasures = measures.get(component.getRef());
     ListMultimap<String, BatchReport.Measure> batchMeasures = from(reportReader.readComponentMeasures(component.getRef()))
       .index(BatchMeasureToMetricKey.INSTANCE);
 
@@ -141,27 +153,50 @@ public class MeasureRepositoryImpl implements MeasureRepository {
 
     ImmutableSetMultimap.Builder<String, Measure> builder = ImmutableSetMultimap.builder();
     builder.putAll(rawMeasuresFromBatch);
-    for (Map.Entry<String, Measure> entry : rawMeasures.entrySet()) {
-      builder.put(entry.getKey(), entry.getValue());
+    for (Map.Entry<MeasureKey, Measure> entry : rawMeasures.entrySet()) {
+      builder.put(entry.getKey().metricKey, entry.getValue());
     }
     return builder.build();
   }
 
-  private Optional<Measure> findLocal(Component component, Metric metric) {
-    Map<String, Measure> measuresPerMetric = measures.get(component.getRef());
+  private Optional<Measure> findInBatch(Component component, final Metric metric) {
+    BatchReport.Measure batchMeasure = Iterables.find(
+        reportReader.readComponentMeasures(component.getRef()),
+        new Predicate<BatchReport.Measure>() {
+          @Override
+          public boolean apply(@Nonnull BatchReport.Measure input) {
+            return input.getMetricKey().equals(metric.getKey());
+          }
+        }
+        , null);
+
+    return batchMeasureToMeasure.toMeasure(batchMeasure, metric);
+  }
+
+  private Optional<Measure> findLocal(Component component, Metric metric,
+    @Nullable RuleDto rule, @Nullable Characteristic characteristic) {
+    Map<MeasureKey, Measure> measuresPerMetric = measures.get(component.getRef());
     if (measuresPerMetric == null) {
       return Optional.absent();
     }
-    return Optional.fromNullable(measuresPerMetric.get(metric.getKey()));
+    return Optional.fromNullable(measuresPerMetric.get(new MeasureKey(metric.getKey(), rule, characteristic)));
+  }
+
+  private Optional<Measure> findLocal(Component component, Metric metric, Measure measure) {
+    Map<MeasureKey, Measure> measuresPerMetric = measures.get(component.getRef());
+    if (measuresPerMetric == null) {
+      return Optional.absent();
+    }
+    return Optional.fromNullable(measuresPerMetric.get(new MeasureKey(metric.getKey(), measure.getRuleId(), measure.getCharacteristicId())));
   }
 
   private void addLocal(Component component, Metric metric, Measure measure) {
-    Map<String, Measure> measuresPerMetric = measures.get(component.getRef());
+    Map<MeasureKey, Measure> measuresPerMetric = measures.get(component.getRef());
     if (measuresPerMetric == null) {
       measuresPerMetric = new HashMap<>();
       measures.put(component.getRef(), measuresPerMetric);
     }
-    measuresPerMetric.put(metric.getKey(), measure);
+    measuresPerMetric.put(new MeasureKey(metric.getKey(), measure.getRuleId(), measure.getCharacteristicId()), measure);
   }
 
   private enum BatchMeasureToMetricKey implements Function<BatchReport.Measure, String> {
@@ -173,4 +208,55 @@ public class MeasureRepositoryImpl implements MeasureRepository {
       return input.getMetricKey();
     }
   }
+
+  @Immutable
+  private static final class MeasureKey {
+    private static final int DEFAULT_INT_VALUE = -6253;
+
+    private final String metricKey;
+    private final int ruleId;
+    private final int characteristicId;
+
+    public MeasureKey(String metricKey, @Nullable Integer ruleId, @Nullable Integer characteristicId) {
+      // defensive code in case we badly chose the default value, we want to know it right away!
+      checkArgument(ruleId == null || ruleId != DEFAULT_INT_VALUE, "Unsupported rule id");
+      checkArgument(characteristicId == null || characteristicId != DEFAULT_INT_VALUE, "Unsupported characteristic id");
+
+      this.metricKey = requireNonNull(metricKey, "MetricKey can not be null");
+      this.ruleId = ruleId == null ? DEFAULT_INT_VALUE : ruleId;
+      this.characteristicId = characteristicId == null ? DEFAULT_INT_VALUE : characteristicId;
+    }
+
+    public MeasureKey(String key, @Nullable RuleDto rule, @Nullable Characteristic characteristic) {
+      this(key, rule == null ? null : rule.getId(), characteristic == null ? null : characteristic.getId());
+    }
+
+    @Override
+    public boolean equals(@Nullable Object o) {
+      if (this == o) {
+        return true;
+      }
+      if (o == null || getClass() != o.getClass()) {
+        return false;
+      }
+      MeasureKey that = (MeasureKey) o;
+      return metricKey.equals(that.metricKey)
+        && ruleId == that.ruleId
+        && characteristicId == that.characteristicId;
+    }
+
+    @Override
+    public int hashCode() {
+      return Objects.hash(metricKey, ruleId, characteristicId);
+    }
+
+    @Override
+    public String toString() {
+      return com.google.common.base.Objects.toStringHelper(this)
+          .add("metricKey", metricKey)
+          .add("ruleId", ruleId)
+          .add("characteristicId", characteristicId)
+          .toString();
+    }
+  }
 }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureRepositoryImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureRepositoryImplTest.java
index 488b84113a7..34c9eeb98d6 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureRepositoryImplTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/measure/MeasureRepositoryImplTest.java
@@ -21,22 +21,26 @@ package org.sonar.server.computation.measure;
 
 import com.google.common.base.Optional;
 import com.google.common.collect.ImmutableList;
+import com.google.common.collect.SetMultimap;
 import javax.annotation.CheckForNull;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.sonar.api.rule.RuleKey;
 import org.sonar.batch.protocol.output.BatchReport;
 import org.sonar.core.measure.db.MeasureDto;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.persistence.DbTester;
+import org.sonar.core.rule.RuleDto;
 import org.sonar.server.component.db.ComponentDao;
 import org.sonar.server.component.db.SnapshotDao;
 import org.sonar.server.computation.batch.BatchReportReader;
 import org.sonar.server.computation.batch.BatchReportReaderRule;
 import org.sonar.server.computation.component.Component;
 import org.sonar.server.computation.component.DumbComponent;
+import org.sonar.server.computation.debt.Characteristic;
 import org.sonar.server.computation.issue.RuleCache;
 import org.sonar.server.computation.metric.Metric;
 import org.sonar.server.computation.metric.MetricRepository;
@@ -55,7 +59,6 @@ import static org.mockito.Mockito.when;
 public class MeasureRepositoryImplTest {
   @ClassRule
   public static final DbTester dbTester = new DbTester();
-  public static final String SOME_DATA = "some data";
   @Rule
   public BatchReportReaderRule reportReader = new BatchReportReaderRule();
 
@@ -72,6 +75,9 @@ public class MeasureRepositoryImplTest {
   private static final long OTHER_SNAPSHOT_ID = 369;
   private static final long COMPONENT_ID = 567;
   private static final Measure SOME_MEASURE = Measure.builder().create(Measure.Level.OK);
+  private static final String SOME_DATA = "some data";
+  private static final RuleDto SOME_RULE = RuleDto.createFor(RuleKey.of("A", "1")).setId(963);
+  private static final Characteristic SOME_CHARACTERISTIC = new Characteristic(741, "key");
 
   private DbClient dbClient = new DbClient(dbTester.database(), dbTester.myBatis(), new MeasureDao(), new SnapshotDao(), new MetricDao(), new ComponentDao());
   private MetricRepository metricRepository = mock(MetricRepository.class);
@@ -211,8 +217,8 @@ public class MeasureRepositoryImplTest {
     String value = "trololo";
 
     reportReader.putMeasures(FILE_COMPONENT.getRef(), ImmutableList.of(
-        BatchReport.Measure.newBuilder().setMetricKey(METRIC_KEY_1).setStringValue(value).build()
-    ));
+      BatchReport.Measure.newBuilder().setMetricKey(METRIC_KEY_1).setStringValue(value).build()
+      ));
 
     Optional<Measure> res = underTest.getRawMeasure(FILE_COMPONENT, metric1);
 
@@ -227,8 +233,8 @@ public class MeasureRepositoryImplTest {
   @Test
   public void getRawMeasure_retrieves_added_measure_over_batch_measure() {
     reportReader.putMeasures(FILE_COMPONENT.getRef(), ImmutableList.of(
-        BatchReport.Measure.newBuilder().setMetricKey(METRIC_KEY_1).setStringValue("some value").build()
-    ));
+      BatchReport.Measure.newBuilder().setMetricKey(METRIC_KEY_1).setStringValue("some value").build()
+      ));
 
     Measure addedMeasure = SOME_MEASURE;
     underTest.add(FILE_COMPONENT, metric1, addedMeasure);
@@ -239,11 +245,89 @@ public class MeasureRepositoryImplTest {
     assertThat(res.get()).isSameAs(addedMeasure);
   }
 
+  @Test(expected = NullPointerException.class)
+  public void getRawMeasure_for_rule_throws_NPE_if_Component_arg_is_null() {
+    underTest.getRawMeasure(null, metric1, SOME_RULE);
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void getRawMeasure_for_rule_throws_NPE_if_Metric_arg_is_null() {
+    underTest.getRawMeasure(FILE_COMPONENT, null, SOME_RULE);
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void getRawMeasure_for_rule_throws_NPE_if_Characteristic_arg_is_null() {
+    underTest.getRawMeasure(FILE_COMPONENT, metric1, (RuleDto) null);
+  }
+
+  @Test
+  public void getRawMeasure_for_rule_returns_absent_if_repository_is_empty() {
+    assertThat(underTest.getRawMeasure(FILE_COMPONENT, metric1, SOME_RULE)).isAbsent();
+  }
+
+  @Test
+  public void getRawMeasure_for_rule_returns_measure_for_specified_rule() {
+    Measure measure = Measure.builder().forRule(SOME_RULE.getId()).createNoValue();
+
+    underTest.add(FILE_COMPONENT, metric1, measure);
+    underTest.add(FILE_COMPONENT, metric1, Measure.builder().forRule(222).createNoValue());
+
+    assertThat(underTest.getRawMeasure(FILE_COMPONENT, metric1, SOME_RULE).get()).isSameAs(measure);
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void getRawMeasure_for_characteristic_throws_NPE_if_Component_arg_is_null() {
+    underTest.getRawMeasure(null, metric1, SOME_CHARACTERISTIC);
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void getRawMeasure_for_characteristic_throws_NPE_if_Metric_arg_is_null() {
+    underTest.getRawMeasure(FILE_COMPONENT, null, SOME_CHARACTERISTIC);
+  }
+
+  @Test(expected = NullPointerException.class)
+  public void getRawMeasure_for_characteristic_throws_NPE_if_Characteristic_arg_is_null() {
+    underTest.getRawMeasure(FILE_COMPONENT, metric1, (Characteristic) null);
+  }
+
+  @Test
+  public void getRawMeasure_for_characteristic_returns_absent_if_repository_is_empty() {
+    assertThat(underTest.getRawMeasure(FILE_COMPONENT, metric1, SOME_CHARACTERISTIC)).isAbsent();
+  }
+
+  @Test
+  public void getRawMeasure_for_characteristic_returns_measure_for_specified_rule() {
+    Measure measure = Measure.builder().forCharacteristic(SOME_CHARACTERISTIC.getId()).createNoValue();
+
+    underTest.add(FILE_COMPONENT, metric1, measure);
+    underTest.add(FILE_COMPONENT, metric1, Measure.builder().forCharacteristic(333).createNoValue());
+
+    assertThat(underTest.getRawMeasure(FILE_COMPONENT, metric1, SOME_CHARACTERISTIC).get()).isSameAs(measure);
+  }
+
+  @Test
+  public void getRawMeasures_returns_added_measures_over_batch_measures() {
+    BatchReport.Measure batchMeasure1 = BatchReport.Measure.newBuilder().setMetricKey(METRIC_KEY_1).setStringValue("some value").build();
+    BatchReport.Measure batchMeasure2 = BatchReport.Measure.newBuilder().setMetricKey(METRIC_KEY_2).setStringValue("some value").build();
+    reportReader.putMeasures(FILE_COMPONENT.getRef(), ImmutableList.of(batchMeasure1, batchMeasure2));
+
+    Measure addedMeasure = SOME_MEASURE;
+    underTest.add(FILE_COMPONENT, metric1, addedMeasure);
+    Measure addedMeasure2 = Measure.builder().forCharacteristic(SOME_CHARACTERISTIC.getId()).createNoValue();
+    underTest.add(FILE_COMPONENT, metric1, addedMeasure2);
+
+    SetMultimap<String, Measure> rawMeasures = underTest.getRawMeasures(FILE_COMPONENT);
+
+    assertThat(rawMeasures.keySet()).hasSize(2);
+    assertThat(rawMeasures.get(METRIC_KEY_1)).containsOnly(addedMeasure, addedMeasure2);
+    assertThat(rawMeasures.get(METRIC_KEY_2)).containsOnly(Measure.builder().create("some value"));
+  }
+
   private static MeasureDto createMeasureDto(int metricId, long snapshotId) {
     return new MeasureDto()
-        .setComponentId(COMPONENT_ID)
-        .setSnapshotId(snapshotId)
-        .setData(SOME_DATA)
-        .setMetricId(metricId);
+      .setComponentId(COMPONENT_ID)
+      .setSnapshotId(snapshotId)
+      .setData(SOME_DATA)
+      .setMetricId(metricId);
   }
 }