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

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 e8bcdcbdaea9b3db69f0b6ff5e362e22b5d9366e..c419b2e600fe5416ae9a0f701fbf9c0c88de9395 100644 (file)
--- 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 09c73b50494cd08af0a2f886a1aa17673fcf7409..9c1e504685fe066ec103d964688f542286128b7f 100644 (file)
--- 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 d1daf99d5549de50d366269cd3f37c28b74ad736..960f1c9aa35f3672e6c663cdc3fbb7e98b0c8cd8 100644 (file)
--- 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()

diff --git a/sonar-core/src/main/java/org/sonar/core/custommeasure/db/CustomMeasureMapper.java b/sonar-core/src/main/java/org/sonar/core/custommeasure/db/CustomMeasureMapper.java
index 95bb4fd3b2350d0a4abfb8e279616ef761c7e7ef..07a87ff2d0c0705aec46c1449b0bf9ce3b42440c 100644 (file)
--- a/sonar-core/src/main/java/org/sonar/core/custommeasure/db/CustomMeasureMapper.java
+++ b/sonar-core/src/main/java/org/sonar/core/custommeasure/db/CustomMeasureMapper.java
@@ -29,4 +29,6 @@ public interface CustomMeasureMapper {
   void deleteByMetricIds(@Param("metricIds") List<Integer> metricIds);
 
   CustomMeasureDto selectById(long id);
+
+  List<CustomMeasureDto> selectByMetricId(int id);
 }

diff --git a/sonar-core/src/main/resources/org/sonar/core/custommeasure/db/CustomMeasureMapper.xml b/sonar-core/src/main/resources/org/sonar/core/custommeasure/db/CustomMeasureMapper.xml
index 4aefce1188d3c47d0e93b1175e8798d75e2fa655..e6e37419cb151ef730a2eb5a87c2be97a64bacd8 100644 (file)
--- a/sonar-core/src/main/resources/org/sonar/core/custommeasure/db/CustomMeasureMapper.xml
+++ b/sonar-core/src/main/resources/org/sonar/core/custommeasure/db/CustomMeasureMapper.xml
@@ -21,6 +21,13 @@
     where m.id=#{id}
   </select>
 
+  <select id="selectByMetricId" resultType="CustomMeasure">
+    select
+    <include refid="selectColumns"/>
+    from manual_measures m
+    where m.metric_id=#{metricId}
+  </select>
+
   <insert id="insert" parameterType="CustomMeasure" useGeneratedKeys="true" keyColumn="id" keyProperty="id">
     INSERT INTO manual_measures (
     metric_id, resource_id, value, text_value, user_login, description, created_at, updated_at