SONAR-9049 Prevent using DATA and DISTRIB metrics in api/measures/component_tree

5 files changed, 65 insertions, 25 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeAction.java b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeAction.java
index 2a3ae7fcced..5dc534db461 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeAction.java
@@ -19,7 +19,9 @@
  */
 package org.sonar.server.measure.ws;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedSet;
 import java.util.Map;
 import java.util.Set;
@@ -38,6 +40,8 @@ import org.sonarqube.ws.WsMeasures.ComponentTreeWsResponse;
 import org.sonarqube.ws.client.measure.ComponentTreeWsRequest;
 
 import static java.lang.String.format;
+import static org.sonar.api.measures.Metric.ValueType.DATA;
+import static org.sonar.api.measures.Metric.ValueType.DISTRIB;
 import static org.sonar.core.util.Uuids.UUID_EXAMPLE_02;
 import static org.sonar.db.component.ComponentTreeQuery.Strategy.CHILDREN;
 import static org.sonar.db.component.ComponentTreeQuery.Strategy.LEAVES;
@@ -103,6 +107,8 @@ public class ComponentTreeAction implements MeasuresWsAction {
   static final String ALL_METRIC_SORT_FILTER = "all";
   static final String WITH_MEASURES_ONLY_METRIC_SORT_FILTER = "withMeasuresOnly";
   static final Set<String> METRIC_SORT_FILTERS = ImmutableSortedSet.of(ALL_METRIC_SORT_FILTER, WITH_MEASURES_ONLY_METRIC_SORT_FILTER);
+  static final Set<String> FORBIDDEN_METRIC_TYPES = ImmutableSet.of(DISTRIB.name(), DATA.name());
+  private static final Joiner COMMA_JOINER = Joiner.on(" , ");
 
   private final ComponentTreeDataLoader dataLoader;
   private final I18n i18n;
@@ -167,7 +173,7 @@ public class ComponentTreeAction implements MeasuresWsAction {
       .setDefaultValue(ALL_METRIC_SORT_FILTER)
       .setPossibleValues(METRIC_SORT_FILTERS);
 
-    createMetricKeysParameter(action);
+    createMetricKeysParameter(action).setDescription("Metric keys. Types %s are not allowed", COMMA_JOINER.join(FORBIDDEN_METRIC_TYPES));
     createAdditionalFieldsParameter(action);
     createDeveloperParameters(action);
     createQualifiersParameter(action, newQualifierParameterContext(i18n, resourceTypes));
diff --git a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeDataLoader.java b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeDataLoader.java
index 34364cba926..7bd4d20f950 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeDataLoader.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeDataLoader.java
@@ -22,6 +22,7 @@ package org.sonar.server.measure.ws;
 import com.google.common.base.Joiner;
 import com.google.common.collect.FluentIterable;
 import com.google.common.collect.HashBasedTable;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
@@ -60,8 +61,8 @@ import org.sonar.server.exceptions.NotFoundException;
 import org.sonar.server.user.UserSession;
 import org.sonarqube.ws.client.measure.ComponentTreeWsRequest;
 
+import static com.google.common.base.Preconditions.checkArgument;
 import static com.google.common.base.Preconditions.checkState;
-import static com.google.common.collect.Sets.newHashSet;
 import static java.lang.String.format;
 import static java.util.Collections.emptyMap;
 import static java.util.Objects.requireNonNull;
@@ -74,7 +75,8 @@ import static org.sonar.server.measure.ws.ComponentTreeAction.WITH_MEASURES_ONLY
 import static org.sonar.server.measure.ws.SnapshotDtoToWsPeriods.snapshotToWsPeriods;
 
 public class ComponentTreeDataLoader {
-  private static final Set<String> QUALIFIERS_ELIGIBLE_FOR_BEST_VALUE = newHashSet(Qualifiers.FILE, Qualifiers.UNIT_TEST_FILE);
+  private static final Set<String> QUALIFIERS_ELIGIBLE_FOR_BEST_VALUE = ImmutableSet.of(Qualifiers.FILE, Qualifiers.UNIT_TEST_FILE);
+  private static final Joiner COMA_JOINER = Joiner.on(", ");
 
   private final DbClient dbClient;
   private final ComponentFinder componentFinder;
@@ -166,9 +168,14 @@ public class ComponentTreeDataLoader {
         new LinkedHashSet<>(metricKeys),
         new LinkedHashSet<>(foundMetricKeys));
 
-      throw new NotFoundException(format("The following metric keys are not found: %s", Joiner.on(", ").join(missingMetricKeys)));
+      throw new NotFoundException(format("The following metric keys are not found: %s", COMA_JOINER.join(missingMetricKeys)));
     }
-
+    String forbiddenMetrics = metrics.stream()
+      .filter(metric -> ComponentTreeAction.FORBIDDEN_METRIC_TYPES.contains(metric.getValueType()))
+      .map(MetricDto::getKey)
+      .sorted()
+      .collect(Collectors.join(COMA_JOINER));
+    checkArgument(forbiddenMetrics.isEmpty(), "Metrics %s can't be requested in this web service. Please use api/measures/component", forbiddenMetrics);
     return metrics;
   }
 
diff --git a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeSort.java b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeSort.java
index 50a42682c5b..fd354fa1603 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeSort.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/measure/ws/ComponentTreeSort.java
@@ -41,8 +41,6 @@ import org.sonarqube.ws.client.measure.ComponentTreeWsRequest;
 import static java.lang.String.CASE_INSENSITIVE_ORDER;
 import static java.lang.String.format;
 import static org.sonar.api.measures.Metric.ValueType.BOOL;
-import static org.sonar.api.measures.Metric.ValueType.DATA;
-import static org.sonar.api.measures.Metric.ValueType.DISTRIB;
 import static org.sonar.api.measures.Metric.ValueType.FLOAT;
 import static org.sonar.api.measures.Metric.ValueType.INT;
 import static org.sonar.api.measures.Metric.ValueType.MILLISEC;
@@ -59,7 +57,7 @@ import static org.sonar.server.measure.ws.ComponentTreeAction.QUALIFIER_SORT;
 public class ComponentTreeSort {
 
   private static final Set<ValueType> NUMERIC_VALUE_TYPES = EnumSet.of(BOOL, FLOAT, INT, MILLISEC, WORK_DUR, PERCENT, RATING);
-  private static final Set<ValueType> TEXTUAL_VALUE_TYPES = EnumSet.of(DATA, DISTRIB, STRING);
+  private static final Set<ValueType> TEXTUAL_VALUE_TYPES = EnumSet.of(STRING);
 
   private ComponentTreeSort() {
     // static method only
@@ -243,8 +241,7 @@ public class ComponentTreeSort {
       if (measure == null) {
         return null;
       }
-      Double variation = measure.getVariation();
-      return (variation == null) ? null : variation;
+      return measure.getVariation();
     }
   }
 
diff --git a/server/sonar-server/src/test/java/org/sonar/server/measure/ws/ComponentTreeActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/measure/ws/ComponentTreeActionTest.java
index fdb07c83daf..ddf6642ba81 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/measure/ws/ComponentTreeActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/measure/ws/ComponentTreeActionTest.java
@@ -40,6 +40,7 @@ import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.ResourceTypesRule;
 import org.sonar.db.component.SnapshotDto;
 import org.sonar.db.metric.MetricDto;
+import org.sonar.db.metric.MetricTesting;
 import org.sonar.server.component.ComponentFinder;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
@@ -64,7 +65,6 @@ import static org.sonar.db.component.ComponentTesting.newFileDto;
 import static org.sonar.db.component.ComponentTesting.newProjectDto;
 import static org.sonar.db.component.SnapshotTesting.newAnalysis;
 import static org.sonar.db.measure.MeasureTesting.newMeasureDto;
-import static org.sonar.db.metric.MetricTesting.newMetricDto;
 import static org.sonar.server.measure.ws.ComponentTreeAction.CHILDREN_STRATEGY;
 import static org.sonar.server.measure.ws.ComponentTreeAction.LEAVES_STRATEGY;
 import static org.sonar.server.measure.ws.ComponentTreeAction.METRIC_PERIOD_SORT;
@@ -183,13 +183,13 @@ public class ComponentTreeActionTest {
     ComponentDto file = newFileDto(directoryDto, null, "file-uuid").setName("file-1");
     componentDb.insertComponent(file);
     MetricDto coverage = insertCoverageMetric();
-    dbClient.metricDao().insert(dbSession, newMetricDto()
+    dbClient.metricDao().insert(dbSession, MetricTesting.newMetricDto()
       .setKey("ncloc")
       .setValueType(ValueType.INT.name())
       .setOptimizedBestValue(true)
       .setBestValue(100d)
       .setWorstValue(1000d));
-    dbClient.metricDao().insert(dbSession, newMetricDtoWithoutOptimization()
+    dbClient.metricDao().insert(dbSession, newMetricDto()
       .setKey("new_violations")
       .setOptimizedBestValue(true)
       .setBestValue(1984.0d)
@@ -229,7 +229,7 @@ public class ComponentTreeActionTest {
     componentDb.insertComponent(directoryDto);
     ComponentDto file = newFileDto(directoryDto, null, "file-uuid").setName("file-1");
     componentDb.insertComponent(file);
-    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDtoWithoutOptimization()
+    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDto()
       .setKey(NEW_SECURITY_RATING_KEY)
       .setOptimizedBestValue(true)
       .setBestValue(1d)
@@ -298,7 +298,7 @@ public class ComponentTreeActionTest {
     ComponentDto file3 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-3"));
     ComponentDto file1 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-1"));
     ComponentDto file2 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-2"));
-    MetricDto ncloc = newMetricDtoWithoutOptimization().setKey("ncloc").setValueType(ValueType.INT.name()).setDirection(1);
+    MetricDto ncloc = newMetricDto().setKey("ncloc").setValueType(ValueType.INT.name()).setDirection(1);
     dbClient.metricDao().insert(dbSession, ncloc);
     dbClient.measureDao().insert(dbSession,
       newMeasureDto(ncloc, file1, projectSnapshot).setValue(1.0d),
@@ -328,7 +328,7 @@ public class ComponentTreeActionTest {
     componentDb.insertComponent(file2);
     componentDb.insertComponent(file3);
     componentDb.insertComponent(file4);
-    MetricDto ncloc = newMetricDtoWithoutOptimization().setKey("ncloc").setValueType(ValueType.INT.name()).setDirection(1);
+    MetricDto ncloc = newMetricDto().setKey("ncloc").setValueType(ValueType.INT.name()).setDirection(1);
     dbClient.metricDao().insert(dbSession, ncloc);
     dbClient.measureDao().insert(dbSession,
       newMeasureDto(ncloc, file1, projectSnapshot).setValue(1.0d),
@@ -358,7 +358,7 @@ public class ComponentTreeActionTest {
     ComponentDto file3 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-3"));
     ComponentDto file1 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-1"));
     ComponentDto file2 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-2"));
-    MetricDto ncloc = newMetricDtoWithoutOptimization().setKey("ncloc").setValueType(ValueType.INT.name()).setDirection(1);
+    MetricDto ncloc = newMetricDto().setKey("ncloc").setValueType(ValueType.INT.name()).setDirection(1);
     dbClient.metricDao().insert(dbSession, ncloc);
     dbClient.measureDao().insert(dbSession,
       newMeasureDto(ncloc, file1, projectSnapshot).setVariation(1.0d),
@@ -384,7 +384,7 @@ public class ComponentTreeActionTest {
     ComponentDto file3 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-3"));
     ComponentDto file2 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-2"));
     ComponentDto file1 = componentDb.insertComponent(newFileDto(projectDto, null, "file-uuid-1"));
-    MetricDto ncloc = newMetricDtoWithoutOptimization().setKey("new_ncloc").setValueType(ValueType.INT.name()).setDirection(1);
+    MetricDto ncloc = newMetricDto().setKey("new_ncloc").setValueType(ValueType.INT.name()).setDirection(1);
     dbClient.metricDao().insert(dbSession, ncloc);
     dbClient.measureDao().insert(dbSession,
       newMeasureDto(ncloc, file1, projectSnapshot).setVariation(1.0d),
@@ -555,6 +555,36 @@ public class ComponentTreeActionTest {
   }
 
   @Test
+  public void fail_when_using_DISTRIB_metrics() {
+    componentDb.insertProjectAndSnapshot(newProjectDto(db.getDefaultOrganization(), "project-uuid"));
+    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("distrib1").setValueType(ValueType.DISTRIB.name()));
+    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("distrib2").setValueType(ValueType.DISTRIB.name()));
+    db.commit();
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Metrics distrib1, distrib2 can't be requested in this web service. Please use api/measures/component");
+
+    call(ws.newRequest()
+      .setParam(PARAM_BASE_COMPONENT_ID, "project-uuid")
+      .setParam(PARAM_METRIC_KEYS, "distrib1,distrib2"));
+  }
+
+  @Test
+  public void fail_when_using_DATA_metrics() {
+    componentDb.insertProjectAndSnapshot(newProjectDto(db.getDefaultOrganization(), "project-uuid"));
+    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("data1").setValueType(ValueType.DISTRIB.name()));
+    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("data2").setValueType(ValueType.DISTRIB.name()));
+    db.commit();
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Metrics data1, data2 can't be requested in this web service. Please use api/measures/component");
+
+    call(ws.newRequest()
+      .setParam(PARAM_BASE_COMPONENT_ID, "project-uuid")
+      .setParam(PARAM_METRIC_KEYS, "data1,data2"));
+  }
+
+  @Test
   public void fail_when_search_query_have_less_than_3_characters() {
     componentDb.insertProjectAndSnapshot(newProjectDto(db.getDefaultOrganization(), "project-uuid"));
     insertNclocMetric();
@@ -659,8 +689,8 @@ public class ComponentTreeActionTest {
     }
   }
 
-  private static MetricDto newMetricDtoWithoutOptimization() {
-    return newMetricDto()
+  private static MetricDto newMetricDto() {
+    return MetricTesting.newMetricDto()
       .setWorstValue(null)
       .setBestValue(null)
       .setOptimizedBestValue(false)
@@ -730,7 +760,7 @@ public class ComponentTreeActionTest {
   }
 
   private MetricDto insertNewViolationsMetric() {
-    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDtoWithoutOptimization()
+    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDto()
       .setKey("new_violations")
       .setShortName("New issues")
       .setDescription("New Issues")
@@ -747,7 +777,7 @@ public class ComponentTreeActionTest {
   }
 
   private MetricDto insertNclocMetric() {
-    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDtoWithoutOptimization()
+    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDto()
       .setKey("ncloc")
       .setShortName("Lines of code")
       .setDescription("Non Commenting Lines of Code")
@@ -762,7 +792,7 @@ public class ComponentTreeActionTest {
   }
 
   private MetricDto insertComplexityMetric() {
-    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDtoWithoutOptimization()
+    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDto()
       .setKey("complexity")
       .setShortName("Complexity")
       .setDescription("Cyclomatic complexity")
@@ -777,7 +807,7 @@ public class ComponentTreeActionTest {
   }
 
   private MetricDto insertCoverageMetric() {
-    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDtoWithoutOptimization()
+    MetricDto metric = dbClient.metricDao().insert(dbSession, newMetricDto()
       .setKey("coverage")
       .setShortName("Coverage")
       .setDescription("Code Coverage")
diff --git a/server/sonar-server/src/test/java/org/sonar/server/measure/ws/ComponentTreeSortTest.java b/server/sonar-server/src/test/java/org/sonar/server/measure/ws/ComponentTreeSortTest.java
index 8e16f4bf355..247d50280e1 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/measure/ws/ComponentTreeSortTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/measure/ws/ComponentTreeSortTest.java
@@ -70,7 +70,7 @@ public class ComponentTreeSortTest {
       .setValueType(ValueType.INT.name());
     MetricDto sqaleIndexMetric = newMetricDto()
       .setKey(TEXT_METRIC_KEY)
-      .setValueType(ValueType.DATA.name());
+      .setValueType(ValueType.STRING.name());
 
     metrics = newArrayList(violationsMetric, sqaleIndexMetric);