diff options
36 files changed, 461 insertions, 535 deletions
diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/api/posttask/ConditionToCondition.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/api/posttask/ConditionToCondition.java index 2226440dbcc..769089445b1 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/api/posttask/ConditionToCondition.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/api/posttask/ConditionToCondition.java @@ -73,10 +73,6 @@ class ConditionToCondition implements Function<Condition, QualityGate.Condition> private static QualityGate.Operator convert(Condition.Operator operator) { switch (operator) { - case EQUALS: - return QualityGate.Operator.EQUALS; - case NOT_EQUALS: - return QualityGate.Operator.NOT_EQUALS; case GREATER_THAN: return QualityGate.Operator.GREATER_THAN; case LESS_THAN: diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/Condition.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/Condition.java index 9aa090afc39..707450bfd62 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/Condition.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/Condition.java @@ -31,7 +31,7 @@ import static java.util.Objects.requireNonNull; public class Condition { public enum Operator { - EQUALS("EQ"), NOT_EQUALS("NE"), GREATER_THAN("GT"), LESS_THAN("LT"); + GREATER_THAN("GT"), LESS_THAN("LT"); private final String dbValue; diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionEvaluator.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionEvaluator.java index 2f5b44d83b8..c713e0abd06 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionEvaluator.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionEvaluator.java @@ -19,6 +19,7 @@ */ package org.sonar.ce.task.projectanalysis.qualitygate; +import java.util.EnumSet; import java.util.Optional; import javax.annotation.CheckForNull; import org.sonar.ce.task.projectanalysis.measure.Measure; @@ -26,14 +27,23 @@ import org.sonar.ce.task.projectanalysis.metric.Metric; import static com.google.common.base.Preconditions.checkArgument; import static java.util.Optional.of; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.FLOAT; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.INT; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.LEVEL; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.MILLISEC; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.PERCENT; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.RATING; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.WORK_DUR; public final class ConditionEvaluator { + private static final EnumSet<Metric.MetricType> SUPPORTED_METRIC_TYPE = EnumSet.of(INT, MILLISEC, RATING, WORK_DUR, FLOAT, PERCENT, LEVEL); + /** * Evaluates the condition for the specified measure */ public EvaluationResult evaluate(Condition condition, Measure measure) { - checkArgument(condition.getMetric().getType() != Metric.MetricType.DATA, "Conditions on MetricType DATA are not supported"); + checkArgument(SUPPORTED_METRIC_TYPE.contains(condition.getMetric().getType()), "Conditions on MetricType %s are not supported", condition.getMetric().getType()); Comparable measureComparable = parseMeasure(condition, measure); if (measureComparable == null) { @@ -61,10 +71,6 @@ public final class ConditionEvaluator { private static boolean doesReachThresholds(Comparable measureValue, Comparable criteriaValue, Condition condition) { int comparison = measureValue.compareTo(criteriaValue); switch (condition.getOperator()) { - case EQUALS: - return comparison == 0; - case NOT_EQUALS: - return comparison != 0; case GREATER_THAN: return comparison > 0; case LESS_THAN: @@ -76,15 +82,12 @@ public final class ConditionEvaluator { private static Comparable parseConditionValue(Metric metric, String value) { switch (metric.getType().getValueType()) { - case BOOLEAN: - return Integer.parseInt(value) == 1; case INT: return parseInteger(value); case LONG: return Long.parseLong(value); case DOUBLE: return Double.parseDouble(value); - case STRING: case LEVEL: return value; default: @@ -102,16 +105,12 @@ public final class ConditionEvaluator { return parseMeasureFromVariation(condition, measure); } switch (measure.getValueType()) { - case BOOLEAN: - return measure.getBooleanValue(); case INT: return measure.getIntValue(); case LONG: return measure.getLongValue(); case DOUBLE: return measure.getDoubleValue(); - case STRING: - return measure.getStringValue(); case LEVEL: return measure.getLevelValue().name(); case NO_VALUE: @@ -131,8 +130,6 @@ public final class ConditionEvaluator { Double variation = measure.getVariation(); Metric.MetricType metricType = condition.getMetric().getType(); switch (metricType.getValueType()) { - case BOOLEAN: - return variation.intValue() == 1; case INT: return variation.intValue(); case LONG: @@ -140,7 +137,6 @@ public final class ConditionEvaluator { case DOUBLE: return variation; case NO_VALUE: - case STRING: case LEVEL: default: throw new IllegalArgumentException("Unsupported metric type " + metricType); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/EvaluationResultTextConverterImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/EvaluationResultTextConverterImpl.java index 4bb8911286a..32d4cfbab5b 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/EvaluationResultTextConverterImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/qualitygate/EvaluationResultTextConverterImpl.java @@ -33,8 +33,6 @@ import static java.util.Objects.requireNonNull; public final class EvaluationResultTextConverterImpl implements EvaluationResultTextConverter { private static final Map<Condition.Operator, String> OPERATOR_LABELS = ImmutableMap.of( - Condition.Operator.EQUALS, "=", - Condition.Operator.NOT_EQUALS, "!=", Condition.Operator.GREATER_THAN, ">", Condition.Operator.LESS_THAN, "<"); @@ -59,14 +57,9 @@ public final class EvaluationResultTextConverterImpl implements EvaluationResult private String getAlertLabel(Condition condition) { String metric = i18n.message(Locale.ENGLISH, "metric." + condition.getMetric().getKey() + ".name", condition.getMetric().getName()); - StringBuilder stringBuilder = new StringBuilder(); - stringBuilder.append(metric); - - stringBuilder - .append(" ").append(OPERATOR_LABELS.get(condition.getOperator())).append(" ") - .append(alertValue(condition)); - - return stringBuilder.toString(); + return metric + + " " + OPERATOR_LABELS.get(condition.getOperator()) + " " + + alertValue(condition); } private String alertValue(Condition condition) { diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/api/posttask/ConditionToConditionTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/api/posttask/ConditionToConditionTest.java index 3fd169ee0ca..e721ae850f2 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/api/posttask/ConditionToConditionTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/api/posttask/ConditionToConditionTest.java @@ -45,7 +45,7 @@ public class ConditionToConditionTest { private static final Map<Condition, ConditionStatus> NO_STATUS_PER_CONDITIONS = Collections.emptyMap(); private static final String SOME_VALUE = "some value"; private static final ConditionStatus SOME_CONDITION_STATUS = ConditionStatus.create(ConditionStatus.EvaluationStatus.OK, SOME_VALUE); - private static final Condition SOME_CONDITION = new Condition(newMetric(METRIC_KEY), Condition.Operator.EQUALS.getDbValue(), ERROR_THRESHOLD); + private static final Condition SOME_CONDITION = new Condition(newMetric(METRIC_KEY), Condition.Operator.LESS_THAN.getDbValue(), ERROR_THRESHOLD); @Rule public ExpectedException expectedException = ExpectedException.none(); @@ -103,7 +103,7 @@ public class ConditionToConditionTest { @Test public void apply_copies_value() { - Condition otherCondition = new Condition(newMetric(METRIC_KEY), Condition.Operator.NOT_EQUALS.getDbValue(), ERROR_THRESHOLD); + Condition otherCondition = new Condition(newMetric(METRIC_KEY), Condition.Operator.LESS_THAN.getDbValue(), ERROR_THRESHOLD); ConditionToCondition underTest = new ConditionToCondition(of( SOME_CONDITION, SOME_CONDITION_STATUS, otherCondition, ConditionStatus.NO_VALUE_STATUS)); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/api/posttask/PostProjectAnalysisTasksExecutorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/api/posttask/PostProjectAnalysisTasksExecutorTest.java index 6a460d59425..2f578d0701b 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/api/posttask/PostProjectAnalysisTasksExecutorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/api/posttask/PostProjectAnalysisTasksExecutorTest.java @@ -391,7 +391,7 @@ public class PostProjectAnalysisTasksExecutorTest { private static Condition createCondition(String metricKey) { Metric metric = mock(Metric.class); when(metric.getKey()).thenReturn(metricKey); - return new Condition(metric, Condition.Operator.EQUALS.getDbValue(), "error threshold"); + return new Condition(metric, Condition.Operator.LESS_THAN.getDbValue(), "error threshold"); } } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/measure/qualitygatedetails/EvaluatedConditionTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/measure/qualitygatedetails/EvaluatedConditionTest.java index 8e85b43bb05..9b333c33515 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/measure/qualitygatedetails/EvaluatedConditionTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/measure/qualitygatedetails/EvaluatedConditionTest.java @@ -34,7 +34,7 @@ public class EvaluatedConditionTest { static { when(SOME_METRIC.getKey()).thenReturn("dummy key"); } - private static final Condition SOME_CONDITION = new Condition(SOME_METRIC, Condition.Operator.EQUALS.getDbValue(), "1"); + private static final Condition SOME_CONDITION = new Condition(SOME_METRIC, Condition.Operator.LESS_THAN.getDbValue(), "1"); private static final Measure.Level SOME_LEVEL = Measure.Level.OK; private static final String SOME_VALUE = "some value"; diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionEvaluatorTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionEvaluatorTest.java index d8d2f9de383..476ed506cb0 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionEvaluatorTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionEvaluatorTest.java @@ -23,6 +23,7 @@ import com.google.common.collect.ImmutableSet; import com.tngtech.java.junit.dataprovider.DataProvider; import com.tngtech.java.junit.dataprovider.DataProviderRunner; import com.tngtech.java.junit.dataprovider.UseDataProvider; +import java.util.EnumSet; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -42,18 +43,18 @@ import static org.sonar.ce.task.projectanalysis.measure.Measure.Level.OK; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.BOOL; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.DATA; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.DISTRIB; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.FLOAT; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.INT; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.LEVEL; +import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.MILLISEC; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.PERCENT; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.RATING; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.STRING; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.WORK_DUR; import static org.sonar.ce.task.projectanalysis.metric.Metric.MetricType.values; -import static org.sonar.ce.task.projectanalysis.qualitygate.Condition.Operator.EQUALS; import static org.sonar.ce.task.projectanalysis.qualitygate.Condition.Operator.GREATER_THAN; import static org.sonar.ce.task.projectanalysis.qualitygate.Condition.Operator.LESS_THAN; -import static org.sonar.ce.task.projectanalysis.qualitygate.Condition.Operator.NOT_EQUALS; import static org.sonar.ce.task.projectanalysis.qualitygate.EvaluationResultAssert.assertThat; @RunWith(DataProviderRunner.class) @@ -68,7 +69,7 @@ public class ConditionEvaluatorTest { try { Metric metric = createMetric(FLOAT); Measure measure = newMeasureBuilder().create(10.2d, 1, null); - underTest.evaluate(createErrorCondition(metric, LESS_THAN, "20"), measure); + underTest.evaluate(createCondition(metric, LESS_THAN, "20"), measure); } catch (NumberFormatException ex) { fail(); } @@ -76,7 +77,7 @@ public class ConditionEvaluatorTest { try { Metric metric = createMetric(INT); Measure measure = newMeasureBuilder().create(5, null); - underTest.evaluate(createErrorCondition(metric, LESS_THAN, "20.1"), measure); + underTest.evaluate(createCondition(metric, LESS_THAN, "20.1"), measure); } catch (NumberFormatException ex) { fail(); } @@ -84,56 +85,20 @@ public class ConditionEvaluatorTest { try { Metric metric = createMetric(PERCENT); Measure measure = newMeasureBuilder().create(10.2d, 1, null); - underTest.evaluate(createErrorCondition(metric, LESS_THAN, "20.1"), measure); + underTest.evaluate(createCondition(metric, LESS_THAN, "20.1"), measure); } catch (NumberFormatException ex) { fail(); } } @Test - public void testEquals_for_double() { - Metric metric = createMetric(FLOAT); - Measure measure = newMeasureBuilder().create(10.2d, 1, null); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR).hasValue(10.2d); - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.1"), measure)).hasLevel(OK).hasValue(10.2d); - } - - @Test - public void testEquals_for_String() { - Metric metric = createMetric(STRING); - Measure measure = newMeasureBuilder().create("TEST"); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "TEST"), measure)).hasLevel(ERROR).hasValue("TEST"); - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "TEST2"), measure)).hasLevel(OK).hasValue("TEST"); - } - - @Test - public void testNotEquals_for_double() { - Metric metric = createMetric(FLOAT); - Measure measure = newMeasureBuilder().create(10.2d, 1, null); - - assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "10.2"), measure)).hasLevel(OK).hasValue(10.2d); - assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "10.1"), measure)).hasLevel(ERROR).hasValue(10.2d); - } - - @Test - public void testNotEquals() { - Metric metric = createMetric(STRING); - Measure measure = newMeasureBuilder().create("TEST"); - - assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "TEST"), measure)).hasLevel(OK).hasValue("TEST"); - assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "TEST2"), measure)).hasLevel(ERROR).hasValue("TEST"); - } - - @Test public void testGreater() { Metric metric = createMetric(FLOAT); Measure measure = newMeasureBuilder().create(10.2d, 1, null); - assertThat(underTest.evaluate(createErrorCondition(metric, GREATER_THAN, "10.1"), measure)).hasLevel(ERROR).hasValue(10.2d); - assertThat(underTest.evaluate(createErrorCondition(metric, GREATER_THAN, "10.2"), measure)).hasLevel(OK).hasValue(10.2d); - assertThat(underTest.evaluate(createErrorCondition(metric, GREATER_THAN, "10.3"), measure)).hasLevel(OK).hasValue(10.2d); + assertThat(underTest.evaluate(createCondition(metric, GREATER_THAN, "10.1"), measure)).hasLevel(ERROR).hasValue(10.2d); + assertThat(underTest.evaluate(createCondition(metric, GREATER_THAN, "10.2"), measure)).hasLevel(OK).hasValue(10.2d); + assertThat(underTest.evaluate(createCondition(metric, GREATER_THAN, "10.3"), measure)).hasLevel(OK).hasValue(10.2d); } @Test @@ -141,89 +106,9 @@ public class ConditionEvaluatorTest { Metric metric = createMetric(FLOAT); Measure measure = newMeasureBuilder().create(10.2d, 1, null); - assertThat(underTest.evaluate(createErrorCondition(metric, LESS_THAN, "10.1"), measure)).hasLevel(OK).hasValue(10.2d); - assertThat(underTest.evaluate(createErrorCondition(metric, LESS_THAN, "10.2"), measure)).hasLevel(OK).hasValue(10.2d); - assertThat(underTest.evaluate(createErrorCondition(metric, LESS_THAN, "10.3"), measure)).hasLevel(ERROR).hasValue(10.2d); - } - - @Test - public void testEquals_Percent() { - Metric metric = createMetric(PERCENT); - Measure measure = newMeasureBuilder().create(10.2d, 1, null); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR).hasValue(10.2d); - } - - @Test - public void testEquals_Float() { - Metric metric = createMetric(PERCENT); - Measure measure = newMeasureBuilder().create(10.2d, 1, null); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR).hasValue(10.2d); - } - - @Test - public void testEquals_Int() { - Metric metric = createMetric(INT); - Measure measure = newMeasureBuilder().create(10, null); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10"), measure)).hasLevel(ERROR).hasValue(10); - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR).hasValue(10); - } - - @Test - public void testEquals_Level() { - Metric metric = createMetric(LEVEL); - Measure measure = newMeasureBuilder().create(ERROR); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, ERROR.name()), measure)).hasLevel(ERROR).hasValue(ERROR.name()); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, OK.name()), measure)).hasLevel(OK).hasValue(ERROR.name()); - } - - @Test - public void testNotEquals_Level() { - Metric metric = createMetric(LEVEL); - Measure measure = newMeasureBuilder().create(ERROR); - - assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, OK.name()), measure)).hasLevel(ERROR).hasValue(ERROR.name()); - } - - @Test - public void testEquals_BOOL() { - Metric metric = createMetric(BOOL); - Measure measure = newMeasureBuilder().create(false, null); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "1"), measure)).hasLevel(OK).hasValue(false); - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "0"), measure)).hasLevel(ERROR).hasValue(false); - } - - @Test - public void testNotEquals_BOOL() { - Metric metric = createMetric(BOOL); - Measure measure = newMeasureBuilder().create(false, null); - - assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "1"), measure)).hasLevel(ERROR).hasValue(false); - assertThat(underTest.evaluate(createErrorCondition(metric, NOT_EQUALS, "0"), measure)).hasLevel(OK).hasValue(false); - } - - @Test - public void getLevel_throws_IEA_if_error_threshold_is_not_parsable_boolean() { - Metric metric = createMetric(BOOL); - Measure measure = newMeasureBuilder().create(false, null); - - expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Quality Gate: Unable to parse value 'polop' to compare against name"); - - underTest.evaluate(createErrorCondition(metric, EQUALS, "polop"), measure); - } - - @Test - public void testEquals_work_duration() { - Metric metric = createMetric(WORK_DUR); - Measure measure = newMeasureBuilder().create(60l, null); - - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "60"), measure)).hasLevel(ERROR); + assertThat(underTest.evaluate(createCondition(metric, LESS_THAN, "10.1"), measure)).hasLevel(OK).hasValue(10.2d); + assertThat(underTest.evaluate(createCondition(metric, LESS_THAN, "10.2"), measure)).hasLevel(OK).hasValue(10.2d); + assertThat(underTest.evaluate(createCondition(metric, LESS_THAN, "10.3"), measure)).hasLevel(ERROR).hasValue(10.2d); } @Test @@ -234,7 +119,7 @@ public class ConditionEvaluatorTest { expectedException.expect(IllegalArgumentException.class); expectedException.expectMessage("Quality Gate: Unable to parse value 'polop' to compare against name"); - underTest.evaluate(createErrorCondition(metric, EQUALS, "polop"), measure); + underTest.evaluate(createCondition(metric, LESS_THAN, "polop"), measure); } @Test @@ -242,32 +127,42 @@ public class ConditionEvaluatorTest { Metric metric = createMetric(FLOAT); Measure measure = newMeasureBuilder().create(10.2d, 1, null); - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(ERROR); - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.1"), measure)).hasLevel(OK); + assertThat(underTest.evaluate(createCondition(metric, LESS_THAN, "10.3"), measure)).hasLevel(ERROR); + assertThat(underTest.evaluate(createCondition(metric, LESS_THAN, "10.1"), measure)).hasLevel(OK); - assertThat(underTest.evaluate(new Condition(metric, EQUALS.getDbValue(), "10.3"), measure)).hasLevel(Measure.Level.OK); assertThat(underTest.evaluate(new Condition(metric, LESS_THAN.getDbValue(), "10.3"), measure)).hasLevel(Measure.Level.ERROR); } @Test public void condition_is_always_ok_when_measure_is_noValue() { - for (MetricType metricType : from(asList(values())).filter(not(in(ImmutableSet.of(DATA, LEVEL))))) { + for (MetricType metricType : from(asList(values())).filter(not(in(ImmutableSet.of(BOOL, DATA, DISTRIB, STRING))))) { Metric metric = createMetric(metricType); Measure measure = newMeasureBuilder().createNoValue(); - assertThat(underTest.evaluate(createErrorCondition(metric, EQUALS, "10.2"), measure)).hasLevel(OK); + assertThat(underTest.evaluate(createCondition(metric, LESS_THAN, "10.2"), measure)).hasLevel(OK); } } @Test - public void testUnsupportedType() { - Metric metric = createMetric(DATA); + @UseDataProvider("unsupportedMetricTypes") + public void fail_when_metric_is_not_supported(MetricType metricType) { + Metric metric = createMetric(metricType); Measure measure = newMeasureBuilder().create("3.14159265358"); expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Conditions on MetricType DATA are not supported"); + expectedException.expectMessage(String.format("Conditions on MetricType %s are not supported", metricType)); - underTest.evaluate(createErrorCondition(metric, EQUALS, "1.60217657"), measure); + underTest.evaluate(createCondition(metric, LESS_THAN, "1.60217657"), measure); + } + + @DataProvider + public static Object[][] unsupportedMetricTypes() { + return new Object[][] { + {BOOL}, + {STRING}, + {DATA}, + {DISTRIB} + }; } @Test @@ -299,23 +194,14 @@ public class ConditionEvaluatorTest { } @Test - @UseDataProvider("unsupportedNewMetricTypes") - public void condition_on_new_metric_with_unsupported_type(MetricType metricType) { - Metric metric = createNewMetric(metricType); + public void fail_when_condition_on_leak_period_is_using_unsupported_metric() { + Metric metric = createNewMetric(LEVEL); Measure measure = newMeasureBuilder().setVariation(0d).createNoValue(); expectedException.expect(IllegalArgumentException.class); - expectedException.expectMessage("Unsupported metric type " + metricType); - - underTest.evaluate(new Condition(metric, EQUALS.getDbValue(), "3"), measure); - } + expectedException.expectMessage("Unsupported metric type LEVEL"); - @DataProvider - public static Object[][] unsupportedNewMetricTypes() { - return new Object[][] { - {STRING}, - {LEVEL}, - }; + underTest.evaluate(new Condition(metric, LESS_THAN.getDbValue(), "3"), measure); } @Test @@ -327,7 +213,7 @@ public class ConditionEvaluatorTest { assertThat(underTest.evaluate(new Condition(metric, GREATER_THAN.getDbValue(), "2"), measure)).hasLevel(ERROR).hasValue(4); } - private static Condition createErrorCondition(Metric metric, Condition.Operator operator, String errorThreshold) { + private static Condition createCondition(Metric metric, Condition.Operator operator, String errorThreshold) { return new Condition(metric, operator.getDbValue(), errorThreshold); } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionTest.java index e6acb186e2e..3e763512b14 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/ConditionTest.java @@ -35,7 +35,7 @@ public class ConditionTest { public ExpectedException expectedException = ExpectedException.none(); private static final Metric SOME_METRIC = mock(Metric.class); - private static final String SOME_OPERATOR = "EQ"; + private static final String SOME_OPERATOR = "LT"; @Before public void setUp() { @@ -67,7 +67,7 @@ public class ConditionTest { Condition condition = new Condition(SOME_METRIC, SOME_OPERATOR, error); assertThat(condition.getMetric()).isSameAs(SOME_METRIC); - assertThat(condition.getOperator()).isSameAs(Condition.Operator.EQUALS); + assertThat(condition.getOperator()).isSameAs(Condition.Operator.LESS_THAN); assertThat(condition.getErrorThreshold()).isEqualTo(error); } @@ -76,7 +76,8 @@ public class ConditionTest { when(SOME_METRIC.toString()).thenReturn("metric1"); assertThat(new Condition(SOME_METRIC, SOME_OPERATOR, "error_l").toString()) - .isEqualTo("Condition{metric=metric1, operator=EQUALS, errorThreshold=error_l}"); + .isEqualTo("Condition{metric=metric1, operator=LESS_THAN, errorThreshold=error_l}"); } + } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/EvaluationResultTextConverterTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/EvaluationResultTextConverterTest.java index 5a9213c5255..fe5e3819f9c 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/EvaluationResultTextConverterTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/EvaluationResultTextConverterTest.java @@ -42,7 +42,7 @@ import static org.sonar.ce.task.projectanalysis.measure.Measure.Level.ERROR; public class EvaluationResultTextConverterTest { private static final Metric INT_METRIC = new MetricImpl(1, "key", "int_metric_name", Metric.MetricType.INT); private static final Metric SOME_VARIATION_METRIC = new MetricImpl(2, "new_variation_of_trololo", "variation_of_trololo_name", Metric.MetricType.INT); - private static final Condition EQ_10_CONDITION = new Condition(INT_METRIC, Condition.Operator.EQUALS.getDbValue(), "10"); + private static final Condition LT_10_CONDITION = new Condition(INT_METRIC, Condition.Operator.LESS_THAN.getDbValue(), "10"); private static final EvaluationResult OK_EVALUATION_RESULT = new EvaluationResult(Measure.Level.OK, null); private static final String ERROR_THRESHOLD = "error_threshold"; @@ -57,12 +57,12 @@ public class EvaluationResultTextConverterTest { @Test(expected = NullPointerException.class) public void evaluate_throws_NPE_if_EvaluationResult_arg_is_null() { - underTest.asText(EQ_10_CONDITION, null); + underTest.asText(LT_10_CONDITION, null); } @Test public void evaluate_returns_null_if_EvaluationResult_has_level_OK() { - assertThat(underTest.asText(EQ_10_CONDITION, OK_EVALUATION_RESULT)).isNull(); + assertThat(underTest.asText(LT_10_CONDITION, OK_EVALUATION_RESULT)).isNull(); } @DataProvider @@ -104,10 +104,6 @@ public class EvaluationResultTextConverterTest { private static String toSign(Condition.Operator operator) { switch (operator) { - case EQUALS: - return "="; - case NOT_EQUALS: - return "!="; case GREATER_THAN: return ">"; case LESS_THAN: diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/QualityGateServiceImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/QualityGateServiceImplTest.java index efcf5054b30..934bf7f99bc 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/QualityGateServiceImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/qualitygate/QualityGateServiceImplTest.java @@ -53,9 +53,9 @@ public class QualityGateServiceImplTest { private static final long METRIC_ID_2 = 753; private static final Metric METRIC_1 = mock(Metric.class); private static final Metric METRIC_2 = mock(Metric.class); - private static final QualityGateConditionDto CONDITION_1 = new QualityGateConditionDto().setId(321).setMetricId(METRIC_ID_1).setOperator("EQ") + private static final QualityGateConditionDto CONDITION_1 = new QualityGateConditionDto().setId(321).setMetricId(METRIC_ID_1).setOperator("LT") .setErrorThreshold("error_th"); - private static final QualityGateConditionDto CONDITION_2 = new QualityGateConditionDto().setId(456).setMetricId(METRIC_ID_2).setOperator("NE").setErrorThreshold("error_th"); + private static final QualityGateConditionDto CONDITION_2 = new QualityGateConditionDto().setId(456).setMetricId(METRIC_ID_2).setOperator("GT").setErrorThreshold("error_th"); private QualityGateDao qualityGateDao = mock(QualityGateDao.class); private QualityGateConditionDao qualityGateConditionDao = mock(QualityGateConditionDao.class); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/QualityGateMeasuresStepTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/QualityGateMeasuresStepTest.java index eddec0e7051..ee87d157d35 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/QualityGateMeasuresStepTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/step/QualityGateMeasuresStepTest.java @@ -23,7 +23,6 @@ import java.util.Optional; import java.util.Collections; import java.util.Map; import java.util.Objects; -import java.util.Optional; import javax.annotation.Nullable; import org.assertj.core.api.AbstractAssert; import org.junit.Before; @@ -150,7 +149,7 @@ public class QualityGateMeasuresStepTest { @Test public void new_measures_are_created_even_if_there_is_no_rawMeasure_for_metric_of_condition() { - Condition equals2Condition = createEqualsCondition(INT_METRIC_1, "2"); + Condition equals2Condition = createLessThanCondition(INT_METRIC_1, "2"); qualityGateHolder.setQualityGate(new QualityGate(SOME_QG_ID, SOME_QG_NAME, of(equals2Condition))); underTest.execute(new TestComputationStepContext()); @@ -172,8 +171,8 @@ public class QualityGateMeasuresStepTest { @Test public void rawMeasure_is_updated_if_present_and_new_measures_are_created_if_project_has_measure_for_metric_of_condition() { - int rawValue = 1; - Condition equals2Condition = createEqualsCondition(INT_METRIC_1, "2"); + int rawValue = 3; + Condition equals2Condition = createLessThanCondition(INT_METRIC_1, "2"); Measure rawMeasure = newMeasureBuilder().create(rawValue, null); qualityGateHolder.setQualityGate(new QualityGate(SOME_QG_ID, SOME_QG_NAME, of(equals2Condition))); @@ -200,9 +199,9 @@ public class QualityGateMeasuresStepTest { @Test public void new_measures_have_ERROR_level_if_at_least_one_updated_measure_has_ERROR_level() { - int rawValue = 1; - Condition equalsOneErrorCondition = createEqualsCondition(INT_METRIC_1, "1"); - Condition equalsOneOkCondition = createEqualsCondition(INT_METRIC_2, "2"); + int rawValue = 3; + Condition equalsOneErrorCondition = createLessThanCondition(INT_METRIC_1, "4"); + Condition equalsOneOkCondition = createLessThanCondition(INT_METRIC_2, "2"); Measure rawMeasure = newMeasureBuilder().create(rawValue, null); qualityGateHolder.setQualityGate(new QualityGate(SOME_QG_ID, SOME_QG_NAME, of(equalsOneErrorCondition, equalsOneOkCondition))); @@ -238,9 +237,9 @@ public class QualityGateMeasuresStepTest { @Test public void new_measure_has_ERROR_level_of_all_conditions_for_a_specific_metric_if_its_the_worst() { - int rawValue = 1; - Condition fixedCondition = createEqualsCondition(INT_METRIC_1, "1"); - Condition periodCondition = createEqualsCondition(INT_METRIC_1, "2"); + int rawValue = 3; + Condition fixedCondition = createLessThanCondition(INT_METRIC_1, "4"); + Condition periodCondition = createLessThanCondition(INT_METRIC_1, "2"); qualityGateHolder.setQualityGate(new QualityGate(SOME_QG_ID, SOME_QG_NAME, of(fixedCondition, periodCondition))); Measure measure = newMeasureBuilder().create(rawValue, null); @@ -256,9 +255,9 @@ public class QualityGateMeasuresStepTest { @Test public void new_measure_has_condition_on_leak_period_when_all_conditions_on_specific_metric_has_same_QG_level() { - int rawValue = 1; - Condition fixedCondition = createEqualsCondition(INT_METRIC_1, "1"); - Condition periodCondition = createEqualsCondition(INT_METRIC_1, "1"); + int rawValue = 0; + Condition fixedCondition = createLessThanCondition(INT_METRIC_1, "1"); + Condition periodCondition = createLessThanCondition(INT_METRIC_1, "1"); qualityGateHolder.setQualityGate(new QualityGate(SOME_QG_ID, SOME_QG_NAME, of(fixedCondition, periodCondition))); Measure measure = newMeasureBuilder() @@ -282,8 +281,8 @@ public class QualityGateMeasuresStepTest { return measureRepository.getAddedRawMeasure(PROJECT_REF, CoreMetrics.QUALITY_GATE_DETAILS_KEY); } - private static Condition createEqualsCondition(Metric metric, String errorThreshold) { - return new Condition(metric, Condition.Operator.EQUALS.getDbValue(), errorThreshold); + private static Condition createLessThanCondition(Metric metric, String errorThreshold) { + return new Condition(metric, Condition.Operator.LESS_THAN.getDbValue(), errorThreshold); } private static MetricImpl createIntMetric(int index) { diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTaskTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTaskTest.java index e967a56a6f2..e47d7a97533 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTaskTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/webhook/WebhookPostTaskTest.java @@ -85,7 +85,7 @@ public class WebhookPostTaskTest { public void call_webhooks_with_analysis_and_qualitygate() { QualityGate.Condition condition = newConditionBuilder() .setMetricKey(randomAlphanumeric(96)) - .setOperator(QualityGate.Operator.values()[random.nextInt(QualityGate.Operator.values().length)]) + .setOperator(QualityGate.Operator.LESS_THAN) .setErrorThreshold(randomAlphanumeric(22)) .setOnLeakPeriod(random.nextBoolean()) .build(QualityGate.EvaluationStatus.OK, randomAlphanumeric(33)); diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDto.java index 275d086adf4..33db00e3d25 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDto.java @@ -216,8 +216,4 @@ public class MetricDto { return this; } - public boolean isDataType() { - return DATA.name().equals(valueType) || DISTRIB.name().equals(valueType) || STRING.name().equals(valueType); - } - } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/qualitygate/QualityGateConditionDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/qualitygate/QualityGateConditionDto.java index 853cfe174c8..25d7d75b254 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/qualitygate/QualityGateConditionDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/qualitygate/QualityGateConditionDto.java @@ -19,70 +19,18 @@ */ package org.sonar.db.qualitygate; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import java.util.Collection; -import java.util.Collections; import java.util.Date; -import java.util.List; -import java.util.Map; import javax.annotation.CheckForNull; import javax.annotation.Nullable; -import org.sonar.api.measures.Metric.ValueType; /** * @since 4.3 */ public class QualityGateConditionDto { - public static final String OPERATOR_EQUALS = "EQ"; - - public static final String OPERATOR_NOT_EQUALS = "NE"; - public static final String OPERATOR_GREATER_THAN = "GT"; - public static final String OPERATOR_LESS_THAN = "LT"; - public static final List<String> ALL_OPERATORS = ImmutableList.of( - OPERATOR_LESS_THAN, - OPERATOR_GREATER_THAN, - OPERATOR_EQUALS, - OPERATOR_NOT_EQUALS); - - private static final List<String> NUMERIC_OPERATORS = ImmutableList.of( - OPERATOR_LESS_THAN, - OPERATOR_GREATER_THAN, - OPERATOR_EQUALS, - OPERATOR_NOT_EQUALS); - - private static final List<String> STRING_OPERATORS = ImmutableList.of( - OPERATOR_EQUALS, - OPERATOR_NOT_EQUALS, - OPERATOR_LESS_THAN, - OPERATOR_GREATER_THAN); - - private static final List<String> LEVEL_OPERATORS = ImmutableList.of( - OPERATOR_EQUALS, - OPERATOR_NOT_EQUALS); - - private static final List<String> BOOLEAN_OPERATORS = ImmutableList.of( - OPERATOR_EQUALS); - - private static final List<String> RATING_OPERATORS = ImmutableList.of( - OPERATOR_GREATER_THAN); - - private static final Map<ValueType, List<String>> OPERATORS_BY_TYPE = ImmutableMap.<ValueType, List<String>>builder() - .put(ValueType.BOOL, BOOLEAN_OPERATORS) - .put(ValueType.LEVEL, LEVEL_OPERATORS) - .put(ValueType.STRING, STRING_OPERATORS) - .put(ValueType.INT, NUMERIC_OPERATORS) - .put(ValueType.FLOAT, NUMERIC_OPERATORS) - .put(ValueType.PERCENT, NUMERIC_OPERATORS) - .put(ValueType.MILLISEC, NUMERIC_OPERATORS) - .put(ValueType.RATING, RATING_OPERATORS) - .put(ValueType.WORK_DUR, NUMERIC_OPERATORS) - .build(); - private long id; private long qualityGateId; @@ -172,15 +120,4 @@ public class QualityGateConditionDto { return this; } - public static boolean isOperatorAllowed(String operator, ValueType metricType) { - return getOperatorsForType(metricType).contains(operator); - } - - public static Collection<String> getOperatorsForType(ValueType metricType) { - if (OPERATORS_BY_TYPE.containsKey(metricType)) { - return OPERATORS_BY_TYPE.get(metricType); - } else { - return Collections.emptySet(); - } - } } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDtoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDtoTest.java index 7aa45f40797..a496b042d71 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDtoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDtoTest.java @@ -72,15 +72,6 @@ public class MetricDtoTest { } @Test - public void is_data_type() { - assertThat(MetricTesting.newMetricDto().setValueType(INT.name()).isDataType()).isFalse(); - - assertThat(MetricTesting.newMetricDto().setValueType(DATA.name()).isDataType()).isTrue(); - assertThat(MetricTesting.newMetricDto().setValueType(STRING.name()).isDataType()).isTrue(); - assertThat(MetricTesting.newMetricDto().setValueType(STRING.name()).isDataType()).isTrue(); - } - - @Test public void fail_if_key_longer_than_64_characters() { String a65 = repeat("a", 65); diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/qualitygate/QualityGateConditionDtoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/qualitygate/QualityGateConditionDtoTest.java deleted file mode 100644 index 6a3a753e95a..00000000000 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/qualitygate/QualityGateConditionDtoTest.java +++ /dev/null @@ -1,79 +0,0 @@ -/* - * SonarQube - * Copyright (C) 2009-2019 SonarSource SA - * mailto:info 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.db.qualitygate; - -import java.util.Arrays; -import org.junit.Test; -import org.sonar.api.measures.Metric.ValueType; - -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.FLOAT; -import static org.sonar.api.measures.Metric.ValueType.INT; -import static org.sonar.api.measures.Metric.ValueType.LEVEL; -import static org.sonar.api.measures.Metric.ValueType.MILLISEC; -import static org.sonar.api.measures.Metric.ValueType.PERCENT; -import static org.sonar.api.measures.Metric.ValueType.RATING; -import static org.sonar.api.measures.Metric.ValueType.STRING; -import static org.sonar.db.qualitygate.QualityGateConditionDto.isOperatorAllowed; - -public class QualityGateConditionDtoTest { - - @Test - public void validate_operators_for_DATA() { - assertThat(isOperatorAllowed("WHATEVER", DATA)).isFalse(); - } - - @Test - public void validate_operators_for_BOOL() { - assertThat(isOperatorAllowed("EQ", BOOL)).isTrue(); - assertThat(isOperatorAllowed("NE", BOOL)).isFalse(); - assertThat(isOperatorAllowed("LT", BOOL)).isFalse(); - assertThat(isOperatorAllowed("GT", BOOL)).isFalse(); - } - - @Test - public void validate_operators_for_LEVEL() { - assertThat(isOperatorAllowed("EQ", LEVEL)).isTrue(); - assertThat(isOperatorAllowed("NE", LEVEL)).isTrue(); - assertThat(isOperatorAllowed("LT", LEVEL)).isFalse(); - assertThat(isOperatorAllowed("GT", LEVEL)).isFalse(); - } - - @Test - public void validate_operators_for_RATING() { - assertThat(isOperatorAllowed("EQ", RATING)).isFalse(); - assertThat(isOperatorAllowed("NE", RATING)).isFalse(); - assertThat(isOperatorAllowed("LT", RATING)).isFalse(); - assertThat(isOperatorAllowed("GT", RATING)).isTrue(); - } - - @Test - public void validate_operators_for_other_types() { - Arrays.stream(new ValueType[] {STRING, INT, FLOAT, PERCENT, MILLISEC}).forEach(type -> { - assertThat(isOperatorAllowed("EQ", type)).isTrue(); - assertThat(isOperatorAllowed("NE", type)).isTrue(); - assertThat(isOperatorAllowed("LT", type)).isTrue(); - assertThat(isOperatorAllowed("GT", type)).isTrue(); - }); - } - -} 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()); 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 42126a88fbd..be80c7906d4 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 @@ -19,13 +19,16 @@ */ package org.sonar.server.qualitygate; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import java.util.ArrayList; import java.util.Collection; +import java.util.EnumSet; import java.util.List; +import java.util.Map; import java.util.Objects; import java.util.stream.Collectors; import javax.annotation.Nullable; -import org.apache.commons.lang.BooleanUtils; import org.sonar.api.measures.CoreMetrics; import org.sonar.api.measures.Metric.ValueType; import org.sonar.db.DbClient; @@ -33,25 +36,45 @@ 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.measure.Rating; import org.sonar.server.exceptions.NotFoundException; +import org.sonar.server.measure.Rating; import static com.google.common.base.Strings.isNullOrEmpty; import static java.lang.Double.parseDouble; import static java.lang.Integer.parseInt; import static java.lang.Long.parseLong; import static java.lang.String.format; +import static java.lang.String.valueOf; import static java.util.Arrays.stream; import static java.util.Objects.requireNonNull; +import static org.sonar.api.measures.Metric.DIRECTION_BETTER; +import static org.sonar.api.measures.Metric.DIRECTION_NONE; +import static org.sonar.api.measures.Metric.DIRECTION_WORST; 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.measure.Rating.E; +import static org.sonar.server.qualitygate.Condition.Operator.GREATER_THAN; +import static org.sonar.server.qualitygate.Condition.Operator.LESS_THAN; import static org.sonar.server.qualitygate.ValidRatingMetrics.isCoreRatingMetric; import static org.sonar.server.ws.WsUtils.checkRequest; public class QualityGateConditionsUpdater { + private static final Map<Integer, ImmutableSet<Condition.Operator>> VALID_OPERATORS_BY_DIRECTION = ImmutableMap.<Integer, ImmutableSet<Condition.Operator>>builder() + .put(DIRECTION_NONE, ImmutableSet.of(GREATER_THAN, LESS_THAN)) + .put(DIRECTION_BETTER, ImmutableSet.of(LESS_THAN)) + .put(DIRECTION_WORST, ImmutableSet.of(GREATER_THAN)) + .build(); + + private static final EnumSet<ValueType> VALID_METRIC_TYPES = EnumSet.of( + ValueType.INT, + ValueType.FLOAT, + ValueType.PERCENT, + ValueType.MILLISEC, + ValueType.LEVEL, + ValueType.RATING, + ValueType.WORK_DUR + ); + 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; @@ -114,16 +137,17 @@ public class QualityGateConditionsUpdater { } private static boolean isAlertable(MetricDto metric) { - return isAvailableForInit(metric) && BooleanUtils.isFalse(metric.isHidden()); - } - - private static boolean isAvailableForInit(MetricDto metric) { - return !metric.isDataType() && !CoreMetrics.ALERT_STATUS_KEY.equals(metric.getKey()); + return !metric.isHidden() + && VALID_METRIC_TYPES.contains(ValueType.valueOf(metric.getValueType())) + && !CoreMetrics.ALERT_STATUS_KEY.equals(metric.getKey()); } private static void checkOperator(MetricDto metric, String operator, List<String> errors) { - ValueType valueType = valueOf(metric.getValueType()); - check(isOperatorAllowed(operator, valueType), errors, "Operator %s is not allowed for metric type %s.", operator, metric.getValueType()); + check( + Condition.Operator.isValid(operator) && isAllowedOperator(operator, metric), + errors, + "Operator %s is not allowed for this metric.", operator + ); } private static void checkErrorThreshold(MetricDto metric, String errorThreshold, List<String> errors) { @@ -140,6 +164,14 @@ public class QualityGateConditionsUpdater { checkRequest(!conditionExists, format("Condition on metric '%s' already exists.", metric.getShortName())); } + private static boolean isAllowedOperator(String operator, MetricDto metric) { + if (VALID_OPERATORS_BY_DIRECTION.containsKey(metric.getDirection())) { + return VALID_OPERATORS_BY_DIRECTION.get(metric.getDirection()).contains(Condition.Operator.fromDbValue(operator)); + } + + return false; + } + private static void validateErrorThresholdValue(MetricDto metric, String errorThreshold, List<String> errors) { try { ValueType valueType = ValueType.valueOf(metric.getValueType()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWs.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWs.java index 456c5c4b8cf..ef8a3561bfc 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWs.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/QualityGatesWs.java @@ -19,11 +19,14 @@ */ package org.sonar.server.qualitygate.ws; +import java.util.Set; +import java.util.stream.Collectors; +import java.util.stream.Stream; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.WebService; -import org.sonar.db.qualitygate.QualityGateConditionDto; import org.sonar.server.exceptions.BadRequestException; +import org.sonar.server.qualitygate.Condition; import org.sonar.server.ws.RemovedWebServiceHandler; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.CONTROLLER_QUALITY_GATES; @@ -69,20 +72,28 @@ public class QualityGatesWs implements WebService { static void addConditionParams(NewAction action) { action .createParam(PARAM_METRIC) - .setDescription("Condition metric") + .setDescription("Condition metric.<br/>" + + " Only metric of the following types are allowed:" + + "<ul>" + + "<li>INT</li>" + + "<li>MILLISEC</li>" + + "<li>RATING</li>" + + "<li>WORK_DUR</li>" + + "<li>FLOAT</li>" + + "<li>PERCENT</li>" + + "<li>LEVEL</li>" + + "") .setRequired(true) .setExampleValue("blocker_violations"); action.createParam(PARAM_OPERATOR) .setDescription("Condition operator:<br/>" + "<ul>" + - "<li>EQ = equals</li>" + - "<li>NE = is not</li>" + "<li>LT = is lower than</li>" + "<li>GT = is greater than</li>" + "</ui>") - .setExampleValue(QualityGateConditionDto.OPERATOR_EQUALS) - .setPossibleValues(QualityGateConditionDto.ALL_OPERATORS); + .setExampleValue(Condition.Operator.GREATER_THAN.getDbValue()) + .setPossibleValues(getPossibleOperators()); action.createParam(PARAM_ERROR) .setMaximumLength(CONDITION_MAX_LENGTH) @@ -99,4 +110,9 @@ public class QualityGatesWs implements WebService { } } + private static Set<String> getPossibleOperators() { + return Stream.of(Condition.Operator.values()) + .map(Condition.Operator::getDbValue) + .collect(Collectors.toSet()); + } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/measure/live/LiveQualityGateComputerImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/measure/live/LiveQualityGateComputerImplTest.java index 5e96dc14446..d4097977303 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/measure/live/LiveQualityGateComputerImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/measure/live/LiveQualityGateComputerImplTest.java @@ -114,7 +114,7 @@ public class LiveQualityGateComputerImplTest { @Test public void getMetricsRelatedTo() { - Condition condition = new Condition("metric1", Condition.Operator.EQUALS, "10"); + Condition condition = new Condition("metric1", Condition.Operator.GREATER_THAN, "10"); QualityGate gate = new QualityGate("1", "foo", ImmutableSet.of(condition)); Set<String> result = underTest.getMetricsRelatedTo(gate); 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 2269165109d..d9c67647ab1 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 @@ -22,7 +22,6 @@ 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 javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -37,16 +36,19 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.NotFoundException; import static java.lang.String.format; +import static java.lang.String.valueOf; import static org.assertj.core.api.AssertionsForInterfaceTypes.assertThat; import static org.sonar.api.measures.CoreMetrics.ALERT_STATUS_KEY; import static org.sonar.api.measures.CoreMetrics.SQALE_RATING_KEY; 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; import static org.sonar.api.measures.Metric.ValueType.PERCENT; import static org.sonar.api.measures.Metric.ValueType.RATING; +import static org.sonar.api.measures.Metric.ValueType.STRING; import static org.sonar.api.measures.Metric.ValueType.WORK_DUR; @RunWith(DataProviderRunner.class) @@ -62,7 +64,7 @@ public class QualityGateConditionsUpdaterTest { @Test public void create_error_condition() { - MetricDto metric = db.measures().insertMetric(m -> m.setKey("new_coverage").setValueType(INT.name()).setHidden(false)); + MetricDto metric = insertMetric(INT, "new_coverage"); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); QualityGateConditionDto result = underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "LT", "80"); @@ -71,8 +73,19 @@ public class QualityGateConditionsUpdaterTest { } @Test + @UseDataProvider("valid_operators_and_direction") + public void create_condition_with_valid_operators_and_direction(String operator, int direction) { + MetricDto metric = db.measures().insertMetric(m -> m.setKey("key").setValueType(INT.name()).setHidden(false).setDirection(direction)); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); + + QualityGateConditionDto result = underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), operator, "80"); + + verifyCondition(result, qualityGate, metric, operator, "80"); + } + + @Test public void create_condition_throws_NPE_if_errorThreshold_is_null() { - MetricDto metric = db.measures().insertMetric(m -> m.setKey(SQALE_RATING_KEY).setValueType(RATING.name()).setHidden(false)); + MetricDto metric = insertMetric(RATING, SQALE_RATING_KEY); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); expectedException.expect(NullPointerException.class); @@ -83,14 +96,14 @@ public class QualityGateConditionsUpdaterTest { @Test public void fail_to_create_condition_when_condition_on_same_metric_already_exist() { - MetricDto metric = db.measures().insertMetric(m -> m.setValueType(PERCENT.name()).setHidden(false)); + MetricDto metric = insertMetric(PERCENT); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); db.qualityGates().addCondition(qualityGate, metric); expectedException.expect(BadRequestException.class); expectedException.expectMessage(format("Condition on metric '%s' already exists.", metric.getShortName())); - underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "LT", "90"); + underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "LT", "80"); } @Test @@ -106,29 +119,30 @@ public class QualityGateConditionsUpdaterTest { @Test @UseDataProvider("invalid_metrics") public void fail_to_create_condition_on_invalid_metric(String metricKey, Metric.ValueType valueType, boolean hidden) { - MetricDto metric = db.measures().insertMetric(m -> m.setKey(metricKey).setValueType(valueType.name()).setHidden(hidden)); + MetricDto metric = db.measures().insertMetric(m -> m.setKey(metricKey).setValueType(valueType.name()).setHidden(hidden).setDirection(0)); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); expectedException.expect(BadRequestException.class); - expectedException.expectMessage(format("Metric '%s' cannot be used to define a condition.", metric.getKey())); + expectedException.expectMessage(format("Metric '%s' cannot be used to define a condition", metric.getKey())); - underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "EQ", "80"); + underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "LT", "80"); } @Test - public void fail_to_create_condition_on_not_allowed_operator() { - MetricDto metric = db.measures().insertMetric(m -> m.setValueType(PERCENT.name()).setHidden(false)); + @UseDataProvider("invalid_operators_and_direction") + public void fail_to_create_condition_on_not_allowed_operator_for_metric_direction(String operator, int direction) { + MetricDto metric = db.measures().insertMetric(m -> m.setKey("key").setValueType(INT.name()).setHidden(false).setDirection(direction)); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); expectedException.expect(BadRequestException.class); - expectedException.expectMessage("Operator UNKNOWN is not allowed for metric type PERCENT."); + expectedException.expectMessage(format("Operator %s is not allowed for this metric.", operator)); - underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "UNKNOWN", "90"); + underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), operator, "90"); } @Test public void create_condition_on_rating_metric() { - MetricDto metric = db.measures().insertMetric(m -> m.setKey(SQALE_RATING_KEY).setValueType(RATING.name()).setHidden(false)); + MetricDto metric = insertMetric(RATING, SQALE_RATING_KEY); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); QualityGateConditionDto result = underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "GT", "3"); @@ -138,7 +152,7 @@ public class QualityGateConditionsUpdaterTest { @Test public void fail_to_create_error_condition_on_invalid_rating_metric() { - MetricDto metric = db.measures().insertMetric(m -> m.setKey(SQALE_RATING_KEY).setValueType(RATING.name()).setHidden(false)); + MetricDto metric = insertMetric(RATING, SQALE_RATING_KEY); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); expectedException.expect(BadRequestException.class); @@ -149,7 +163,7 @@ public class QualityGateConditionsUpdaterTest { @Test public void fail_to_create_condition_on_greater_than_E() { - MetricDto metric = db.measures().insertMetric(m -> m.setKey(SQALE_RATING_KEY).setValueType(RATING.name()).setHidden(false)); + MetricDto metric = insertMetric(RATING, SQALE_RATING_KEY); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); expectedException.expect(BadRequestException.class); @@ -161,29 +175,29 @@ public class QualityGateConditionsUpdaterTest { @Test @UseDataProvider("valid_values") public void create_error_condition(Metric.ValueType valueType, String value) { - MetricDto metric = db.measures().insertMetric(m -> m.setValueType(valueType.name()).setHidden(false)); + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(valueType.name()).setHidden(false).setDirection(0)); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); - QualityGateConditionDto result = underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "EQ", value); + QualityGateConditionDto result = underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "LT", value); - verifyCondition(result, qualityGate, metric, "EQ", value); + verifyCondition(result, qualityGate, metric, "LT", value); } @Test @UseDataProvider("invalid_values") public void fail_to_create_error_INT_condition_when_value_is_not_an_integer(Metric.ValueType valueType, String value) { - MetricDto metric = db.measures().insertMetric(m -> m.setValueType(valueType.name()).setHidden(false)); + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(valueType.name()).setHidden(false).setDirection(0)); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); expectedException.expect(BadRequestException.class); expectedException.expectMessage(format("Invalid value '%s' for metric '%s'", value, metric.getShortName())); - underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "EQ", value); + underTest.createCondition(db.getSession(), qualityGate, metric.getKey(), "LT", value); } @Test public void update_condition() { - MetricDto metric = db.measures().insertMetric(m -> m.setValueType(PERCENT.name()).setHidden(false)); + MetricDto metric = insertMetric(PERCENT); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, c -> c.setOperator("LT").setErrorThreshold("80")); @@ -195,7 +209,7 @@ public class QualityGateConditionsUpdaterTest { @Test public void update_condition_throws_NPE_if_errorThreshold_is_null() { - MetricDto metric = db.measures().insertMetric(m -> m.setValueType(PERCENT.name()).setHidden(false)); + MetricDto metric = insertMetric(PERCENT); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, c -> c.setOperator("LT").setErrorThreshold("80")); @@ -208,7 +222,7 @@ public class QualityGateConditionsUpdaterTest { @Test public void update_condition_on_rating_metric() { - MetricDto metric = db.measures().insertMetric(m -> m.setKey(SQALE_RATING_KEY).setValueType(RATING.name()).setHidden(false)); + MetricDto metric = insertMetric(RATING, SQALE_RATING_KEY); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, c -> c.setOperator("LT").setErrorThreshold("80")); @@ -219,8 +233,34 @@ public class QualityGateConditionsUpdaterTest { } @Test + @UseDataProvider("update_invalid_operators_and_direction") + public void fail_to_update_condition_on_not_allowed_operator_for_metric_direction(String validOperator, String updatedOperator, int direction) { + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(PERCENT.name()).setHidden(false).setDirection(direction)); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); + QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, + c -> c.setOperator(validOperator).setErrorThreshold("80")); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(format("Operator %s is not allowed for this metric", updatedOperator)); + + underTest.updateCondition(db.getSession(), condition, metric.getKey(), updatedOperator, "70"); + } + + @Test + public void fail_to_update_condition_on_rating_metric_on_leak_period() { + MetricDto metric = insertMetric(RATING, SQALE_RATING_KEY); + QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); + QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, + c -> c.setOperator("LT").setErrorThreshold("3")); + + QualityGateConditionDto result = underTest.updateCondition(db.getSession(), condition, metric.getKey(), "GT", "4"); + + verifyCondition(result, qualityGate, metric, "GT", "4"); + } + + @Test public void fail_to_update_condition_on_rating_metric_on_not_core_rating_metric() { - MetricDto metric = db.measures().insertMetric(m -> m.setKey("not_core_rating_metric").setValueType(RATING.name()).setHidden(false)); + MetricDto metric = insertMetric(RATING, "not_core_rating_metric"); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, c -> c.setOperator("LT").setErrorThreshold("3")); @@ -240,7 +280,7 @@ public class QualityGateConditionsUpdaterTest { c -> c.setOperator("LT").setErrorThreshold("80")); expectedException.expect(BadRequestException.class); - expectedException.expectMessage(format("Metric '%s' cannot be used to define a condition.", metric.getKey())); + expectedException.expectMessage(format("Metric '%s' cannot be used to define a condition", metric.getKey())); underTest.updateCondition(db.getSession(), condition, metric.getKey(), "GT", "60"); } @@ -248,20 +288,20 @@ public class QualityGateConditionsUpdaterTest { @Test @UseDataProvider("valid_values") public void update_error_condition(Metric.ValueType valueType, String value) { - MetricDto metric = db.measures().insertMetric(m -> m.setValueType(valueType.name()).setHidden(false)); + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(valueType.name()).setHidden(false).setDirection(0)); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, c -> c.setOperator("LT").setErrorThreshold("80")); - QualityGateConditionDto result = underTest.updateCondition(db.getSession(), condition, metric.getKey(), "EQ", value); + QualityGateConditionDto result = underTest.updateCondition(db.getSession(), condition, metric.getKey(), "LT", value); - verifyCondition(result, qualityGate, metric, "EQ", value); + verifyCondition(result, qualityGate, metric, "LT", value); } @Test @UseDataProvider("invalid_values") public void fail_to_update_error_INT_condition_when_value_is_not_an_integer(Metric.ValueType valueType, String value) { - MetricDto metric = db.measures().insertMetric(m -> m.setValueType(valueType.name()).setHidden(false)); + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(valueType.name()).setHidden(false).setDirection(0)); QualityGateDto qualityGate = db.qualityGates().insertQualityGate(db.getDefaultOrganization()); QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, c -> c.setOperator("LT").setErrorThreshold("80")); @@ -269,14 +309,17 @@ public class QualityGateConditionsUpdaterTest { expectedException.expect(BadRequestException.class); expectedException.expectMessage(format("Invalid value '%s' for metric '%s'", value, metric.getShortName())); - underTest.updateCondition(db.getSession(), condition, metric.getKey(), "EQ", value); + underTest.updateCondition(db.getSession(), condition, metric.getKey(), "LT", value); } @DataProvider public static Object[][] invalid_metrics() { return new Object[][] { {ALERT_STATUS_KEY, INT, false}, + {"boolean", BOOL, false}, + {"string", STRING, false}, {"data_metric", DATA, false}, + {"distrib", DISTRIB, false}, {"hidden", INT, true} }; } @@ -285,7 +328,6 @@ public class QualityGateConditionsUpdaterTest { public static Object[][] valid_values() { return new Object[][] { {INT, "10"}, - {BOOL, "1"}, {MILLISEC, "1000"}, {WORK_DUR, "1000"}, {FLOAT, "5.12"}, @@ -297,7 +339,6 @@ public class QualityGateConditionsUpdaterTest { public static Object[][] invalid_values() { return new Object[][] { {INT, "ABCD"}, - {BOOL, "ABCD"}, {MILLISEC, "ABCD"}, {WORK_DUR, "ABCD"}, {FLOAT, "ABCD"}, @@ -305,6 +346,49 @@ public class QualityGateConditionsUpdaterTest { }; } + @DataProvider + public static Object[][] invalid_operators_and_direction() { + return new Object[][] { + {"EQ", 0}, + {"NE", 0}, + {"LT", -1}, + {"GT", 1}, + }; + } + + @DataProvider + public static Object[][] update_invalid_operators_and_direction() { + return new Object[][] { + {"LT", "EQ", 0}, + {"LT", "NE", 0}, + {"GT", "LT", -1}, + {"LT", "GT", 1}, + }; + } + + @DataProvider + public static Object[][] valid_operators_and_direction() { + return new Object[][] { + {"LT", 0}, + {"GT", 0}, + {"GT", -1}, + {"LT", 1}, + }; + } + + private MetricDto insertMetric(Metric.ValueType type) { + return insertMetric(type, "key"); + } + + private MetricDto insertMetric(Metric.ValueType type, String key) { + return db.measures().insertMetric(m -> m + .setKey(key) + .setValueType(type.name()) + .setHidden(false) + .setDirection(0) + ); + } + private void verifyCondition(QualityGateConditionDto dto, QualityGateDto qualityGate, MetricDto metric, String operator, String error) { QualityGateConditionDto reloaded = db.getDbClient().gateConditionDao().selectById(dto.getId(), db.getSession()); assertThat(reloaded.getQualityGateId()).isEqualTo(qualityGate.getId()); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/RegisterQualityGatesTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/RegisterQualityGatesTest.java index ce299186f24..176e7f3e623 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/RegisterQualityGatesTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/RegisterQualityGatesTest.java @@ -228,12 +228,12 @@ public class RegisterQualityGatesTest { } private void insertMetrics() { - dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_RELIABILITY_RATING_KEY).setValueType(INT.name()).setHidden(false)); - dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_SECURITY_RATING_KEY).setValueType(INT.name()).setHidden(false)); - dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_SECURITY_REMEDIATION_EFFORT_KEY).setValueType(INT.name()).setHidden(false)); - dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_MAINTAINABILITY_RATING_KEY).setValueType(PERCENT.name()).setHidden(false)); - dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_COVERAGE_KEY).setValueType(PERCENT.name()).setHidden(false)); - dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_DUPLICATED_LINES_DENSITY_KEY).setValueType(PERCENT.name()).setHidden(false)); + dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_RELIABILITY_RATING_KEY).setValueType(INT.name()).setHidden(false).setDirection(0)); + dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_SECURITY_RATING_KEY).setValueType(INT.name()).setHidden(false).setDirection(0)); + dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_SECURITY_REMEDIATION_EFFORT_KEY).setValueType(INT.name()).setHidden(false).setDirection(0)); + dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_MAINTAINABILITY_RATING_KEY).setValueType(PERCENT.name()).setHidden(false).setDirection(0)); + dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_COVERAGE_KEY).setValueType(PERCENT.name()).setHidden(false).setDirection(0)); + dbClient.metricDao().insert(dbSession, newMetricDto().setKey(NEW_DUPLICATED_LINES_DENSITY_KEY).setValueType(PERCENT.name()).setHidden(false).setDirection(0)); dbSession.commit(); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/CreateConditionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/CreateConditionActionTest.java index f0b1e768e3b..f8b8327af49 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/CreateConditionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/CreateConditionActionTest.java @@ -19,10 +19,14 @@ */ package org.sonar.server.qualitygate.ws; +import com.tngtech.java.junit.dataprovider.DataProvider; +import com.tngtech.java.junit.dataprovider.DataProviderRunner; +import com.tngtech.java.junit.dataprovider.UseDataProvider; import java.util.ArrayList; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; import org.sonar.api.server.ws.WebService; import org.sonar.db.DbClient; import org.sonar.db.DbSession; @@ -32,6 +36,7 @@ import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualitygate.QGateWithOrgDto; import org.sonar.db.qualitygate.QualityGateConditionDto; import org.sonar.db.qualitygate.QualityGateDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.qualitygate.QualityGateConditionsUpdater; @@ -50,6 +55,7 @@ import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_MET import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_OPERATOR; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_ORGANIZATION; +@RunWith(DataProviderRunner.class) public class CreateConditionActionTest { @Rule @@ -144,6 +150,45 @@ public class CreateConditionActionTest { } @Test + public void fail_with_unknown_operator() { + OrganizationDto organization = db.organizations().insert(); + logInAsQualityGateAdmin(organization); + QGateWithOrgDto qualityGate = db.qualityGates().insertQualityGate(organization); + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(INT.name()).setHidden(false).setDirection(0)); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Value of parameter 'op' (ABC) must be one of: [LT, GT]"); + + ws.newRequest() + .setParam(PARAM_GATE_ID, qualityGate.getId().toString()) + .setParam(PARAM_METRIC, metric.getKey()) + .setParam(PARAM_OPERATOR, "ABC") + .setParam(PARAM_ERROR, "90") + .setParam(PARAM_ORGANIZATION, organization.getKey()) + .execute(); + } + + @Test + @UseDataProvider("invalid_operators_for_direction") + public void fail_with_invalid_operators_for_direction(String operator, int direction) { + OrganizationDto organization = db.organizations().insert(); + logInAsQualityGateAdmin(organization); + QGateWithOrgDto qualityGate = db.qualityGates().insertQualityGate(organization); + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(INT.name()).setHidden(false).setDirection(direction)); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(format("Operator %s is not allowed for this metric.", operator)); + + ws.newRequest() + .setParam(PARAM_GATE_ID, qualityGate.getId().toString()) + .setParam(PARAM_METRIC, metric.getKey()) + .setParam(PARAM_OPERATOR, operator) + .setParam(PARAM_ERROR, "90") + .setParam(PARAM_ORGANIZATION, organization.getKey()) + .execute(); + } + + @Test public void test_response() { OrganizationDto organization = db.organizations().insert(); logInAsQualityGateAdmin(organization); @@ -201,6 +246,14 @@ public class CreateConditionActionTest { tuple("organization", false)); } + @DataProvider + public static Object[][] invalid_operators_for_direction() { + return new Object[][] { + {"LT", -1}, + {"GT", 1}, + }; + } + private void assertCondition(QualityGateDto qualityGate, MetricDto metric, String operator, String error) { assertThat(dbClient.gateConditionDao().selectForQualityGate(dbSession, qualityGate.getId())) .extracting(QualityGateConditionDto::getQualityGateId, QualityGateConditionDto::getMetricId, QualityGateConditionDto::getOperator, @@ -213,6 +266,9 @@ public class CreateConditionActionTest { } private MetricDto insertMetric() { - return db.measures().insertMetric(m -> m.setValueType(INT.name()).setHidden(false)); + return db.measures().insertMetric(m -> m + .setValueType(INT.name()) + .setHidden(false) + .setDirection(0)); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest.java index 7721f3c6871..1d4dcf8e316 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest.java @@ -53,23 +53,21 @@ public class QualityGateDetailsFormatterTest { assertThat(result.getStatus()).isEqualTo(ProjectStatusResponse.Status.ERROR); // check conditions - assertThat(result.getConditionsCount()).isEqualTo(4); + assertThat(result.getConditionsCount()).isEqualTo(3); List<ProjectStatusResponse.Condition> conditions = result.getConditionsList(); assertThat(conditions).extracting("status").containsExactly( ProjectStatusResponse.Status.ERROR, ProjectStatusResponse.Status.WARN, - ProjectStatusResponse.Status.OK, ProjectStatusResponse.Status.OK); - assertThat(conditions).extracting("metricKey").containsExactly("new_coverage", "new_blocker_violations", "new_critical_violations", "new_sqale_debt_ratio"); + assertThat(conditions).extracting("metricKey").containsExactly("new_coverage", "new_blocker_violations", "new_critical_violations"); assertThat(conditions).extracting("comparator").containsExactly( ProjectStatusResponse.Comparator.LT, ProjectStatusResponse.Comparator.GT, - ProjectStatusResponse.Comparator.NE, - ProjectStatusResponse.Comparator.EQ); - assertThat(conditions).extracting("periodIndex").containsExactly(1, 1, 1, 1); + ProjectStatusResponse.Comparator.GT); + assertThat(conditions).extracting("periodIndex").containsExactly(1, 1, 1); assertThat(conditions).extracting("warningThreshold").containsOnly("80", ""); - assertThat(conditions).extracting("errorThreshold").containsOnly("85", "0", "5"); - assertThat(conditions).extracting("actualValue").containsExactly("82.2985024398452", "1", "0", "0.5670339761248853"); + assertThat(conditions).extracting("errorThreshold").containsOnly("85", "0", "0"); + assertThat(conditions).extracting("actualValue").containsExactly("82.2985024398452", "1", "0"); // check periods assertThat(result.getPeriodsCount()).isEqualTo(1); @@ -98,7 +96,6 @@ public class QualityGateDetailsFormatterTest { assertThat(conditions).extracting("metricKey").containsExactly("new_coverage"); assertThat(conditions).extracting("comparator").containsExactly(ProjectStatusResponse.Comparator.LT); assertThat(conditions).extracting("periodIndex").containsExactly(1); - assertThat(conditions).extracting("warningThreshold").containsOnly("80"); assertThat(conditions).extracting("errorThreshold").containsOnly("85"); assertThat(conditions).extracting("actualValue").containsExactly("82.2985024398452"); } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/UpdateConditionActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/UpdateConditionActionTest.java index 7d8353aa39b..b487dc5755f 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/UpdateConditionActionTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/UpdateConditionActionTest.java @@ -25,6 +25,7 @@ import com.tngtech.java.junit.dataprovider.UseDataProvider; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; +import org.junit.runner.RunWith; import org.sonar.api.server.ws.WebService; import org.sonar.api.utils.System2; import org.sonar.db.DbClient; @@ -35,6 +36,7 @@ import org.sonar.db.organization.OrganizationDto; import org.sonar.db.qualitygate.QGateWithOrgDto; import org.sonar.db.qualitygate.QualityGateConditionDto; import org.sonar.db.qualitygate.QualityGateDto; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.organization.TestDefaultOrganizationProvider; @@ -49,11 +51,13 @@ import static org.assertj.core.api.Assertions.tuple; import static org.sonar.api.measures.Metric.ValueType.INT; import static org.sonar.db.permission.OrganizationPermission.ADMINISTER_QUALITY_GATES; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_ERROR; +import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_GATE_ID; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_ID; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_METRIC; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_OPERATOR; import static org.sonar.server.qualitygate.ws.QualityGatesWsParameters.PARAM_ORGANIZATION; +@RunWith(DataProviderRunner.class) public class UpdateConditionActionTest { @Rule @@ -195,6 +199,49 @@ public class UpdateConditionActionTest { } @Test + public void fail_with_unknown_operator() { + OrganizationDto organization = db.organizations().insert(); + userSession.addPermission(ADMINISTER_QUALITY_GATES, organization); + QGateWithOrgDto qualityGate = db.qualityGates().insertQualityGate(organization); + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(INT.name()).setHidden(false).setDirection(0)); + QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, + c -> c.setOperator("LT").setErrorThreshold("80")); + + expectedException.expect(IllegalArgumentException.class); + expectedException.expectMessage("Value of parameter 'op' (ABC) must be one of: [LT, GT]"); + + ws.newRequest() + .setParam(PARAM_ORGANIZATION, organization.getKey()) + .setParam(PARAM_ID, Long.toString(condition.getId())) + .setParam(PARAM_METRIC, metric.getKey()) + .setParam(PARAM_OPERATOR, "ABC") + .setParam(PARAM_ERROR, "90") + .execute(); + } + + @Test + @UseDataProvider("update_invalid_operators_and_direction") + public void fail_with_invalid_operators_for_direction(String validOperator, String updateOperator, int direction) { + OrganizationDto organization = db.organizations().insert(); + userSession.addPermission(ADMINISTER_QUALITY_GATES, organization); + QGateWithOrgDto qualityGate = db.qualityGates().insertQualityGate(organization); + MetricDto metric = db.measures().insertMetric(m -> m.setValueType(INT.name()).setHidden(false).setDirection(direction)); + QualityGateConditionDto condition = db.qualityGates().addCondition(qualityGate, metric, + c -> c.setOperator(validOperator).setErrorThreshold("80")); + + expectedException.expect(BadRequestException.class); + expectedException.expectMessage(format("Operator %s is not allowed for this metric.", updateOperator)); + + ws.newRequest() + .setParam(PARAM_ORGANIZATION, organization.getKey()) + .setParam(PARAM_ID, Long.toString(condition.getId())) + .setParam(PARAM_METRIC, metric.getKey()) + .setParam(PARAM_OPERATOR, updateOperator) + .setParam(PARAM_ERROR, "90") + .execute(); + } + + @Test public void throw_ForbiddenException_if_not_gate_administrator() { userSession.logIn(); OrganizationDto organization = db.organizations().insert(); @@ -231,6 +278,14 @@ public class UpdateConditionActionTest { tuple("organization", false)); } + @DataProvider + public static Object[][] update_invalid_operators_and_direction() { + return new Object[][] { + {"GT", "LT", -1}, + {"LT", "GT", 1}, + }; + } + private void assertCondition(QualityGateDto qualityGate, MetricDto metric, String operator, String error) { assertThat(dbClient.gateConditionDao().selectForQualityGate(dbSession, qualityGate.getId())) .extracting(QualityGateConditionDto::getQualityGateId, QualityGateConditionDto::getMetricId, QualityGateConditionDto::getOperator, @@ -239,6 +294,6 @@ public class UpdateConditionActionTest { } private MetricDto insertMetric() { - return db.measures().insertMetric(m -> m.setValueType(INT.name()).setHidden(false)); + return db.measures().insertMetric(m -> m.setValueType(INT.name()).setHidden(false).setDirection(0)); } } diff --git a/server/sonar-server/src/test/resources/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest/non_leak_period.json b/server/sonar-server/src/test/resources/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest/non_leak_period.json index 3dda0a73a86..f4c59dfc7ab 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest/non_leak_period.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest/non_leak_period.json @@ -21,7 +21,7 @@ }, { "metric": "new_critical_violations", - "op": "NE", + "op": "LT", "period": 3, "warning": "", "error": "0", @@ -30,7 +30,7 @@ }, { "metric": "new_sqale_debt_ratio", - "op": "EQ", + "op": "GT", "period": 4, "warning": "", "error": "5", diff --git a/server/sonar-server/src/test/resources/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest/quality_gate_details.json b/server/sonar-server/src/test/resources/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest/quality_gate_details.json index a959264939c..f19fbbcf362 100644 --- a/server/sonar-server/src/test/resources/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest/quality_gate_details.json +++ b/server/sonar-server/src/test/resources/org/sonar/server/qualitygate/ws/QualityGateDetailsFormatterTest/quality_gate_details.json @@ -21,21 +21,12 @@ }, { "metric": "new_critical_violations", - "op": "NE", + "op": "GT", "period": 1, "warning": "", "error": "0", "actual": "0", "level": "OK" - }, - { - "metric": "new_sqale_debt_ratio", - "op": "EQ", - "period": 1, - "warning": "", - "error": "5", - "actual": "0.5670339761248853", - "level": "OK" } ] } diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/ce/posttask/QualityGate.java b/sonar-plugin-api/src/main/java/org/sonar/api/ce/posttask/QualityGate.java index 7fd579c151a..2f45a5c9115 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/ce/posttask/QualityGate.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/ce/posttask/QualityGate.java @@ -120,7 +120,18 @@ public interface QualityGate { * Quality Gate condition operator. */ enum Operator { - EQUALS, NOT_EQUALS, GREATER_THAN, LESS_THAN + /** + * @deprecated in 7.6. Using this operator will have no effect. + */ + @Deprecated + EQUALS, + /** + * @deprecated in 7.6. Using this operator will have no effect. + */ + @Deprecated + NOT_EQUALS, + GREATER_THAN, + LESS_THAN } /** diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/qualitygates/CreateConditionRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/qualitygates/CreateConditionRequest.java index ec5c5b4f5a5..6a2ffefee19 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/qualitygates/CreateConditionRequest.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/qualitygates/CreateConditionRequest.java @@ -75,13 +75,11 @@ public class CreateConditionRequest { } /** - * Example value: "EQ" + * Example value: "LT" * Possible values: * <ul> * <li>"LT"</li> * <li>"GT"</li> - * <li>"EQ"</li> - * <li>"NE"</li> * </ul> */ public CreateConditionRequest setOp(String op) { diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/qualitygates/UpdateConditionRequest.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/qualitygates/UpdateConditionRequest.java index 43c77559bca..49573819540 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/qualitygates/UpdateConditionRequest.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/qualitygates/UpdateConditionRequest.java @@ -76,13 +76,11 @@ public class UpdateConditionRequest { } /** - * Example value: "EQ" + * Example value: "LT" * Possible values: * <ul> * <li>"LT"</li> * <li>"GT"</li> - * <li>"EQ"</li> - * <li>"NE"</li> * </ul> */ public UpdateConditionRequest setOp(String op) { |