From fb63d5de0de140c0f7c38e59520760511388111f Mon Sep 17 00:00:00 2001 From: Julien Lancelot Date: Fri, 24 Nov 2017 17:05:00 +0100 Subject: [PATCH] SONAR-10088 Replace id by dto when creating condition --- .../QualityGateConditionsUpdater.java | 15 ++-------- .../qualitygate/RegisterQualityGates.java | 9 ++---- .../qualitygate/ws/CreateConditionAction.java | 10 +++++-- .../QualityGateConditionsUpdaterTest.java | 28 +++++++++---------- .../qualitygate/RegisterQualityGatesTest.java | 12 ++++---- .../ws/CreateConditionActionTest.java | 4 ++- .../qualitygate/ws/QualityGatesWsTest.java | 24 +--------------- 7 files changed, 37 insertions(+), 65 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/QualityGateConditionsUpdater.java index 430891455a30..a0c530ef5326 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 @@ -58,14 +58,13 @@ public QualityGateConditionsUpdater(DbClient dbClient) { this.dbClient = dbClient; } - public QualityGateConditionDto createCondition(DbSession dbSession, long qGateId, String metricKey, String operator, + public QualityGateConditionDto createCondition(DbSession dbSession, QualityGateDto qualityGate, String metricKey, String operator, @Nullable String warningThreshold, @Nullable String errorThreshold, @Nullable Integer period) { - getNonNullQgate(dbSession, qGateId); MetricDto metric = getNonNullMetric(dbSession, metricKey); validateCondition(metric, operator, warningThreshold, errorThreshold, period); - checkConditionDoesNotAlreadyExistOnSameMetricAndPeriod(getConditions(dbSession, qGateId, null), metric, period); + checkConditionDoesNotAlreadyExistOnSameMetricAndPeriod(getConditions(dbSession, qualityGate.getId(), null), metric, period); - QualityGateConditionDto newCondition = new QualityGateConditionDto().setQualityGateId(qGateId) + QualityGateConditionDto newCondition = new QualityGateConditionDto().setQualityGateId(qualityGate.getId()) .setMetricId(metric.getId()).setMetricKey(metric.getKey()) .setOperator(operator) .setWarningThreshold(warningThreshold) @@ -92,14 +91,6 @@ public QualityGateConditionDto updateCondition(DbSession dbSession, QualityGateC return condition; } - private QualityGateDto getNonNullQgate(DbSession dbSession, long id) { - QualityGateDto qGate = dbClient.qualityGateDao().selectById(dbSession, id); - if (qGate == null) { - throw new NotFoundException(format("There is no quality gate with id=%s", id)); - } - return qGate; - } - private MetricDto getNonNullMetric(DbSession dbSession, String metricKey) { MetricDto metric = dbClient.metricDao().selectByKey(dbSession, metricKey); if (metric == null) { diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/RegisterQualityGates.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/RegisterQualityGates.java index 8e79e62d61ea..2d22f446ce6e 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/RegisterQualityGates.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/RegisterQualityGates.java @@ -63,8 +63,7 @@ public class RegisterQualityGates implements Startable { new QualityGateCondition().setMetricKey(NEW_RELIABILITY_RATING_KEY).setOperator(OPERATOR_GREATER_THAN).setPeriod(LEAK_PERIOD).setErrorThreshold(A_RATING), new QualityGateCondition().setMetricKey(NEW_MAINTAINABILITY_RATING_KEY).setOperator(OPERATOR_GREATER_THAN).setPeriod(LEAK_PERIOD).setErrorThreshold(A_RATING), new QualityGateCondition().setMetricKey(NEW_COVERAGE_KEY).setOperator(OPERATOR_LESS_THAN).setPeriod(LEAK_PERIOD).setErrorThreshold("80"), - new QualityGateCondition().setMetricKey(NEW_DUPLICATED_LINES_DENSITY_KEY).setOperator(OPERATOR_GREATER_THAN).setPeriod(LEAK_PERIOD).setErrorThreshold("3") - ); + new QualityGateCondition().setMetricKey(NEW_DUPLICATED_LINES_DENSITY_KEY).setOperator(OPERATOR_GREATER_THAN).setPeriod(LEAK_PERIOD).setErrorThreshold("3")); private final DbClient dbClient; private final QualityGateConditionsUpdater qualityGateConditionsUpdater; @@ -138,10 +137,8 @@ private void updateQualityConditionsIfRequired(DbSession dbSession, QualityGateD List qgConditionsToBeCreated = new ArrayList<>(QUALITY_GATE_CONDITIONS); qgConditionsToBeCreated.removeAll(qualityGateConditions); qgConditionsToBeCreated.stream() - .forEach(qgc -> - qualityGateConditionsUpdater.createCondition(dbSession, builtin.getId(), qgc.getMetricKey(), qgc.getOperator(), qgc.getWarningThreshold(), - qgc.getErrorThreshold(), qgc.getPeriod()) - ); + .forEach(qgc -> qualityGateConditionsUpdater.createCondition(dbSession, builtin, qgc.getMetricKey(), qgc.getOperator(), qgc.getWarningThreshold(), + qgc.getErrorThreshold(), qgc.getPeriod())); if (!qgConditionsToBeCreated.isEmpty() || !qgConditionsToBeDeleted.isEmpty()) { LOGGER.info("Built-in quality gate's conditions of [{}] has been updated", BUILTIN_QUALITY_GATE_NAME); diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/CreateConditionAction.java b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/CreateConditionAction.java index 6509351d6211..268af34e7639 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/CreateConditionAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/qualitygate/ws/CreateConditionAction.java @@ -26,8 +26,10 @@ import org.sonar.db.DbSession; import org.sonar.db.permission.OrganizationPermission; import org.sonar.db.qualitygate.QualityGateConditionDto; +import org.sonar.db.qualitygate.QualityGateDto; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.qualitygate.QualityGateConditionsUpdater; +import org.sonar.server.qualitygate.QualityGateFinder; import org.sonar.server.user.UserSession; import org.sonarqube.ws.Qualitygates.CreateConditionResponse; @@ -48,13 +50,15 @@ public class CreateConditionAction implements QualityGatesWsAction { private final DbClient dbClient; private final QualityGateConditionsUpdater qualityGateConditionsUpdater; private final DefaultOrganizationProvider defaultOrganizationProvider; + private final QualityGateFinder qualityGateFinder; public CreateConditionAction(UserSession userSession, DbClient dbClient, QualityGateConditionsUpdater qualityGateConditionsUpdater, - DefaultOrganizationProvider defaultOrganizationProvider) { + DefaultOrganizationProvider defaultOrganizationProvider, QualityGateFinder qualityGateFinder) { this.userSession = userSession; this.dbClient = dbClient; this.qualityGateConditionsUpdater = qualityGateConditionsUpdater; this.defaultOrganizationProvider = defaultOrganizationProvider; + this.qualityGateFinder = qualityGateFinder; } @Override @@ -88,8 +92,8 @@ public void handle(Request request, Response response) { Integer period = request.paramAsInt(PARAM_PERIOD); try (DbSession dbSession = dbClient.openSession(false)) { - QualityGateConditionDto condition = qualityGateConditionsUpdater.createCondition(dbSession, gateId, metric, operator, warning, error, period); - + QualityGateDto qualityGate = qualityGateFinder.getById(dbSession, gateId); + QualityGateConditionDto condition = qualityGateConditionsUpdater.createCondition(dbSession, qualityGate, metric, operator, warning, error, period); CreateConditionResponse.Builder createConditionResponse = CreateConditionResponse.newBuilder() .setId(condition.getId()) .setMetric(condition.getMetricKey()) 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 2e4fb9751385..6bb3284df741 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 @@ -85,7 +85,7 @@ public void setUp() throws Exception { @Test public void create_warning_condition_without_period() { - QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto.getId(), "coverage", "LT", "90", null, null); + QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto, "coverage", "LT", "90", null, null); verifyCondition(result, coverageMetricDto.getId(), "LT", "90", null, null); } @@ -98,7 +98,7 @@ public void create_error_condition_with_period() { .setHidden(false)); dbSession.commit(); - QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto.getId(), "new_coverage", "LT", null, "80", 1); + QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto, "new_coverage", "LT", null, "80", 1); verifyCondition(result, metricDto.getId(), "LT", null, "80", 1); } @@ -113,7 +113,7 @@ public void fail_to_create_condition_when_condition_on_same_metric_already_exist expectedException.expect(BadRequestException.class); expectedException.expectMessage("Condition on metric 'Coverage' already exists."); - underTest.createCondition(dbSession, qualityGateDto.getId(), coverageMetricDto.getKey(), "LT", "90", null, null); + underTest.createCondition(dbSession, qualityGateDto, coverageMetricDto.getKey(), "LT", "90", null, null); } @Test @@ -126,14 +126,14 @@ public void fail_to_create_condition_when_condition_on_same_metric_and_on_leak_p expectedException.expect(BadRequestException.class); expectedException.expectMessage("Condition on metric 'Coverage' over leak period already exists."); - underTest.createCondition(dbSession, qualityGateDto.getId(), coverageMetricDto.getKey(), "LT", "90", null, 1); + underTest.createCondition(dbSession, qualityGateDto, coverageMetricDto.getKey(), "LT", "90", null, 1); } @Test public void fail_to_create_condition_on_missing_metric() { expectedException.expect(NotFoundException.class); expectedException.expectMessage("There is no metric with key=new_coverage"); - underTest.createCondition(dbSession, qualityGateDto.getId(), "new_coverage", "LT", null, "80", 2); + underTest.createCondition(dbSession, qualityGateDto, "new_coverage", "LT", null, "80", 2); } @Test @@ -147,14 +147,14 @@ public void fail_to_create_condition_on_invalid_metric(String metricKey, Metric. expectedException.expect(BadRequestException.class); expectedException.expectMessage("Metric '" + metricKey + "' cannot be used to define a condition."); - underTest.createCondition(dbSession, qualityGateDto.getId(), metricKey, "EQ", null, "80", null); + underTest.createCondition(dbSession, qualityGateDto, metricKey, "EQ", null, "80", null); } @Test public void fail_to_create_condition_on_not_allowed_operator() { expectedException.expect(BadRequestException.class); expectedException.expectMessage("Operator UNKNOWN is not allowed for metric type PERCENT."); - underTest.createCondition(dbSession, qualityGateDto.getId(), "coverage", "UNKNOWN", null, "80", 2); + underTest.createCondition(dbSession, qualityGateDto, "coverage", "UNKNOWN", null, "80", 2); } @Test @@ -167,19 +167,19 @@ public void fail_to_create_condition_on_missing_period() { expectedException.expect(BadRequestException.class); expectedException.expectMessage("A period must be selected for differential metrics."); - underTest.createCondition(dbSession, qualityGateDto.getId(), "new_coverage", "EQ", null, "90", null); + underTest.createCondition(dbSession, qualityGateDto, "new_coverage", "EQ", null, "90", null); } @Test public void fail_to_create_condition_on_invalid_period() { expectedException.expect(BadRequestException.class); expectedException.expectMessage("The only valid quality gate period is 1, the leak period."); - underTest.createCondition(dbSession, qualityGateDto.getId(), "coverage", "EQ", null, "90", 6); + underTest.createCondition(dbSession, qualityGateDto, "coverage", "EQ", null, "90", 6); } @Test public void create_condition_on_rating_metric() { - QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "GT", null, "3", null); + QualityGateConditionDto result = underTest.createCondition(dbSession, qualityGateDto, ratingMetricDto.getKey(), "GT", null, "3", null); verifyCondition(result, ratingMetricDto.getId(), "GT", null, "3", null); } @@ -188,28 +188,28 @@ public void create_condition_on_rating_metric() { public void fail_to_create_condition_on_rating_metric_on_leak_period() { expectedException.expect(BadRequestException.class); expectedException.expectMessage("The metric 'Reliability Rating' cannot be used on the leak period"); - underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "GT", null, "3", 1); + underTest.createCondition(dbSession, qualityGateDto, ratingMetricDto.getKey(), "GT", null, "3", 1); } @Test public void fail_to_create_warning_condition_on_invalid_rating_metric() { expectedException.expect(BadRequestException.class); expectedException.expectMessage("'6' is not a valid rating"); - underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "GT", "6", null, null); + underTest.createCondition(dbSession, qualityGateDto, ratingMetricDto.getKey(), "GT", "6", null, null); } @Test public void fail_to_create_error_condition_on_invalid_rating_metric() { expectedException.expect(BadRequestException.class); expectedException.expectMessage("'80' is not a valid rating"); - underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "GT", null, "80", null); + underTest.createCondition(dbSession, qualityGateDto, ratingMetricDto.getKey(), "GT", null, "80", null); } @Test public void fail_to_create_condition_on_greater_than_E() { expectedException.expect(BadRequestException.class); expectedException.expectMessage("There's no worse rating than E (5)"); - underTest.createCondition(dbSession, qualityGateDto.getId(), ratingMetricDto.getKey(), "GT", "5", null, null); + underTest.createCondition(dbSession, qualityGateDto, ratingMetricDto.getKey(), "GT", "5", null, null); } @Test 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 731c5ef61295..599f7b09b91c 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 @@ -123,7 +123,7 @@ public void upgrade_should_remove_deleted_condition() { createBuiltInConditions(builtin); // Add another condition - qualityGateConditionsUpdater.createCondition(dbSession, builtin.getId(), + qualityGateConditionsUpdater.createCondition(dbSession, builtin, NEW_SECURITY_REMEDIATION_EFFORT_KEY, OPERATOR_GREATER_THAN, null, "5", LEAK_PERIOD); dbSession.commit(); @@ -301,15 +301,15 @@ private void verifyCorrectBuiltInQualityGate() { private List createBuiltInConditions(QualityGateDto builtin) { List conditions = new ArrayList<>(); - conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin.getId(), + conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin, NEW_SECURITY_RATING_KEY, OPERATOR_GREATER_THAN, null, "1", LEAK_PERIOD)); - conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin.getId(), + conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin, NEW_RELIABILITY_RATING_KEY, OPERATOR_GREATER_THAN, null, "1", LEAK_PERIOD)); - conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin.getId(), + conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin, NEW_MAINTAINABILITY_RATING_KEY, OPERATOR_GREATER_THAN, null, "1", LEAK_PERIOD)); - conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin.getId(), + conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin, NEW_COVERAGE_KEY, OPERATOR_LESS_THAN, null, "80", LEAK_PERIOD)); - conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin.getId(), + conditions.add(qualityGateConditionsUpdater.createCondition(dbSession, builtin, NEW_DUPLICATED_LINES_DENSITY_KEY, OPERATOR_GREATER_THAN, null, "3", LEAK_PERIOD)); return conditions; 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 0d94f7bbda3a..b455ba4af882 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 @@ -35,6 +35,7 @@ import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.qualitygate.QualityGateConditionsUpdater; +import org.sonar.server.qualitygate.QualityGateFinder; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; @@ -65,7 +66,8 @@ public class CreateConditionActionTest { private TestDefaultOrganizationProvider defaultOrganizationProvider = TestDefaultOrganizationProvider.from(db); private DbClient dbClient = db.getDbClient(); private DbSession dbSession = db.getSession(); - private CreateConditionAction underTest = new CreateConditionAction(userSession, dbClient, new QualityGateConditionsUpdater(dbClient), defaultOrganizationProvider); + private CreateConditionAction underTest = new CreateConditionAction(userSession, dbClient, new QualityGateConditionsUpdater(dbClient), defaultOrganizationProvider, + new QualityGateFinder(dbClient)); private WsActionTester ws = new WsActionTester(underTest); diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java index 12616f7cd2db..9eaf85f4a1b0 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualitygate/ws/QualityGatesWsTest.java @@ -72,7 +72,6 @@ public void setUp() { new CopyAction(qGates), new DestroyAction(qGates), new SetAsDefaultAction(qGates), - new CreateConditionAction(null, null, null, null), new DeleteConditionAction(null, null, null), selectAction, new DeselectAction(qGates, mock(DbClient.class), mock(ComponentFinder.class)))); @@ -84,15 +83,7 @@ public void define_ws() { assertThat(controller).isNotNull(); assertThat(controller.path()).isEqualTo("api/qualitygates"); assertThat(controller.description()).isNotEmpty(); - assertThat(controller.actions()).hasSize(10); - - Action create = controller.action("create"); - assertThat(create).isNotNull(); - assertThat(create.handler()).isNotNull(); - assertThat(create.since()).isEqualTo("4.3"); - assertThat(create.isPost()).isTrue(); - assertThat(create.param("name")).isNotNull(); - assertThat(create.isInternal()).isFalse(); + assertThat(controller.actions()).hasSize(9); Action copy = controller.action("copy"); assertThat(copy).isNotNull(); @@ -132,19 +123,6 @@ public void define_ws() { assertThat(unsetDefault.handler()).isEqualTo(RemovedWebServiceHandler.INSTANCE); assertThat(unsetDefault.responseExample()).isEqualTo(RemovedWebServiceHandler.INSTANCE.getResponseExample()); assertThat(unsetDefault.isInternal()).isFalse(); - - Action createCondition = controller.action("create_condition"); - assertThat(createCondition).isNotNull(); - assertThat(createCondition.handler()).isNotNull(); - assertThat(createCondition.since()).isEqualTo("4.3"); - assertThat(createCondition.isPost()).isTrue(); - assertThat(createCondition.param("gateId")).isNotNull(); - assertThat(createCondition.param("metric")).isNotNull(); - assertThat(createCondition.param("op")).isNotNull(); - assertThat(createCondition.param("warning")).isNotNull(); - assertThat(createCondition.param("error")).isNotNull(); - assertThat(createCondition.param("period")).isNotNull(); - assertThat(createCondition.isInternal()).isFalse(); } @Test