From 8fae1ede3f39a29731237c0d89b76f0b447d9547 Mon Sep 17 00:00:00 2001 From: =?utf8?q?L=C3=A9o=20Geoffroy?= Date: Wed, 6 Nov 2024 10:51:12 +0100 Subject: [PATCH] SONAR-23527 impacts severity is updated when issue is set on manual severity --- .../sonar/server/issue/IssueFieldsSetter.java | 13 +++++ .../server/issue/IssueFieldsSetterTest.java | 43 ++++++++++++++ .../QProfileImpactSeverityMapper.java | 19 ++----- .../sonar/core/rule/ImpactSeverityMapper.java | 50 +++++++++++++++++ .../core/rule/ImpactSeverityMapperTest.java | 56 +++++++++++++++++++ 5 files changed, 167 insertions(+), 14 deletions(-) create mode 100644 sonar-core/src/main/java/org/sonar/core/rule/ImpactSeverityMapper.java create mode 100644 sonar-core/src/test/java/org/sonar/core/rule/ImpactSeverityMapperTest.java diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java index 482fc0791a0..3711d2f439e 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/IssueFieldsSetter.java @@ -39,6 +39,7 @@ import org.sonar.core.issue.DefaultImpact; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.DefaultIssueComment; import org.sonar.core.issue.IssueChangeContext; +import org.sonar.core.rule.ImpactSeverityMapper; import org.sonar.db.protobuf.DbIssues; import org.sonar.db.user.UserDto; import org.sonar.db.user.UserIdDto; @@ -46,6 +47,7 @@ import org.sonar.db.user.UserIdDto; import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Strings.isNullOrEmpty; import static java.util.Objects.requireNonNull; +import static org.sonar.api.server.rule.internal.ImpactMapper.convertToSoftwareQuality; /** * Updates issue fields and chooses if changes must be kept in history. @@ -465,6 +467,17 @@ public class IssueFieldsSetter { .stream().filter(DefaultImpact::manualSeverity) .forEach(i -> issue.addImpact(i.softwareQuality(), i.severity(), true)); + // If the severity of the issue is manually set but not the impact, we need to update the impacts with the same severity. + // This happens for user migrating from lower than 10.8 and having already customized the rule severity + if (issue.manualSeverity() + && issue.severity() != null + && issue.getImpacts().stream().noneMatch(DefaultImpact::manualSeverity)) { + issue.getImpacts() + .stream() + .filter(i -> convertToSoftwareQuality(issue.type()).equals(i.softwareQuality())) + .forEach(i -> issue.addImpact(i.softwareQuality(), ImpactSeverityMapper.mapImpactSeverity(issue.severity()), true)); + } + if (!previousImpacts.equals(issue.getImpacts())) { issue.setUpdateDate(context.date()); issue.setChanged(true); diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java index 6d80dba6d82..2695654c10b 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/IssueFieldsSetterTest.java @@ -625,6 +625,49 @@ class IssueFieldsSetterTest { new DefaultImpact(SoftwareQuality.RELIABILITY, Severity.HIGH, false))); } + @Test + void setImpacts_whenIssueHasManualSeverityAndHasEquivalentImpact_ImpactShouldBeSetToManualSeverity() { + Set currentImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, false)); + Set newImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, false)); + + newImpacts + .forEach(e -> issue.addImpact(e.softwareQuality(), e.severity(), e.manualSeverity())); + issue.setManualSeverity(true); + issue.setSeverity(org.sonar.api.rule.Severity.BLOCKER); + issue.setType(RuleType.CODE_SMELL); + boolean updated = underTest.setImpacts(issue, currentImpacts, context); + assertThat(updated).isTrue(); + assertThat(issue.getImpacts()).isEqualTo(Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.BLOCKER, true))); + } + + @Test + void setImpacts_whenIssueHasManualSeverityAndHasNoEquivalentImpact_ImpactShouldNotBeUpdated() { + Set currentImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, false)); + Set newImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, false)); + + newImpacts + .forEach(e -> issue.addImpact(e.softwareQuality(), e.severity(), e.manualSeverity())); + issue.setManualSeverity(true); + issue.setSeverity(org.sonar.api.rule.Severity.BLOCKER); + issue.setType(RuleType.BUG); + boolean updated = underTest.setImpacts(issue, currentImpacts, context); + assertThat(updated).isFalse(); + } + + @Test + void setImpacts_whenIssueHasManualSeverityAndImpactHasManualSeverity_ImpactShouldNotBeUpdated() { + Set currentImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, true)); + Set newImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, true)); + + newImpacts + .forEach(e -> issue.addImpact(e.softwareQuality(), e.severity(), e.manualSeverity())); + issue.setManualSeverity(true); + issue.setSeverity(org.sonar.api.rule.Severity.BLOCKER); + issue.setType(RuleType.BUG); + boolean updated = underTest.setImpacts(issue, currentImpacts, context); + assertThat(updated).isFalse(); + } + @Test void setCodeVariants_whenCodeVariantsUnchanged_shouldNotBeUpdated() { Set currentCodeVariants = new HashSet<>(Arrays.asList("linux", "windows")); diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java index 571bf550852..5f570e02dd4 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/qualityprofile/QProfileImpactSeverityMapper.java @@ -19,8 +19,6 @@ */ package org.sonar.server.qualityprofile; -import com.google.common.collect.BiMap; -import com.google.common.collect.ImmutableBiMap; import java.util.EnumMap; import java.util.Map; import javax.annotation.CheckForNull; @@ -29,6 +27,7 @@ import org.sonar.api.issue.impact.Severity; import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rules.RuleType; import org.sonar.api.server.rule.internal.ImpactMapper; +import org.sonar.core.rule.ImpactSeverityMapper; /** * Class to map impact severity and rule severity during the override of severity of quality profile. @@ -36,14 +35,6 @@ import org.sonar.api.server.rule.internal.ImpactMapper; */ public class QProfileImpactSeverityMapper { - private static final BiMap SEVERITY_MAPPING = new ImmutableBiMap.Builder() - .put(Severity.INFO, org.sonar.api.rule.Severity.INFO) - .put(Severity.LOW, org.sonar.api.rule.Severity.MINOR) - .put(Severity.MEDIUM, org.sonar.api.rule.Severity.MAJOR) - .put(Severity.HIGH, org.sonar.api.rule.Severity.CRITICAL) - .put(Severity.BLOCKER, org.sonar.api.rule.Severity.BLOCKER) - .build(); - private QProfileImpactSeverityMapper() { } @@ -54,9 +45,9 @@ public class QProfileImpactSeverityMapper { } SoftwareQuality softwareQuality = ImpactMapper.convertToSoftwareQuality(ruleType); if (ruleImpacts.containsKey(softwareQuality)) { - result.put(softwareQuality, SEVERITY_MAPPING.inverse().get(severity)); + result.put(softwareQuality, ImpactSeverityMapper.mapImpactSeverity(severity)); } else if (ruleImpacts.size() == 1) { - result.replaceAll((sq, sev) -> SEVERITY_MAPPING.inverse().get(severity)); + result.replaceAll((sq, sev) -> ImpactSeverityMapper.mapImpactSeverity(severity)); } return result; } @@ -65,9 +56,9 @@ public class QProfileImpactSeverityMapper { public static String mapSeverity(Map impacts, RuleType ruleType, @Nullable String ruleSeverity) { SoftwareQuality softwareQuality = ImpactMapper.convertToSoftwareQuality(ruleType); if (impacts.containsKey(softwareQuality)) { - return SEVERITY_MAPPING.get(impacts.get(softwareQuality)); + return ImpactSeverityMapper.mapRuleSeverity(impacts.get(softwareQuality)); } else if (impacts.size() == 1) { - return SEVERITY_MAPPING.get(impacts.entrySet().iterator().next().getValue()); + return ImpactSeverityMapper.mapRuleSeverity(impacts.entrySet().iterator().next().getValue()); } return ruleSeverity; } diff --git a/sonar-core/src/main/java/org/sonar/core/rule/ImpactSeverityMapper.java b/sonar-core/src/main/java/org/sonar/core/rule/ImpactSeverityMapper.java new file mode 100644 index 00000000000..b68057cf429 --- /dev/null +++ b/sonar-core/src/main/java/org/sonar/core/rule/ImpactSeverityMapper.java @@ -0,0 +1,50 @@ +/* + * SonarQube + * Copyright (C) 2009-2024 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.core.rule; + +import com.google.common.collect.BiMap; +import com.google.common.collect.ImmutableBiMap; +import java.util.Optional; +import org.sonar.api.issue.impact.Severity; + +public class ImpactSeverityMapper { + private static final BiMap SEVERITY_MAPPING = new ImmutableBiMap.Builder() + .put(Severity.INFO, org.sonar.api.rule.Severity.INFO) + .put(Severity.LOW, org.sonar.api.rule.Severity.MINOR) + .put(Severity.MEDIUM, org.sonar.api.rule.Severity.MAJOR) + .put(Severity.HIGH, org.sonar.api.rule.Severity.CRITICAL) + .put(Severity.BLOCKER, org.sonar.api.rule.Severity.BLOCKER) + .build(); + + private ImpactSeverityMapper() { + + } + + public static Severity mapImpactSeverity(String severity) { + return Optional.ofNullable(SEVERITY_MAPPING.inverse().get(severity)) + .orElseThrow(() -> new IllegalArgumentException("Severity not supported: " + severity)); + } + + public static String mapRuleSeverity(Severity severity) { + return Optional.ofNullable(SEVERITY_MAPPING.get(severity)) + .orElseThrow(() -> new IllegalArgumentException("Impact Severity not supported: " + severity)); + } + +} diff --git a/sonar-core/src/test/java/org/sonar/core/rule/ImpactSeverityMapperTest.java b/sonar-core/src/test/java/org/sonar/core/rule/ImpactSeverityMapperTest.java new file mode 100644 index 00000000000..300c3376196 --- /dev/null +++ b/sonar-core/src/test/java/org/sonar/core/rule/ImpactSeverityMapperTest.java @@ -0,0 +1,56 @@ +/* + * SonarQube + * Copyright (C) 2009-2024 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.core.rule; + +import org.junit.jupiter.api.Test; +import org.sonar.api.issue.impact.Severity; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +class ImpactSeverityMapperTest { + + @Test + void mapImpactSeverity_shouldReturnExpectedValues() { + + assertThat(ImpactSeverityMapper.mapRuleSeverity(Severity.INFO)).isEqualTo(org.sonar.api.rule.Severity.INFO); + } + + @Test + void mapImpactSeverity_shouldThrowExceptionWhenValueIsUnknownOrNull() { + + assertThatThrownBy(() -> ImpactSeverityMapper.mapImpactSeverity("unknown")).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Severity not supported: unknown"); + + assertThatThrownBy(() -> ImpactSeverityMapper.mapImpactSeverity(null)).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Severity not supported: null"); + } + + @Test + void mapRuleSeverity_shouldReturnExpectedValues() { + assertThat(ImpactSeverityMapper.mapImpactSeverity(org.sonar.api.rule.Severity.INFO)).isEqualTo(Severity.INFO); + } + + @Test + void mapRuleSeverity_shouldThrowExceptionWhenValueIsUnknownOrNull() { + assertThatThrownBy(() -> ImpactSeverityMapper.mapRuleSeverity(null)).isInstanceOf(IllegalArgumentException.class) + .hasMessage("Impact Severity not supported: null"); + } +} -- 2.39.5