From 4d75df2feac832421fa6e6b54167e441f1e51836 Mon Sep 17 00:00:00 2001
From: Jean-Baptiste Lievremont <jean-baptiste.lievremont@sonarsource.com>
Date: Thu, 13 Mar 2014 14:30:23 +0100
Subject: [PATCH] SONAR-4366 Remove conditions when the associated metric
 disappears

---
 .../db/QualityGateConditionDao.java           | 14 ++++++++++++
 .../db/QualityGateConditionMapper.java        |  2 ++
 .../db/QualityGateConditionMapper.xml         |  5 +++++
 .../db/QualityGateConditionDaoTest.java       |  5 +++++
 .../shouldCleanConditions-result.xml          | 11 ++++------
 .../shouldCleanConditions.xml                 | 12 ++++------
 .../sonar/server/startup/RegisterMetrics.java |  9 +++++++-
 .../server/startup/RegisterMetricsTest.java   | 22 +++++++++----------
 8 files changed, 52 insertions(+), 28 deletions(-)
 rename sonar-server/src/test/resources/org/sonar/server/startup/RegisterMetricsTest/shouldCleanAlerts-result.xml => sonar-core/src/test/resources/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest/shouldCleanConditions-result.xml (51%)
 rename sonar-server/src/test/resources/org/sonar/server/startup/RegisterMetricsTest/shouldCleanAlerts.xml => sonar-core/src/test/resources/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest/shouldCleanConditions.xml (52%)

diff --git a/sonar-core/src/main/java/org/sonar/core/qualitygate/db/QualityGateConditionDao.java b/sonar-core/src/main/java/org/sonar/core/qualitygate/db/QualityGateConditionDao.java
index 8d532995e0e..280cf141865 100644
--- a/sonar-core/src/main/java/org/sonar/core/qualitygate/db/QualityGateConditionDao.java
+++ b/sonar-core/src/main/java/org/sonar/core/qualitygate/db/QualityGateConditionDao.java
@@ -104,6 +104,20 @@ public class QualityGateConditionDao {
     getMapper(session).update(qGate.setUpdatedAt(new Date()));
   }
 
+  public void deleteConditionsWithInvalidMetrics() {
+    SqlSession session = myBatis.openSession();
+    try {
+      deleteConditionsWithInvalidMetrics(session);
+      session.commit();
+    } finally {
+      MyBatis.closeQuietly(session);
+    }
+  }
+
+  public void deleteConditionsWithInvalidMetrics(SqlSession session) {
+    getMapper(session).deleteConditionsWithInvalidMetrics();
+  }
+
   private QualityGateConditionMapper getMapper(SqlSession session) {
     return session.getMapper(QualityGateConditionMapper.class);
   }
diff --git a/sonar-core/src/main/java/org/sonar/core/qualitygate/db/QualityGateConditionMapper.java b/sonar-core/src/main/java/org/sonar/core/qualitygate/db/QualityGateConditionMapper.java
index 27202c5c8b7..5d2d348fb51 100644
--- a/sonar-core/src/main/java/org/sonar/core/qualitygate/db/QualityGateConditionMapper.java
+++ b/sonar-core/src/main/java/org/sonar/core/qualitygate/db/QualityGateConditionMapper.java
@@ -32,4 +32,6 @@ public interface QualityGateConditionMapper {
   QualityGateConditionDto selectById(long id);
 
   void delete(long id);
+
+  void deleteConditionsWithInvalidMetrics();
 }
diff --git a/sonar-core/src/main/resources/org/sonar/core/qualitygate/db/QualityGateConditionMapper.xml b/sonar-core/src/main/resources/org/sonar/core/qualitygate/db/QualityGateConditionMapper.xml
index afc56564b42..d709af2693d 100644
--- a/sonar-core/src/main/resources/org/sonar/core/qualitygate/db/QualityGateConditionMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/qualitygate/db/QualityGateConditionMapper.xml
@@ -37,5 +37,10 @@
     where id=#{id}
   </update>
 
+  <delete id="deleteConditionsWithInvalidMetrics">
+    delete from quality_gate_conditions
+    where metric_id not in (select id from metrics where enabled=true)
+  </delete>
+
 </mapper>
 
diff --git a/sonar-core/src/test/java/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest.java b/sonar-core/src/test/java/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest.java
index c4a67a2215e..5ac75ac4b01 100644
--- a/sonar-core/src/test/java/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest.java
+++ b/sonar-core/src/test/java/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest.java
@@ -84,4 +84,9 @@ public class QualityGateConditionDaoTest extends AbstractDaoTestCase {
     checkTable("update", "quality_gate_conditions", COLUMNS_WITHOUT_TIMESTAMPS);
   }
 
+  public void shouldCleanConditions() throws Exception {
+    setupData("shouldCleanConditions");
+    dao.deleteConditionsWithInvalidMetrics();
+    checkTables("shouldCleanConditions", "quality_gate_conditions");
+  }
 }
diff --git a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterMetricsTest/shouldCleanAlerts-result.xml b/sonar-core/src/test/resources/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest/shouldCleanConditions-result.xml
similarity index 51%
rename from sonar-server/src/test/resources/org/sonar/server/startup/RegisterMetricsTest/shouldCleanAlerts-result.xml
rename to sonar-core/src/test/resources/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest/shouldCleanConditions-result.xml
index 4d716fe29cc..1dd295d9e98 100644
--- a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterMetricsTest/shouldCleanAlerts-result.xml
+++ b/sonar-core/src/test/resources/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest/shouldCleanConditions-result.xml
@@ -6,17 +6,14 @@
   <metrics delete_historical_data="false" id="2" name="disabledkey2" val_type="INT" description="description"  domain="domain"
            short_name="name2" qualitative="false" user_managed="false" enabled="false" origin="JAV" worst_value="[null]" optimized_best_value="false" best_value="[null]" direction="1" hidden="false"/>
 
-  <rules_profiles id="1" version="1" used_profile="true" name="profile1" language="JAV" />
-  <rules_profiles id="2" version="1" used_profile="true" name="profile2" language="JAV" />
-
   <!-- ok -->
-  <alerts id="1" profile_id="1" metric_id="1" operator=">" value_error="30" value_warning="[null]" period="[null]"/>
-  <alerts id="2" profile_id="2" metric_id="1" operator=">" value_error="[null]" value_warning="150" period="[null]"/>
+  <quality_gate_conditions id="1" qgate_id="1" metric_id="1" operator="GT" value_error="30" value_warning="[null]" period="[null]"/>
+  <quality_gate_conditions id="2" qgate_id="2" metric_id="1" operator="GT" value_error="[null]" value_warning="150" period="[null]"/>
 
   <!-- disabled metric -->
-  <!--<alerts id="3" profile_id="1" metric_id="2" operator=">" value_error="30" value_warning="[null]" period="[null]"/>-->
+  <!--<quality_gate_conditions id="3" qgate_id="1" metric_id="2" operator="GT" value_error="30" value_warning="[null]" period="[null]"/>-->
 
   <!-- unknown metric -->
-  <!--<alerts id="4" profile_id="1" metric_id="999" operator=">" value_error="30" value_warning="[null]" period="[null]"/>-->
+  <!--<quality_gate_conditions id="4" qgate_id="1" metric_id="999" operator="GT" value_error="30" value_warning="[null]" period="[null]"/>-->
 
 </dataset>
\ No newline at end of file
diff --git a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterMetricsTest/shouldCleanAlerts.xml b/sonar-core/src/test/resources/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest/shouldCleanConditions.xml
similarity index 52%
rename from sonar-server/src/test/resources/org/sonar/server/startup/RegisterMetricsTest/shouldCleanAlerts.xml
rename to sonar-core/src/test/resources/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest/shouldCleanConditions.xml
index 1443f991c02..73a9a3647f1 100644
--- a/sonar-server/src/test/resources/org/sonar/server/startup/RegisterMetricsTest/shouldCleanAlerts.xml
+++ b/sonar-core/src/test/resources/org/sonar/core/qualitygate/db/QualityGateConditionDaoTest/shouldCleanConditions.xml
@@ -6,18 +6,14 @@
   <metrics delete_historical_data="false" id="2" name="disabledkey2" val_type="INT" description="description"  domain="domain"
            short_name="name2" qualitative="false" user_managed="false" enabled="false" origin="JAV" worst_value="[null]" optimized_best_value="false" best_value="[null]" direction="1" hidden="false"/>
 
-
-  <rules_profiles id="1" version="1" used_profile="true" name="profile1" language="JAV" />
-  <rules_profiles id="2" version="1" used_profile="true" name="profile2" language="JAV" />
-
   <!-- ok -->
-  <alerts id="1" profile_id="1" metric_id="1" operator=">" value_error="30" value_warning="[null]" period="[null]"/>
-  <alerts id="2" profile_id="2" metric_id="1" operator=">" value_error="[null]" value_warning="150" period="[null]"/>
+  <quality_gate_conditions id="1" qgate_id="1" metric_id="1" operator="GT" value_error="30" value_warning="[null]" period="[null]"/>
+  <quality_gate_conditions id="2" qgate_id="2" metric_id="1" operator="GT" value_error="[null]" value_warning="150" period="[null]"/>
 
   <!-- disabled metric -->
-  <alerts id="3" profile_id="1" metric_id="2" operator=">" value_error="30" value_warning="[null]" period="[null]"/>
+  <quality_gate_conditions id="3" qgate_id="1" metric_id="2" operator="GT" value_error="30" value_warning="[null]" period="[null]"/>
 
   <!-- unknown metric -->
-  <alerts id="4" profile_id="1" metric_id="999" operator=">" value_error="30" value_warning="[null]" period="[null]"/>
+  <quality_gate_conditions id="4" qgate_id="1" metric_id="999" operator="GT" value_error="30" value_warning="[null]" period="[null]"/>
 
 </dataset>
\ No newline at end of file
diff --git a/sonar-server/src/main/java/org/sonar/server/startup/RegisterMetrics.java b/sonar-server/src/main/java/org/sonar/server/startup/RegisterMetrics.java
index 43027bf58cf..5668b8e297e 100644
--- a/sonar-server/src/main/java/org/sonar/server/startup/RegisterMetrics.java
+++ b/sonar-server/src/main/java/org/sonar/server/startup/RegisterMetrics.java
@@ -19,6 +19,8 @@
  */
 package org.sonar.server.startup;
 
+import org.sonar.core.qualitygate.db.QualityGateConditionDao;
+
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
@@ -47,11 +49,13 @@ public class RegisterMetrics {
   private final MeasuresDao measuresDao;
   private final Metrics[] metricsRepositories;
   private final DatabaseSession session;
+  private final QualityGateConditionDao conditionDao;
 
-  public RegisterMetrics(DatabaseSession session, MeasuresDao measuresDao, Metrics[] metricsRepositories) {
+  public RegisterMetrics(DatabaseSession session, MeasuresDao measuresDao, QualityGateConditionDao conditionDao, Metrics[] metricsRepositories) {
     this.session = session;
     this.measuresDao = measuresDao;
     this.metricsRepositories = metricsRepositories;
+    this.conditionDao = conditionDao;
   }
 
   public void start() {
@@ -105,6 +109,8 @@ public class RegisterMetrics {
 
   protected void cleanAlerts() {
     LOG.info("cleaning alert thresholds...");
+    conditionDao.deleteConditionsWithInvalidMetrics();
+/*
     Query query = session.createQuery("delete from " + Alert.class.getSimpleName() + " a where NOT EXISTS(FROM Metric m WHERE m=a.metric))");
     query.executeUpdate();
 
@@ -112,6 +118,7 @@ public class RegisterMetrics {
     query.setParameter("enabled", Boolean.TRUE);
     query.executeUpdate();
     session.commit();
+*/
   }
 
   protected void register(List<Metric> metrics) {
diff --git a/sonar-server/src/test/java/org/sonar/server/startup/RegisterMetricsTest.java b/sonar-server/src/test/java/org/sonar/server/startup/RegisterMetricsTest.java
index 51d7fb06485..59a4e6d3ce1 100644
--- a/sonar-server/src/test/java/org/sonar/server/startup/RegisterMetricsTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/startup/RegisterMetricsTest.java
@@ -23,6 +23,7 @@ import org.junit.Test;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.measures.Metric;
 import org.sonar.api.measures.Metrics;
+import org.sonar.core.qualitygate.db.QualityGateConditionDao;
 import org.sonar.jpa.dao.MeasuresDao;
 import org.sonar.jpa.test.AbstractDbUnitTestCase;
 
@@ -31,8 +32,7 @@ import java.util.List;
 
 import static com.google.common.collect.Lists.newArrayList;
 import static org.fest.assertions.Assertions.assertThat;
-import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 
 public class RegisterMetricsTest extends AbstractDbUnitTestCase {
 
@@ -55,7 +55,7 @@ public class RegisterMetricsTest extends AbstractDbUnitTestCase {
         .setUserManaged(false)
         .create();
 
-    RegisterMetrics synchronizer = new RegisterMetrics(getSession(), new MeasuresDao(getSession()), null);
+    RegisterMetrics synchronizer = new RegisterMetrics(getSession(), new MeasuresDao(getSession()), mock(QualityGateConditionDao.class), null);
     synchronizer.register(Arrays.asList(metric1, metric2));
     checkTables("shouldSaveIfNew", "metrics");
   }
@@ -64,7 +64,7 @@ public class RegisterMetricsTest extends AbstractDbUnitTestCase {
   public void shouldUpdateIfAlreadyExists() {
     setupData("shouldUpdateIfAlreadyExists");
 
-    RegisterMetrics synchronizer = new RegisterMetrics(getSession(), new MeasuresDao(getSession()), null);
+    RegisterMetrics synchronizer = new RegisterMetrics(getSession(), new MeasuresDao(getSession()), mock(QualityGateConditionDao.class), null);
     synchronizer.register(newArrayList(new Metric.Builder("key", "new short name", Metric.ValueType.FLOAT)
         .setDescription("new description")
         .setDirection(-1)
@@ -88,7 +88,7 @@ public class RegisterMetricsTest extends AbstractDbUnitTestCase {
         .create()));
 
     MeasuresDao measuresDao = new MeasuresDao(getSession());
-    RegisterMetrics loader = new RegisterMetrics(getSession(), measuresDao, new Metrics[]{metrics});
+    RegisterMetrics loader = new RegisterMetrics(getSession(), measuresDao, mock(QualityGateConditionDao.class), new Metrics[]{metrics});
     List<Metric> result = loader.getMetricsRepositories();
 
     assertThat(result).hasSize(1);
@@ -108,7 +108,7 @@ public class RegisterMetricsTest extends AbstractDbUnitTestCase {
         .create()));
 
     MeasuresDao measuresDao = new MeasuresDao(getSession());
-    RegisterMetrics loader = new RegisterMetrics(getSession(), measuresDao, new Metrics[]{metrics});
+    RegisterMetrics loader = new RegisterMetrics(getSession(), measuresDao, mock(QualityGateConditionDao.class), new Metrics[]{metrics});
     List<Metric> result = loader.getMetricsRepositories();
 
     assertThat(result).isEmpty();
@@ -119,7 +119,7 @@ public class RegisterMetricsTest extends AbstractDbUnitTestCase {
     setupData("shouldEnableOnlyLoadedMetrics");
 
     MeasuresDao measuresDao = new MeasuresDao(getSession());
-    RegisterMetrics loader = new RegisterMetrics(getSession(), measuresDao, null);
+    RegisterMetrics loader = new RegisterMetrics(getSession(), measuresDao, mock(QualityGateConditionDao.class), null);
     loader.start();
 
     assertThat(measuresDao.getMetric("deprecated").getEnabled()).isFalse();
@@ -128,11 +128,9 @@ public class RegisterMetricsTest extends AbstractDbUnitTestCase {
 
   @Test
   public void shouldCleanAlerts() {
-    setupData("shouldCleanAlerts");
-
-    RegisterMetrics loader = new RegisterMetrics(getSession(), new MeasuresDao(getSession()), null);
+    QualityGateConditionDao conditionDao = mock(QualityGateConditionDao.class);
+    RegisterMetrics loader = new RegisterMetrics(getSession(), new MeasuresDao(getSession()), conditionDao, null);
     loader.cleanAlerts();
-
-    checkTables("shouldCleanAlerts", "metrics", "alerts");
+    verify(conditionDao).deleteConditionsWithInvalidMetrics();
   }
 }
-- 
2.39.5