From 93c69969195abe9ae4399e7930c0c58c59ffedf6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?L=C3=A9o=20Geoffroy?= Date: Wed, 6 Nov 2024 16:20:51 +0100 Subject: [PATCH] SONAR-23363 handle manual impact flag during project analysis --- .../projectanalysis/issue/IssueLifecycle.java | 7 ++- .../util/cache/ProtobufIssueDiskCache.java | 11 +++-- .../src/main/protobuf/issue_cache.proto | 1 + .../issue/IssueLifecycleTest.java | 13 +++-- .../cache/ProtobufIssueDiskCacheTest.java | 24 ++++----- .../java/org/sonar/db/issue/IssueDto.java | 33 ++++++------- .../java/org/sonar/db/issue/IssueDtoTest.java | 32 ++++++------ .../sonar/server/issue/IssueFieldsSetter.java | 24 ++++----- .../server/issue/IssueFieldsSetterTest.java | 49 +++++++++++++++++-- .../org/sonar/core/issue/DefaultImpact.java | 26 ++++++++++ .../org/sonar/core/issue/DefaultIssue.java | 20 ++++++-- .../sonar/core/issue/DefaultIssueTest.java | 8 +++ 12 files changed, 171 insertions(+), 77 deletions(-) create mode 100644 sonar-core/src/main/java/org/sonar/core/issue/DefaultImpact.java diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java index 00a2a117a42..80937105adb 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycle.java @@ -29,6 +29,7 @@ import org.sonar.api.issue.Issue; import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolder; +import org.sonar.core.issue.DefaultImpact; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.DefaultIssueComment; import org.sonar.core.issue.FieldDiffs; @@ -136,6 +137,10 @@ public class IssueLifecycle { to.setManualSeverity(true); to.setSeverity(from.severity()); } + + from.getImpacts() + .stream().filter(DefaultImpact::manualSeverity) + .forEach(i -> to.addImpact(i.softwareQuality(), i.severity(), true)); to.setCleanCodeAttribute(from.getCleanCodeAttribute()); copyChangesOfIssueFromOtherBranch(to, from); } @@ -212,7 +217,7 @@ public class IssueLifecycle { updater.setPastGap(raw, base.gap(), changeContext); updater.setPastEffort(raw, base.effort(), changeContext); updater.setCodeVariants(raw, requireNonNull(base.codeVariants()), changeContext); - updater.setImpacts(raw, base.impacts(), changeContext); + updater.setImpacts(raw, base.getImpacts(), changeContext); updater.setCleanCodeAttribute(raw, base.getCleanCodeAttribute(), changeContext); updater.setPrioritizedRule(raw, base.isPrioritizedRule(), changeContext); } diff --git a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java index 5b2760f1750..15cdef2b114 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java +++ b/server/sonar-ce-task-projectanalysis/src/main/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCache.java @@ -39,6 +39,7 @@ import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; import org.sonar.api.utils.System2; +import org.sonar.core.issue.DefaultImpact; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.DefaultIssueComment; import org.sonar.core.issue.FieldDiffs; @@ -146,7 +147,7 @@ public class ProtobufIssueDiskCache implements DiskCache { } for (IssueCache.Impact impact : next.getImpactsList()) { - defaultIssue.addImpact(SoftwareQuality.valueOf(impact.getSoftwareQuality()), Severity.valueOf(impact.getSeverity())); + defaultIssue.addImpact(SoftwareQuality.valueOf(impact.getSoftwareQuality()), Severity.valueOf(impact.getSeverity()), impact.getManualSeverity()); } for (IssueCache.FieldDiffs protoFieldDiffs : next.getChangesList()) { defaultIssue.addChange(toDefaultIssueChanges(protoFieldDiffs)); @@ -204,11 +205,11 @@ public class ProtobufIssueDiskCache implements DiskCache { builder.setIsNoLongerNewCodeReferenceIssue(defaultIssue.isNoLongerNewCodeReferenceIssue()); defaultIssue.getAnticipatedTransitionUuid().ifPresent(builder::setAnticipatedTransitionUuid); - - for (Map.Entry impact : defaultIssue.impacts().entrySet()) { + for (DefaultImpact impact : defaultIssue.getImpacts()) { builder.addImpacts(IssueCache.Impact.newBuilder() - .setSoftwareQuality(impact.getKey().name()) - .setSeverity(impact.getValue().name()) + .setSoftwareQuality(impact.softwareQuality().name()) + .setSeverity(impact.severity().name()) + .setManualSeverity(impact.manualSeverity()) .build()); } for (FieldDiffs fieldDiffs : defaultIssue.changes()) { diff --git a/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto b/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto index 8ef383a8687..6b1e4f5e709 100644 --- a/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto +++ b/server/sonar-ce-task-projectanalysis/src/main/protobuf/issue_cache.proto @@ -115,4 +115,5 @@ message Diff { message Impact { required string software_quality = 1; required string severity = 2; + required bool manual_severity = 3; } diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java index dcfa537892f..66a1e930f1c 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/issue/IssueLifecycleTest.java @@ -30,6 +30,7 @@ import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; import org.sonar.ce.task.projectanalysis.analysis.AnalysisMetadataHolderRule; import org.sonar.ce.task.projectanalysis.analysis.Branch; +import org.sonar.core.issue.DefaultImpact; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.DefaultIssueComment; import org.sonar.core.issue.FieldDiffs; @@ -397,7 +398,8 @@ public class IssueLifecycleTest { .setRuleDescriptionContextKey("spring") .setCleanCodeAttribute(CleanCodeAttribute.IDENTIFIABLE) .setCodeVariants(Set.of("foo", "bar")) - .addImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH) + .addImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW) + .addImpact(SoftwareQuality.RELIABILITY, Severity.HIGH) .setCreationDate(parseDate("2015-10-01")) .setUpdateDate(parseDate("2015-10-02")) .setCloseDate(parseDate("2015-10-03")); @@ -438,7 +440,7 @@ public class IssueLifecycleTest { .setGap(15d) .setRuleDescriptionContextKey("hibernate") .setCodeVariants(Set.of("donut")) - .addImpact(SoftwareQuality.RELIABILITY, Severity.LOW) + .addImpact(SoftwareQuality.RELIABILITY, Severity.LOW, true) .setEffort(Duration.create(15L)) .setManualSeverity(false) .setLocations(issueLocations) @@ -469,8 +471,10 @@ public class IssueLifecycleTest { .containsOnly(entry("foo", new FieldDiffs.Diff<>("bar", "donut"))); assertThat(raw.changes().get(1).diffs()) .containsOnly(entry("file", new FieldDiffs.Diff<>("A", "B"))); - assertThat(raw.impacts()) - .containsEntry(SoftwareQuality.MAINTAINABILITY, Severity.HIGH); + verifyUpdater(raw, messageFormattings, issueLocations); + } + + private void verifyUpdater(DefaultIssue raw, DbIssues.MessageFormattings messageFormattings, DbIssues.Locations issueLocations) { verify(updater).setPastSeverity(raw, BLOCKER, issueChangeContext); verify(updater).setPastLine(raw, 10); verify(updater).setRuleDescriptionContextKey(raw, "hibernate"); @@ -479,6 +483,7 @@ public class IssueLifecycleTest { verify(updater).setPastEffort(raw, Duration.create(15L), issueChangeContext); verify(updater).setPastLocations(raw, issueLocations); verify(updater).setCleanCodeAttribute(raw, CleanCodeAttribute.FOCUSED, issueChangeContext); + verify(updater).setImpacts(raw, Set.of(new DefaultImpact(SoftwareQuality.RELIABILITY, Severity.LOW, true)), issueChangeContext); } @Test diff --git a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCacheTest.java b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCacheTest.java index 2b9639ca815..7e6dab430d1 100644 --- a/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCacheTest.java +++ b/server/sonar-ce-task-projectanalysis/src/test/java/org/sonar/ce/task/projectanalysis/util/cache/ProtobufIssueDiskCacheTest.java @@ -20,13 +20,13 @@ package org.sonar.ce.task.projectanalysis.util.cache; import java.util.Date; -import java.util.Map; import org.junit.Test; import org.sonar.api.issue.impact.Severity; import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; +import org.sonar.core.issue.DefaultImpact; import org.sonar.core.issue.DefaultIssue; import static org.assertj.core.api.Assertions.assertThat; @@ -59,13 +59,16 @@ public class ProtobufIssueDiskCacheTest { @Test public void toDefaultIssue_whenImpactIsSet_shouldSetItInDefaultIssue() { IssueCache.Issue issue = prepareIssueWithCompulsoryFields() - .addImpacts(toImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH)) - .addImpacts(toImpact(SoftwareQuality.RELIABILITY, Severity.LOW)) + .addImpacts(toImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, true)) + .addImpacts(toImpact(SoftwareQuality.RELIABILITY, Severity.LOW, false)) .build(); DefaultIssue defaultIssue = ProtobufIssueDiskCache.toDefaultIssue(issue); - assertThat(defaultIssue.impacts()).containsExactlyInAnyOrderEntriesOf(Map.of(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, SoftwareQuality.RELIABILITY, Severity.LOW)); + assertThat(defaultIssue.getImpacts()) + .containsExactly( + new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, true), + new DefaultImpact(SoftwareQuality.RELIABILITY, Severity.LOW, false)); } @Test @@ -103,15 +106,14 @@ public class ProtobufIssueDiskCacheTest { @Test public void toProto_whenRuleDescriptionContextKeyIsSet_shouldCopyToIssueProto() { DefaultIssue defaultIssue = createDefaultIssueWithMandatoryFields(); - defaultIssue.addImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH); - defaultIssue.addImpact(SoftwareQuality.RELIABILITY, Severity.LOW); + defaultIssue.addImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, true); + defaultIssue.addImpact(SoftwareQuality.RELIABILITY, Severity.LOW, false); IssueCache.Issue issue = ProtobufIssueDiskCache.toProto(IssueCache.Issue.newBuilder(), defaultIssue); assertThat(issue.getImpactsList()).containsExactly( - toImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH), - toImpact(SoftwareQuality.RELIABILITY, Severity.LOW) - ); + toImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, true), + toImpact(SoftwareQuality.RELIABILITY, Severity.LOW, false)); } @Test @@ -124,8 +126,8 @@ public class ProtobufIssueDiskCacheTest { assertThat(issue.getCleanCodeAttribute()).isEqualTo(CleanCodeAttribute.FOCUSED.name()); } - private IssueCache.Impact toImpact(SoftwareQuality softwareQuality, Severity severity) { - return IssueCache.Impact.newBuilder().setSoftwareQuality(softwareQuality.name()).setSeverity(severity.name()).build(); + private IssueCache.Impact toImpact(SoftwareQuality softwareQuality, Severity severity, boolean manualSeverity) { + return IssueCache.Impact.newBuilder().setSoftwareQuality(softwareQuality.name()).setSeverity(severity.name()).setManualSeverity(manualSeverity).build(); } private static DefaultIssue createDefaultIssueWithMandatoryFields() { 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 8f78835585b..537b28db3a2 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 @@ -28,6 +28,7 @@ import java.io.Serializable; import java.util.Collection; import java.util.Date; import java.util.HashSet; +import java.util.LinkedHashSet; import java.util.Map; import java.util.Optional; import java.util.Set; @@ -36,7 +37,6 @@ import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.commons.lang3.builder.ToStringBuilder; import org.apache.commons.lang3.builder.ToStringStyle; -import org.jetbrains.annotations.NotNull; import org.sonar.api.issue.IssueStatus; import org.sonar.api.issue.impact.Severity; import org.sonar.api.issue.impact.SoftwareQuality; @@ -112,14 +112,14 @@ public final class IssueDto implements Serializable { // populate only when retrieving closed issue for issue tracking private String closedChangeData; - private Set impacts = new HashSet<>(); + private Set impacts = new LinkedHashSet<>(); - //non-persisted fields - private Set ruleDefaultImpacts = new HashSet<>(); + // non-persisted fields + private Set ruleDefaultImpacts = new LinkedHashSet<>(); private CleanCodeAttribute cleanCodeAttribute; private CleanCodeAttribute ruleCleanCodeAttribute; - //issues dependency fields, one-one relationship + // issues dependency fields, one-one relationship private String cveId; public IssueDto() { @@ -130,7 +130,7 @@ public final class IssueDto implements Serializable { * On batch side, component keys and uuid are useless */ public static IssueDto toDtoForComputationInsert(DefaultIssue issue, String ruleUuid, long now) { - return new IssueDto() + IssueDto issueDto = new IssueDto() .setKee(issue.key()) .setType(issue.type()) .setLine(issue.line()) @@ -162,20 +162,14 @@ public final class IssueDto implements Serializable { .setQuickFixAvailable(issue.isQuickFixAvailable()) .setIsNewCodeReferenceIssue(issue.isNewCodeReferenceIssue()) .setCodeVariants(issue.codeVariants()) - .replaceAllImpacts(mapToImpactDto(issue.impacts())) .setCleanCodeAttribute(issue.getCleanCodeAttribute()) // technical dates .setCreatedAt(now) .setUpdatedAt(now) .setCveId(issue.getCveId()); - } - @NotNull - private static Set mapToImpactDto(Map impacts) { - return impacts.entrySet().stream().map(e -> new ImpactDto() - .setSoftwareQuality(e.getKey()) - .setSeverity(e.getValue())) - .collect(Collectors.toSet()); + issue.getImpacts().forEach(i -> issueDto.addImpact(new ImpactDto(i.softwareQuality(), i.severity(), i.manualSeverity()))); + return issueDto; } /** @@ -189,7 +183,7 @@ public final class IssueDto implements Serializable { public static IssueDto toDtoForUpdate(DefaultIssue issue, long now) { // Invariant fields, like key and rule, can't be updated - return new IssueDto() + IssueDto issueDto = new IssueDto() .setKee(issue.key()) .setType(issue.type()) .setLine(issue.line()) @@ -220,11 +214,14 @@ public final class IssueDto implements Serializable { .setQuickFixAvailable(issue.isQuickFixAvailable()) .setIsNewCodeReferenceIssue(issue.isNewCodeReferenceIssue()) .setCodeVariants(issue.codeVariants()) - .replaceAllImpacts(mapToImpactDto(issue.impacts())) .setCleanCodeAttribute(issue.getCleanCodeAttribute()) .setPrioritizedRule(issue.isPrioritizedRule()) // technical date .setUpdatedAt(now); + + issue.getImpacts().forEach(i -> issueDto.addImpact(new ImpactDto(i.softwareQuality(), i.severity(), i.manualSeverity()))); + return issueDto; + } public String getKey() { @@ -833,13 +830,11 @@ public final class IssueDto implements Serializable { return this; } - public IssueDto setRuleDefaultImpacts(Set ruleDefaultImpacts) { this.ruleDefaultImpacts = new HashSet<>(ruleDefaultImpacts); return this; } - public IssueDto replaceAllImpacts(Collection newImpacts) { Set newSoftwareQuality = newImpacts.stream().map(ImpactDto::getSoftwareQuality).collect(Collectors.toSet()); if (newSoftwareQuality.size() != newImpacts.size()) { @@ -928,7 +923,7 @@ public final class IssueDto implements Serializable { issue.setIsNewCodeReferenceIssue(isNewCodeReferenceIssue); issue.setCodeVariants(getCodeVariants()); issue.setCleanCodeAttribute(cleanCodeAttribute); - impacts.forEach(i -> issue.addImpact(i.getSoftwareQuality(), i.getSeverity())); + impacts.forEach(i -> issue.addImpact(i.getSoftwareQuality(), i.getSeverity(), i.isManualSeverity())); issue.setCveId(cveId); return issue; } 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 a320ae28522..537a2d5e963 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 @@ -91,7 +91,8 @@ class IssueDtoTest { .setIssueUpdateDate(updatedAt) .setIssueCloseDate(closedAt) .setRuleDescriptionContextKey(TEST_CONTEXT_KEY) - .addImpact(new ImpactDto().setSoftwareQuality(MAINTAINABILITY).setSeverity(HIGH)); + .addImpact(new ImpactDto().setSoftwareQuality(MAINTAINABILITY).setSeverity(HIGH).setManualSeverity(true)) + .addImpact(new ImpactDto().setSoftwareQuality(RELIABILITY).setSeverity(LOW).setManualSeverity(false)); DefaultIssue expected = new DefaultIssue() .setKey("100") @@ -122,7 +123,8 @@ class IssueDtoTest { .setRuleDescriptionContextKey(TEST_CONTEXT_KEY) .setCodeVariants(Set.of()) .setTags(Set.of()) - .addImpact(MAINTAINABILITY, HIGH); + .addImpact(MAINTAINABILITY, HIGH, true) + .addImpact(RELIABILITY, LOW, false); DefaultIssue issue = dto.toDefaultIssue(); @@ -291,26 +293,26 @@ class IssueDtoTest { RuleType.BUG.getDbConstant(), RuleKey.of("repo", "rule")); assertThat(issueDto).extracting(IssueDto::getIssueCreationDate, IssueDto::getIssueCloseDate, - IssueDto::getIssueUpdateDate, IssueDto::getSelectedAt, IssueDto::getUpdatedAt, IssueDto::getCreatedAt) + IssueDto::getIssueUpdateDate, IssueDto::getSelectedAt, IssueDto::getUpdatedAt, IssueDto::getCreatedAt) .containsExactly(dateNow, dateNow, dateNow, dateNow.getTime(), now, now); assertThat(issueDto).extracting(IssueDto::getLine, IssueDto::getMessage, - IssueDto::getGap, IssueDto::getEffort, IssueDto::getResolution, IssueDto::getStatus, IssueDto::getSeverity) + IssueDto::getGap, IssueDto::getEffort, IssueDto::getResolution, IssueDto::getStatus, IssueDto::getSeverity) .containsExactly(1, "message", 1.0, 1L, Issue.RESOLUTION_FALSE_POSITIVE, Issue.STATUS_CLOSED, "BLOCKER"); assertThat(issueDto).extracting(IssueDto::getTags, IssueDto::getCodeVariants, IssueDto::getAuthorLogin) .containsExactly(Set.of("todo"), Set.of("variant1", "variant2"), "admin"); assertThat(issueDto).extracting(IssueDto::isManualSeverity, IssueDto::getChecksum, IssueDto::getAssigneeUuid, - IssueDto::isExternal, IssueDto::getComponentUuid, IssueDto::getComponentKey, - IssueDto::getProjectUuid, IssueDto::getProjectKey, IssueDto::getRuleUuid) + IssueDto::isExternal, IssueDto::getComponentUuid, IssueDto::getComponentKey, + IssueDto::getProjectUuid, IssueDto::getProjectKey, IssueDto::getRuleUuid) .containsExactly(true, "123", "123", true, "123", "componentKey", "123", "projectKey", "ruleUuid"); assertThat(issueDto.isQuickFixAvailable()).isTrue(); assertThat(issueDto.isNewCodeReferenceIssue()).isTrue(); assertThat(issueDto.getOptionalRuleDescriptionContextKey()).contains(TEST_CONTEXT_KEY); - assertThat(issueDto.getImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity) - .containsExactlyInAnyOrder(tuple(MAINTAINABILITY, HIGH), tuple(RELIABILITY, LOW)); + assertThat(issueDto.getImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity, ImpactDto::isManualSeverity) + .containsExactlyInAnyOrder(tuple(MAINTAINABILITY, HIGH, true), tuple(RELIABILITY, LOW, false)); } @Test @@ -325,25 +327,25 @@ class IssueDtoTest { RuleType.BUG.getDbConstant(), RuleKey.of("repo", "rule")); assertThat(issueDto).extracting(IssueDto::getIssueCreationDate, IssueDto::getIssueCloseDate, - IssueDto::getIssueUpdateDate, IssueDto::getSelectedAt, IssueDto::getUpdatedAt) + IssueDto::getIssueUpdateDate, IssueDto::getSelectedAt, IssueDto::getUpdatedAt) .containsExactly(dateNow, dateNow, dateNow, dateNow.getTime(), now); assertThat(issueDto).extracting(IssueDto::getLine, IssueDto::getMessage, - IssueDto::getGap, IssueDto::getEffort, IssueDto::getResolution, IssueDto::getStatus, IssueDto::getSeverity) + IssueDto::getGap, IssueDto::getEffort, IssueDto::getResolution, IssueDto::getStatus, IssueDto::getSeverity) .containsExactly(1, "message", 1.0, 1L, Issue.RESOLUTION_FALSE_POSITIVE, Issue.STATUS_CLOSED, "BLOCKER"); assertThat(issueDto).extracting(IssueDto::getTags, IssueDto::getCodeVariants, IssueDto::getAuthorLogin) .containsExactly(Set.of("todo"), Set.of("variant1", "variant2"), "admin"); assertThat(issueDto).extracting(IssueDto::isManualSeverity, IssueDto::getChecksum, IssueDto::getAssigneeUuid, - IssueDto::isExternal, IssueDto::getComponentUuid, IssueDto::getComponentKey, IssueDto::getProjectUuid, IssueDto::getProjectKey) + IssueDto::isExternal, IssueDto::getComponentUuid, IssueDto::getComponentKey, IssueDto::getProjectUuid, IssueDto::getProjectKey) .containsExactly(true, "123", "123", true, "123", "componentKey", "123", "projectKey"); assertThat(issueDto.isQuickFixAvailable()).isTrue(); assertThat(issueDto.isNewCodeReferenceIssue()).isTrue(); assertThat(issueDto.getOptionalRuleDescriptionContextKey()).contains(TEST_CONTEXT_KEY); - assertThat(issueDto.getImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity) - .containsExactlyInAnyOrder(tuple(MAINTAINABILITY, HIGH), tuple(RELIABILITY, LOW)); + assertThat(issueDto.getImpacts()).extracting(ImpactDto::getSoftwareQuality, ImpactDto::getSeverity, ImpactDto::isManualSeverity) + .containsExactlyInAnyOrder(tuple(MAINTAINABILITY, HIGH, true), tuple(RELIABILITY, LOW, false)); } @Test @@ -399,8 +401,8 @@ class IssueDtoTest { .setIsNewCodeReferenceIssue(true) .setRuleDescriptionContextKey(TEST_CONTEXT_KEY) .setCodeVariants(List.of("variant1", "variant2")) - .addImpact(MAINTAINABILITY, HIGH) - .addImpact(RELIABILITY, LOW); + .addImpact(MAINTAINABILITY, HIGH, true) + .addImpact(RELIABILITY, LOW, false); return defaultIssue; } 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 7e9e63543af..482fc0791a0 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 @@ -23,22 +23,19 @@ import com.google.common.base.Joiner; import java.time.temporal.ChronoUnit; import java.util.Collection; import java.util.Date; -import java.util.EnumMap; import java.util.HashSet; -import java.util.Map; import java.util.Objects; import java.util.Set; import java.util.stream.Collectors; import javax.annotation.Nullable; import org.sonar.api.ce.ComputeEngineSide; import org.sonar.api.issue.IssueStatus; -import org.sonar.api.issue.impact.Severity; -import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; import org.sonar.api.server.ServerSide; import org.sonar.api.server.rule.RuleTagFormat; import org.sonar.api.utils.Duration; +import org.sonar.core.issue.DefaultImpact; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.DefaultIssueComment; import org.sonar.core.issue.IssueChangeContext; @@ -262,7 +259,7 @@ public class IssueFieldsSetter { public boolean setIssueStatus(DefaultIssue issue, @Nullable IssueStatus previousIssueStatus, @Nullable IssueStatus newIssueStatus, IssueChangeContext context) { if (!Objects.equals(newIssueStatus, previousIssueStatus)) { - //Currently, issue status is not persisted in database, but is considered as an issue change + // Currently, issue status is not persisted in database, but is considered as an issue change issue.setFieldChange(context, ISSUE_STATUS, previousIssueStatus, issue.issueStatus()); return true; } @@ -346,8 +343,8 @@ public class IssueFieldsSetter { for (int i = 0; i < l1c.getMessageFormattingCount(); i++) { if (l1c.getMessageFormatting(i).getStart() != l2.getMessageFormatting(i).getStart() - || l1c.getMessageFormatting(i).getEnd() != l2.getMessageFormatting(i).getEnd() - || l1c.getMessageFormatting(i).getType() != l2.getMessageFormatting(i).getType()) { + || l1c.getMessageFormatting(i).getEnd() != l2.getMessageFormatting(i).getEnd() + || l1c.getMessageFormatting(i).getType() != l2.getMessageFormatting(i).getType()) { return false; } } @@ -372,7 +369,7 @@ public class IssueFieldsSetter { public void setPrioritizedRule(DefaultIssue issue, boolean prioritizedRule, IssueChangeContext context) { if (!Objects.equals(prioritizedRule, issue.isPrioritizedRule())) { issue.setPrioritizedRule(prioritizedRule); - if (!issue.isNew()){ + if (!issue.isNew()) { issue.setUpdateDate(context.date()); issue.setChanged(true); } @@ -463,14 +460,17 @@ public class IssueFieldsSetter { return false; } - public boolean setImpacts(DefaultIssue issue, Map previousImpacts, IssueChangeContext context) { - Map currentImpacts = new EnumMap<>(issue.impacts()); - if (!previousImpacts.equals(currentImpacts)) { - issue.replaceImpacts(currentImpacts); + public boolean setImpacts(DefaultIssue issue, Set previousImpacts, IssueChangeContext context) { + previousImpacts + .stream().filter(DefaultImpact::manualSeverity) + .forEach(i -> issue.addImpact(i.softwareQuality(), i.severity(), true)); + + if (!previousImpacts.equals(issue.getImpacts())) { issue.setUpdateDate(context.date()); issue.setChanged(true); return true; } + return false; } 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 85e08425108..6d80dba6d82 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 @@ -24,7 +24,6 @@ import java.util.Calendar; import java.util.Date; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Random; import java.util.Set; import org.apache.commons.lang3.time.DateUtils; @@ -36,6 +35,7 @@ import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rules.CleanCodeAttribute; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.Duration; +import org.sonar.core.issue.DefaultImpact; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.FieldDiffs; import org.sonar.core.issue.IssueChangeContext; @@ -577,13 +577,52 @@ class IssueFieldsSetterTest { @Test void setImpacts_whenImpactAdded_shouldBeUpdated() { - Map currentImpacts = Map.of(SoftwareQuality.RELIABILITY, Severity.LOW); - Map newImpacts = Map.of(SoftwareQuality.MAINTAINABILITY, Severity.HIGH); + Set currentImpacts = Set.of(new DefaultImpact(SoftwareQuality.RELIABILITY, Severity.LOW, false)); + Set newImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, false)); - issue.replaceImpacts(newImpacts); + newImpacts + .forEach(e -> issue.addImpact(e.softwareQuality(), e.severity(), e.manualSeverity())); boolean updated = underTest.setImpacts(issue, currentImpacts, context); assertThat(updated).isTrue(); - assertThat(issue.impacts()).isEqualTo(newImpacts); + assertThat(issue.getImpacts()).isEqualTo(newImpacts); + } + + @Test + void setImpacts_whenImpactExists_shouldNotBeUpdated() { + 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())); + boolean updated = underTest.setImpacts(issue, currentImpacts, context); + assertThat(updated).isFalse(); + assertThat(issue.getImpacts()).isEqualTo(newImpacts); + } + + @Test + void setImpacts_whenImpactExistsWithManualSeverity_shouldNotBeUpdated() { + Set currentImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, true)); + Set newImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, false)); + + newImpacts + .forEach(e -> issue.addImpact(e.softwareQuality(), e.severity(), e.manualSeverity())); + boolean updated = underTest.setImpacts(issue, currentImpacts, context); + assertThat(updated).isFalse(); + assertThat(issue.getImpacts()).isEqualTo(currentImpacts); + } + + @Test + void setImpacts_whenImpactExistsWithManualSeverityAndNewImpact_shouldBeUpdated() { + Set currentImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, true)); + Set newImpacts = Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, false), + new DefaultImpact(SoftwareQuality.RELIABILITY, Severity.HIGH, false)); + + newImpacts + .forEach(e -> issue.addImpact(e.softwareQuality(), e.severity(), e.manualSeverity())); + boolean updated = underTest.setImpacts(issue, currentImpacts, context); + assertThat(updated).isTrue(); + assertThat(issue.getImpacts()).isEqualTo(Set.of(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH, true), + new DefaultImpact(SoftwareQuality.RELIABILITY, Severity.HIGH, false))); } @Test diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultImpact.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultImpact.java new file mode 100644 index 00000000000..36ea21eca93 --- /dev/null +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultImpact.java @@ -0,0 +1,26 @@ +/* + * 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.issue; + +import org.sonar.api.issue.impact.Severity; +import org.sonar.api.issue.impact.SoftwareQuality; + +public record DefaultImpact(SoftwareQuality softwareQuality, Severity severity, boolean manualSeverity) { +} diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java index b8c46a299af..dc4fb271e3d 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java @@ -28,14 +28,15 @@ import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.Date; -import java.util.EnumMap; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.List; import java.util.Map; import java.util.Objects; import java.util.Optional; import java.util.Set; +import java.util.stream.Collectors; import javax.annotation.CheckForNull; import javax.annotation.Nullable; import org.apache.commons.lang3.StringUtils; @@ -137,7 +138,7 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. private String anticipatedTransitionUuid = null; - private Map impacts = new EnumMap<>(SoftwareQuality.class); + private final Map impacts = new LinkedHashMap<>(); private CleanCodeAttribute cleanCodeAttribute = null; private String cveId = null; @@ -159,17 +160,26 @@ public class DefaultIssue implements Issue, Trackable, org.sonar.api.ce.measure. @Override public Map impacts() { - return impacts; + return impacts.values().stream().collect(Collectors.toMap(DefaultImpact::softwareQuality, DefaultImpact::severity)); + } + + public Set getImpacts() { + return new LinkedHashSet<>(this.impacts.values()); } public DefaultIssue replaceImpacts(Map impacts) { this.impacts.clear(); - this.impacts.putAll(impacts); + this.impacts.putAll(impacts.entrySet().stream().collect(Collectors.toMap(Map.Entry::getKey, e -> new DefaultImpact(e.getKey(), e.getValue(), false)))); return this; } public DefaultIssue addImpact(SoftwareQuality softwareQuality, org.sonar.api.issue.impact.Severity severity) { - impacts.put(softwareQuality, severity); + impacts.put(softwareQuality, new DefaultImpact(softwareQuality, severity, false)); + return this; + } + + public DefaultIssue addImpact(SoftwareQuality softwareQuality, org.sonar.api.issue.impact.Severity severity, boolean manualSeverity) { + impacts.put(softwareQuality, new DefaultImpact(softwareQuality, severity, manualSeverity)); return this; } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java index a50efdc2ef4..ad935a1f7ad 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java @@ -323,6 +323,14 @@ class DefaultIssueTest { assertThat(issue.impacts()).containsExactlyEntriesOf(Map.of(SoftwareQuality.SECURITY, Severity.LOW)); } + @Test + void addImpact_shouldReplaceExistingSoftwareQuality() { + issue.addImpact(SoftwareQuality.MAINTAINABILITY, Severity.HIGH); + issue.addImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, true); + assertThat(issue.getImpacts()) + .containsExactly(new DefaultImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, true)); + } + @Test void prioritizedRule_shouldHaveCorrectDefaultValue() { assertThat(issue.isPrioritizedRule()).isFalse(); -- 2.39.5