From 149c8efb8cc053ac615f5512c6533447273614b6 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Thu, 13 Apr 2017 17:50:32 +0200 Subject: [PATCH] SONAR-9049 Prevent using DATA and DISTRIB metrics in api/measures/component_tree --- .../measure/ws/ComponentTreeAction.java | 8 ++- .../measure/ws/ComponentTreeDataLoader.java | 15 +++-- .../server/measure/ws/ComponentTreeSort.java | 7 +-- .../measure/ws/ComponentTreeActionTest.java | 58 ++++++++++++++----- .../measure/ws/ComponentTreeSortTest.java | 2 +- 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 METRIC_SORT_FILTERS = ImmutableSortedSet.of(ALL_METRIC_SORT_FILTER, WITH_MEASURES_ONLY_METRIC_SORT_FILTER); + static final Set 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 QUALIFIERS_ELIGIBLE_FOR_BEST_VALUE = newHashSet(Qualifiers.FILE, Qualifiers.UNIT_TEST_FILE); + private static final Set 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 NUMERIC_VALUE_TYPES = EnumSet.of(BOOL, FLOAT, INT, MILLISEC, WORK_DUR, PERCENT, RATING); - private static final Set TEXTUAL_VALUE_TYPES = EnumSet.of(DATA, DISTRIB, STRING); + private static final Set 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), @@ -554,6 +554,36 @@ public class ComponentTreeActionTest { .setParam(PARAM_METRIC_KEYS, "ncloc, new_violations, unknown-metric, another-unknown-metric")); } + @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")); @@ -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); -- 2.39.5