SONAR-8973 Validate key, name and description when creating or updating a custom metric

server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDto.java

@@ -25,6 +25,9 @@ import javax.annotation.Nullable;
 import static org.sonar.api.measures.Metric.ValueType.DATA;
 import static org.sonar.api.measures.Metric.ValueType.DISTRIB;
 import static org.sonar.api.measures.Metric.ValueType.STRING;
+import static org.sonar.db.metric.MetricDtoValidator.validateDescription;
+import static org.sonar.db.metric.MetricDtoValidator.validateKey;
+import static org.sonar.db.metric.MetricDtoValidator.validateShortName;
 
 public class MetricDto {
 
@@ -73,8 +76,8 @@ public class MetricDto {
     return kee;
   }
 
-  public MetricDto setKey(String name) {
-    this.kee = name;
+  public MetricDto setKey(String key) {
+    this.kee = validateKey(key);
     return this;
   }
 
@@ -83,7 +86,7 @@ public class MetricDto {
   }
 
   public MetricDto setShortName(String shortName) {
-    this.shortName = shortName;
+    this.shortName = validateShortName(shortName);
     return this;
   }
 
@@ -105,7 +108,7 @@ public class MetricDto {
   }
 
   public MetricDto setDescription(@Nullable String description) {
-    this.description = description;
+    this.description = validateDescription(description);
     return this;
   }
 

server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDtoValidator.java

@@ -0,0 +1,63 @@
+/*
+ * SonarQube
+ * Copyright (C) 2009-2017 SonarSource SA
+ * mailto:info AT sonarsource DOT com
+ *
+ * This program 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.
+ *
+ * This program 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.db.metric;
+
+import javax.annotation.CheckForNull;
+import javax.annotation.Nullable;
+
+import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.base.Strings.isNullOrEmpty;
+
+public class MetricDtoValidator {
+  private static final int MAX_KEY_LENGTH = 64;
+  private static final int MAX_NAME_LENGTH = 64;
+  private static final int MAX_DESCRIPTION_LENGTH = 255;
+
+  private MetricDtoValidator() {
+    // static utility methods only
+  }
+
+  static String validateKey(String key) {
+    checkArgument(!isNullOrEmpty(key), "Metric key cannot be empty");
+    checkArgument(key.length() <= MAX_NAME_LENGTH, "Metric key length (%s) is longer than the maximum authorized (%s). '%s' was provided.",
+      key.length(), MAX_KEY_LENGTH, key);
+    return key;
+  }
+
+  static String validateShortName(String name) {
+    checkArgument(!isNullOrEmpty(name), "Metric name cannot be empty");
+    checkArgument(name.length() <= MAX_NAME_LENGTH, "Metric name length (%s) is longer than the maximum authorized (%s). '%s' was provided.",
+      name.length(), MAX_NAME_LENGTH, name);
+    return name;
+  }
+
+  @CheckForNull
+  static String validateDescription(@Nullable String description) {
+    if (description == null) {
+      return null;
+    }
+
+    checkArgument(description.length() <= MAX_DESCRIPTION_LENGTH, "Metric description length (%s) is longer than the maximum authorized (%s). '%s' was provided.",
+      description.length(), MAX_DESCRIPTION_LENGTH, description);
+
+    return description;
+  }
+}

server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDaoTest.java

@@ -21,8 +21,6 @@ package org.sonar.db.metric;
 
 import java.util.Arrays;
 import java.util.List;
-import org.junit.After;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.ExpectedException;
@@ -35,34 +33,22 @@ import static com.google.common.collect.Sets.newHashSet;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.sonar.db.metric.MetricTesting.newMetricDto;
 
-
 public class MetricDaoTest {
 
   @Rule
-  public DbTester dbTester = DbTester.create(System2.INSTANCE);
+  public DbTester db = DbTester.create(System2.INSTANCE);
   @Rule
-  public ExpectedException thrown = ExpectedException.none();
-
-  DbSession session;
+  public ExpectedException expectedException = ExpectedException.none();
 
-  MetricDao underTest;
-
-  @Before
-  public void createDao() {
-    session = dbTester.myBatis().openSession(false);
-    underTest = new MetricDao();
-  }
+  private DbSession dbSession = db.getSession();
 
-  @After
-  public void tearDown() {
-    session.close();
-  }
+  private MetricDao underTest = new MetricDao();
 
   @Test
   public void select_by_key() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
-    MetricDto result = underTest.selectByKey(session, "coverage");
+    MetricDto result = underTest.selectByKey(dbSession, "coverage");
     assertThat(result.getId()).isEqualTo(2);
     assertThat(result.getKey()).isEqualTo("coverage");
     assertThat(result.getShortName()).isEqualTo("Coverage");
@@ -81,21 +67,23 @@ public class MetricDaoTest {
     assertThat(result.getDecimalScale()).isEqualTo(3);
 
     // Disabled metrics are returned
-    result = underTest.selectByKey(session, "disabled");
+    result = underTest.selectByKey(dbSession, "disabled");
     assertThat(result.getId()).isEqualTo(3);
     assertThat(result.isEnabled()).isFalse();
   }
 
-  @Test(expected = RowNotFoundException.class)
+  @Test
   public void select_or_fail_by_key() {
-    underTest.selectOrFailByKey(session, "unknown");
+    expectedException.expect(RowNotFoundException.class);
+
+    underTest.selectOrFailByKey(dbSession, "unknown");
   }
 
   @Test
   public void get_manual_metric() {
-    dbTester.prepareDbUnit(getClass(), "manual_metric.xml");
+    db.prepareDbUnit(getClass(), "manual_metric.xml");
 
-    MetricDto result = underTest.selectByKey(session, "manual");
+    MetricDto result = underTest.selectByKey(dbSession, "manual");
     assertThat(result.getId()).isEqualTo(1);
     assertThat(result.getKey()).isEqualTo("manual");
     assertThat(result.getShortName()).isEqualTo("Manual metric");
@@ -115,21 +103,21 @@ public class MetricDaoTest {
 
   @Test
   public void find_all_enabled() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
-    assertThat(underTest.selectEnabled(session)).hasSize(2);
+    assertThat(underTest.selectEnabled(dbSession)).hasSize(2);
   }
 
   @Test
   public void find_all() {
-    dbTester.prepareDbUnit(getClass(), "shared.xml");
+    db.prepareDbUnit(getClass(), "shared.xml");
 
-    assertThat(underTest.selectAll(session)).extracting("id").containsExactly(2, 3, 1);
+    assertThat(underTest.selectAll(dbSession)).extracting("id").containsExactly(2, 3, 1);
   }
 
   @Test
   public void insert() {
-    underTest.insert(session, new MetricDto()
+    underTest.insert(dbSession, new MetricDto()
       .setKey("coverage")
       .setShortName("Coverage")
       .setDescription("Coverage by unit tests")
@@ -145,7 +133,7 @@ public class MetricDaoTest {
       .setDeleteHistoricalData(true)
       .setEnabled(true));
 
-    MetricDto result = underTest.selectByKey(session, "coverage");
+    MetricDto result = underTest.selectByKey(dbSession, "coverage");
     assertThat(result.getId()).isNotNull();
     assertThat(result.getKey()).isEqualTo("coverage");
     assertThat(result.getShortName()).isEqualTo("Coverage");
@@ -165,7 +153,7 @@ public class MetricDaoTest {
 
   @Test
   public void insert_metrics() {
-    underTest.insert(session, new MetricDto()
+    underTest.insert(dbSession, new MetricDto()
       .setKey("coverage")
       .setShortName("Coverage")
       .setDescription("Coverage by unit tests")
@@ -195,86 +183,86 @@ public class MetricDaoTest {
         .setHidden(true)
         .setDeleteHistoricalData(true)
         .setEnabled(true));
-    session.commit();
+    dbSession.commit();
 
-    assertThat(dbTester.countRowsOfTable("metrics")).isEqualTo(2);
+    assertThat(db.countRowsOfTable("metrics")).isEqualTo(2);
   }
 
   @Test
   public void selectById() {
-    MetricDto metric = underTest.insert(session, newMetricDto());
+    MetricDto metric = underTest.insert(dbSession, newMetricDto());
 
-    MetricDto result = underTest.selectById(session, metric.getId());
+    MetricDto result = underTest.selectById(dbSession, metric.getId());
 
     assertThat(result).isNotNull();
   }
 
   @Test
   public void selectOrFailById() {
-    MetricDto metric = underTest.insert(session, newMetricDto());
+    MetricDto metric = underTest.insert(dbSession, newMetricDto());
 
-    MetricDto result = underTest.selectOrFailById(session, metric.getId());
+    MetricDto result = underTest.selectOrFailById(dbSession, metric.getId());
 
     assertThat(result).isNotNull();
   }
 
   @Test
   public void fail_when_no_id_selectOrFailById() {
-    thrown.expect(RowNotFoundException.class);
+    expectedException.expect(RowNotFoundException.class);
 
-    underTest.selectOrFailById(session, 42L);
+    underTest.selectOrFailById(dbSession, 42L);
   }
 
   @Test
   public void selectByIds() {
-    MetricDto metric1 = underTest.insert(session, newMetricDto());
-    MetricDto metric2 = underTest.insert(session, newMetricDto());
+    MetricDto metric1 = underTest.insert(dbSession, newMetricDto());
+    MetricDto metric2 = underTest.insert(dbSession, newMetricDto());
 
-    List<MetricDto> result = underTest.selectByIds(session, newHashSet(metric1.getId(), metric2.getId()));
+    List<MetricDto> result = underTest.selectByIds(dbSession, newHashSet(metric1.getId(), metric2.getId()));
 
     assertThat(result).hasSize(2);
   }
 
   @Test
   public void update() {
-    MetricDto metric = underTest.insert(session, newMetricDto().setKey("first-key"));
+    MetricDto metric = underTest.insert(dbSession, newMetricDto().setKey("first-key"));
 
-    underTest.update(session, metric.setKey("second-key"));
+    underTest.update(dbSession, metric.setKey("second-key"));
 
-    MetricDto result = underTest.selectByKey(session, "second-key");
+    MetricDto result = underTest.selectByKey(dbSession, "second-key");
     assertThat(result).isNotNull();
   }
 
   @Test
   public void countEnabled() {
-    underTest.insert(session, newMetricDto().setEnabled(true).setUserManaged(true));
-    underTest.insert(session, newMetricDto().setEnabled(true).setUserManaged(true));
-    underTest.insert(session, newMetricDto().setEnabled(false));
+    underTest.insert(dbSession, newMetricDto().setEnabled(true).setUserManaged(true));
+    underTest.insert(dbSession, newMetricDto().setEnabled(true).setUserManaged(true));
+    underTest.insert(dbSession, newMetricDto().setEnabled(false));
 
-    int result = underTest.countEnabled(session, true);
+    int result = underTest.countEnabled(dbSession, true);
 
     assertThat(result).isEqualTo(2);
   }
 
   @Test
   public void selectDomains() {
-    underTest.insert(session, newMetricDto().setDomain("first-domain").setEnabled(true));
-    underTest.insert(session, newMetricDto().setDomain("second-domain").setEnabled(true));
-    underTest.insert(session, newMetricDto().setDomain("second-domain").setEnabled(true));
-    underTest.insert(session, newMetricDto().setDomain("third-domain").setEnabled(true));
+    underTest.insert(dbSession, newMetricDto().setDomain("first-domain").setEnabled(true));
+    underTest.insert(dbSession, newMetricDto().setDomain("second-domain").setEnabled(true));
+    underTest.insert(dbSession, newMetricDto().setDomain("second-domain").setEnabled(true));
+    underTest.insert(dbSession, newMetricDto().setDomain("third-domain").setEnabled(true));
 
-    List<String> domains = underTest.selectEnabledDomains(session);
+    List<String> domains = underTest.selectEnabledDomains(dbSession);
 
     assertThat(domains).hasSize(3).containsOnly("first-domain", "second-domain", "third-domain");
   }
 
   @Test
   public void selectByKeys() {
-    underTest.insert(session, newMetricDto().setKey("first-key"));
-    underTest.insert(session, newMetricDto().setKey("second-key"));
-    underTest.insert(session, newMetricDto().setKey("third-key"));
+    underTest.insert(dbSession, newMetricDto().setKey("first-key"));
+    underTest.insert(dbSession, newMetricDto().setKey("second-key"));
+    underTest.insert(dbSession, newMetricDto().setKey("third-key"));
 
-    List<MetricDto> result = underTest.selectByKeys(session, Arrays.asList("first-key", "second-key", "third-key"));
+    List<MetricDto> result = underTest.selectByKeys(dbSession, Arrays.asList("first-key", "second-key", "third-key"));
 
     assertThat(result).hasSize(3)
       .extracting("key").containsOnly("first-key", "second-key", "third-key");
@@ -282,36 +270,36 @@ public class MetricDaoTest {
 
   @Test
   public void disableByIds() {
-    MetricDto metric1 = underTest.insert(session, newMetricDto().setEnabled(true).setUserManaged(true));
-    MetricDto metric2 = underTest.insert(session, newMetricDto().setEnabled(true).setUserManaged(true));
+    MetricDto metric1 = underTest.insert(dbSession, newMetricDto().setEnabled(true).setUserManaged(true));
+    MetricDto metric2 = underTest.insert(dbSession, newMetricDto().setEnabled(true).setUserManaged(true));
 
-    underTest.disableCustomByIds(session, Arrays.asList(metric1.getId(), metric2.getId()));
+    underTest.disableCustomByIds(dbSession, Arrays.asList(metric1.getId(), metric2.getId()));
 
-    List<MetricDto> result = underTest.selectByIds(session, newHashSet(metric1.getId(), metric2.getId()));
+    List<MetricDto> result = underTest.selectByIds(dbSession, newHashSet(metric1.getId(), metric2.getId()));
     assertThat(result).hasSize(2);
     assertThat(result).extracting("enabled").containsOnly(false);
   }
 
   @Test
   public void disableByKey() {
-    underTest.insert(session, newMetricDto().setKey("metric-key").setEnabled(true).setUserManaged(true));
+    underTest.insert(dbSession, newMetricDto().setKey("metric-key").setEnabled(true).setUserManaged(true));
 
-    boolean updated = underTest.disableCustomByKey(session, "metric-key");
+    boolean updated = underTest.disableCustomByKey(dbSession, "metric-key");
     assertThat(updated).isTrue();
 
-    MetricDto result = underTest.selectByKey(session, "metric-key");
+    MetricDto result = underTest.selectByKey(dbSession, "metric-key");
     assertThat(result.isEnabled()).isFalse();
 
     // disable again -> zero rows are touched
-    updated = underTest.disableCustomByKey(session, "metric-key");
+    updated = underTest.disableCustomByKey(dbSession, "metric-key");
     assertThat(updated).isFalse();
   }
 
   @Test
   public void selectOrFailByKey() {
-    underTest.insert(session, newMetricDto().setKey("metric-key"));
+    underTest.insert(dbSession, newMetricDto().setKey("metric-key"));
 
-    MetricDto result = underTest.selectOrFailByKey(session, "metric-key");
+    MetricDto result = underTest.selectOrFailByKey(dbSession, "metric-key");
 
     assertThat(result).isNotNull();
     assertThat(result.getKey()).isEqualTo("metric-key");
@@ -319,27 +307,26 @@ public class MetricDaoTest {
 
   @Test
   public void selectEnabled_with_paging_and_custom() {
-    underTest.insert(session, newMetricDto().setUserManaged(true).setEnabled(true));
-    underTest.insert(session, newMetricDto().setUserManaged(true).setEnabled(true));
-    underTest.insert(session, newMetricDto().setUserManaged(true).setEnabled(true));
-    underTest.insert(session, newMetricDto().setUserManaged(false).setEnabled(true));
-    underTest.insert(session, newMetricDto().setUserManaged(true).setEnabled(false));
+    underTest.insert(dbSession, newMetricDto().setUserManaged(true).setEnabled(true));
+    underTest.insert(dbSession, newMetricDto().setUserManaged(true).setEnabled(true));
+    underTest.insert(dbSession, newMetricDto().setUserManaged(true).setEnabled(true));
+    underTest.insert(dbSession, newMetricDto().setUserManaged(false).setEnabled(true));
+    underTest.insert(dbSession, newMetricDto().setUserManaged(true).setEnabled(false));
 
-    List<MetricDto> result = underTest.selectEnabled(session, true, 0, 100);
+    List<MetricDto> result = underTest.selectEnabled(dbSession, true, 0, 100);
 
     assertThat(result).hasSize(3);
   }
 
   @Test
   public void selectAvailableByComponentUuid() {
-    underTest.insert(session, newMetricDto().setUserManaged(true).setEnabled(true).setKey("metric-key"));
-    underTest.insert(session, newMetricDto().setUserManaged(false).setEnabled(true).setKey("another-metric-key"));
-    underTest.insert(session, newMetricDto().setUserManaged(true).setEnabled(false).setKey("third-metric-key"));
+    underTest.insert(dbSession, newMetricDto().setUserManaged(true).setEnabled(true).setKey("metric-key"));
+    underTest.insert(dbSession, newMetricDto().setUserManaged(false).setEnabled(true).setKey("another-metric-key"));
+    underTest.insert(dbSession, newMetricDto().setUserManaged(true).setEnabled(false).setKey("third-metric-key"));
 
-    List<MetricDto> result = underTest.selectAvailableCustomMetricsByComponentUuid(session, "project-uuid");
+    List<MetricDto> result = underTest.selectAvailableCustomMetricsByComponentUuid(dbSession, "project-uuid");
 
     assertThat(result).hasSize(1)
       .extracting("key").containsOnly("metric-key");
-
   }
 }

server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDtoTest.java

@@ -19,14 +19,21 @@
  */
 package org.sonar.db.metric;
 
+import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.ExpectedException;
 
+import static com.google.common.base.Strings.repeat;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.sonar.api.measures.Metric.ValueType.DATA;
 import static org.sonar.api.measures.Metric.ValueType.INT;
 import static org.sonar.api.measures.Metric.ValueType.STRING;
 
 public class MetricDtoTest {
+  @Rule
+  public ExpectedException expectedException = ExpectedException.none();
+
+  private MetricDto underTest = new MetricDto();
 
   @Test
   public void getters_and_setters() {
@@ -72,4 +79,34 @@ public class MetricDtoTest {
     assertThat(MetricTesting.newMetricDto().setValueType(STRING.name()).isDataType()).isTrue();
     assertThat(MetricTesting.newMetricDto().setValueType(STRING.name()).isDataType()).isTrue();
   }
+
+  @Test
+  public void fail_if_key_longer_than_64_characters() {
+    String a65 = repeat("a", 65);
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Metric key length (65) is longer than the maximum authorized (64). '" + a65 + "' was provided.");
+
+    underTest.setKey(a65);
+  }
+
+  @Test
+  public void fail_if_name_longer_than_64_characters() {
+    String a65 = repeat("a", 65);
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Metric name length (65) is longer than the maximum authorized (64). '" + a65 + "' was provided.");
+
+    underTest.setShortName(a65);
+  }
+
+  @Test
+  public void fail_if_description_longer_than_255_characters() {
+    String a256 = repeat("a", 256);
+
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Metric description length (256) is longer than the maximum authorized (255). '" + a256 + "' was provided.");
+
+    underTest.setDescription(a256);
+  }
 }

server/sonar-server/src/main/java/org/sonar/server/util/MetricKeyValidator.java

@@ -20,7 +20,6 @@
 package org.sonar.server.util;
 
 public class MetricKeyValidator {
-
   /*
    * Allowed characters are alphanumeric, '-', '_', with at least one non-digit
    */

server/sonar-server/src/test/java/org/sonar/server/metric/DefaultMetricFinderTest.java

@@ -20,52 +20,61 @@
 package org.sonar.server.metric;
 
 import java.util.Arrays;
-import org.junit.Before;
 import org.junit.Rule;
 import org.junit.Test;
+import org.sonar.api.measures.Metric;
 import org.sonar.api.utils.System2;
-import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
-import org.sonar.db.TestDBSessions;
-import org.sonar.db.metric.MetricDao;
+import org.sonar.db.metric.MetricDto;
 
-import static org.hamcrest.core.Is.is;
-import static org.hamcrest.core.IsNull.nullValue;
-import static org.junit.Assert.assertThat;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.sonar.db.metric.MetricTesting.newMetricDto;
 
 
 public class DefaultMetricFinderTest {
 
   @Rule
-  public DbTester dbTester = DbTester.create(System2.INSTANCE);
+  public DbTester db = DbTester.create(System2.INSTANCE);
 
-  DefaultMetricFinder finder;
-
-  @Before
-  public void setUp() {
-    dbTester.prepareDbUnit(DefaultMetricFinderTest.class, "shared.xml");
-    finder = new DefaultMetricFinder(new DbClient(dbTester.database(), dbTester.myBatis(), new TestDBSessions(dbTester.myBatis()), new MetricDao()));
-  }
+  private DefaultMetricFinder underTest = new DefaultMetricFinder(db.getDbClient());
 
   @Test
-  public void shouldFindAll() {
-    assertThat(finder.findAll().size(), is(2));
+  public void findAll_enabled() {
+    db.getDbClient().metricDao().insert(db.getSession(), newMetricDto());
+    db.getDbClient().metricDao().insert(db.getSession(), newMetricDto());
+    db.getDbClient().metricDao().insert(db.getSession(), newMetricDto().setEnabled(false));
+    db.commit();
+
+    assertThat(underTest.findAll()).hasSize(2);
   }
 
   @Test
-  public void shouldFindByKeys() {
-    assertThat(finder.findAll(Arrays.asList("ncloc", "foo", "coverage")).size(), is(2));
+  public void findAll_by_keys() {
+    db.getDbClient().metricDao().insert(db.getSession(), newMetricDto().setKey("ncloc"));
+    db.getDbClient().metricDao().insert(db.getSession(), newMetricDto().setKey("foo"));
+    db.getDbClient().metricDao().insert(db.getSession(), newMetricDto().setKey("coverage"));
+    db.commit();
+
+    assertThat(underTest.findAll(Arrays.asList("ncloc", "foo"))).extracting(Metric::getKey).containsExactlyInAnyOrder("ncloc", "foo")
+      .doesNotContain("coverage");
+
   }
 
   @Test
-  public void shouldFindById() {
-    assertThat(finder.findById(1).getKey(), is("ncloc"));
-    assertThat(finder.findById(3), nullValue());
+  public void findById() {
+    MetricDto firstMetric = db.getDbClient().metricDao().insert(db.getSession(), newMetricDto());
+    MetricDto secondMetric = db.getDbClient().metricDao().insert(db.getSession(), newMetricDto());
+    db.commit();
+
+    assertThat(underTest.findById(firstMetric.getId())).extracting(Metric::getKey).containsExactly(firstMetric.getKey());
   }
 
   @Test
-  public void shouldFindByKey() {
-    assertThat(finder.findByKey("ncloc").getKey(), is("ncloc"));
-    assertThat(finder.findByKey("disabled"), nullValue());
+  public void findByKey() {
+    MetricDto firstMetric = db.getDbClient().metricDao().insert(db.getSession(), newMetricDto());
+    MetricDto secondMetric = db.getDbClient().metricDao().insert(db.getSession(), newMetricDto());
+    db.commit();
+
+    assertThat(underTest.findByKey(secondMetric.getKey())).extracting(Metric::getKey).containsExactly(secondMetric.getKey());
   }
 }

server/sonar-server/src/test/java/org/sonar/server/metric/ws/CreateActionTest.java

@@ -258,6 +258,7 @@ public class CreateActionTest {
   @Test
   public void fail_when_ill_formatted_key() throws Exception {
     expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Malformed metric key '123:456'. Allowed characters are alphanumeric, '-', '_', with at least one non-digit.");
 
     newRequest()
       .setParam(PARAM_KEY, "123:456")

server/sonar-server/src/test/java/org/sonar/server/metric/ws/UpdateActionTest.java

@@ -203,12 +203,13 @@ public class UpdateActionTest {
 
   @Test
   public void fail_when_metric_key_is_not_well_formatted() throws Exception {
-    expectedException.expect(IllegalArgumentException.class);
-
     int id = insertMetric(newDefaultMetric());
     dbClient.customMeasureDao().insert(dbSession, newCustomMeasureDto().setMetricId(id));
     dbSession.commit();
 
+    expectedException.expect(IllegalArgumentException.class);
+    expectedException.expectMessage("Malformed metric key 'not well formatted key'. Allowed characters are alphanumeric, '-', '_', with at least one non-digit.");
+
     newRequest()
       .setParam(PARAM_ID, String.valueOf(id))
       .setParam(PARAM_KEY, "not well formatted key")

server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java

@@ -318,7 +318,7 @@ public class SetActionTest {
   @Test
   public void persist_multi_value_with_type_metric() {
     definitions.addComponent(PropertyDefinition
-      .builder("my.key")
+      .builder("my_key")
       .name("foo")
       .description("desc")
       .category("cat")
@@ -327,13 +327,13 @@ public class SetActionTest {
       .defaultValue("default")
       .multiValues(true)
       .build());
-    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("metric.key.1"));
-    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("metric.key.2"));
+    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("metric_key_1"));
+    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("metric_key_2"));
     dbSession.commit();
 
-    callForMultiValueGlobalSetting("my.key", newArrayList("metric.key.1", "metric.key.2"));
+    callForMultiValueGlobalSetting("my_key", newArrayList("metric_key_1", "metric_key_2"));
 
-    assertGlobalSetting("my.key", "metric.key.1,metric.key.2");
+    assertGlobalSetting("my_key", "metric_key_1,metric_key_2");
   }
 
   @Test
@@ -464,7 +464,7 @@ public class SetActionTest {
   @Test
   public void fail_when_data_and_metric_type_with_invalid_key() {
     definitions.addComponent(PropertyDefinition
-      .builder("my.key")
+      .builder("my_key")
       .name("foo")
       .description("desc")
       .category("cat")
@@ -473,14 +473,14 @@ public class SetActionTest {
       .defaultValue("default")
       .multiValues(true)
       .build());
-    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("metric.key"));
-    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("metric.disabled.key").setEnabled(false));
+    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("metric_key"));
+    dbClient.metricDao().insert(dbSession, newMetricDto().setKey("metric_disabled_key").setEnabled(false));
     dbSession.commit();
 
     expectedException.expect(BadRequestException.class);
-    expectedException.expectMessage("Error when validating metric setting with key 'my.key' and values [metric.key, metric.disabled.key]. A value is not a valid metric key.");
+    expectedException.expectMessage("Error when validating metric setting with key 'my_key' and values [metric_key, metric_disabled_key]. A value is not a valid metric key.");
 
-    callForMultiValueGlobalSetting("my.key", newArrayList("metric.key", "metric.disabled.key"));
+    callForMultiValueGlobalSetting("my_key", newArrayList("metric_key", "metric_disabled_key"));
   }
 
   @Test