From 422912b59e4ffd99d05f45eae4b9289f2e7f6646 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Fri, 30 Jun 2017 17:42:45 +0200 Subject: [PATCH] SONAR-8973 Validate key, name and description when creating or updating a custom metric --- .../java/org/sonar/db/metric/MetricDto.java | 11 +- .../sonar/db/metric/MetricDtoValidator.java | 63 ++++++++ .../org/sonar/db/metric/MetricDaoTest.java | 147 ++++++++---------- .../org/sonar/db/metric/MetricDtoTest.java | 37 +++++ .../sonar/server/util/MetricKeyValidator.java | 1 - .../metric/DefaultMetricFinderTest.java | 59 ++++--- .../server/metric/ws/CreateActionTest.java | 1 + .../server/metric/ws/UpdateActionTest.java | 5 +- .../server/setting/ws/SetActionTest.java | 20 +-- 9 files changed, 222 insertions(+), 122 deletions(-) create mode 100644 server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDtoValidator.java diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDto.java index 04f45f93cbf..7a62894bbb2 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDto.java +++ b/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; } diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDtoValidator.java b/server/sonar-db-dao/src/main/java/org/sonar/db/metric/MetricDtoValidator.java new file mode 100644 index 00000000000..96069dd6a07 --- /dev/null +++ b/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; + } +} diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDaoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDaoTest.java index 9c9a7c8ba66..0cb9b8f97b1 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDaoTest.java +++ b/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 result = underTest.selectByIds(session, newHashSet(metric1.getId(), metric2.getId())); + List 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 domains = underTest.selectEnabledDomains(session); + List 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 result = underTest.selectByKeys(session, Arrays.asList("first-key", "second-key", "third-key")); + List 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 result = underTest.selectByIds(session, newHashSet(metric1.getId(), metric2.getId())); + List 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 result = underTest.selectEnabled(session, true, 0, 100); + List 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 result = underTest.selectAvailableCustomMetricsByComponentUuid(session, "project-uuid"); + List result = underTest.selectAvailableCustomMetricsByComponentUuid(dbSession, "project-uuid"); assertThat(result).hasSize(1) .extracting("key").containsOnly("metric-key"); - } } diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDtoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDtoTest.java index 99a6bc5e518..41a2cb454cd 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/metric/MetricDtoTest.java +++ b/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); + } } diff --git a/server/sonar-server/src/main/java/org/sonar/server/util/MetricKeyValidator.java b/server/sonar-server/src/main/java/org/sonar/server/util/MetricKeyValidator.java index 2632867e6ff..6a16e60cd06 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/util/MetricKeyValidator.java +++ b/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 */ diff --git a/server/sonar-server/src/test/java/org/sonar/server/metric/DefaultMetricFinderTest.java b/server/sonar-server/src/test/java/org/sonar/server/metric/DefaultMetricFinderTest.java index 504249c23cf..3ceea1d6e93 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/metric/DefaultMetricFinderTest.java +++ b/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()); } } 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 abf404de18d..cda1a84aa88 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 @@ -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") 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 index defa61c563f..5e0ed44b905 100644 --- 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 @@ -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") diff --git a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java b/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java index 0463e100451..97e03d13c64 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/setting/ws/SetActionTest.java +++ b/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 -- 2.39.5