[SONAR-18811] Remove duplicated conditions on built-in quality gate at startup

diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualitygate/RegisterQualityGatesIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualitygate/RegisterQualityGatesIT.java
index 9f56986821b7ab038a53005901532317b7071449..73e651eca7cb9a240d75242e4c85f6ea88e9f7f5 100644 (file)
--- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualitygate/RegisterQualityGatesIT.java
+++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/qualitygate/RegisterQualityGatesIT.java
@@ -119,6 +119,23 @@ public class RegisterQualityGatesIT {
       logTester.logs(Level.INFO)).contains("Built-in quality gate's conditions of [Sonar way] has been updated");
   }
 
+  @Test
+  public void upgrade_should_remove_duplicated_conditions_if_found() {
+    insertMetrics();
+    QualityGateDto builtInQualityGate = db.qualityGates().insertBuiltInQualityGate();
+    createBuiltInConditions(builtInQualityGate);
+    //Conditions added twice as found in some DB instances
+    createBuiltInConditionsWithoutCheckingDuplicates(builtInQualityGate);
+    dbSession.commit();
+
+    underTest.start();
+
+    assertThat(db.countRowsOfTable("quality_gates")).isOne();
+    verifyCorrectBuiltInQualityGate();
+    assertThat(
+      logTester.logs(Level.INFO)).contains("Built-in quality gate's conditions of [Sonar way] has been updated");
+  }
+
   @Test
   public void upgrade_should_add_missing_condition() {
     insertMetrics();
@@ -272,4 +289,37 @@ public class RegisterQualityGatesIT {
 
     return conditions;
   }
+
+  private List<QualityGateConditionDto> createBuiltInConditionsWithoutCheckingDuplicates(QualityGateDto qg) {
+    List<QualityGateConditionDto> conditions = new ArrayList<>();
+
+    conditions.add(createConditionWithoutCheckingDuplicates(qg,
+      NEW_SECURITY_RATING_KEY, OPERATOR_GREATER_THAN, "1"));
+    conditions.add(createConditionWithoutCheckingDuplicates(qg,
+      NEW_RELIABILITY_RATING_KEY, OPERATOR_GREATER_THAN, "1"));
+    conditions.add(createConditionWithoutCheckingDuplicates(qg,
+      NEW_MAINTAINABILITY_RATING_KEY, OPERATOR_GREATER_THAN, "1"));
+    conditions.add(createConditionWithoutCheckingDuplicates(qg,
+      NEW_COVERAGE_KEY, OPERATOR_LESS_THAN, "80"));
+    conditions.add(createConditionWithoutCheckingDuplicates(qg,
+      NEW_DUPLICATED_LINES_DENSITY_KEY, OPERATOR_GREATER_THAN, "3"));
+    conditions.add(createConditionWithoutCheckingDuplicates(qg,
+      NEW_SECURITY_HOTSPOTS_REVIEWED_KEY, OPERATOR_LESS_THAN, "100"));
+
+    return conditions;
+  }
+
+  private QualityGateConditionDto createConditionWithoutCheckingDuplicates(QualityGateDto qualityGate, String metricKey, String operator,
+    String errorThreshold) {
+    MetricDto metric = metricDao.selectByKey(dbSession, metricKey);
+
+    QualityGateConditionDto newCondition = new QualityGateConditionDto().setQualityGateUuid(qualityGate.getUuid())
+      .setUuid(Uuids.create())
+      .setMetricUuid(metric.getUuid()).setMetricKey(metric.getKey())
+      .setOperator(operator)
+      .setErrorThreshold(errorThreshold);
+    gateConditionDao.insert(newCondition, dbSession);
+    return newCondition;
+  }
+
 }

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualitygate/RegisterQualityGates.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualitygate/RegisterQualityGates.java
index 19b78c29fe8fb382634f92ccddfd5eca916899f3..92b2a56e9590985a6fe8e930c3b0a52df55f9cdd 100644 (file)
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualitygate/RegisterQualityGates.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualitygate/RegisterQualityGates.java
@@ -19,16 +19,20 @@
  */
 package org.sonar.server.qualitygate;
 
+import com.google.common.collect.ImmutableList;
 import java.util.ArrayList;
+import java.util.Collections;
 import java.util.Date;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Set;
+import java.util.stream.Collectors;
 import javax.annotation.CheckForNull;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 import org.sonar.api.Startable;
 import org.sonar.api.utils.System2;
-import org.sonar.api.utils.log.Logger;
-import org.sonar.api.utils.log.Loggers;
 import org.sonar.core.util.UuidFactory;
 import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.DbClient;
@@ -53,7 +57,7 @@ import static org.sonar.db.qualitygate.QualityGateConditionDto.OPERATOR_LESS_THA
 
 public class RegisterQualityGates implements Startable {
 
-  private static final Logger LOGGER = Loggers.get(RegisterQualityGates.class);
+  private static final Logger LOGGER = LoggerFactory.getLogger(RegisterQualityGates.class);
 
   private static final String BUILTIN_QUALITY_GATE_NAME = "Sonar way";
   private static final String A_RATING = Integer.toString(Rating.A.getIndex());
@@ -85,22 +89,22 @@ public class RegisterQualityGates implements Startable {
   @Override
   public void start() {
     try (DbSession dbSession = dbClient.openSession(false)) {
-      QualityGateDto builtin = qualityGateDao.selectByName(dbSession, BUILTIN_QUALITY_GATE_NAME);
+      QualityGateDto builtinQualityGate = qualityGateDao.selectByName(dbSession, BUILTIN_QUALITY_GATE_NAME);
 
-      // Create builtin if not present
-      if (builtin == null) {
+      // Create builtinQualityGate if not present
+      if (builtinQualityGate == null) {
         LOGGER.info("Built-in quality gate [{}] has been created", BUILTIN_QUALITY_GATE_NAME);
-        builtin = createQualityGate(dbSession, BUILTIN_QUALITY_GATE_NAME);
+        builtinQualityGate = createQualityGate(dbSession, BUILTIN_QUALITY_GATE_NAME);
       }
 
-      // Set builtin if missing
-      if (!builtin.isBuiltIn()) {
-        builtin.setBuiltIn(true);
-        dbClient.qualityGateDao().update(builtin, dbSession);
+      // Set builtinQualityGate if missing
+      if (!builtinQualityGate.isBuiltIn()) {
+        builtinQualityGate.setBuiltIn(true);
+        dbClient.qualityGateDao().update(builtinQualityGate, dbSession);
         LOGGER.info("Quality gate [{}] has been set as built-in", BUILTIN_QUALITY_GATE_NAME);
       }
 
-      updateQualityConditionsIfRequired(dbSession, builtin);
+      updateQualityConditionsIfRequired(dbSession, builtinQualityGate);
 
       qualityGateDao.ensureOneBuiltInQualityGate(dbSession, BUILTIN_QUALITY_GATE_NAME);
 
@@ -108,33 +112,60 @@ public class RegisterQualityGates implements Startable {
     }
   }
 
-  private void updateQualityConditionsIfRequired(DbSession dbSession, QualityGateDto builtin) {
+  private void updateQualityConditionsIfRequired(DbSession dbSession, QualityGateDto builtinQualityGate) {
+    List<QualityGateCondition> qualityGateConditions = getQualityGateConditions(dbSession, builtinQualityGate);
+
+    List<QualityGateCondition> qgConditionsDeleted = removeExtraConditions(dbSession, builtinQualityGate, qualityGateConditions);
+    qgConditionsDeleted.addAll(removeDuplicatedConditions(dbSession, builtinQualityGate, qualityGateConditions));
+
+    List<QualityGateCondition> qgConditionsAdded = addMissingConditions(dbSession, builtinQualityGate, qualityGateConditions);
+
+    if (!qgConditionsAdded.isEmpty() || !qgConditionsDeleted.isEmpty()) {
+      LOGGER.info("Built-in quality gate's conditions of [{}] has been updated", BUILTIN_QUALITY_GATE_NAME);
+    }
+  }
+
+  private ImmutableList<QualityGateCondition> getQualityGateConditions(DbSession dbSession, QualityGateDto builtinQualityGate) {
     Map<String, String> uuidToKeyMetric = dbClient.metricDao().selectAll(dbSession).stream()
       .collect(toMap(MetricDto::getUuid, MetricDto::getKey));
 
-    List<QualityGateCondition> qualityGateConditions = qualityGateConditionDao.selectForQualityGate(dbSession, builtin.getUuid())
+    return qualityGateConditionDao.selectForQualityGate(dbSession, builtinQualityGate.getUuid())
       .stream()
       .map(dto -> QualityGateCondition.from(dto, uuidToKeyMetric))
       .collect(MoreCollectors.toList());
+  }
 
+  private List<QualityGateCondition> removeExtraConditions(DbSession dbSession, QualityGateDto builtinQualityGate, List<QualityGateCondition> qualityGateConditions) {
     // Find all conditions that are not present in QUALITY_GATE_CONDITIONS
     // Those conditions must be deleted
     List<QualityGateCondition> qgConditionsToBeDeleted = new ArrayList<>(qualityGateConditions);
     qgConditionsToBeDeleted.removeAll(QUALITY_GATE_CONDITIONS);
     qgConditionsToBeDeleted
-      .forEach(qgc -> qualityGateConditionDao.delete(qgc.toQualityGateDto(builtin.getUuid()), dbSession));
+      .forEach(qgc -> qualityGateConditionDao.delete(qgc.toQualityGateDto(builtinQualityGate.getUuid()), dbSession));
+    return qgConditionsToBeDeleted;
+  }
 
+  private Set<QualityGateCondition> removeDuplicatedConditions(DbSession dbSession, QualityGateDto builtinQualityGate, List<QualityGateCondition> qualityGateConditions) {
+    Set<QualityGateCondition> qgConditionsDuplicated = qualityGateConditions
+      .stream()
+      .filter(qualityGateCondition -> Collections.frequency(qualityGateConditions, qualityGateCondition) > 1)
+      .collect(Collectors.toSet());
+
+    qgConditionsDuplicated
+      .forEach(qgc -> qualityGateConditionDao.delete(qgc.toQualityGateDto(builtinQualityGate.getUuid()), dbSession));
+
+    return qgConditionsDuplicated;
+  }
+
+  private List<QualityGateCondition> addMissingConditions(DbSession dbSession, QualityGateDto builtinQualityGate, List<QualityGateCondition> qualityGateConditions) {
     // Find all conditions that are not present in qualityGateConditions
-    // Those conditions must be created
-    List<QualityGateCondition> qgConditionsToBeCreated = new ArrayList<>(QUALITY_GATE_CONDITIONS);
-    qgConditionsToBeCreated.removeAll(qualityGateConditions);
-    qgConditionsToBeCreated
-      .forEach(qgc -> qualityGateConditionsUpdater.createCondition(dbSession, builtin, qgc.getMetricKey(), qgc.getOperator(),
+    // Those conditions must be added to the built-in quality gate
+    List<QualityGateCondition> qgConditionsToBeAdded = new ArrayList<>(QUALITY_GATE_CONDITIONS);
+    qgConditionsToBeAdded.removeAll(qualityGateConditions);
+    qgConditionsToBeAdded
+      .forEach(qgc -> qualityGateConditionsUpdater.createCondition(dbSession, builtinQualityGate, qgc.getMetricKey(), qgc.getOperator(),
         qgc.getErrorThreshold()));
-
-    if (!qgConditionsToBeCreated.isEmpty() || !qgConditionsToBeDeleted.isEmpty()) {
-      LOGGER.info("Built-in quality gate's conditions of [{}] has been updated", BUILTIN_QUALITY_GATE_NAME);
-    }
+    return qgConditionsToBeAdded;
   }
 
   @Override