SONAR-6827 Validate uniqueness of output metrics

Output metric can only by generated by one measure computer

2 files changed, 39 insertions, 11 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedMeasureComputers.java b/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedMeasureComputers.java
index 7e9caac486d..a9901ac5ca5 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedMeasureComputers.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/computation/step/FeedMeasureComputers.java
@@ -26,6 +26,7 @@ import com.google.common.base.Predicates;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
@@ -40,6 +41,7 @@ import org.sonar.server.computation.measure.MutableMeasureComputersHolder;
 import org.sonar.server.computation.measure.api.MeasureComputerDefinitionImpl;
 import org.sonar.server.computation.measure.api.MeasureComputerWrapper;
 
+import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.FluentIterable.from;
 import static org.sonar.api.ce.measure.MeasureComputer.MeasureComputerDefinition;
 import static org.sonar.api.ce.measure.MeasureComputer.MeasureComputerDefinitionContext;
@@ -110,7 +112,7 @@ public class FeedMeasureComputers implements ComputationStep {
   }
 
   private static void feedComputersByMetric(List<MeasureComputerWrapper> wrappers, Map<String, MeasureComputerWrapper> computersByOutputMetric,
-    Map<String, MeasureComputerWrapper> computersByInputMetric) {
+                                            Map<String, MeasureComputerWrapper> computersByInputMetric) {
     for (MeasureComputerWrapper computer : wrappers) {
       for (String outputMetric : computer.getDefinition().getOutputMetrics()) {
         computersByOutputMetric.put(outputMetric, computer);
@@ -124,6 +126,7 @@ public class FeedMeasureComputers implements ComputationStep {
   private void validateMetrics(List<MeasureComputerWrapper> wrappers) {
     from(wrappers).transformAndConcat(ToInputMetrics.INSTANCE).filter(new ValidateInputMetric()).size();
     from(wrappers).transformAndConcat(ToOutputMetrics.INSTANCE).filter(new ValidateOutputMetric()).size();
+    from(wrappers).filter(new ValidateUniqueOutputMetric()).size();
   }
 
   private static Iterable<MeasureComputerWrapper> getDependencies(MeasureComputerWrapper measureComputer, ToComputerByKey toComputerByOutputMetricKey) {
@@ -203,10 +206,9 @@ public class FeedMeasureComputers implements ComputationStep {
 
   private class ValidateInputMetric implements Predicate<String> {
     @Override
-    public boolean apply(@Nullable String metric) {
-      if (!pluginMetricKeys.contains(metric) && !CORE_METRIC_KEYS.contains(metric)) {
-        throw new IllegalStateException(String.format("Metric '%s' cannot be used as an input metric as it's not a core metric and no plugin declare this metric", metric));
-      }
+    public boolean apply(@Nonnull String metric) {
+      checkState(pluginMetricKeys.contains(metric) || CORE_METRIC_KEYS.contains(metric),
+        String.format("Metric '%s' cannot be used as an input metric as it's not a core metric and no plugin declare this metric", metric));
       return true;
     }
   }
@@ -222,12 +224,22 @@ public class FeedMeasureComputers implements ComputationStep {
 
   private class ValidateOutputMetric implements Predicate<String> {
     @Override
-    public boolean apply(@Nullable String metric) {
-      if (CORE_METRIC_KEYS.contains(metric)) {
-        throw new IllegalStateException(String.format("Metric '%s' cannot be used as an output metric as it's a core metric", metric));
-      }
-      if (!pluginMetricKeys.contains(metric)) {
-        throw new IllegalStateException(String.format("Metric '%s' cannot be used as an output metric as no plugin declare this metric", metric));
+    public boolean apply(@Nonnull String metric) {
+      checkState(!CORE_METRIC_KEYS.contains(metric), String.format("Metric '%s' cannot be used as an output metric as it's a core metric", metric));
+      checkState(pluginMetricKeys.contains(metric), String.format("Metric '%s' cannot be used as an output metric as no plugin declare this metric", metric));
+      return true;
+    }
+  }
+
+  private static class ValidateUniqueOutputMetric implements Predicate<MeasureComputerWrapper> {
+    private Set<String> allOutputMetrics = new HashSet<>();
+
+    @Override
+    public boolean apply(@Nonnull MeasureComputerWrapper wrapper ) {
+      for (String outputMetric : wrapper.getDefinition().getOutputMetrics()) {
+        checkState(!allOutputMetrics.contains(outputMetric),
+          String.format("Output metric '%s' is already defined by another measure computer '%s'", outputMetric, wrapper.getComputer()));
+        allOutputMetrics.add(outputMetric);
       }
       return true;
     }
diff --git a/server/sonar-server/src/test/java/org/sonar/server/computation/step/FeedMeasureComputersTest.java b/server/sonar-server/src/test/java/org/sonar/server/computation/step/FeedMeasureComputersTest.java
index 2b3d0163ced..b8eac165d79 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/computation/step/FeedMeasureComputersTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/computation/step/FeedMeasureComputersTest.java
@@ -37,6 +37,7 @@ import org.sonar.server.computation.measure.api.MeasureComputerWrapper;
 import static com.google.common.collect.Lists.newArrayList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.util.Arrays.array;
+import static org.sonar.api.measures.CoreMetrics.CLASSES_KEY;
 import static org.sonar.api.measures.CoreMetrics.NCLOC_KEY;
 import static org.sonar.api.measures.Metric.ValueType.DATA;
 import static org.sonar.api.measures.Metric.ValueType.FLOAT;
@@ -179,6 +180,16 @@ public class FeedMeasureComputersTest {
   }
 
   @Test
+  public void fail_with_ISE_when_two_measure_computers_generate_the_same_output_metric() throws Exception {
+    thrown.expect(IllegalStateException.class);
+    thrown.expectMessage("Output metric 'metric1' is already defined by another measure computer 'TestMeasureComputer'");
+
+    MeasureComputer[] computers = new MeasureComputer[] {newMeasureComputer(array(NCLOC_KEY), array(NEW_METRIC_1)), newMeasureComputer(array(CLASSES_KEY), array(NEW_METRIC_1))};
+    ComputationStep underTest = new FeedMeasureComputers(holder, array(new TestMetrics()), computers);
+    underTest.execute();
+  }
+
+  @Test
   public void fail_with_IAE_when_creating_measure_computer_definition_without_using_the_builder_and_with_invalid_output_metrics() throws Exception {
     thrown.expect(IllegalArgumentException.class);
     thrown.expectMessage("At least one output metric must be defined");
@@ -226,6 +237,11 @@ public class FeedMeasureComputersTest {
       public void compute(MeasureComputerContext context) {
         // Nothing needs to be done as we're only testing metada
       }
+
+      @Override
+      public String toString() {
+        return "TestMeasureComputer";
+      }
     };
   }