From 4798c9728f3cf8cba89b1298f583101ceafadaa2 Mon Sep 17 00:00:00 2001 From: Lukasz Jarocki Date: Mon, 10 May 2021 16:35:00 +0200 Subject: [PATCH] SONAR-14807 fixed an issue where not only ratings were converted in the email message to letters --- .../metric/MetricRepository.java | 6 +++ .../metric/MetricRepositoryImpl.java | 7 +++ .../step/QualityGateEventsStep.java | 11 ++++- .../metric/MetricRepositoryImplTest.java | 46 +++++++++++++++++-- .../metric/MetricRepositoryRule.java | 8 ++++ .../notification/QGChangeEmailTemplate.java | 31 ++++++++----- .../notification/QGChangeNotification.java | 1 - .../QGChangeEmailTemplateTest.java | 39 ++++++++++++---- 8 files changed, 120 insertions(+), 29 deletions(-) diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/metric/MetricRepository.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/metric/MetricRepository.java index 93d4272062e..4bd383cfbd9 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/metric/MetricRepository.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/metric/MetricRepository.java @@ -19,6 +19,7 @@ */ package org.sonar.ce.task.projectanalysis.metric; +import java.util.List; import java.util.Optional; public interface MetricRepository { @@ -51,4 +52,9 @@ public interface MetricRepository { */ Iterable getAll(); + /** + * Returns all the {@link Metric}s for the specific type. + */ + List getMetricsByType(Metric.MetricType type); + } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryImpl.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryImpl.java index 9ebfe1adadc..801018d541d 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryImpl.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryImpl.java @@ -87,6 +87,13 @@ public class MetricRepositoryImpl implements MetricRepository, Startable { return metricsByKey.values(); } + @Override + public List getMetricsByType(Metric.MetricType type) { + verifyMetricsInitialized(); + + return metricsByKey.values().stream().filter(m -> m.getType() == type).collect(Collectors.toList()); + } + private void verifyMetricsInitialized() { if (this.metricsByKey == null) { throw new IllegalStateException("Metric cache has not been initialized"); diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/QualityGateEventsStep.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/QualityGateEventsStep.java index 339f4d1d103..b217dd1d793 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/QualityGateEventsStep.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/step/QualityGateEventsStep.java @@ -19,8 +19,6 @@ */ package org.sonar.ce.task.projectanalysis.step; -import java.util.Optional; -import javax.annotation.Nullable; import org.sonar.api.measures.CoreMetrics; import org.sonar.api.utils.log.Logger; import org.sonar.api.utils.log.Loggers; @@ -38,12 +36,17 @@ import org.sonar.ce.task.projectanalysis.measure.Measure; import org.sonar.ce.task.projectanalysis.measure.MeasureRepository; import org.sonar.ce.task.projectanalysis.measure.QualityGateStatus; import org.sonar.ce.task.projectanalysis.metric.Metric; +import org.sonar.ce.task.projectanalysis.metric.Metric.MetricType; import org.sonar.ce.task.projectanalysis.metric.MetricRepository; import org.sonar.ce.task.step.ComputationStep; import org.sonar.server.notification.NotificationService; import org.sonar.server.qualitygate.notification.QGChangeNotification; import static java.util.Collections.singleton; +import javax.annotation.Nullable; +import java.util.List; +import java.util.Optional; +import java.util.stream.Collectors; /** * This step must be executed after computation of quality gate measure {@link QualityGateMeasuresStep} @@ -142,6 +145,10 @@ public class QualityGateEventsStep implements ComputationStep { if (!branch.isMain()) { notification.setFieldValue("branch", branch.getName()); } + + List ratingMetrics = metricRepository.getMetricsByType(MetricType.RATING); + String ratingMetricsInOneString = ratingMetrics.stream().map(Metric::getName).collect(Collectors.joining(",")); + notification.setFieldValue("ratingMetrics", ratingMetricsInOneString); notificationService.deliverEmails(singleton(notification)); // compatibility with old API diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryImplTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryImplTest.java index 0f6fc4f9113..fe2f5866c95 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryImplTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryImplTest.java @@ -19,10 +19,6 @@ */ package org.sonar.ce.task.projectanalysis.metric; -import java.util.List; -import java.util.Random; -import java.util.stream.Collectors; -import java.util.stream.IntStream; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -32,6 +28,10 @@ import org.sonar.db.DbTester; import org.sonar.db.metric.MetricDto; import static org.assertj.core.api.Assertions.assertThat; +import java.util.List; +import java.util.Random; +import java.util.stream.Collectors; +import java.util.stream.IntStream; public class MetricRepositoryImplTest { private static final String SOME_KEY = "some_key"; @@ -181,4 +181,42 @@ public class MetricRepositoryImplTest { .containsOnly(enabledMetrics.stream().map(MetricDto::getKey).toArray(String[]::new)); } + @Test + public void getMetricsByType_givenRatingType_returnRatingMetrics() { + List enabledMetrics = IntStream.range(0, 1 + new Random().nextInt(12)) + .mapToObj(i -> dbTester.measures().insertMetric(t -> t.setKey("key_enabled_" + i).setEnabled(true).setValueType("RATING"))) + .collect(Collectors.toList()); + + underTest.start(); + assertThat(underTest.getMetricsByType(Metric.MetricType.RATING)) + .extracting(Metric::getKey) + .containsOnly(enabledMetrics.stream().map(MetricDto::getKey).toArray(String[]::new)); + } + + @Test + public void getMetricsByType_givenRatingTypeAndWantedMilisecType_returnEmptyList() { + IntStream.range(0, 1 + new Random().nextInt(12)) + .mapToObj(i -> dbTester.measures().insertMetric(t -> t.setKey("key_enabled_" + i).setEnabled(true).setValueType("RATING"))) + .collect(Collectors.toList()); + + underTest.start(); + assertThat(underTest.getMetricsByType(Metric.MetricType.MILLISEC).size()).isZero(); + } + + @Test + public void getMetricsByType_givenOnlyMilisecTypeAndWantedRatingMetrics_returnEmptyList() { + IntStream.range(0, 1 + new Random().nextInt(12)) + .mapToObj(i -> dbTester.measures().insertMetric(t -> t.setKey("key_enabled_" + i).setEnabled(true).setValueType("MILISEC"))); + + underTest.start(); + assertThat(underTest.getMetricsByType(Metric.MetricType.RATING).size()).isZero(); + } + + @Test + public void getMetricsByType_givenMetricsAreNull_throwException() { + expectedException.expect(IllegalStateException.class); + + underTest.getMetricsByType(Metric.MetricType.RATING); + } + } diff --git a/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryRule.java b/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryRule.java index d1a386fa71e..ab9192bcba4 100644 --- a/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryRule.java +++ b/server/sonar-ce-task-projectanalysis/src/testFixtures/java/org/sonar/ce/task/projectanalysis/metric/MetricRepositoryRule.java @@ -20,8 +20,11 @@ package org.sonar.ce.task.projectanalysis.metric; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.stream.Collectors; + import org.junit.rules.ExternalResource; import static com.google.common.base.Preconditions.checkState; @@ -112,4 +115,9 @@ public class MetricRepositoryRule extends ExternalResource implements MetricRepo public Iterable getAll() { return metricsByKey.values(); } + + @Override + public List getMetricsByType(Metric.MetricType type) { + return metricsByKey.values().stream().filter(m -> m.getType() == type).collect(Collectors.toList()); + } } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/notification/QGChangeEmailTemplate.java b/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/notification/QGChangeEmailTemplate.java index 4766df2eecb..b9860094a00 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/notification/QGChangeEmailTemplate.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/notification/QGChangeEmailTemplate.java @@ -21,6 +21,7 @@ package org.sonar.server.qualitygate.notification; import javax.annotation.CheckForNull; import javax.annotation.Nullable; + import org.apache.commons.lang.StringUtils; import org.sonar.api.config.EmailSettings; import org.sonar.api.measures.Metric; @@ -29,7 +30,7 @@ import org.sonar.server.issue.notification.EmailMessage; import org.sonar.server.issue.notification.EmailTemplate; import org.sonar.server.measure.Rating; -import java.util.regex.Pattern; +import java.util.Optional; /** * Creates email message for notification "alerts". @@ -38,8 +39,6 @@ import java.util.regex.Pattern; */ public class QGChangeEmailTemplate implements EmailTemplate { - private static final Pattern alertRatingRegex = Pattern.compile(".*>\\s\\d$"); - private final EmailSettings configuration; public QGChangeEmailTemplate(EmailSettings configuration) { @@ -62,12 +61,13 @@ public class QGChangeEmailTemplate implements EmailTemplate { String alertName = notification.getFieldValue("alertName"); String alertText = notification.getFieldValue("alertText"); String alertLevel = notification.getFieldValue("alertLevel"); + String ratingMetricsInOneString = notification.getFieldValue("ratingMetrics"); boolean isNewAlert = Boolean.parseBoolean(notification.getFieldValue("isNewAlert")); String fullProjectName = computeFullProjectName(projectName, branchName); // Generate text String subject = generateSubject(fullProjectName, alertLevel, isNewAlert); - String messageBody = generateMessageBody(projectName, projectKey, projectVersion, branchName, alertName, alertText, isNewAlert); + String messageBody = generateMessageBody(projectName, projectKey, projectVersion, branchName, alertName, alertText, isNewAlert, ratingMetricsInOneString); // And finally return the email that will be sent return new EmailMessage() @@ -97,7 +97,7 @@ public class QGChangeEmailTemplate implements EmailTemplate { private String generateMessageBody(String projectName, String projectKey, @Nullable String projectVersion, @Nullable String branchName, - String alertName, String alertText, boolean isNewAlert) { + String alertName, String alertText, boolean isNewAlert, String ratingMetricsInOneString) { StringBuilder messageBody = new StringBuilder(); messageBody.append("Project: ").append(projectName).append("\n"); if (branchName != null) { @@ -116,11 +116,11 @@ public class QGChangeEmailTemplate implements EmailTemplate { messageBody.append("Quality gate threshold"); } if (alerts.length == 1) { - messageBody.append(": ").append(formatRating(alerts[0].trim())).append("\n"); + messageBody.append(": ").append(formatRating(alerts[0].trim(), ratingMetricsInOneString)).append("\n"); } else { messageBody.append("s:\n"); for (String alert : alerts) { - messageBody.append(" - ").append(formatRating(alert.trim())).append("\n"); + messageBody.append(" - ").append(formatRating(alert.trim(), ratingMetricsInOneString)).append("\n"); } } } @@ -143,16 +143,23 @@ public class QGChangeEmailTemplate implements EmailTemplate { * Code Coverage < 50% will not be converted and returned as is. * * @param alert + * @param ratingMetricsInOneString * @return full raw alert with converted ratings */ - private static String formatRating(String alert) { - if(!alertRatingRegex.matcher(alert).matches()) { + private static String formatRating(String alert, String ratingMetricsInOneString) { + Optional ratingToFormat = Optional.empty(); + for(String rating : ratingMetricsInOneString.split(",")) { + if (alert.matches(rating + " > \\d$")) { + ratingToFormat = Optional.of(rating); + break; + } + } + if(!ratingToFormat.isPresent()){ return alert; } - StringBuilder builder = new StringBuilder(); - builder.append(alert, 0, alert.length() - 3); - builder.append("worse than "); + StringBuilder builder = new StringBuilder(ratingToFormat.get()); + builder.append(" worse than "); String rating = alert.substring(alert.length() - 1); builder.append(Rating.valueOf(Integer.parseInt(rating)).name()); return builder.toString(); diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/notification/QGChangeNotification.java b/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/notification/QGChangeNotification.java index 7ece5f0afbf..7067912c47a 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/notification/QGChangeNotification.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/qualitygate/notification/QGChangeNotification.java @@ -21,7 +21,6 @@ package org.sonar.server.qualitygate.notification; import javax.annotation.CheckForNull; import org.sonar.api.notifications.Notification; - public class QGChangeNotification extends Notification { public QGChangeNotification() { super("alerts"); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/notification/QGChangeEmailTemplateTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/notification/QGChangeEmailTemplateTest.java index 5a956a4df10..45f6c37e01c 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/notification/QGChangeEmailTemplateTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/qualitygate/notification/QGChangeEmailTemplateTest.java @@ -67,7 +67,7 @@ public class QGChangeEmailTemplateTest { "Quality gate status: Failed\n" + "\n" + "Quality gate thresholds:\n" + - " - violations worse than D\n" + + " - violations > 4\n" + " - coverage < 75%\n" + "\n" + "More details at: http://nemo.sonarsource.org/dashboard?id=org.sonar.foo:foo")); @@ -88,7 +88,7 @@ public class QGChangeEmailTemplateTest { "Quality gate status: Failed\n" + "\n" + "Quality gate thresholds:\n" + - " - violations worse than D\n" + + " - violations > 4\n" + " - coverage < 75%\n" + "\n" + "More details at: http://nemo.sonarsource.org/dashboard?id=org.sonar.foo:foo&branch=feature")); @@ -107,7 +107,7 @@ public class QGChangeEmailTemplateTest { "Quality gate status: Failed\n" + "\n" + "New quality gate thresholds:\n" + - " - violations worse than D\n" + + " - violations > 4\n" + " - coverage < 75%\n" + "\n" + "More details at: http://nemo.sonarsource.org/dashboard?id=org.sonar.foo:foo")); @@ -125,7 +125,7 @@ public class QGChangeEmailTemplateTest { "Version: V1-SNAP\n" + "Quality gate status: Failed\n" + "\n" + - "New quality gate threshold: violations worse than D\n" + + "New quality gate threshold: violations > 4\n" + "\n" + "More details at: http://nemo.sonarsource.org/dashboard?id=org.sonar.foo:foo")); } @@ -142,7 +142,7 @@ public class QGChangeEmailTemplateTest { "Project: Foo\n" + "Quality gate status: Failed\n" + "\n" + - "New quality gate threshold: violations worse than D\n" + + "New quality gate threshold: violations > 4\n" + "\n" + "More details at: http://nemo.sonarsource.org/dashboard?id=org.sonar.foo:foo")); } @@ -177,7 +177,7 @@ public class QGChangeEmailTemplateTest { "Version: V1-SNAP\n" + "Quality gate status: Failed\n" + "\n" + - "New quality gate threshold: violations worse than D\n" + + "New quality gate threshold: violations > 4\n" + "\n" + "More details at: http://nemo.sonarsource.org/dashboard?id=org.sonar.foo:foo&branch=feature")); } @@ -203,10 +203,25 @@ public class QGChangeEmailTemplateTest { @DataProvider public static Object[][] alertTextAndFormattedText() { return new Object[][] { - {"violations > 1", "violations worse than A"}, - {"violations > 4", "violations worse than D"}, + {"violations > 0", "violations > 0"}, + {"violations > 1", "violations > 1"}, + {"violations > 4", "violations > 4"}, + {"violations > 5", "violations > 5"}, + {"violations > 6", "violations > 6"}, + {"violations > 10", "violations > 10"}, + {"Code Coverage < 0%", "Code Coverage < 0%"}, + {"Code Coverage < 1%", "Code Coverage < 1%"}, {"Code Coverage < 50%", "Code Coverage < 50%"}, - {"custom metric condition not met", "custom metric condition not met"} + {"Code Coverage < 100%", "Code Coverage < 100%"}, + {"Custom metric with big number > 100000000000", "Custom metric with big number > 100000000000"}, + {"Custom metric with negative number > -1", "Custom metric with negative number > -1"}, + {"custom metric condition not met", "custom metric condition not met"}, + + {"Security Review Rating > 1", "Security Review Rating worse than A"}, + {"Security Review Rating on New Code > 4", "Security Review Rating on New Code worse than D"}, + {"Security Rating > 1", "Security Rating worse than A"}, + {"Maintainability Rating > 3", "Maintainability Rating worse than C"}, + {"Reliability Rating > 4", "Reliability Rating worse than D" } }; } @@ -238,7 +253,11 @@ public class QGChangeEmailTemplateTest { .setFieldValue("alertName", alertName) .setFieldValue("alertText", alertText) .setFieldValue("alertLevel", alertLevel) - .setFieldValue("isNewAlert", isNewAlert); + .setFieldValue("isNewAlert", isNewAlert) + .setFieldValue("ratingMetrics", "Maintainability Rating,Reliability Rating on New Code," + + "Maintainability Rating on New Code,Reliability Rating," + + "Security Rating on New Code,Security Review Rating," + + "Security Review Rating on New Code,Security Rating"); } } -- 2.39.5