From 899dcc0578adfb5c6a7cd2285b6c4090b618c228 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Thu, 4 Jun 2015 09:50:56 +0200 Subject: [PATCH] SONAR-6572 ws metrics/update update an existing custom metric --- .../server/metric/persistence/MetricDao.java | 4 + .../sonar/server/metric/ws/CreateAction.java | 2 +- .../server/metric/ws/MetricsWsModule.java | 1 + .../sonar/server/metric/ws/UpdateAction.java | 215 ++++++++++++++++++ .../server/metric/ws/MetricsWsModuleTest.java | 2 +- .../server/metric/ws/UpdateActionTest.java | 214 +++++++++++++++++ .../metric/ws/UpdateActionTest/metric.json | 7 + .../sonar/core/metric/db/MetricMapper.java | 2 + .../org/sonar/core/metric/db/MetricMapper.xml | 12 +- 9 files changed, 456 insertions(+), 3 deletions(-) create mode 100644 server/sonar-server/src/main/java/org/sonar/server/metric/ws/UpdateAction.java create mode 100644 server/sonar-server/src/test/java/org/sonar/server/metric/ws/UpdateActionTest.java create mode 100644 server/sonar-server/src/test/resources/org/sonar/server/metric/ws/UpdateActionTest/metric.json diff --git a/server/sonar-server/src/main/java/org/sonar/server/metric/persistence/MetricDao.java b/server/sonar-server/src/main/java/org/sonar/server/metric/persistence/MetricDao.java index f25def734f5..63611986243 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/metric/persistence/MetricDao.java +++ b/server/sonar-server/src/main/java/org/sonar/server/metric/persistence/MetricDao.java @@ -113,4 +113,8 @@ public class MetricDao implements DaoComponent { public void update(DbSession session, MetricDto metric) { mapper(session).update(metric); } + + public MetricDto selectNullableById(DbSession session, int id) { + return mapper(session).selectById(id); + } } 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 2504bdca3ae..e9b253ef47e 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 @@ -217,7 +217,7 @@ public class CreateAction implements MetricsWsAction { private static void writeMetric(JsonWriter json, MetricDto metric) { json.beginObject(); - json.prop(FIELD_ID, String.valueOf(metric.getId())); + json.prop(FIELD_ID, metric.getId().toString()); json.prop(FIELD_KEY, metric.getKey()); json.prop(FIELD_NAME, metric.getShortName()); json.prop(FIELD_TYPE, metric.getValueType()); diff --git a/server/sonar-server/src/main/java/org/sonar/server/metric/ws/MetricsWsModule.java b/server/sonar-server/src/main/java/org/sonar/server/metric/ws/MetricsWsModule.java index 4ac2423a008..17a5056a597 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/metric/ws/MetricsWsModule.java +++ b/server/sonar-server/src/main/java/org/sonar/server/metric/ws/MetricsWsModule.java @@ -31,6 +31,7 @@ public class MetricsWsModule extends Module { DeleteAction.class, DomainsAction.class, SearchAction.class, + UpdateAction.class, TypesAction.class); } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/metric/ws/UpdateAction.java b/server/sonar-server/src/main/java/org/sonar/server/metric/ws/UpdateAction.java new file mode 100644 index 00000000000..4a06fac6138 --- /dev/null +++ b/server/sonar-server/src/main/java/org/sonar/server/metric/ws/UpdateAction.java @@ -0,0 +1,215 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +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; +import org.sonar.core.persistence.MyBatis; +import org.sonar.server.db.DbClient; +import org.sonar.server.exceptions.ServerException; +import org.sonar.server.user.UserSession; + +public class UpdateAction implements MetricsWsAction { + private static final String ACTION = "update"; + + public static final String PARAM_ID = "id"; + public static final String PARAM_NAME = "name"; + public static final String PARAM_KEY = "key"; + public static final String PARAM_TYPE = "type"; + public static final String PARAM_DESCRIPTION = "description"; + public static final String PARAM_DOMAIN = "domain"; + + private static final String FIELD_ID = PARAM_ID; + private static final String FIELD_NAME = PARAM_NAME; + private static final String FIELD_KEY = PARAM_KEY; + private static final String FIELD_TYPE = PARAM_TYPE; + private static final String FIELD_DESCRIPTION = PARAM_DESCRIPTION; + private static final String FIELD_DOMAIN = PARAM_DOMAIN; + + private final DbClient dbClient; + private final UserSession userSession; + + public UpdateAction(DbClient dbClient, UserSession userSession) { + this.dbClient = dbClient; + this.userSession = userSession; + } + + @Override + public void define(WebService.NewController context) { + WebService.NewAction action = context.createAction(ACTION) + .setPost(true) + .setDescription("Update a custom metric.
Requires 'Administer System' permission.") + .setSince("5.2") + .setHandler(this); + + action.createParam(PARAM_ID) + .setRequired(true) + .setDescription("Id of the custom metric to update") + .setExampleValue("42"); + + action.createParam(PARAM_NAME) + .setDescription("Name") + .setExampleValue("Team Size"); + + action.createParam(PARAM_KEY) + .setDescription("Key") + .setExampleValue("team_size"); + + action.createParam(PARAM_TYPE) + .setDescription("Type") + .setPossibleValues(Metric.ValueType.names()); + + action.createParam(PARAM_DESCRIPTION) + .setDescription("Description") + .setExampleValue("Size of the team"); + + action.createParam(PARAM_DOMAIN) + .setDescription("Domain") + .setExampleValue("Tests"); + } + + @Override + public void handle(Request request, Response response) throws Exception { + userSession.checkLoggedIn().checkGlobalPermission(GlobalPermissions.SYSTEM_ADMIN); + int id = request.mandatoryParamAsInt(PARAM_ID); + + DbSession dbSession = dbClient.openSession(false); + try { + MetricDto metricTemplate = newMetricTemplate(request); + MetricDto metricInDb = dbClient.metricDao().selectNullableById(dbSession, id); + checkMetricInDbAndTemplate(dbSession, metricInDb, metricTemplate); + + updateMetricInDb(dbSession, metricInDb, metricTemplate); + JsonWriter json = response.newJsonWriter(); + writeMetric(json, metricInDb); + json.close(); + } finally { + MyBatis.closeQuietly(dbSession); + } + } + + private static MetricDto newMetricTemplate(Request request) { + int id = request.mandatoryParamAsInt(PARAM_ID); + String key = request.param(PARAM_KEY); + String type = request.param(PARAM_TYPE); + String name = request.param(PARAM_NAME); + String domain = request.param(PARAM_DOMAIN); + String description = request.param(PARAM_DESCRIPTION); + + MetricDto metricTemplate = new MetricDto().setId(id); + if (key != null) { + metricTemplate.setKey(key); + } + if (type != null) { + metricTemplate.setValueType(type); + } + if (name != null) { + metricTemplate.setShortName(name); + } + if (domain != null) { + metricTemplate.setDomain(domain); + } + if (description != null) { + metricTemplate.setDescription(description); + } + return metricTemplate; + } + + private void updateMetricInDb(DbSession dbSession, MetricDto metricInDb, MetricDto metricTemplate) { + String key = metricTemplate.getKey(); + String name = metricTemplate.getShortName(); + String type = metricTemplate.getValueType(); + String domain = metricTemplate.getDomain(); + String description = metricTemplate.getDescription(); + if (key != null) { + metricInDb.setKey(key); + } + if (name != null) { + metricInDb.setShortName(name); + } + if (type != null) { + metricInDb.setValueType(type); + } + if (domain != null) { + metricInDb.setDomain(domain); + } + if (description != null) { + metricInDb.setDescription(description); + } + dbClient.metricDao().update(dbSession, metricInDb); + dbSession.commit(); + } + + private void checkMetricInDbAndTemplate(DbSession dbSession, @Nullable MetricDto metricInDb, MetricDto template) { + if (isMetricFoundInDb(metricInDb) || isMetricDisabled(metricInDb) || isMetricNonCustom(metricInDb)) { + throw new ServerException(HttpURLConnection.HTTP_CONFLICT, String.format("No active custom metric has been found for id '%d'. Maybe you want to create a metric ?", + template.getId())); + } + if (haveMetricTypeChanged(metricInDb, template)) { + List customMeasures = dbClient.customMeasureDao().selectByMetricId(dbSession, metricInDb.getId()); + if (haveAssociatedCustomMeasures(customMeasures)) { + throw new ServerException(HttpURLConnection.HTTP_CONFLICT, String.format("You're trying to change the type '%s' while there are associated custom measures.", + metricInDb.getValueType())); + } + } + } + + private static void writeMetric(JsonWriter json, MetricDto metric) { + json.beginObject(); + json.prop(FIELD_ID, String.valueOf(metric.getId())); + json.prop(FIELD_KEY, metric.getKey()); + json.prop(FIELD_TYPE, metric.getValueType()); + json.prop(FIELD_NAME, metric.getShortName()); + json.prop(FIELD_DOMAIN, metric.getDomain()); + json.prop(FIELD_DESCRIPTION, metric.getDescription()); + json.endObject(); + } + + private static boolean isMetricNonCustom(MetricDto metricInDb) { + return !metricInDb.isUserManaged(); + } + + private static boolean isMetricDisabled(MetricDto metricInDb) { + return !metricInDb.isEnabled(); + } + + private static boolean isMetricFoundInDb(@Nullable MetricDto metricInDb) { + return metricInDb == null; + } + + private static boolean haveAssociatedCustomMeasures(List customMeasures) { + return !customMeasures.isEmpty(); + } + + private static boolean haveMetricTypeChanged(MetricDto metricInDb, MetricDto template) { + return !metricInDb.getValueType().equals(template.getValueType()) && template.getValueType() != null; + } +} diff --git a/server/sonar-server/src/test/java/org/sonar/server/metric/ws/MetricsWsModuleTest.java b/server/sonar-server/src/test/java/org/sonar/server/metric/ws/MetricsWsModuleTest.java index fba94669370..8714df5608d 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/metric/ws/MetricsWsModuleTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/metric/ws/MetricsWsModuleTest.java @@ -30,6 +30,6 @@ public class MetricsWsModuleTest { public void verify_count_of_added_components() { ComponentContainer container = new ComponentContainer(); new MetricsWsModule().configure(container); - assertThat(container.size()).isEqualTo(8); + assertThat(container.size()).isEqualTo(9); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/metric/ws/UpdateActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/metric/ws/UpdateActionTest.java new file mode 100644 index 00000000000..1c1a47238e8 --- /dev/null +++ b/server/sonar-server/src/test/java/org/sonar/server/metric/ws/UpdateActionTest.java @@ -0,0 +1,214 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2014 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.metric.ws; + +import org.junit.After; +import org.junit.Before; +import org.junit.ClassRule; +import org.junit.Rule; +import org.junit.Test; +import org.junit.experimental.categories.Category; +import org.junit.rules.ExpectedException; +import org.sonar.api.measures.Metric.ValueType; +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.db.DbClient; +import org.sonar.server.exceptions.ForbiddenException; +import org.sonar.server.exceptions.ServerException; +import org.sonar.server.metric.persistence.MetricDao; +import org.sonar.server.tester.UserSessionRule; +import org.sonar.server.ws.WsTester; +import org.sonar.test.DbTests; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.sonar.server.custommeasure.persistence.CustomMeasureTesting.newCustomMeasureDto; +import static org.sonar.server.metric.ws.UpdateAction.PARAM_DESCRIPTION; +import static org.sonar.server.metric.ws.UpdateAction.PARAM_DOMAIN; +import static org.sonar.server.metric.ws.UpdateAction.PARAM_ID; +import static org.sonar.server.metric.ws.UpdateAction.PARAM_KEY; +import static org.sonar.server.metric.ws.UpdateAction.PARAM_NAME; +import static org.sonar.server.metric.ws.UpdateAction.PARAM_TYPE; + +@Category(DbTests.class) +public class UpdateActionTest { + + private static final String DEFAULT_KEY = "custom-metric-key"; + private static final String DEFAULT_NAME = "custom-metric-name"; + private static final String DEFAULT_DOMAIN = "custom-metric-domain"; + private static final String DEFAULT_DESCRIPTION = "custom-metric-description"; + private static final String DEFAULT_TYPE = ValueType.INT.name(); + + @ClassRule + public static DbTester db = new DbTester(); + @Rule + public ExpectedException expectedException = ExpectedException.none(); + @Rule + public UserSessionRule userSessionRule = UserSessionRule.standalone(); + WsTester ws; + DbClient dbClient; + DbSession dbSession; + + @Before + public void setUp() { + dbClient = new DbClient(db.database(), db.myBatis(), new MetricDao(), new CustomMeasureDao()); + dbSession = dbClient.openSession(false); + db.truncateTables(); + + ws = new WsTester(new MetricsWs(new UpdateAction(dbClient, userSessionRule))); + userSessionRule.login("login").setGlobalPermissions(GlobalPermissions.SYSTEM_ADMIN); + } + + @After + public void tearDown() { + dbSession.close(); + } + + @Test + public void update_all_fields() throws Exception { + int id = insertMetric(newDefaultMetric()); + + newRequest() + .setParam(PARAM_ID, String.valueOf(id)) + .setParam(PARAM_KEY, "another-key") + .setParam(PARAM_NAME, "another-name") + .setParam(PARAM_TYPE, ValueType.BOOL.name()) + .setParam(PARAM_DOMAIN, "another-domain") + .setParam(PARAM_DESCRIPTION, "another-description") + .execute(); + dbSession.commit(); + + MetricDto result = dbClient.metricDao().selectNullableById(dbSession, id); + assertThat(result.getKey()).isEqualTo("another-key"); + assertThat(result.getShortName()).isEqualTo("another-name"); + assertThat(result.getValueType()).isEqualTo(ValueType.BOOL.name()); + assertThat(result.getDomain()).isEqualTo("another-domain"); + assertThat(result.getDescription()).isEqualTo("another-description"); + } + + @Test + public void update_one_field() throws Exception { + int id = insertMetric(newDefaultMetric()); + dbClient.customMeasureDao().insert(dbSession, newCustomMeasureDto().setMetricId(id)); + dbSession.commit(); + + newRequest() + .setParam(PARAM_ID, String.valueOf(id)) + .setParam(PARAM_DESCRIPTION, "another-description") + .execute(); + dbSession.commit(); + + MetricDto result = dbClient.metricDao().selectNullableById(dbSession, id); + assertThat(result.getKey()).isEqualTo(DEFAULT_KEY); + assertThat(result.getShortName()).isEqualTo(DEFAULT_NAME); + assertThat(result.getValueType()).isEqualTo(DEFAULT_TYPE); + assertThat(result.getDomain()).isEqualTo(DEFAULT_DOMAIN); + assertThat(result.getDescription()).isEqualTo("another-description"); + } + + @Test + public void update_return_the_full_object_with_id() throws Exception { + int id = insertMetric(newDefaultMetric().setDescription("another-description")); + + WsTester.Result requestResult = newRequest() + .setParam(PARAM_ID, String.valueOf(id)) + .setParam(PARAM_DESCRIPTION, DEFAULT_DESCRIPTION) + .execute(); + dbSession.commit(); + + requestResult.assertJson(getClass(), "metric.json"); + assertThat(requestResult.outputAsString()).matches(".*\"id\"\\s*:\\s*\"" + id + "\".*"); + } + + @Test + public void fail_when_metric_not_in_db() throws Exception { + expectedException.expect(ServerException.class); + + newRequest().setParam(PARAM_ID, "42").execute(); + } + + @Test + public void fail_when_metric_is_deactivated() throws Exception { + expectedException.expect(ServerException.class); + int id = insertMetric(newDefaultMetric().setEnabled(false)); + + newRequest().setParam(PARAM_ID, String.valueOf(id)).execute(); + } + + @Test + public void fail_when_metric_is_not_custom() throws Exception { + expectedException.expect(ServerException.class); + int id = insertMetric(newDefaultMetric().setUserManaged(false)); + + newRequest().setParam(PARAM_ID, String.valueOf(id)).execute(); + } + + @Test + public void fail_when_custom_measures_and_type_changed() throws Exception { + expectedException.expect(ServerException.class); + int id = insertMetric(newDefaultMetric()); + dbClient.customMeasureDao().insert(dbSession, newCustomMeasureDto().setMetricId(id)); + dbSession.commit(); + + newRequest() + .setParam(PARAM_ID, String.valueOf(id)) + .setParam(PARAM_TYPE, ValueType.BOOL.name()) + .execute(); + } + + @Test + public void fail_when_no_id() throws Exception { + expectedException.expect(IllegalArgumentException.class); + + newRequest().execute(); + } + + @Test + public void fail_when_insufficient_privileges() throws Exception { + expectedException.expect(ForbiddenException.class); + userSessionRule.login("login"); + + newRequest().execute(); + } + + private MetricDto newDefaultMetric() { + return new MetricDto() + .setKey(DEFAULT_KEY) + .setShortName(DEFAULT_NAME) + .setValueType(DEFAULT_TYPE) + .setDomain(DEFAULT_DOMAIN) + .setDescription(DEFAULT_DESCRIPTION) + .setUserManaged(true) + .setEnabled(true); + } + + private int insertMetric(MetricDto metricDto) { + dbClient.metricDao().insert(dbSession, metricDto); + dbSession.commit(); + return metricDto.getId(); + } + + private WsTester.TestRequest newRequest() { + return ws.newPostRequest("api/metrics", "update"); + } +} diff --git a/server/sonar-server/src/test/resources/org/sonar/server/metric/ws/UpdateActionTest/metric.json b/server/sonar-server/src/test/resources/org/sonar/server/metric/ws/UpdateActionTest/metric.json new file mode 100644 index 00000000000..ac8905bea70 --- /dev/null +++ b/server/sonar-server/src/test/resources/org/sonar/server/metric/ws/UpdateActionTest/metric.json @@ -0,0 +1,7 @@ +{ + "key": "custom-metric-key", + "name": "custom-metric-name", + "type": "INT", + "domain": "custom-metric-domain", + "description": "custom-metric-description" +} diff --git a/sonar-core/src/main/java/org/sonar/core/metric/db/MetricMapper.java b/sonar-core/src/main/java/org/sonar/core/metric/db/MetricMapper.java index 360c5c40ded..abf450248e8 100644 --- a/sonar-core/src/main/java/org/sonar/core/metric/db/MetricMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/metric/db/MetricMapper.java @@ -44,4 +44,6 @@ public interface MetricMapper { int countCustom(); void update(MetricDto metric); + + MetricDto selectById(int id); } diff --git a/sonar-core/src/main/resources/org/sonar/core/metric/db/MetricMapper.xml b/sonar-core/src/main/resources/org/sonar/core/metric/db/MetricMapper.xml index 38f07455cf6..56f4ec6c2a5 100644 --- a/sonar-core/src/main/resources/org/sonar/core/metric/db/MetricMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/metric/db/MetricMapper.xml @@ -73,9 +73,10 @@ update metrics set - enabled=#{enabled, jdbcType=BOOLEAN}, + name=#{key, jdbcType=VARCHAR}, short_name=#{shortName, jdbcType=VARCHAR}, val_type=#{valueType, jdbcType=VARCHAR}, + enabled=#{enabled, jdbcType=BOOLEAN}, domain=#{domain, jdbcType=VARCHAR}, description=#{description, jdbcType=VARCHAR} where id=#{id} @@ -111,4 +112,13 @@ + + -- 2.39.5