From f251a4ff3671c30c2ed51f37bf12c0627008bc75 Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 28 Oct 2016 16:39:21 +0200 Subject: [PATCH] SONAR-5857 It should only be possible to use rating metrics defined in core for quality gates --- .../QualityGateConditionsUpdater.java | 4 ++ .../qualitygate/ValidRatingMetrics.java | 43 +++++++++++++++++++ .../server/qualitygate/ws/AppAction.java | 5 ++- .../QualityGateConditionsUpdaterTest.java | 29 +++++++++---- .../server/qualitygate/ws/AppActionTest.java | 32 +++++++------- 5 files changed, 88 insertions(+), 25 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/qualitygate/ValidRatingMetrics.java diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java index 4cd15ce1941..62677ca2e1e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java @@ -48,6 +48,7 @@ import static org.sonar.api.measures.Metric.ValueType.RATING; import static org.sonar.api.measures.Metric.ValueType.valueOf; import static org.sonar.db.qualitygate.QualityGateConditionDto.isOperatorAllowed; import static org.sonar.server.computation.task.projectanalysis.qualitymodel.RatingGrid.Rating.E; +import static org.sonar.server.qualitygate.ValidRatingMetrics.isCoreRatingMetric; public class QualityGateConditionsUpdater { @@ -187,6 +188,9 @@ public class QualityGateConditionsUpdater { if (!metric.getValueType().equals(RATING.name())) { return; } + if (!isCoreRatingMetric(metric.getKey())) { + errors.add(Message.of(format("The metric '%s' cannot be used", metric.getShortName()))); + } if (period != null && !metric.getKey().startsWith("new_")) { errors.add(Message.of(format("The metric '%s' cannot be used on the leak period", metric.getShortName()))); } diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ValidRatingMetrics.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ValidRatingMetrics.java new file mode 100644 index 00000000000..c33f3b771c5 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ValidRatingMetrics.java @@ -0,0 +1,43 @@ +/* + * SonarQube + * Copyright (C) 2009-2016 SonarSource SA + * mailto:contact AT sonarsource DOT com + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.qualitygate; + +import java.util.Set; +import org.sonar.api.measures.CoreMetrics; + +import static org.sonar.api.measures.Metric.ValueType.RATING; +import static org.sonar.core.util.stream.Collectors.toSet; + +public class ValidRatingMetrics { + + private static final Set CORE_RATING_METRICS = CoreMetrics.getMetrics().stream() + .filter(metric -> metric.getType().equals(RATING)) + .map(org.sonar.api.measures.Metric::getKey) + .collect(toSet()); + + private ValidRatingMetrics() { + // only static methods + } + + public static boolean isCoreRatingMetric(String metricKey) { + return CORE_RATING_METRICS.contains(metricKey); + } +} diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/AppAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/AppAction.java index c0d5900ce54..90dc4c66cfb 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/AppAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/AppAction.java @@ -31,7 +31,9 @@ import org.sonar.server.user.UserSession; import org.sonarqube.ws.WsQualityGates.AppWsResponse.Metric; import static org.sonar.api.measures.CoreMetrics.ALERT_STATUS_KEY; +import static org.sonar.api.measures.Metric.ValueType.RATING; import static org.sonar.core.permission.GlobalPermissions.QUALITY_GATE_ADMIN; +import static org.sonar.server.qualitygate.ValidRatingMetrics.isCoreRatingMetric; import static org.sonar.server.ws.WsUtils.writeProtobuf; import static org.sonarqube.ws.WsQualityGates.AppWsResponse; @@ -84,7 +86,8 @@ public class AppAction implements QualityGatesWsAction { DbSession dbSession = dbClient.openSession(false); try { return dbClient.metricDao().selectEnabled(dbSession).stream() - .filter(metric -> !metric.isDataType() && !ALERT_STATUS_KEY.equals(metric.getKey())) + .filter(metric -> !metric.isDataType() && !ALERT_STATUS_KEY.equals(metric.getKey()) && + (!RATING.name().equals(metric.getValueType()) || isCoreRatingMetric(metric.getKey()))) .collect(Collectors.toList()); } finally { dbClient.closeSession(dbSession); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGateConditionsUpdaterTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGateConditionsUpdaterTest.java index 4f5a81e5797..f987aa6347f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGateConditionsUpdaterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGateConditionsUpdaterTest.java @@ -70,8 +70,8 @@ public class QualityGateConditionsUpdaterTest { .setHidden(false); MetricDto ratingMetricDto = newMetricDto() - .setKey("rating_metric") - .setShortName("Rating") + .setKey("reliability_rating") + .setShortName("Reliability Rating") .setValueType(RATING.name()) .setHidden(false); @@ -180,7 +180,7 @@ public class QualityGateConditionsUpdaterTest { @Test public void create_condition_on_rating_metric() { - QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto.getId(), "rating_metric", "GT", null, "3", null); + QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "GT", null, "3", null); verifyCondition(result, ratingMetricDto.getId(), "GT", null, "3", null); } @@ -188,8 +188,8 @@ public class QualityGateConditionsUpdaterTest { @Test public void fail_to_create_condition_on_rating_metric_on_leak_period() { expectedException.expect(BadRequestException.class); - expectedException.expectMessage("The metric 'Rating' cannot be used on the leak period"); - underTest.createCondition(dbSession, qualityGateDto.getId(), "rating_metric", "GT", null, "3", 1); + expectedException.expectMessage("The metric 'Reliability Rating' cannot be used on the leak period"); + underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "GT", null, "3", 1); } @Test @@ -235,7 +235,7 @@ public class QualityGateConditionsUpdaterTest { public void update_condition_on_rating_metric() { QualityGateConditionDto condition = insertCondition(ratingMetricDto.getId(), "LT", null, "3", null); - QualityGateConditionDto result = underTest.updateCondition(dbSession, condition.getId(), "rating_metric", "GT", "4", null, null); + QualityGateConditionDto result = underTest.updateCondition(dbSession, condition.getId(), ratingMetricDto.getKey(), "GT", "4", null, null); verifyCondition(result, ratingMetricDto.getId(), "GT", "4", null, null); } @@ -245,8 +245,21 @@ public class QualityGateConditionsUpdaterTest { QualityGateConditionDto condition = insertCondition(ratingMetricDto.getId(), "LT", null, "3", null); expectedException.expect(BadRequestException.class); - expectedException.expectMessage("The metric 'Rating' cannot be used on the leak period"); - underTest.updateCondition(dbSession, condition.getId(), "rating_metric", "GT", "4", null, 1); + expectedException.expectMessage("The metric 'Reliability Rating' cannot be used on the leak period"); + underTest.updateCondition(dbSession, condition.getId(), ratingMetricDto.getKey(), "GT", "4", null, 1); + } + + @Test + public void fail_to_update_condition_on_rating_metric_on_not_core_rating_metric() { + MetricDto metricDto = dbClient.metricDao().insert(dbSession, newMetricDto().setKey("rating_metric") + .setShortName("Not core rating") + .setValueType(RATING.name()).setHidden(false)); + QualityGateConditionDto condition = insertCondition(metricDto.getId(), "LT", null, "3", null); + dbSession.commit(); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("The metric 'Not core rating' cannot be used"); + underTest.updateCondition(dbSession, condition.getId(), metricDto.getKey(), "GT", "4", null, 1); } @Test diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/AppActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/AppActionTest.java index c4c29438eb8..3f9d5fc860f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/AppActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/AppActionTest.java @@ -29,6 +29,7 @@ import org.sonar.core.permission.GlobalPermissions; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.DbTester; +import org.sonar.db.metric.MetricDto; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsActionTester; import org.sonarqube.ws.MediaTypes; @@ -102,25 +103,17 @@ public class AppActionTest { } @Test - public void return_rating_metrics() throws Exception { - dbClient.metricDao().insert(dbSession, newMetricDto() - .setKey("reliability_rating") - .setShortName("Reliability Rating") - .setDomain("Reliability") - .setValueType(RATING.name()) - .setHidden(false)); - dbSession.commit(); + public void return_rating_metrics_only_from_core_metrics() throws Exception { + insertMetrics( + newMetricDto().setKey("reliability_rating").setValueType(RATING.name()).setHidden(false), + newMetricDto().setKey("new_reliability_rating").setValueType(RATING.name()).setHidden(false), + newMetricDto().setKey("sqale_rating").setValueType(RATING.name()).setHidden(false), + newMetricDto().setKey("none_core_rating").setValueType(RATING.name()).setHidden(false)); AppWsResponse response = executeRequest(); - List metrics = response.getMetricsList(); - assertThat(metrics).hasSize(1); - AppWsResponse.Metric metric = metrics.get(0); - assertThat(metric.getKey()).isEqualTo("reliability_rating"); - assertThat(metric.getName()).isEqualTo("Reliability Rating"); - assertThat(metric.getDomain()).isEqualTo("Reliability"); - assertThat(metric.getType()).isEqualTo(RATING.name()); - assertThat(metric.getHidden()).isFalse(); + assertThat(response.getMetricsList()).extracting(AppWsResponse.Metric::getKey).containsOnly( + "reliability_rating", "new_reliability_rating", "sqale_rating"); } @Test @@ -221,6 +214,13 @@ public class AppActionTest { assertThat(action.params()).isEmpty(); } + private void insertMetrics(MetricDto... metricDtos) { + for (MetricDto metricDto : metricDtos) { + dbClient.metricDao().insert(dbSession, metricDto); + } + dbSession.commit(); + } + private AppWsResponse executeRequest() { try { return AppWsResponse.parseFrom(ws.newRequest().setMediaType(MediaTypes.PROTOBUF).execute().getInputStream()); -- 2.39.5