From 78d5368a7d2344fd758278c9c69971bfbe7f4f3d Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Wed, 11 May 2016 15:56:31 +0200 Subject: [PATCH] SONAR-6336 No check should be done when updating condition --- .../server/qualitygate/QualityGates.java | 20 +++++++------ .../server/qualitygate/QualityGatesTest.java | 28 ++++++++----------- 2 files changed, 23 insertions(+), 25 deletions(-) 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 6df826fda8f..1bd05457571 100644 --- 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 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 1d7d0ac1102..e034eabe131 100644 --- 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); } -- 2.39.5