SONAR-6571 ws metrics/create change update conditions relating to metric type change

a disabled custom metric can be reactivated if:
- the type is not changed
- the type is changed and there are not associated custom measures

3 files changed, 53 insertions, 15 deletions

diff --git a/server/sonar-server/src/main/java/org/sonar/server/custommeasure/persistence/CustomMeasureDao.java b/server/sonar-server/src/main/java/org/sonar/server/custommeasure/persistence/CustomMeasureDao.java
index e8bcdcbdaea..c419b2e600f 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/custommeasure/persistence/CustomMeasureDao.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/custommeasure/persistence/CustomMeasureDao.java
@@ -52,6 +52,10 @@ public class CustomMeasureDao implements DaoComponent {
     return mapper(session).selectById(id);
   }
 
+  public List<CustomMeasureDto> selectByMetricId(DbSession session, int id) {
+    return mapper(session).selectByMetricId(id);
+  }
+
   private CustomMeasureMapper mapper(DbSession session) {
     return session.getMapper(CustomMeasureMapper.class);
   }
diff --git a/server/sonar-server/src/main/java/org/sonar/server/metric/ws/CreateAction.java b/server/sonar-server/src/main/java/org/sonar/server/metric/ws/CreateAction.java
index 09c73b50494..9c1e504685f 100644
--- a/server/sonar-server/src/main/java/org/sonar/server/metric/ws/CreateAction.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/metric/ws/CreateAction.java
@@ -21,12 +21,14 @@
 package org.sonar.server.metric.ws;
 
 import java.net.HttpURLConnection;
+import java.util.List;
 import javax.annotation.Nullable;
 import org.sonar.api.measures.Metric;
 import org.sonar.api.server.ws.Request;
 import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.text.JsonWriter;
+import org.sonar.core.custommeasure.db.CustomMeasureDto;
 import org.sonar.core.metric.db.MetricDto;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.DbSession;
@@ -100,9 +102,9 @@ public class CreateAction implements MetricsWsAction {
     try {
       MetricDto metricTemplate = newMetricTemplate(request);
       MetricDto metricInDb = dbClient.metricDao().selectNullableByKey(dbSession, key);
-      checkMetricInDbAndTemplate(metricInDb, metricTemplate);
+      checkMetricInDbAndTemplate(dbSession, metricInDb, metricTemplate);
 
-      if (metricInDb == null) {
+      if (metricIsNotInDb(metricInDb)) {
         metricInDb = insertNewMetric(dbSession, metricTemplate);
       } else {
         updateMetric(dbSession, metricInDb, metricTemplate);
@@ -168,24 +170,53 @@ public class CreateAction implements MetricsWsAction {
     return metric;
   }
 
-  private void checkMetricInDbAndTemplate(@Nullable MetricDto metricInDb, MetricDto template) {
-    if (template.getValueType().isEmpty() || template.getShortName().isEmpty() || template.getKey().isEmpty()) {
+  private void checkMetricInDbAndTemplate(DbSession dbSession, @Nullable MetricDto metricInDb, MetricDto template) {
+    if (areOneOfTheMandatoryArgumentsEmpty(template)) {
       throw new IllegalArgumentException(String.format("The mandatory arguments '%s','%s' and '%s' must not be empty", PARAM_KEY, PARAM_NAME, PARAM_TYPE));
     }
-    if (metricInDb == null) {
+    if (
+      metricIsNotInDb(metricInDb)) {
       return;
     }
-    if (metricInDb.isEnabled()) {
+    if (isMetricEnabled(metricInDb)) {
       throw new ServerException(HttpURLConnection.HTTP_CONFLICT, "An active metric already exist with key: " + metricInDb.getKey());
     }
-    if (!metricInDb.isUserManaged()) {
+    if (isMetricNonCustom(metricInDb)) {
       throw new ServerException(HttpURLConnection.HTTP_CONFLICT, "An non custom metric already exist with key: " + metricInDb.getKey());
     }
-    if (!metricInDb.getValueType().equals(template.getValueType())) {
-      throw new ServerException(HttpURLConnection.HTTP_CONFLICT, "An existing metric exist with a different type: " + metricInDb.getValueType());
+    if (hasMetricTypeChanged(metricInDb, template)) {
+      List<CustomMeasureDto> customMeasures = dbClient.customMeasureDao().selectByMetricId(dbSession, metricInDb.getId());
+      if (hasAssociatedCustomMeasures(customMeasures)) {
+        throw new ServerException(HttpURLConnection.HTTP_CONFLICT, String.format("You're trying to change the type '%s' while there are associated measures.",
+          metricInDb.getValueType()));
+      }
     }
   }
 
+  private static boolean hasAssociatedCustomMeasures(List<CustomMeasureDto> customMeasures) {
+    return !customMeasures.isEmpty();
+  }
+
+  private static boolean hasMetricTypeChanged(@Nullable MetricDto metricInDb, MetricDto template) {
+    return !metricInDb.getValueType().equals(template.getValueType());
+  }
+
+  private static boolean isMetricNonCustom(@Nullable MetricDto metricInDb) {
+    return !metricInDb.isUserManaged();
+  }
+
+  private static boolean isMetricEnabled(@Nullable MetricDto metricInDb) {
+    return metricInDb.isEnabled();
+  }
+
+  private static boolean metricIsNotInDb(@Nullable MetricDto metricInDb) {
+    return metricInDb == null;
+  }
+
+  private static boolean areOneOfTheMandatoryArgumentsEmpty(MetricDto template) {
+    return template.getValueType().isEmpty() || template.getShortName().isEmpty() || template.getKey().isEmpty();
+  }
+
   private void writeMetric(JsonWriter json, MetricDto metric) {
     json.beginObject();
     json.prop(FIELD_ID, metric.getId());
diff --git a/server/sonar-server/src/test/java/org/sonar/server/metric/ws/CreateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/metric/ws/CreateActionTest.java
index d1daf99d554..960f1c9aa35 100644
--- a/server/sonar-server/src/test/java/org/sonar/server/metric/ws/CreateActionTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/metric/ws/CreateActionTest.java
@@ -32,6 +32,8 @@ import org.sonar.core.metric.db.MetricDto;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.DbSession;
 import org.sonar.core.persistence.DbTester;
+import org.sonar.server.custommeasure.persistence.CustomMeasureDao;
+import org.sonar.server.custommeasure.persistence.CustomMeasureTesting;
 import org.sonar.server.db.DbClient;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.ServerException;
@@ -68,7 +70,7 @@ public class CreateActionTest {
 
   @Before
   public void setUp() {
-    dbClient = new DbClient(db.database(), db.myBatis(), new MetricDao());
+    dbClient = new DbClient(db.database(), db.myBatis(), new MetricDao(), new CustomMeasureDao());
     dbSession = dbClient.openSession(false);
     db.truncateTables();
 
@@ -138,7 +140,7 @@ public class CreateActionTest {
   public void update_existing_metric_when_custom_and_disabled() throws Exception {
     MetricDto metricInDb = MetricTesting.newMetricDto()
       .setKey(DEFAULT_KEY)
-      .setValueType(DEFAULT_TYPE)
+      .setValueType(ValueType.BOOL.name())
       .setUserManaged(true)
       .setEnabled(false);
     dbClient.metricDao().insert(dbSession, metricInDb);
@@ -195,14 +197,15 @@ public class CreateActionTest {
   }
 
   @Test
-  public void fail_when_metric_type_is_changed() throws Exception {
+  public void fail_when_metric_type_is_changed_and_associated_measures_exist() throws Exception {
     expectedException.expect(ServerException.class);
-    dbClient.metricDao().insert(dbSession, MetricTesting.newMetricDto()
+    MetricDto metric = MetricTesting.newMetricDto()
       .setKey(DEFAULT_KEY)
       .setValueType(ValueType.BOOL.name())
       .setUserManaged(true)
-      .setEnabled(false)
-      );
+      .setEnabled(false);
+    dbClient.metricDao().insert(dbSession, metric);
+    dbClient.customMeasureDao().insert(dbSession, CustomMeasureTesting.newDto().setMetricId(metric.getId()));
     dbSession.commit();
 
     newRequest()