SONAR-10088 Replace id by dto when creating condition

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 430891455a3..a0c530ef532 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 class QualityGateConditionsUpdater {
     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 class QualityGateConditionsUpdater {
     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 8e79e62d61e..2d22f446ce6 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 @@ public class RegisterQualityGates implements Startable {
     List<QualityGateCondition> 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 6509351d621..268af34e763 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.DbClient;
 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 class CreateConditionAction implements QualityGatesWsAction {
     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 2e4fb975138..6bb3284df74 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 class QualityGateConditionsUpdaterTest {
 
   @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 class QualityGateConditionsUpdaterTest {
       .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 class QualityGateConditionsUpdaterTest {
 
     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 class QualityGateConditionsUpdaterTest {
 
     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 class QualityGateConditionsUpdaterTest {
 
     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 class QualityGateConditionsUpdaterTest {
 
     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 class QualityGateConditionsUpdaterTest {
   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 731c5ef6129..599f7b09b91 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 class RegisterQualityGatesTest {
     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 @@ public class RegisterQualityGatesTest {
   private List<QualityGateConditionDto> createBuiltInConditions(QualityGateDto builtin) {
     List<QualityGateConditionDto> 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 0d94f7bbda3..b455ba4af88 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.db.qualitygate.QualityGateDto;
 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 12616f7cd2d..9eaf85f4a1b 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 class QualityGatesWsTest {
       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 class QualityGatesWsTest {
     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 class QualityGatesWsTest {
     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