SONAR-6336 No check should be done when updating condition

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java
index 6df826fda8f147402e45964c642e23cc11d61380..1bd05457571d699141c96b1377f275dd229133f7 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGates.java
@@ -180,7 +180,8 @@ public class QualityGates {
     checkPermission();
     getNonNullQgate(qGateId);
     Metric metric = getNonNullMetric(metricKey);
-    validateCondition(qGateId, metric, operator, warningThreshold, errorThreshold, period);
+    validateCondition(metric, operator, warningThreshold, errorThreshold, period);
+    checkConditionDoesNotAlreadyExistOnSameMetricAndPeriod(qGateId, metric, period);
     QualityGateConditionDto newCondition = new QualityGateConditionDto().setQualityGateId(qGateId)
       .setMetricId(metric.getId()).setMetricKey(metric.getKey())
       .setOperator(operator)
@@ -196,7 +197,7 @@ public class QualityGates {
     checkPermission();
     QualityGateConditionDto condition = getNonNullCondition(condId);
     Metric metric = getNonNullMetric(metricKey);
-    validateCondition(condition.getQualityGateId(), metric, operator, warningThreshold, errorThreshold, period);
+    validateCondition(metric, operator, warningThreshold, errorThreshold, period);
     condition
       .setMetricId(metric.getId())
       .setMetricKey(metric.getKey())
@@ -267,29 +268,30 @@ public class QualityGates {
     return hasWritePermission;
   }
 
-  private void validateCondition(long qGateId, Metric metric, String operator, @Nullable String warningThreshold, @Nullable String errorThreshold, @Nullable Integer period) {
+  private void validateCondition(Metric metric, String operator, @Nullable String warningThreshold, @Nullable String errorThreshold, @Nullable Integer period) {
     Errors errors = new Errors();
     validateMetric(metric, errors);
     checkOperator(metric, operator, errors);
     checkThresholds(warningThreshold, errorThreshold, errors);
     checkPeriod(metric, period, errors);
-    checkConditionDoesNotAlreadyExistOnSameMetricAndPeriod(qGateId, metric, period, errors);
     if (!errors.isEmpty()) {
       throw new BadRequestException(errors);
     }
   }
 
-  private void checkConditionDoesNotAlreadyExistOnSameMetricAndPeriod(long qGateId, Metric metric, @Nullable final Integer period, Errors errors) {
+  private void checkConditionDoesNotAlreadyExistOnSameMetricAndPeriod(long qGateId, Metric metric, @Nullable final Integer period) {
     Collection<QualityGateConditionDto> conditions = conditionDao.selectForQualityGate(qGateId);
     if (conditions.isEmpty()) {
       return;
     }
 
     boolean conditionExists = from(conditions).anyMatch(new MatchMetricAndPeriod(metric.getId(), period));
-    String errorMessage = period == null
-      ? format("Condition on metric '%s' already exists.", metric.getName())
-      : format("Condition on metric '%s' over leak period already exists.", metric.getName());
-    errors.check(!conditionExists, errorMessage);
+    if (conditionExists) {
+      String errorMessage = period == null
+        ? format("Condition on metric '%s' already exists.", metric.getName())
+        : format("Condition on metric '%s' over leak period already exists.", metric.getName());
+      throw new BadRequestException(errorMessage);
+    }
   }
 
   private void checkPeriod(Metric metric, @Nullable Integer period, Errors errors) {

diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java
index 1d7d0ac1102a283a70c8a1de18556b9a7785a181..e034eabe1313af9646f3d7e586221ee55fff8ba4 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/QualityGatesTest.java
@@ -485,27 +485,23 @@ public class QualityGatesTest {
 
   @Test
   public void should_update_condition() {
-    long condId = QUALITY_GATE_ID;
+    long condId = 43;
     String metricKey = "new_coverage";
     String operator = "LT";
     String errorThreshold = "80";
-    final QualityGateConditionDto condition = new QualityGateConditionDto().setId(condId)
-      .setMetricId(666L).setOperator("GT").setWarningThreshold("123");
+    addMetric(metricKey, "New Coverage");
+
+    QualityGateConditionDto condition = new QualityGateConditionDto().setId(condId)
+      .setQualityGateId(QUALITY_GATE_ID)
+      .setMetricId(METRIC_ID)
+      .setMetricKey(metricKey)
+      .setOperator("GT")
+      .setWarningThreshold("123")
+      .setPeriod(1);
     when(conditionDao.selectById(condId)).thenReturn(condition);
-    int metricId = 10;
-    Metric newCoverage = Mockito.spy(CoreMetrics.NEW_COVERAGE);
-    when(newCoverage.getId()).thenReturn(metricId);
-    when(metricFinder.findByKey(metricKey)).thenReturn(newCoverage);
-    int period = 1;
+    when(conditionDao.selectForQualityGate(QUALITY_GATE_ID)).thenReturn(singletonList(condition));
 
-    assertThat(underTest.updateCondition(condId, metricKey, operator, null, errorThreshold, period)).isEqualTo(condition);
-    assertThat(condition.getId()).isEqualTo(condId);
-    assertThat(condition.getMetricId()).isEqualTo((long) metricId);
-    assertThat(condition.getMetricKey()).isEqualTo(metricKey);
-    assertThat(condition.getOperator()).isEqualTo(operator);
-    assertThat(condition.getWarningThreshold()).isNull();
-    assertThat(condition.getErrorThreshold()).isEqualTo(errorThreshold);
-    assertThat(condition.getPeriod()).isEqualTo(period);
+    assertThat(underTest.updateCondition(condId, metricKey, operator, null, errorThreshold, 1)).isEqualTo(condition);
     verify(conditionDao).update(condition);
   }