SONAR-8117 Validate rating conditions

Check that warning and error value is a valid rating
Check that conditions "< A" and "> E" are not used

2 files changed, 102 insertions, 24 deletions

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 039dca7f630..c1482037138 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
@@ -21,26 +21,41 @@
 package org.sonar.server.qualitygate;
 
 import java.util.Collection;
+import java.util.List;
+import java.util.Objects;
 import java.util.stream.Collectors;
 import javax.annotation.Nullable;
 import org.apache.commons.lang.BooleanUtils;
 import org.apache.commons.lang.ObjectUtils;
 import org.sonar.api.measures.CoreMetrics;
-import org.sonar.api.measures.Metric;
+import org.sonar.api.measures.Metric.ValueType;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.metric.MetricDto;
 import org.sonar.db.qualitygate.QualityGateConditionDto;
 import org.sonar.db.qualitygate.QualityGateDto;
+import org.sonar.server.computation.task.projectanalysis.qualitymodel.RatingGrid.Rating;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.Errors;
+import org.sonar.server.exceptions.Message;
 import org.sonar.server.exceptions.NotFoundException;
 
+import static com.google.common.base.Strings.isNullOrEmpty;
+import static java.lang.Integer.parseInt;
 import static java.lang.String.format;
+import static java.util.Arrays.stream;
+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.OPERATOR_GREATER_THAN;
+import static org.sonar.db.qualitygate.QualityGateConditionDto.OPERATOR_LESS_THAN;
 import static org.sonar.db.qualitygate.QualityGateConditionDto.isOperatorAllowed;
+import static org.sonar.server.computation.task.projectanalysis.qualitymodel.RatingGrid.Rating.A;
+import static org.sonar.server.computation.task.projectanalysis.qualitymodel.RatingGrid.Rating.E;
 
 public class QualityGateConditionsUpdater {
 
+  private static final List<String> RATING_VALID_INT_VALUES = stream(Rating.values()).map(r -> Integer.toString(r.getIndex())).collect(Collectors.toList());
+
   private final DbClient dbClient;
 
   public QualityGateConditionsUpdater(DbClient dbClient) {
@@ -122,6 +137,7 @@ public class QualityGateConditionsUpdater {
     checkOperator(metric, operator, errors);
     checkThresholds(warningThreshold, errorThreshold, errors);
     checkPeriod(metric, period, errors);
+    checkRatingMetric(metric, operator, warningThreshold, errorThreshold, errors);
     if (!errors.isEmpty()) {
       throw new BadRequestException(errors);
     }
@@ -140,7 +156,7 @@ public class QualityGateConditionsUpdater {
   }
 
   private static void checkOperator(MetricDto metric, String operator, Errors errors) {
-    Metric.ValueType valueType = Metric.ValueType.valueOf(metric.getValueType());
+    ValueType valueType = valueOf(metric.getValueType());
     errors.check(isOperatorAllowed(operator, valueType), format("Operator %s is not allowed for metric type %s.", operator, metric.getValueType()));
   }
 
@@ -170,4 +186,44 @@ public class QualityGateConditionsUpdater {
     }
   }
 
+  private static void checkRatingMetric(MetricDto metric, String operator, @Nullable String warningThreshold, @Nullable String errorThreshold, Errors errors) {
+    if (!metric.getValueType().equals(RATING.name())) {
+      return;
+    }
+    if (!isValidRating(warningThreshold)) {
+      addInvalidRatingError(warningThreshold, errors);
+      return;
+    }
+    if (!isValidRating(errorThreshold)) {
+      addInvalidRatingError(errorThreshold, errors);
+      return;
+    }
+    if (OPERATOR_GREATER_THAN.equals(operator)) {
+      checkRatingGreaterThanOperator(warningThreshold, errors);
+      checkRatingGreaterThanOperator(errorThreshold, errors);
+    } else if (OPERATOR_LESS_THAN.equals(operator)) {
+      checkRatingLesserThanOperator(warningThreshold, errors);
+      checkRatingLesserThanOperator(errorThreshold, errors);
+    }
+  }
+
+  private static void addInvalidRatingError(@Nullable String value, Errors errors) {
+    errors.add(Message.of(format("'%s' is not a valid rating", value)));
+  }
+
+  private static void checkRatingGreaterThanOperator(@Nullable String value, Errors errors) {
+    errors.check(isNullOrEmpty(value) || !Objects.equals(toRating(value), E), format("There's no worse rating than E (%s)", value));
+  }
+
+  private static void checkRatingLesserThanOperator(@Nullable String value, Errors errors) {
+    errors.check(isNullOrEmpty(value) || !Objects.equals(toRating(value), A), format("There's no better rating than A (%s)", value));
+  }
+
+  private static Rating toRating(String value) {
+    return Rating.valueOf(parseInt(value));
+  }
+
+  private static boolean isValidRating(@Nullable String value) {
+    return isNullOrEmpty(value) || RATING_VALID_INT_VALUES.contains(value);
+  }
 }
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 aef9ffac6e1..09a3993fee9 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
@@ -69,12 +69,17 @@ public class QualityGateConditionsUpdaterTest {
     .setValueType(PERCENT.name())
     .setHidden(false);
 
+  MetricDto ratingMetricDto = newMetricDto()
+    .setKey("rating_metric")
+    .setValueType(RATING.name())
+    .setHidden(false);
+
   QualityGateConditionsUpdater underTest = new QualityGateConditionsUpdater(dbClient);
 
   @Before
   public void setUp() throws Exception {
     qualityGateDto = qualityGateDbTester.insertQualityGate();
-    dbClient.metricDao().insert(dbSession, coverageMetricDto);
+    dbClient.metricDao().insert(dbSession, coverageMetricDto, ratingMetricDto);
     dbSession.commit();
   }
 
@@ -100,23 +105,17 @@ public class QualityGateConditionsUpdaterTest {
 
   @Test
   public void create_condition_on_rating_metric() {
-    MetricDto metricDto = dbClient.metricDao().insert(dbSession, newMetricDto()
-      .setKey("rating_metric")
-      .setValueType(RATING.name())
-      .setHidden(false));
-    dbSession.commit();
-
     QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto.getId(), "rating_metric", "LT", null, "3", 1);
 
-    verifyCondition(result, metricDto.getId(), "LT", null, "3", 1);
+    verifyCondition(result, ratingMetricDto.getId(), "LT", null, "3", 1);
   }
 
   @Test
   public void fail_to_create_condition_when_condition_on_same_metric_already_exist() throws Exception {
     dbClient.gateConditionDao().insert(new QualityGateConditionDto()
-        .setQualityGateId(qualityGateDto.getId())
-        .setMetricId(coverageMetricDto.getId())
-        .setPeriod(null),
+      .setQualityGateId(qualityGateDto.getId())
+      .setMetricId(coverageMetricDto.getId())
+      .setPeriod(null),
       dbSession);
 
     expectedException.expect(BadRequestException.class);
@@ -127,9 +126,9 @@ public class QualityGateConditionsUpdaterTest {
   @Test
   public void fail_to_create_condition_when_condition_on_same_metric_and_on_leak_period_already_exist() throws Exception {
     dbClient.gateConditionDao().insert(new QualityGateConditionDto()
-        .setQualityGateId(qualityGateDto.getId())
-        .setMetricId(coverageMetricDto.getId())
-        .setPeriod(1),
+      .setQualityGateId(qualityGateDto.getId())
+      .setMetricId(coverageMetricDto.getId())
+      .setPeriod(1),
       dbSession);
 
     expectedException.expect(BadRequestException.class);
@@ -186,6 +185,34 @@ public class QualityGateConditionsUpdaterTest {
   }
 
   @Test
+  public void fail_to_create_warning_condition_on_invalid_rating_metric() {
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("'6' is not a valid rating");
+    underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "LT", "6", null, null);
+  }
+
+  @Test
+  public void fail_to_create_error_condition_on_invalid_rating_metric() {
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("'80' is not a valid rating");
+    underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "LT", null, "80", null);
+  }
+
+  @Test
+  public void fail_to_create_condition_on_greater_than_E() {
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("There's no worse rating than E (5)");
+    underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "GT", "5", null, null);
+  }
+
+  @Test
+  public void fail_to_create_condition_on_lesser_than_A() {
+    expectedException.expect(BadRequestException.class);
+    expectedException.expectMessage("There's no better rating than A (1)");
+    underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "LT", null, "1", null);
+  }
+
+  @Test
   public void update_condition() {
     QualityGateConditionDto condition = insertCondition(coverageMetricDto.getId(), "LT", null, "80", null);
 
@@ -205,16 +232,11 @@ public class QualityGateConditionsUpdaterTest {
 
   @Test
   public void update_condition_on_rating_metric() {
-    MetricDto metricDto = dbClient.metricDao().insert(dbSession, newMetricDto()
-      .setKey("rating_metric")
-      .setValueType(RATING.name())
-      .setHidden(false));
-    dbSession.commit();
-    QualityGateConditionDto condition = insertCondition(metricDto.getId(), "LT", null, "3", null);
+    QualityGateConditionDto condition = insertCondition(ratingMetricDto.getId(), "LT", null, "3", null);
 
     QualityGateConditionDto result = underTest.updateCondition(dbSession, condition.getId(), "rating_metric", "GT", "4", null, 1);
 
-    verifyCondition(result, metricDto.getId(), "GT", "4", null, 1);
+    verifyCondition(result, ratingMetricDto.getId(), "GT", "4", null, 1);
   }
 
   @Test
@@ -253,7 +275,7 @@ public class QualityGateConditionsUpdaterTest {
   }
 
   private QualityGateConditionDto insertCondition(long metricId, String operator, @Nullable String warning, @Nullable String error,
-                                                  @Nullable Integer period) {
+    @Nullable Integer period) {
     QualityGateConditionDto qualityGateConditionDto = new QualityGateConditionDto().setQualityGateId(qualityGateDto.getId())
       .setMetricId(metricId)
       .setOperator(operator)