From: antoine.vinot Date: Tue, 12 Sep 2023 14:59:23 +0000 (+0200) Subject: SONAR-20424 - Store rules impact within issue X-Git-Tag: 10.3.0.82913~368 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=abc6475f07561c76b346b2bea7b757a48c7cf9b6;p=sonarqube.git SONAR-20424 - Store rules impact within issue --- diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java index 7b8696d3fbd..77f740f6cf3 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactory.java @@ -25,7 +25,6 @@ import java.util.HashMap; import java.util.List; import java.util.Map; import java.util.Optional; -import java.util.stream.Collectors; import javax.annotation.Nullable; import org.jetbrains.annotations.NotNull; import org.slf4j.LoggerFactory; @@ -50,9 +49,11 @@ import org.sonar.db.protobuf.DbIssues; import org.sonar.scanner.protocol.Constants; import org.sonar.scanner.protocol.Constants.Severity; import org.sonar.scanner.protocol.output.ScannerReport; +import org.sonar.scanner.protocol.output.ScannerReport.Impact; import org.sonar.scanner.protocol.output.ScannerReport.IssueType; import org.sonar.server.rule.CommonRuleKeys; +import static java.util.stream.Collectors.toMap; import static org.apache.commons.lang.StringUtils.isNotEmpty; import static org.sonar.api.issue.Issue.STATUS_OPEN; import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; @@ -197,18 +198,24 @@ public class TrackerRawInputFactory { issue.setRuleDescriptionContextKey(reportIssue.hasRuleDescriptionContextKey() ? reportIssue.getRuleDescriptionContextKey() : null); issue.setCodeVariants(reportIssue.getCodeVariantsList()); - issue.replaceImpacts(convertImpacts(issue.ruleKey(), reportIssue.getOverridenImpactsList())); + issue.replaceImpacts(replaceDefaultWithOverridenImpacts(issue.ruleKey(), reportIssue.getOverridenImpactsList())); return issue; } - private Map convertImpacts(RuleKey ruleKey, List overridenImpactsList) { - if (overridenImpactsList.isEmpty()) { - return Collections.emptyMap(); - } - Rule rule = ruleRepository.getByKey(ruleKey); + private Map replaceDefaultWithOverridenImpacts(RuleKey ruleKey, List overridenImpactsList) { + Map overridenImpactMap = getOverridenImpactMap(overridenImpactsList); + return ruleRepository.getByKey(ruleKey).getDefaultImpacts().entrySet() + .stream() + .collect(toMap( + Map.Entry::getKey, + entry -> overridenImpactMap.containsKey(entry.getKey()) ? overridenImpactMap.get(entry.getKey()) : entry.getValue())); + } + + private static Map getOverridenImpactMap(List overridenImpactsList) { return overridenImpactsList.stream() - .filter(i -> rule.getDefaultImpacts().containsKey(SoftwareQuality.valueOf(i.getSoftwareQuality()))) - .collect(Collectors.toMap(i -> SoftwareQuality.valueOf(i.getSoftwareQuality()), i -> org.sonar.api.issue.impact.Severity.valueOf(i.getSeverity()))); + .collect(toMap( + impact -> SoftwareQuality.valueOf(impact.getSoftwareQuality()), + impact -> org.sonar.api.issue.impact.Severity.valueOf(impact.getSeverity()))); } private DbIssues.Flow.Builder convertLocations(ScannerReport.Flow flow) { @@ -233,7 +240,7 @@ public class TrackerRawInputFactory { Rule existingRule = ruleRepository.getByKey(ruleKey); issue.setSeverity(determineDeprecatedSeverity(reportExternalIssue, existingRule)); issue.setType(determineDeprecatedType(reportExternalIssue, existingRule)); - issue.replaceImpacts(convertImpacts(issue.ruleKey(), reportExternalIssue.getImpactsList())); + issue.replaceImpacts(replaceDefaultWithOverridenImpacts(issue.ruleKey(), reportExternalIssue.getImpactsList())); init(issue, issue.type() == RuleType.SECURITY_HOTSPOT ? STATUS_TO_REVIEW : STATUS_OPEN); diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java index 431a717a203..cf12e50945b 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/TrackerRawInputFactoryTest.java @@ -34,7 +34,6 @@ import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; -import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.Severity; import org.sonar.api.rules.RuleType; @@ -66,6 +65,9 @@ import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.STATUS_OPEN; import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; +import static org.sonar.api.issue.impact.Severity.LOW; +import static org.sonar.api.issue.impact.SoftwareQuality.MAINTAINABILITY; +import static org.sonar.api.issue.impact.SoftwareQuality.SECURITY; import static org.sonar.scanner.protocol.output.ScannerReport.MessageFormattingType.CODE; @RunWith(DataProviderRunner.class) @@ -128,13 +130,13 @@ public class TrackerRawInputFactoryTest { public void load_issues_from_report() { RuleKey ruleKey = RuleKey.of("java", "S001"); markRuleAsActive(ruleKey); - registerRule(ruleKey, "name", r -> r.addDefaultImpact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.LOW)); + registerRule(ruleKey, "name", r -> r.addDefaultImpact(MAINTAINABILITY, LOW)); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() .setTextRange(newTextRange(2)) .setMsg("the message") .addMsgFormatting(ScannerReport.MessageFormatting.newBuilder().setStart(0).setEnd(3).setType(CODE).build()) .addOverridenImpacts(ScannerReport.Impact.newBuilder() - .setSoftwareQuality(SoftwareQuality.MAINTAINABILITY.name()) + .setSoftwareQuality(MAINTAINABILITY.name()) .setSeverity(org.sonar.api.issue.impact.Severity.HIGH.name()) .build()) .setRuleRepository(ruleKey.repository()) @@ -171,13 +173,14 @@ public class TrackerRawInputFactoryTest { assertInitializedIssue(issue); assertThat(issue.effort()).isNull(); assertThat(issue.getRuleDescriptionContextKey()).isEmpty(); - assertThat(issue.impacts()).containsExactlyEntriesOf(Map.of(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.HIGH)); + assertThat(issue.impacts()).containsExactlyEntriesOf(Map.of(MAINTAINABILITY, org.sonar.api.issue.impact.Severity.HIGH)); } @Test public void load_issues_from_report_with_locations() { RuleKey ruleKey = RuleKey.of("java", "S001"); markRuleAsActive(ruleKey); + registerRule(ruleKey, "name"); ScannerReport.MessageFormatting messageFormatting = ScannerReport.MessageFormatting.newBuilder().setStart(0).setEnd(4).setType(CODE).build(); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() @@ -223,6 +226,7 @@ public class TrackerRawInputFactoryTest { public void load_issues_from_report_with_rule_description_context_key() { RuleKey ruleKey = RuleKey.of("java", "S001"); markRuleAsActive(ruleKey); + registerRule(ruleKey, "name"); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() .setTextRange(newTextRange(2)) @@ -245,31 +249,30 @@ public class TrackerRawInputFactoryTest { public void create_whenImpactIsNotDefinedAtRuleLevel_shouldDiscardImpacts() { RuleKey ruleKey = RuleKey.of("java", "S001"); markRuleAsActive(ruleKey); - registerRule(ruleKey, "name", r -> r.addDefaultImpact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.LOW)); + registerRule(ruleKey, "name", r -> r.addDefaultImpact(MAINTAINABILITY, LOW)); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() .setTextRange(newTextRange(2)) .setMsg("the message") .setRuleRepository(ruleKey.repository()) .setRuleKey(ruleKey.rule()) .addOverridenImpacts(ScannerReport.Impact.newBuilder() - .setSoftwareQuality(SoftwareQuality.SECURITY.name()) - .setSeverity(org.sonar.api.issue.impact.Severity.LOW.name()).build()) + .setSoftwareQuality(SECURITY.name()) + .setSeverity(LOW.name()).build()) .build(); reportReader.putIssues(FILE.getReportAttributes().getRef(), singletonList(reportIssue)); + Input input = underTest.create(FILE); Collection issues = input.getIssues(); - assertThat(issues) - .hasSize(1); - assertThat(issues.iterator().next().impacts()).isEmpty(); + assertThat(issues).hasSize(1); + assertThat(issues.iterator().next().impacts()).hasSize(1).containsEntry(MAINTAINABILITY, LOW); } @Test public void set_rule_name_as_message_when_issue_message_from_report_is_empty() { RuleKey ruleKey = RuleKey.of("java", "S001"); markRuleAsActive(ruleKey); - registerRule(ruleKey, "Rule 1", r -> { - }); + registerRule(ruleKey, "Rule 1"); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() .setRuleRepository(ruleKey.repository()) .setRuleKey(ruleKey.rule()) @@ -295,6 +298,7 @@ public class TrackerRawInputFactoryTest { public void load_issues_from_report_missing_secondary_location_component() { RuleKey ruleKey = RuleKey.of("java", "S001"); markRuleAsActive(ruleKey); + registerRule(ruleKey, "name"); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() .setTextRange(newTextRange(2)) @@ -337,9 +341,7 @@ public class TrackerRawInputFactoryTest { @Test @UseDataProvider("ruleTypeAndStatusByIssueType") public void load_external_issues_from_report(IssueType issueType, RuleType expectedRuleType, String expectedStatus) { - registerRule(RuleKey.of("external_eslint", "S001"), "rule", r -> { - r.addDefaultImpact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.LOW); - }); + registerRule(RuleKey.of("external_eslint", "S001"), "rule", r -> r.addDefaultImpact(MAINTAINABILITY, LOW)); ScannerReport.ExternalIssue reportIssue = ScannerReport.ExternalIssue.newBuilder() .setTextRange(newTextRange(2)) .setMsg("the message") @@ -350,7 +352,7 @@ public class TrackerRawInputFactoryTest { .setEffort(20L) .setType(issueType) .addFlow(ScannerReport.Flow.newBuilder().setType(FlowType.DATA).addLocation(ScannerReport.IssueLocation.newBuilder().build()).build()) - .addImpacts(ScannerReport.Impact.newBuilder().setSoftwareQuality(SoftwareQuality.MAINTAINABILITY.name()) + .addImpacts(ScannerReport.Impact.newBuilder().setSoftwareQuality(MAINTAINABILITY.name()) .setSeverity(org.sonar.api.issue.impact.Severity.MEDIUM.name()).build()) .build(); reportReader.putExternalIssues(FILE.getReportAttributes().getRef(), asList(reportIssue)); @@ -386,7 +388,7 @@ public class TrackerRawInputFactoryTest { assertThat(issue.tags()).isEmpty(); assertInitializedExternalIssue(issue, expectedStatus); - assertThat(issue.impacts()).containsExactlyEntriesOf(Map.of(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); + assertThat(issue.impacts()).containsExactlyEntriesOf(Map.of(MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); } @DataProvider @@ -402,8 +404,7 @@ public class TrackerRawInputFactoryTest { @Test @UseDataProvider("ruleTypeAndStatusByIssueType") public void load_external_issues_from_report_with_default_effort(IssueType issueType, RuleType expectedRuleType, String expectedStatus) { - registerRule(RuleKey.of("external_eslint", "S001"), "rule", r -> { - }); + registerRule(RuleKey.of("external_eslint", "S001"), "rule"); ScannerReport.ExternalIssue reportIssue = ScannerReport.ExternalIssue.newBuilder() .setTextRange(newTextRange(2)) .setMsg("the message") @@ -453,9 +454,8 @@ public class TrackerRawInputFactoryTest { @Test public void create_whenSeverityAndTypeNotProvidedByIssueAndRule_shouldTakeFromTheRuleImpact() { - registerRule(RuleKey.of("external_eslint", "S001"), "rule", r -> { - r.addDefaultImpact(SoftwareQuality.MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM); - }); + registerRule(RuleKey.of("external_eslint", "S001"), "rule", + r -> r.addDefaultImpact(MAINTAINABILITY, org.sonar.api.issue.impact.Severity.MEDIUM)); ScannerReport.ExternalIssue reportIssue = createIssue(null, null); reportReader.putExternalIssues(FILE.getReportAttributes().getRef(), asList(reportIssue)); Input input = underTest.create(FILE); @@ -508,6 +508,7 @@ public class TrackerRawInputFactoryTest { public void filter_excludes_issues_from_report() { RuleKey ruleKey = RuleKey.of("java", "S001"); markRuleAsActive(ruleKey); + registerRule(ruleKey, "name"); when(issueFilter.accept(any(), eq(FILE))).thenReturn(false); ScannerReport.Issue reportIssue = ScannerReport.Issue.newBuilder() .setTextRange(newTextRange(2)) @@ -570,6 +571,10 @@ public class TrackerRawInputFactoryTest { activeRulesHolder.put(new ActiveRule(ruleKey, Severity.CRITICAL, emptyMap(), 1_000L, null, "qp1")); } + private void registerRule(RuleKey ruleKey, String name) { + registerRule(ruleKey, name, r -> {}); + } + private void registerRule(RuleKey ruleKey, String name, Consumer dumbRulePopulator) { DumbRule dumbRule = new DumbRule(ruleKey); dumbRule.setName(name); diff --git a/server/sonar-db-dao/src/it/java/org/sonar/db/issue/IssueDaoIT.java b/server/sonar-db-dao/src/it/java/org/sonar/db/issue/IssueDaoIT.java index 0a4a79a5e86..f5ecdafdf3c 100644 --- a/server/sonar-db-dao/src/it/java/org/sonar/db/issue/IssueDaoIT.java +++ b/server/sonar-db-dao/src/it/java/org/sonar/db/issue/IssueDaoIT.java @@ -68,6 +68,12 @@ import static org.sonar.api.issue.Issue.STATUS_OPEN; import static org.sonar.api.issue.Issue.STATUS_REOPENED; import static org.sonar.api.issue.Issue.STATUS_RESOLVED; import static org.sonar.api.issue.Issue.STATUS_REVIEWED; +import static org.sonar.api.issue.impact.Severity.HIGH; +import static org.sonar.api.issue.impact.Severity.LOW; +import static org.sonar.api.issue.impact.Severity.MEDIUM; +import static org.sonar.api.issue.impact.SoftwareQuality.MAINTAINABILITY; +import static org.sonar.api.issue.impact.SoftwareQuality.RELIABILITY; +import static org.sonar.api.issue.impact.SoftwareQuality.SECURITY; import static org.sonar.db.component.BranchType.BRANCH; import static org.sonar.db.component.BranchType.PULL_REQUEST; import static org.sonar.db.component.ComponentTesting.newFileDto; @@ -163,15 +169,11 @@ public class IssueDaoIT { assertThat(issue.getImpacts()) .extracting(ImpactDto::getSeverity, ImpactDto::getSoftwareQuality) .containsExactlyInAnyOrder( - tuple(Severity.MEDIUM, SoftwareQuality.RELIABILITY), - tuple(Severity.LOW, SoftwareQuality.SECURITY)); - - assertThat(issue.getEffectiveImpacts()) - // impacts from rule - .containsEntry(SoftwareQuality.MAINTAINABILITY, Severity.HIGH) - // impacts from issue - .containsEntry(SoftwareQuality.RELIABILITY, Severity.MEDIUM) - .containsEntry(SoftwareQuality.SECURITY, Severity.LOW); + tuple(MEDIUM, RELIABILITY), + tuple(LOW, SECURITY)); + assertThat(issue.getRuleDefaultImpacts()) + .extracting(ImpactDto::getSeverity, ImpactDto::getSoftwareQuality) + .containsExactlyInAnyOrder(tuple(HIGH, MAINTAINABILITY)); } @Test @@ -196,8 +198,8 @@ public class IssueDaoIT { .flatMap(issueImpactDtos -> issueImpactDtos) .extracting(ImpactDto::getSeverity, ImpactDto::getSoftwareQuality) .containsExactlyInAnyOrder( - tuple(Severity.MEDIUM, SoftwareQuality.RELIABILITY), - tuple(Severity.LOW, SoftwareQuality.SECURITY)); + tuple(MEDIUM, RELIABILITY), + tuple(LOW, SECURITY)); } @Test @@ -206,8 +208,8 @@ public class IssueDaoIT { RuleDto rule = db.rules().insert(r -> r.setRepositoryKey("java").setLanguage("java") .addDefaultImpact(new ImpactDto() .setUuid(UuidFactoryFast.getInstance().create()) - .setSoftwareQuality(SoftwareQuality.RELIABILITY) - .setSeverity(Severity.MEDIUM))); + .setSoftwareQuality(RELIABILITY) + .setSeverity(MEDIUM))); ComponentDto branchA = db.components().insertProjectBranch(project, b -> b.setKey("branchA")); ComponentDto fileA = db.components().insertComponent(newFileDto(branchA)); @@ -223,12 +225,12 @@ public class IssueDaoIT { assertThat(next.getRuleDefaultImpacts()).hasSize(2) .extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity) .containsExactlyInAnyOrder( - tuple(SoftwareQuality.RELIABILITY, Severity.MEDIUM), - tuple(SoftwareQuality.MAINTAINABILITY, Severity.HIGH)); + tuple(RELIABILITY, MEDIUM), + tuple(MAINTAINABILITY, HIGH)); assertThat(next.getImpacts()) .extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity) .containsExactlyInAnyOrder( - tuple(SoftwareQuality.MAINTAINABILITY, Severity.HIGH)); + tuple(MAINTAINABILITY, HIGH)); issueCount++; } assertThat(issueCount).isEqualTo(100); @@ -602,12 +604,12 @@ public class IssueDaoIT { public void insert_shouldInsertBatchIssuesWithImpacts() { ImpactDto impact1 = new ImpactDto() .setUuid(UuidFactoryFast.getInstance().create()) - .setSoftwareQuality(SoftwareQuality.MAINTAINABILITY) - .setSeverity(Severity.HIGH); + .setSoftwareQuality(MAINTAINABILITY) + .setSeverity(HIGH); ImpactDto impact2 = new ImpactDto() .setUuid(UuidFactoryFast.getInstance().create()) - .setSoftwareQuality(SoftwareQuality.SECURITY) - .setSeverity(Severity.LOW); + .setSoftwareQuality(SECURITY) + .setSeverity(LOW); IssueDto issue1 = createIssueWithKey(ISSUE_KEY1) .addImpact(impact1) .addImpact(impact2); @@ -828,12 +830,12 @@ public class IssueDaoIT { prepareTables(); IssueDto issueDto = underTest.selectOrFailByKey(db.getSession(), ISSUE_KEY1) .setSelectedAt(1_440_000_000_000L) - .replaceAllImpacts(List.of(new ImpactDto().setUuid(Uuids.createFast()).setSoftwareQuality(SoftwareQuality.RELIABILITY).setSeverity(Severity.LOW))); + .replaceAllImpacts(List.of(new ImpactDto().setUuid(Uuids.createFast()).setSoftwareQuality(RELIABILITY).setSeverity(LOW))); underTest.updateIfBeforeSelectedDate(db.getSession(), issueDto); assertThat(underTest.selectOrFailByKey(db.getSession(), ISSUE_KEY1).getImpacts()).extracting(i -> i.getSoftwareQuality(), i -> i.getSeverity()) - .containsExactly(tuple(SoftwareQuality.RELIABILITY, Severity.LOW)); + .containsExactly(tuple(RELIABILITY, LOW)); } @@ -842,12 +844,12 @@ public class IssueDaoIT { prepareTables(); IssueDto issueDto = underTest.selectOrFailByKey(db.getSession(), ISSUE_KEY1) .setSelectedAt(1_400_000_000_000L) - .replaceAllImpacts(List.of(new ImpactDto().setUuid(Uuids.createFast()).setSoftwareQuality(SoftwareQuality.RELIABILITY).setSeverity(Severity.LOW))); + .replaceAllImpacts(List.of(new ImpactDto().setUuid(Uuids.createFast()).setSoftwareQuality(RELIABILITY).setSeverity(LOW))); underTest.updateIfBeforeSelectedDate(db.getSession(), issueDto); assertThat(underTest.selectOrFailByKey(db.getSession(), ISSUE_KEY1).getImpacts()).extracting(i -> i.getSoftwareQuality(), i -> i.getSeverity()) - .containsExactlyInAnyOrder(tuple(SoftwareQuality.RELIABILITY, Severity.MEDIUM), tuple(SoftwareQuality.SECURITY, Severity.LOW)); + .containsExactlyInAnyOrder(tuple(RELIABILITY, MEDIUM), tuple(SECURITY, LOW)); } private static IssueDto createIssueWithKey(String issueKey) { @@ -898,8 +900,8 @@ public class IssueDaoIT { .setRuleUuid(RULE.getUuid()) .setComponentUuid(FILE_UUID) .setProjectUuid(PROJECT_UUID) - .addImpact(newIssueImpact(SoftwareQuality.RELIABILITY, Severity.MEDIUM)) - .addImpact(newIssueImpact(SoftwareQuality.SECURITY, Severity.LOW))); + .addImpact(newIssueImpact(RELIABILITY, MEDIUM)) + .addImpact(newIssueImpact(SECURITY, LOW))); underTest.insert(db.getSession(), newIssueDto(ISSUE_KEY2) .setRuleUuid(RULE.getUuid()) .setComponentUuid(FILE_UUID) diff --git a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java index e6811c758f4..d933e0b45fd 100644 --- a/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java +++ b/server/sonar-db-dao/src/main/java/org/sonar/db/issue/IssueDto.java @@ -26,9 +26,7 @@ import com.google.common.collect.ImmutableSet; import com.google.protobuf.InvalidProtocolBufferException; import java.io.Serializable; import java.util.Collection; -import java.util.Collections; import java.util.Date; -import java.util.EnumMap; import java.util.HashSet; import java.util.Map; import java.util.Optional; @@ -53,6 +51,7 @@ import org.sonar.db.rule.RuleDto; import static com.google.common.base.Preconditions.checkArgument; import static java.lang.String.format; +import static java.util.stream.Collectors.toUnmodifiableMap; import static org.sonar.api.utils.DateUtils.dateToLong; import static org.sonar.api.utils.DateUtils.longToDate; @@ -840,10 +839,12 @@ public final class IssueDto implements Serializable { * @return Unmodifiable Map of impacts */ public Map getEffectiveImpacts() { - EnumMap effectiveImpacts = new EnumMap<>(SoftwareQuality.class); - ruleDefaultImpacts.forEach(impact -> effectiveImpacts.put(impact.getSoftwareQuality(), impact.getSeverity())); - impacts.forEach(impact -> effectiveImpacts.put(impact.getSoftwareQuality(), impact.getSeverity())); - return Collections.unmodifiableMap(effectiveImpacts); + return impacts.isEmpty() ? toImpactMap(ruleDefaultImpacts) : toImpactMap(impacts); + } + + private static Map toImpactMap(Collection impacts) { + return impacts.stream() + .collect(toUnmodifiableMap(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity)); } @Override diff --git a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDtoTest.java b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDtoTest.java index 5bb26690d80..7530893e97c 100644 --- a/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDtoTest.java +++ b/server/sonar-db-dao/src/test/java/org/sonar/db/issue/IssueDtoTest.java @@ -43,6 +43,12 @@ import org.sonar.db.rule.RuleDto; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.assertj.core.api.Assertions.tuple; +import static org.sonar.api.issue.impact.Severity.HIGH; +import static org.sonar.api.issue.impact.Severity.LOW; +import static org.sonar.api.issue.impact.Severity.MEDIUM; +import static org.sonar.api.issue.impact.SoftwareQuality.MAINTAINABILITY; +import static org.sonar.api.issue.impact.SoftwareQuality.RELIABILITY; +import static org.sonar.api.issue.impact.SoftwareQuality.SECURITY; public class IssueDtoTest { @@ -84,7 +90,7 @@ public class IssueDtoTest { .setIssueUpdateDate(updatedAt) .setIssueCloseDate(closedAt) .setRuleDescriptionContextKey(TEST_CONTEXT_KEY) - .addImpact(new ImpactDto().setSoftwareQuality(SoftwareQuality.MAINTAINABILITY).setSeverity(Severity.HIGH)); + .addImpact(new ImpactDto().setSoftwareQuality(MAINTAINABILITY).setSeverity(HIGH)); DefaultIssue expected = new DefaultIssue() .setKey("100") @@ -114,7 +120,7 @@ public class IssueDtoTest { .setRuleDescriptionContextKey(TEST_CONTEXT_KEY) .setCodeVariants(Set.of()) .setTags(Set.of()) - .addImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH); + .addImpact(MAINTAINABILITY, HIGH); DefaultIssue issue = dto.toDefaultIssue(); @@ -147,7 +153,7 @@ public class IssueDtoTest { assertThat(dto.getTags()).containsOnly("tag1", "tag2", "tag3"); assertThat(dto.getTagsString()).isEqualTo("tag1,tag2,tag3"); - dto.setTags(Arrays.asList()); + dto.setTags(List.of()); assertThat(dto.getTags()).isEmpty(); dto.setTagsString("tag1, tag2 ,,tag3"); @@ -163,38 +169,39 @@ public class IssueDtoTest { @Test public void getEffectiveImpacts_whenNoIssueImpactsOverridden_shouldReturnRuleImpacts() { IssueDto dto = new IssueDto(); - dto.getRuleDefaultImpacts().add(newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.HIGH)); - dto.getRuleDefaultImpacts().add(newImpactDto(SoftwareQuality.SECURITY, Severity.MEDIUM)); - dto.getRuleDefaultImpacts().add(newImpactDto(SoftwareQuality.RELIABILITY, Severity.LOW)); + dto.getRuleDefaultImpacts().add(newImpactDto(MAINTAINABILITY, HIGH)); + dto.getRuleDefaultImpacts().add(newImpactDto(SECURITY, MEDIUM)); + dto.getRuleDefaultImpacts().add(newImpactDto(RELIABILITY, LOW)); assertThat(dto.getEffectiveImpacts()) - .containsEntry(SoftwareQuality.MAINTAINABILITY, Severity.HIGH) - .containsEntry(SoftwareQuality.SECURITY, Severity.MEDIUM) - .containsEntry(SoftwareQuality.RELIABILITY, Severity.LOW); + .hasSize(3) + .containsEntry(MAINTAINABILITY, HIGH) + .containsEntry(SECURITY, MEDIUM) + .containsEntry(RELIABILITY, LOW); } @Test - public void getEffectiveImpacts_whenIssueImpactsOverridden_shouldReturnRuleImpactsOverriddenByIssueImpacts() { + public void getEffectiveImpacts_whenIssueImpactsOverridden_shouldReturnIssueImpacts() { IssueDto dto = new IssueDto(); - dto.getRuleDefaultImpacts().add(newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.HIGH)); - dto.getRuleDefaultImpacts().add(newImpactDto(SoftwareQuality.SECURITY, Severity.MEDIUM)); - dto.getRuleDefaultImpacts().add(newImpactDto(SoftwareQuality.RELIABILITY, Severity.LOW)); + dto.getRuleDefaultImpacts().add(newImpactDto(MAINTAINABILITY, HIGH)); + dto.getRuleDefaultImpacts().add(newImpactDto(SECURITY, MEDIUM)); + dto.getRuleDefaultImpacts().add(newImpactDto(RELIABILITY, LOW)); - dto.addImpact(newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.LOW)); - dto.addImpact(newImpactDto(SoftwareQuality.RELIABILITY, Severity.HIGH)); + dto.addImpact(newImpactDto(MAINTAINABILITY, LOW)); + dto.addImpact(newImpactDto(RELIABILITY, HIGH)); assertThat(dto.getEffectiveImpacts()) - .containsEntry(SoftwareQuality.MAINTAINABILITY, Severity.LOW) - .containsEntry(SoftwareQuality.SECURITY, Severity.MEDIUM) - .containsEntry(SoftwareQuality.RELIABILITY, Severity.HIGH); + .hasSize(2) + .containsEntry(MAINTAINABILITY, LOW) + .containsEntry(RELIABILITY, HIGH); } @Test public void addImpact_whenSoftwareQualityAlreadyDefined_shouldThrowISE() { IssueDto dto = new IssueDto(); - dto.addImpact(newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.LOW)); + dto.addImpact(newImpactDto(MAINTAINABILITY, LOW)); - ImpactDto duplicatedImpact = newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.HIGH); + ImpactDto duplicatedImpact = newImpactDto(MAINTAINABILITY, HIGH); assertThatThrownBy(() -> dto.addImpact(duplicatedImpact)) .isInstanceOf(IllegalStateException.class) @@ -204,13 +211,13 @@ public class IssueDtoTest { @Test public void replaceAllImpacts_whenSoftwareQualityAlreadyDuplicated_shouldThrowISE() { IssueDto dto = new IssueDto(); - dto.addImpact(newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.MEDIUM)); - dto.addImpact(newImpactDto(SoftwareQuality.SECURITY, Severity.HIGH)); - dto.addImpact(newImpactDto(SoftwareQuality.RELIABILITY, Severity.LOW)); + dto.addImpact(newImpactDto(MAINTAINABILITY, MEDIUM)); + dto.addImpact(newImpactDto(SECURITY, HIGH)); + dto.addImpact(newImpactDto(RELIABILITY, LOW)); Set duplicatedImpacts = Set.of( - newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.HIGH), - newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.LOW)); + newImpactDto(MAINTAINABILITY, HIGH), + newImpactDto(MAINTAINABILITY, LOW)); assertThatThrownBy(() -> dto.replaceAllImpacts(duplicatedImpacts)) .isInstanceOf(IllegalStateException.class) .hasMessage("Impacts must have unique Software Quality values"); @@ -219,21 +226,21 @@ public class IssueDtoTest { @Test public void replaceAllImpacts_shouldReplaceExistingImpacts() { IssueDto dto = new IssueDto(); - dto.addImpact(newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.MEDIUM)); - dto.addImpact(newImpactDto(SoftwareQuality.SECURITY, Severity.HIGH)); - dto.addImpact(newImpactDto(SoftwareQuality.RELIABILITY, Severity.LOW)); + dto.addImpact(newImpactDto(MAINTAINABILITY, MEDIUM)); + dto.addImpact(newImpactDto(SECURITY, HIGH)); + dto.addImpact(newImpactDto(RELIABILITY, LOW)); Set duplicatedImpacts = Set.of( - newImpactDto(SoftwareQuality.MAINTAINABILITY, Severity.HIGH), - newImpactDto(SoftwareQuality.SECURITY, Severity.LOW)); + newImpactDto(MAINTAINABILITY, HIGH), + newImpactDto(SECURITY, LOW)); dto.replaceAllImpacts(duplicatedImpacts); assertThat(dto.getImpacts()) .extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity) .containsExactlyInAnyOrder( - tuple(SoftwareQuality.MAINTAINABILITY, Severity.HIGH), - tuple(SoftwareQuality.SECURITY, Severity.LOW)); + tuple(MAINTAINABILITY, HIGH), + tuple(SECURITY, LOW)); } @@ -300,7 +307,7 @@ public class IssueDtoTest { assertThat(issueDto.isNewCodeReferenceIssue()).isTrue(); assertThat(issueDto.getOptionalRuleDescriptionContextKey()).contains(TEST_CONTEXT_KEY); assertThat(issueDto.getImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity) - .containsExactlyInAnyOrder(tuple(SoftwareQuality.MAINTAINABILITY, Severity.HIGH), tuple(SoftwareQuality.RELIABILITY, Severity.LOW)); + .containsExactlyInAnyOrder(tuple(MAINTAINABILITY, HIGH), tuple(RELIABILITY, LOW)); } @Test @@ -332,7 +339,7 @@ public class IssueDtoTest { assertThat(issueDto.isNewCodeReferenceIssue()).isTrue(); assertThat(issueDto.getOptionalRuleDescriptionContextKey()).contains(TEST_CONTEXT_KEY); assertThat(issueDto.getImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity) - .containsExactlyInAnyOrder(tuple(SoftwareQuality.MAINTAINABILITY, Severity.HIGH), tuple(SoftwareQuality.RELIABILITY, Severity.LOW)); + .containsExactlyInAnyOrder(tuple(MAINTAINABILITY, HIGH), tuple(RELIABILITY, LOW)); } private DefaultIssue createExampleDefaultIssue(Date dateNow) { @@ -365,8 +372,8 @@ public class IssueDtoTest { .setIsNewCodeReferenceIssue(true) .setRuleDescriptionContextKey(TEST_CONTEXT_KEY) .setCodeVariants(List.of("variant1", "variant2")) - .addImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH) - .addImpact(SoftwareQuality.RELIABILITY, Severity.LOW); + .addImpact(MAINTAINABILITY, HIGH) + .addImpact(RELIABILITY, LOW); return defaultIssue; }