SONAR-11572 Limit list of operators for QG conditions

6 files changed, 57 insertions, 81 deletions

diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/Condition.java b/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/Condition.java
index 7dd515f6d03..79c33a87b47 100644
--- a/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/Condition.java
+++ b/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/Condition.java
@@ -90,8 +90,6 @@ public class Condition {
   }
 
   public enum Operator {
-    EQUALS(QualityGateConditionDto.OPERATOR_EQUALS),
-    NOT_EQUALS(QualityGateConditionDto.OPERATOR_NOT_EQUALS),
     GREATER_THAN(QualityGateConditionDto.OPERATOR_GREATER_THAN),
     LESS_THAN(QualityGateConditionDto.OPERATOR_LESS_THAN);
 
@@ -111,5 +109,11 @@ public class Condition {
         .findFirst()
         .orElseThrow(() -> new IllegalArgumentException("Unsupported operator db value: " + s));
     }
+
+    public static boolean isValid(String s) {
+      return Stream.of(values())
+        .anyMatch(o -> o.getDbValue().equals(s));
+    }
+
   }
 }
diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/ConditionEvaluator.java b/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/ConditionEvaluator.java
index 8775bd62444..ed41eae97bc 100644
--- a/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/ConditionEvaluator.java
+++ b/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/ConditionEvaluator.java
@@ -26,8 +26,9 @@ import javax.annotation.CheckForNull;
 import org.sonar.api.measures.Metric.ValueType;
 import org.sonar.server.qualitygate.EvaluatedCondition.EvaluationStatus;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.String.format;
 import static java.util.Optional.of;
-import static org.sonar.api.measures.Metric.ValueType.BOOL;
 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;
@@ -37,7 +38,7 @@ import static org.sonar.api.measures.Metric.ValueType.WORK_DUR;
 
 class ConditionEvaluator {
 
-  private static final Set<ValueType> NUMERICAL_TYPES = EnumSet.of(BOOL, INT, RATING, FLOAT, MILLISEC, PERCENT, WORK_DUR);
+  private static final Set<ValueType> NUMERICAL_TYPES = EnumSet.of(INT, RATING, FLOAT, MILLISEC, PERCENT, WORK_DUR);
 
   private ConditionEvaluator() {
     // prevent instantiation
@@ -59,8 +60,7 @@ class ConditionEvaluator {
 
     ValueType type = measure.get().getType();
     return evaluateCondition(condition, type, value.get())
-      .orElseGet(() -> evaluateCondition(condition, type, value.get())
-        .orElseGet(() -> new EvaluatedCondition(condition, EvaluationStatus.OK, value.get().toString())));
+      .orElseGet(() -> new EvaluatedCondition(condition, EvaluationStatus.OK, value.get().toString()));
   }
 
   /**
@@ -79,8 +79,6 @@ class ConditionEvaluator {
     String valString = condition.getErrorThreshold();
     try {
       switch (valueType) {
-        case BOOL:
-          return parseInteger(valString) == 1;
         case INT:
         case RATING:
           return parseInteger(valString);
@@ -90,14 +88,13 @@ class ConditionEvaluator {
         case FLOAT:
         case PERCENT:
           return Double.parseDouble(valString);
-        case STRING:
         case LEVEL:
           return valueType;
         default:
-          throw new IllegalArgumentException(String.format("Unsupported value type %s. Cannot convert condition value", valueType));
+          throw new IllegalArgumentException(format("Unsupported value type %s. Cannot convert condition value", valueType));
       }
     } catch (NumberFormatException badValueFormat) {
-      throw new IllegalArgumentException(String.format(
+      throw new IllegalArgumentException(format(
         "Quality Gate: unable to parse threshold '%s' to compare against %s", valString, condition.getMetricKey()));
     }
   }
@@ -116,14 +113,9 @@ class ConditionEvaluator {
       return measure.getValue().isPresent() ? getNumericValue(measure.getType(), measure.getValue().getAsDouble()) : null;
     }
 
-    switch (measure.getType()) {
-      case LEVEL:
-      case STRING:
-      case DISTRIB:
-        return measure.getStringValue().orElse(null);
-      default:
-        throw new IllegalArgumentException("Condition is not allowed for type " + measure.getType());
-    }
+    checkArgument(ValueType.LEVEL.equals(measure.getType()), "Condition is not allowed for type %s" , measure.getType());
+    return measure.getStringValue().orElse(null);
+
   }
 
   @CheckForNull
@@ -137,8 +129,6 @@ class ConditionEvaluator {
 
   private static Comparable getNumericValue(ValueType type, double value) {
     switch (type) {
-      case BOOL:
-        return Double.compare(value, 1.0) == 1;
       case INT:
       case RATING:
         return (int) value;
@@ -160,16 +150,12 @@ class ConditionEvaluator {
   private static boolean reachThreshold(Comparable measureValue, Comparable threshold, Condition condition) {
     int comparison = measureValue.compareTo(threshold);
     switch (condition.getOperator()) {
-      case EQUALS:
-        return comparison == 0;
-      case NOT_EQUALS:
-        return comparison != 0;
       case GREATER_THAN:
         return comparison > 0;
       case LESS_THAN:
         return comparison < 0;
       default:
-        throw new IllegalArgumentException(String.format("Unsupported operator '%s'", condition.getOperator()));
+        throw new IllegalArgumentException(format("Unsupported operator '%s'", condition.getOperator()));
     }
   }
 }
diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/ConditionEvaluatorTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/ConditionEvaluatorTest.java
index 01a25df60c3..831de831af1 100644
--- a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/ConditionEvaluatorTest.java
+++ b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/ConditionEvaluatorTest.java
@@ -19,36 +19,31 @@
  */
 package org.sonar.server.qualitygate;
 
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
 import java.util.Optional;
 import java.util.OptionalDouble;
 import javax.annotation.Nullable;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
+import org.junit.runner.RunWith;
 import org.sonar.api.measures.Metric;
 
 import static org.assertj.core.api.Assertions.assertThat;
+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.STRING;
 import static org.sonar.server.qualitygate.ConditionEvaluatorTest.FakeMeasure.newFakeMeasureOnLeak;
 
+@RunWith(DataProviderRunner.class)
 public class ConditionEvaluatorTest {
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
 
   @Test
-  public void EQUALS_double() {
-    test(new FakeMeasure(10.1d), Condition.Operator.EQUALS, "10.2", EvaluatedCondition.EvaluationStatus.OK, "10.1");
-    test(new FakeMeasure(10.2d), Condition.Operator.EQUALS, "10.2", EvaluatedCondition.EvaluationStatus.ERROR, "10.2");
-    test(new FakeMeasure(10.3d), Condition.Operator.EQUALS, "10.2", EvaluatedCondition.EvaluationStatus.OK, "10.3");
-  }
-
-  @Test
-  public void NOT_EQUALS_double() {
-    test(new FakeMeasure(10.1d), Condition.Operator.NOT_EQUALS, "10.2", EvaluatedCondition.EvaluationStatus.ERROR, "10.1");
-    test(new FakeMeasure(10.2d), Condition.Operator.NOT_EQUALS, "10.2", EvaluatedCondition.EvaluationStatus.OK, "10.2");
-    test(new FakeMeasure(10.3d), Condition.Operator.NOT_EQUALS, "10.2", EvaluatedCondition.EvaluationStatus.ERROR, "10.3");
-  }
-
-  @Test
   public void GREATER_THAN_double() {
     test(new FakeMeasure(10.1d), Condition.Operator.GREATER_THAN, "10.2", EvaluatedCondition.EvaluationStatus.OK, "10.1");
     test(new FakeMeasure(10.2d), Condition.Operator.GREATER_THAN, "10.2", EvaluatedCondition.EvaluationStatus.OK, "10.2");
@@ -63,30 +58,6 @@ public class ConditionEvaluatorTest {
   }
 
   @Test
-  public void EQUALS_int() {
-    test(new FakeMeasure(10), Condition.Operator.EQUALS, "9", EvaluatedCondition.EvaluationStatus.OK, "10");
-    test(new FakeMeasure(10), Condition.Operator.EQUALS, "10", EvaluatedCondition.EvaluationStatus.ERROR, "10");
-    test(new FakeMeasure(10), Condition.Operator.EQUALS, "11", EvaluatedCondition.EvaluationStatus.OK, "10");
-
-    // badly stored thresholds are truncated
-    test(new FakeMeasure(10), Condition.Operator.EQUALS, "10.4", EvaluatedCondition.EvaluationStatus.ERROR, "10");
-    test(new FakeMeasure(10), Condition.Operator.EQUALS, "10.9", EvaluatedCondition.EvaluationStatus.ERROR, "10");
-    test(new FakeMeasure(11), Condition.Operator.EQUALS, "10.9", EvaluatedCondition.EvaluationStatus.OK, "11");
-  }
-
-  @Test
-  public void NOT_EQUALS_int() {
-    test(new FakeMeasure(10), Condition.Operator.NOT_EQUALS, "9", EvaluatedCondition.EvaluationStatus.ERROR, "10");
-    test(new FakeMeasure(10), Condition.Operator.NOT_EQUALS, "10", EvaluatedCondition.EvaluationStatus.OK, "10");
-    test(new FakeMeasure(10), Condition.Operator.NOT_EQUALS, "11", EvaluatedCondition.EvaluationStatus.ERROR, "10");
-
-    // badly stored thresholds are truncated
-    test(new FakeMeasure(10), Condition.Operator.NOT_EQUALS, "10.4", EvaluatedCondition.EvaluationStatus.OK, "10");
-    test(new FakeMeasure(10), Condition.Operator.NOT_EQUALS, "10.9", EvaluatedCondition.EvaluationStatus.OK, "10");
-    test(new FakeMeasure(10), Condition.Operator.NOT_EQUALS, "9.9", EvaluatedCondition.EvaluationStatus.ERROR, "10");
-  }
-
-  @Test
   public void GREATER_THAN_int() {
     test(new FakeMeasure(10), Condition.Operator.GREATER_THAN, "9", EvaluatedCondition.EvaluationStatus.ERROR, "10");
     test(new FakeMeasure(10), Condition.Operator.GREATER_THAN, "10", EvaluatedCondition.EvaluationStatus.OK, "10");
@@ -102,12 +73,10 @@ public class ConditionEvaluatorTest {
     test(new FakeMeasure(10), Condition.Operator.LESS_THAN, "9", EvaluatedCondition.EvaluationStatus.OK, "10");
     test(new FakeMeasure(10), Condition.Operator.LESS_THAN, "10", EvaluatedCondition.EvaluationStatus.OK, "10");
     test(new FakeMeasure(10), Condition.Operator.LESS_THAN, "11", EvaluatedCondition.EvaluationStatus.ERROR, "10");
-    test(new FakeMeasure(10), Condition.Operator.LESS_THAN, "11", EvaluatedCondition.EvaluationStatus.ERROR, "10");
 
     testOnLeak(newFakeMeasureOnLeak(10), Condition.Operator.LESS_THAN, "9", EvaluatedCondition.EvaluationStatus.OK, "10");
     testOnLeak(newFakeMeasureOnLeak(10), Condition.Operator.LESS_THAN, "10", EvaluatedCondition.EvaluationStatus.OK, "10");
     testOnLeak(newFakeMeasureOnLeak(10), Condition.Operator.LESS_THAN, "11", EvaluatedCondition.EvaluationStatus.ERROR, "10");
-    testOnLeak(newFakeMeasureOnLeak(10), Condition.Operator.LESS_THAN, "11", EvaluatedCondition.EvaluationStatus.ERROR, "10");
   }
 
   @Test
@@ -125,11 +94,24 @@ public class ConditionEvaluatorTest {
     test(null, Condition.Operator.LESS_THAN, "9", EvaluatedCondition.EvaluationStatus.OK, null);
   }
 
+
   @Test
-  public void empty_warning_condition() {
-    test(new FakeMeasure(10), Condition.Operator.LESS_THAN, "9", EvaluatedCondition.EvaluationStatus.OK, "10");
-    test(new FakeMeasure(10), Condition.Operator.LESS_THAN, "9", EvaluatedCondition.EvaluationStatus.OK, "10");
-    test(new FakeMeasure(3), Condition.Operator.LESS_THAN, "9", EvaluatedCondition.EvaluationStatus.ERROR, "3");
+  @UseDataProvider("unsupportedMetricTypes")
+  public void fail_when_condition_is_on_unsupported_metric(Metric.ValueType metricType) {
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage(String.format("Condition is not allowed for type %s", metricType));
+
+    test(new FakeMeasure(metricType), Condition.Operator.LESS_THAN, "9", EvaluatedCondition.EvaluationStatus.OK, "10");
+  }
+
+  @DataProvider
+  public static Object[][] unsupportedMetricTypes() {
+    return new Object[][] {
+      {BOOL},
+      {STRING},
+      {DATA},
+      {DISTRIB}
+    };
   }
 
   private void test(@Nullable QualityGateEvaluator.Measure measure, Condition.Operator operator, String errorThreshold, EvaluatedCondition.EvaluationStatus expectedStatus, @Nullable String expectedValue) {
@@ -181,6 +163,10 @@ public class ConditionEvaluatorTest {
 
     }
 
+    FakeMeasure(Metric.ValueType valueType) {
+      this.valueType = valueType;
+    }
+
     FakeMeasure(@Nullable Double value) {
       this.value = value;
       this.valueType = Metric.ValueType.FLOAT;
diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/ConditionTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/ConditionTest.java
index 0799b033a62..94f0754528f 100644
--- a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/ConditionTest.java
+++ b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/ConditionTest.java
@@ -28,7 +28,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 
 public class ConditionTest {
   private static final String METRIC_KEY = "metric_key";
-  private static final Condition.Operator OPERATOR = Condition.Operator.EQUALS;
+  private static final Condition.Operator OPERATOR = Condition.Operator.GREATER_THAN;
   private static final String ERROR_THRESHOLD = "2";
 
   @Rule
@@ -70,7 +70,7 @@ public class ConditionTest {
   @Test
   public void toString_is_override() {
     assertThat(underTest.toString())
-      .isEqualTo("Condition{metricKey='metric_key', operator=EQUALS, errorThreshold='2'}");
+      .isEqualTo("Condition{metricKey='metric_key', operator=GREATER_THAN, errorThreshold='2'}");
   }
 
   @Test
diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/EvaluatedConditionTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/EvaluatedConditionTest.java
index 1806aa82368..b0de4ed2959 100644
--- a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/EvaluatedConditionTest.java
+++ b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/EvaluatedConditionTest.java
@@ -24,12 +24,12 @@ import org.junit.Test;
 import org.junit.rules.ExpectedException;
 
 import static org.assertj.core.api.Assertions.assertThat;
-import static org.sonar.server.qualitygate.Condition.Operator.EQUALS;
+import static org.sonar.server.qualitygate.Condition.Operator.GREATER_THAN;
 import static org.sonar.server.qualitygate.EvaluatedCondition.EvaluationStatus.ERROR;
 import static org.sonar.server.qualitygate.EvaluatedCondition.EvaluationStatus.OK;
 
 public class EvaluatedConditionTest {
-  private static final Condition CONDITION_1 = new Condition("metricKey", EQUALS, "2");
+  private static final Condition CONDITION_1 = new Condition("metricKey", GREATER_THAN, "2");
 
   @Rule
   public ExpectedException expectedException = ExpectedException.none();
@@ -71,7 +71,7 @@ public class EvaluatedConditionTest {
   @Test
   public void override_toString() {
     assertThat(underTest.toString()).isEqualTo("EvaluatedCondition{condition=" +
-      "Condition{metricKey='metricKey', operator=EQUALS, errorThreshold='2'}, " +
+      "Condition{metricKey='metricKey', operator=GREATER_THAN, errorThreshold='2'}, " +
       "status=ERROR, value='value'}");
   }
 
@@ -80,7 +80,7 @@ public class EvaluatedConditionTest {
     EvaluatedCondition underTest = new EvaluatedCondition(CONDITION_1, ERROR, null);
 
     assertThat(underTest.toString()).isEqualTo("EvaluatedCondition{condition=" +
-      "Condition{metricKey='metricKey', operator=EQUALS, errorThreshold='2'}, " +
+      "Condition{metricKey='metricKey', operator=GREATER_THAN, errorThreshold='2'}, " +
       "status=ERROR, value=null}");
   }
 
@@ -90,7 +90,7 @@ public class EvaluatedConditionTest {
     assertThat(underTest).isEqualTo(new EvaluatedCondition(CONDITION_1, ERROR, "value"));
     assertThat(underTest).isNotEqualTo(null);
     assertThat(underTest).isNotEqualTo(new Object());
-    assertThat(underTest).isNotEqualTo(new EvaluatedCondition(new Condition("other_metric", EQUALS, "a"), ERROR, "value"));
+    assertThat(underTest).isNotEqualTo(new EvaluatedCondition(new Condition("other_metric", GREATER_THAN, "a"), ERROR, "value"));
     assertThat(underTest).isNotEqualTo(new EvaluatedCondition(CONDITION_1, OK, "value"));
     assertThat(underTest).isNotEqualTo(new EvaluatedCondition(CONDITION_1, ERROR, null));
     assertThat(underTest).isNotEqualTo(new EvaluatedCondition(CONDITION_1, ERROR, "other_value"));
@@ -102,7 +102,7 @@ public class EvaluatedConditionTest {
     assertThat(underTest.hashCode()).isEqualTo(new EvaluatedCondition(CONDITION_1, ERROR, "value").hashCode());
     assertThat(underTest.hashCode()).isNotEqualTo(null);
     assertThat(underTest.hashCode()).isNotEqualTo(new Object().hashCode());
-    assertThat(underTest.hashCode()).isNotEqualTo(new EvaluatedCondition(new Condition("other_metric", EQUALS, "a"), ERROR, "value").hashCode());
+    assertThat(underTest.hashCode()).isNotEqualTo(new EvaluatedCondition(new Condition("other_metric", GREATER_THAN, "a"), ERROR, "value").hashCode());
     assertThat(underTest.hashCode()).isNotEqualTo(new EvaluatedCondition(CONDITION_1, OK, "value").hashCode());
     assertThat(underTest.hashCode()).isNotEqualTo(new EvaluatedCondition(CONDITION_1, ERROR, null).hashCode());
     assertThat(underTest.hashCode()).isNotEqualTo(new EvaluatedCondition(CONDITION_1, ERROR, "other_value").hashCode());
diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/QualityGateTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/QualityGateTest.java
index 19cd6cdacca..d3dee0b6939 100644
--- a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/QualityGateTest.java
+++ b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/QualityGateTest.java
@@ -35,7 +35,7 @@ import static org.assertj.core.api.Assertions.assertThat;
 public class QualityGateTest {
   private static final String QUALIGATE_ID = "qg_id";
   private static final String QUALIGATE_NAME = "qg_name";
-  private static final Condition CONDITION_1 = new Condition("m1", Condition.Operator.EQUALS, "1");
+  private static final Condition CONDITION_1 = new Condition("m1", Condition.Operator.GREATER_THAN, "1");
   private static final Condition CONDITION_2 = new Condition("m2", Condition.Operator.LESS_THAN, "2");
 
   @Rule
@@ -74,10 +74,10 @@ public class QualityGateTest {
     Random random = new Random();
     Set<Condition> conditions = Stream.of(
       IntStream.range(0, random.nextInt(5))
-        .mapToObj(i -> new Condition("m_before_" + i, Condition.Operator.EQUALS, "10")),
+        .mapToObj(i -> new Condition("m_before_" + i, Condition.Operator.GREATER_THAN, "10")),
       Stream.of((Condition) null),
       IntStream.range(0, random.nextInt(5))
-        .mapToObj(i -> new Condition("m_after_" + i, Condition.Operator.EQUALS, "10")))
+        .mapToObj(i -> new Condition("m_after_" + i, Condition.Operator.GREATER_THAN, "10")))
       .flatMap(s -> s)
       .collect(Collectors.toSet());