From d6ecbc0f912a8951ebfaabf8e803239db15eff3c Mon Sep 17 00:00:00 2001 From: OrlovAlexander Date: Fri, 8 Nov 2024 12:24:10 +0100 Subject: [PATCH] SONAR-23363 Update set_severity endpoint --- .../sonar/server/issue/IssueFieldsSetter.java | 31 +++ .../server/issue/IssueFieldsSetterTest.java | 45 ++++ .../server/issue/ws/SetSeverityActionIT.java | 234 +++++++++++++++--- .../server/issue/ws/SetSeverityAction.java | 110 +++++++- .../ws/client/issue/IssuesWsParameters.java | 1 + 5 files changed, 383 insertions(+), 38 deletions(-) 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 3711d2f439e..fada386bada 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 @@ -24,12 +24,15 @@ import java.time.temporal.ChronoUnit; import java.util.Collection; import java.util.Date; 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; @@ -84,6 +87,7 @@ public class IssueFieldsSetter { public static final String LINE = "line"; public static final String TAGS = "tags"; public static final String CODE_VARIANTS = "code_variants"; + public static final String IMPACT_SEVERITY = "impactSeverity"; private static final Joiner CHANGELOG_LIST_JOINER = Joiner.on(" ").skipNulls(); @@ -116,6 +120,9 @@ public class IssueFieldsSetter { return setSeverity(issue, currentSeverity, context); } + /** + * @return true if the 'severity' or 'manualSeverity' flag has been changed, else return false + */ public boolean setManualSeverity(DefaultIssue issue, String severity, IssueChangeContext context) { if (!issue.manualSeverity() || !Objects.equals(severity, issue.severity())) { issue.setFieldChange(context, SEVERITY, issue.severity(), severity); @@ -129,6 +136,30 @@ public class IssueFieldsSetter { return false; } + /** + * @return true if the 'severity' or 'manualSeverity' flag of the Impact has been changed, else return false + */ + public boolean setImpactManualSeverity(DefaultIssue issue, SoftwareQuality softwareQuality, Severity severity, + IssueChangeContext context) { + Map oldImpacts = issue.impacts(); + if ((oldImpacts.containsKey(softwareQuality) + && (!Objects.equals(oldImpacts.get(softwareQuality), severity) || !hasManualSeverity(issue, softwareQuality)))) { + issue.addImpact(softwareQuality, severity, true); + issue.setFieldChange(context, IMPACT_SEVERITY, + softwareQuality + ":" + oldImpacts.get(softwareQuality), + softwareQuality + ":" + severity); + issue.setUpdateDate(context.date()); + issue.setChanged(true); + issue.setSendNotifications(true); + return true; + } + return false; + } + + private static boolean hasManualSeverity(DefaultIssue issue, SoftwareQuality softwareQuality) { + return issue.getImpacts().stream().filter(i -> i.softwareQuality().equals(softwareQuality)).anyMatch(DefaultImpact::manualSeverity); + } + public boolean assign(DefaultIssue issue, @Nullable UserDto user, IssueChangeContext context) { String assigneeUuid = user != null ? user.getUuid() : null; if (!Objects.equals(assigneeUuid, issue.assignee())) { 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 2695654c10b..58e957eae74 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,6 +24,7 @@ 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; @@ -51,6 +52,7 @@ import static org.sonar.core.issue.IssueChangeContext.issueChangeContextByUserBu import static org.sonar.db.protobuf.DbIssues.MessageFormattingType.CODE; import static org.sonar.db.user.UserTesting.newUserDto; import static org.sonar.server.issue.IssueFieldsSetter.ASSIGNEE; +import static org.sonar.server.issue.IssueFieldsSetter.IMPACT_SEVERITY; import static org.sonar.server.issue.IssueFieldsSetter.SEVERITY; import static org.sonar.server.issue.IssueFieldsSetter.STATUS; import static org.sonar.server.issue.IssueFieldsSetter.TECHNICAL_DEBT; @@ -240,6 +242,49 @@ class IssueFieldsSetterTest { assertThat(issue.mustSendNotifications()).isFalse(); } + @Test + void set_manual_impact_severity_whenExistingImpactMatchesRequested_shouldUpdateManualImpactSeverity() { + issue.replaceImpacts(Map.of(SoftwareQuality.MAINTAINABILITY, Severity.LOW)); + boolean updated = underTest.setImpactManualSeverity(issue, SoftwareQuality.MAINTAINABILITY, Severity.BLOCKER, context); + + assertThat(updated).isTrue(); + issue.getImpacts().stream().filter(impact -> impact.softwareQuality().equals(SoftwareQuality.MAINTAINABILITY)).forEach(impact -> { + assertThat(impact.severity()).isEqualTo(Severity.BLOCKER); + assertThat(impact.manualSeverity()).isTrue(); + }); + FieldDiffs.Diff diff = issue.currentChange().get(IMPACT_SEVERITY); + assertThat(diff.oldValue()).isEqualTo("MAINTAINABILITY:LOW"); + assertThat(diff.newValue()).isEqualTo("MAINTAINABILITY:BLOCKER"); + } + + @Test + void set_manual_impact_severity_whenExistingImpactHasSameSeverityButNoManualFlag_shouldUpdateManualImpactSeverity() { + issue.replaceImpacts(Map.of(SoftwareQuality.MAINTAINABILITY, Severity.LOW)); + boolean updated = underTest.setImpactManualSeverity(issue, SoftwareQuality.MAINTAINABILITY, Severity.LOW, context); + + assertThat(updated).isTrue(); + issue.getImpacts().stream().filter(impact -> impact.softwareQuality().equals(SoftwareQuality.MAINTAINABILITY)).forEach(impact -> { + assertThat(impact.severity()).isEqualTo(Severity.LOW); + assertThat(impact.manualSeverity()).isTrue(); + }); + assertThat(issue.currentChange()).isNull(); + } + + @Test + void set_manual_impact_severity_whenExistingImpactHasDifferentSeverity_shouldUpdateManualImpactSeverity() { + issue.addImpact(SoftwareQuality.MAINTAINABILITY, Severity.LOW, true); + boolean updated = underTest.setImpactManualSeverity(issue, SoftwareQuality.MAINTAINABILITY, Severity.BLOCKER, context); + + assertThat(updated).isTrue(); + issue.getImpacts().stream().filter(impact -> impact.softwareQuality().equals(SoftwareQuality.MAINTAINABILITY)).forEach(impact -> { + assertThat(impact.severity()).isEqualTo(Severity.BLOCKER); + assertThat(impact.manualSeverity()).isTrue(); + }); + FieldDiffs.Diff diff = issue.currentChange().get(IMPACT_SEVERITY); + assertThat(diff.oldValue()).isEqualTo("MAINTAINABILITY:LOW"); + assertThat(diff.newValue()).isEqualTo("MAINTAINABILITY:BLOCKER"); + } + @Test void unset_line() { int line = 1 + new Random().nextInt(500); diff --git a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/issue/ws/SetSeverityActionIT.java b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/issue/ws/SetSeverityActionIT.java index 68ee1ca0a8f..d8b532d7965 100644 --- a/server/sonar-webserver-webapi/src/it/java/org/sonar/server/issue/ws/SetSeverityActionIT.java +++ b/server/sonar-webserver-webapi/src/it/java/org/sonar/server/issue/ws/SetSeverityActionIT.java @@ -20,10 +20,13 @@ package org.sonar.server.issue.ws; import java.util.List; +import java.util.Set; import javax.annotation.Nullable; -import org.junit.Rule; -import org.junit.Test; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; import org.mockito.ArgumentCaptor; +import org.sonar.api.issue.impact.Severity; +import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.server.ws.Request; import org.sonar.api.server.ws.Response; import org.sonar.api.server.ws.WebService; @@ -35,6 +38,7 @@ import org.sonar.db.DbTester; import org.sonar.db.component.BranchDto; import org.sonar.db.component.BranchType; import org.sonar.db.component.ComponentDto; +import org.sonar.db.issue.ImpactDto; import org.sonar.db.issue.IssueDbTester; import org.sonar.db.issue.IssueDto; import org.sonar.db.project.ProjectDto; @@ -67,23 +71,25 @@ import static org.assertj.core.api.Assertions.assertThatThrownBy; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoInteractions; import static org.sonar.api.rule.Severity.MAJOR; import static org.sonar.api.rule.Severity.MINOR; +import static org.sonar.api.rules.RuleType.BUG; import static org.sonar.api.rules.RuleType.CODE_SMELL; import static org.sonar.api.web.UserRole.ISSUE_ADMIN; import static org.sonar.api.web.UserRole.USER; import static org.sonar.db.component.ComponentTesting.newFileDto; import static org.sonar.db.issue.IssueTesting.newIssue; -public class SetSeverityActionIT { +class SetSeverityActionIT { - @Rule + @RegisterExtension public DbTester dbTester = DbTester.create(); - @Rule + @RegisterExtension public EsTester es = EsTester.create(); - @Rule + @RegisterExtension public UserSessionRule userSession = UserSessionRule.standalone(); private System2 system2 = mock(System2.class); @@ -105,11 +111,11 @@ public class SetSeverityActionIT { responseWriter)); @Test - public void set_severity() { - IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR)); + void set_severity_whenSetSeverity_shouldAlsoUpdateImpactSeverity() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR).setType(CODE_SMELL)); setUserWithBrowseAndAdministerIssuePermission(issueDto); - call(issueDto.getKey(), MINOR); + call(issueDto.getKey(), MINOR, null); verify(responseWriter).write(eq(issueDto.getKey()), preloadedSearchResponseDataCaptor.capture(), any(Request.class), any(Response.class), eq(true)); verifyContentOfPreloadedSearchResponseData(issueDto); @@ -118,13 +124,176 @@ public class SetSeverityActionIT { IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); assertThat(issueReloaded.getSeverity()).isEqualTo(MINOR); assertThat(issueReloaded.isManualSeverity()).isTrue(); + Set impacts = issueReloaded.getImpacts(); + assertThat(impacts).hasSize(1); + ImpactDto impact = impacts.iterator().next(); + assertThat(impact.getSoftwareQuality()).isEqualTo(SoftwareQuality.MAINTAINABILITY); + assertThat(impact.getSeverity()).isEqualTo(Severity.LOW); + assertThat(impact.isManualSeverity()).isTrue(); assertThat(issueChangePostProcessor.calledComponents()) .extracting(ComponentDto::uuid) .containsExactlyInAnyOrder(issueDto.getComponentUuid()); } @Test - public void set_severity_is_not_distributed_for_pull_request() { + void set_severity_whenSetSeverityAndTypeNotMatch_shouldNotUpdateImpactSeverity() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR).setType(BUG)); + setUserWithBrowseAndAdministerIssuePermission(issueDto); + + call(issueDto.getKey(), MINOR, null); + + verify(responseWriter).write(eq(issueDto.getKey()), preloadedSearchResponseDataCaptor.capture(), any(Request.class), + any(Response.class), eq(true)); + verifyContentOfPreloadedSearchResponseData(issueDto); + verify(issueChangeEventService).distributeIssueChangeEvent(any(), any(), any(), any(), any(), any()); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getSeverity()).isEqualTo(MINOR); + assertThat(issueReloaded.isManualSeverity()).isTrue(); + + Set impacts = issueReloaded.getImpacts(); + assertThat(impacts).hasSize(1); + ImpactDto impact = impacts.iterator().next(); + assertThat(impact.getSoftwareQuality()).isEqualTo(SoftwareQuality.MAINTAINABILITY); + assertThat(impact.getSeverity()).isEqualTo(Severity.HIGH); + assertThat(impact.isManualSeverity()).isFalse(); + } + + @Test + void set_severity_whenIssueHasNuImpacts_shouldProperlyUpdate() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR).setType(CODE_SMELL).replaceAllImpacts(List.of())); + setUserWithBrowseAndAdministerIssuePermission(issueDto); + + call(issueDto.getKey(), null, "MAINTAINABILITY=LOW"); + + verify(responseWriter).write(eq(issueDto.getKey()), preloadedSearchResponseDataCaptor.capture(), any(Request.class), + any(Response.class), eq(true)); + verifyContentOfPreloadedSearchResponseData(issueDto); + verify(issueChangeEventService).distributeIssueChangeEvent(any(), any(), any(), any(), any(), any()); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getSeverity()).isEqualTo(MINOR); + assertThat(issueReloaded.isManualSeverity()).isTrue(); + Set impacts = issueReloaded.getImpacts(); + assertThat(impacts).hasSize(1); + ImpactDto impact = impacts.iterator().next(); + assertThat(impact.getSoftwareQuality()).isEqualTo(SoftwareQuality.MAINTAINABILITY); + assertThat(impact.getSeverity()).isEqualTo(Severity.LOW); + assertThat(impact.isManualSeverity()).isTrue(); + assertThat(issueChangePostProcessor.calledComponents()) + .extracting(ComponentDto::uuid) + .containsExactlyInAnyOrder(issueDto.getComponentUuid()); + } + @Test + void set_severity_whenSetImpactSeverity_shouldAlsoUpdateSeverity() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR).setType(CODE_SMELL)); + setUserWithBrowseAndAdministerIssuePermission(issueDto); + + call(issueDto.getKey(), null, "MAINTAINABILITY=LOW"); + + verify(responseWriter).write(eq(issueDto.getKey()), preloadedSearchResponseDataCaptor.capture(), any(Request.class), + any(Response.class), eq(true)); + verifyContentOfPreloadedSearchResponseData(issueDto); + verify(issueChangeEventService).distributeIssueChangeEvent(any(), any(), any(), any(), any(), any()); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getSeverity()).isEqualTo(MINOR); + assertThat(issueReloaded.isManualSeverity()).isTrue(); + Set impacts = issueReloaded.getImpacts(); + assertThat(impacts).hasSize(1); + ImpactDto impact = impacts.iterator().next(); + assertThat(impact.getSoftwareQuality()).isEqualTo(SoftwareQuality.MAINTAINABILITY); + assertThat(impact.getSeverity()).isEqualTo(Severity.LOW); + assertThat(impact.isManualSeverity()).isTrue(); + assertThat(issueChangePostProcessor.calledComponents()) + .extracting(ComponentDto::uuid) + .containsExactlyInAnyOrder(issueDto.getComponentUuid()); + } + + @Test + void set_severity_whenIssueTypeNotMatchSoftwareQuality_shouldNotUpdateImpactSeverity() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR).setType(BUG)); + setUserWithBrowseAndAdministerIssuePermission(issueDto); + + call(issueDto.getKey(), null, "MAINTAINABILITY=LOW"); + + verify(responseWriter).write(eq(issueDto.getKey()), preloadedSearchResponseDataCaptor.capture(), any(Request.class), + any(Response.class), eq(true)); + verifyContentOfPreloadedSearchResponseData(issueDto); + verify(issueChangeEventService).distributeIssueChangeEvent(any(), any(), any(), any(), any(), any()); + + IssueDto issueReloaded = dbClient.issueDao().selectByKey(dbTester.getSession(), issueDto.getKey()).get(); + assertThat(issueReloaded.getSeverity()).isEqualTo(MAJOR); + assertThat(issueReloaded.isManualSeverity()).isFalse(); + Set impacts = issueReloaded.getImpacts(); + assertThat(impacts).hasSize(1); + ImpactDto impact = impacts.iterator().next(); + assertThat(impact.getSoftwareQuality()).isEqualTo(SoftwareQuality.MAINTAINABILITY); + assertThat(impact.getSeverity()).isEqualTo(Severity.LOW); + assertThat(impact.isManualSeverity()).isTrue(); + assertThat(issueChangePostProcessor.calledComponents()) + .extracting(ComponentDto::uuid) + .containsExactlyInAnyOrder(issueDto.getComponentUuid()); + } + + @Test + void set_severity_whenImpactAndManualFlagNotChanged_shouldNotTriggerUpdate() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.replaceAllImpacts(List.of(new ImpactDto(SoftwareQuality.MAINTAINABILITY, + Severity.LOW, true)))); + setUserWithBrowseAndAdministerIssuePermission(issueDto); + + call(issueDto.getKey(), null, "MAINTAINABILITY=LOW"); + + verify(responseWriter).write(eq(issueDto.getKey()), preloadedSearchResponseDataCaptor.capture(), any(Request.class), + any(Response.class), eq(true)); + + verify(issueChangeEventService, times(0)).distributeIssueChangeEvent(any(), any(), any(), any(), any(), any()); + } + + @Test + void set_severity_whenWrongSoftwareQualityReceived_shouldThrowException() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR)); + setUserWithBrowseAndAdministerIssuePermission(issueDto); + String key = issueDto.getKey(); + + assertThatThrownBy(() -> call(key, null, "SECURITY=HIGH")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Issue does not support impact SECURITY"); + } + + @Test + void set_severity_whenSeverityAndImpactsReceived_shouldThrowException() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR).setType(CODE_SMELL)); + setUserWithBrowseAndAdministerIssuePermission(issueDto); + String key = issueDto.getKey(); + + assertThatThrownBy(() -> call(key, "BLOCKER", "MAINTAINABILITY=LOW")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Parameters 'severity' and 'impact' cannot be used at the same time"); + } + + @Test + void set_severity_whenWrongImpactFormat_shouldThrowException() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR).setType(CODE_SMELL)); + setUserWithBrowseAndAdministerIssuePermission(issueDto); + String key = issueDto.getKey(); + + assertThatThrownBy(() -> call(key, null, "MAINTAINABILITY")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid impact format: MAINTAINABILITY"); + assertThatThrownBy(() -> call(key, null, "MAINTAINABILITY=HIGH,RELIABILITY=LOW")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid impact format: MAINTAINABILITY=HIGH,RELIABILITY=LOW"); + assertThatThrownBy(() -> call(key, null, "MAINTAINABILITY:HIGH")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Invalid impact format: MAINTAINABILITY:HIGH"); + assertThatThrownBy(() -> call(key, null, "MAINTAINABILITY=MAJOR")) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("No enum constant org.sonar.api.issue.impact.Severity.MAJOR"); + } + + @Test + void set_severity_is_not_distributed_for_pull_request() { RuleDto rule = dbTester.rules().insertIssueRule(); ComponentDto mainBranch = dbTester.components().insertPrivateProject().getMainBranchComponent(); @@ -138,84 +307,87 @@ public class SetSeverityActionIT { setUserWithBrowseAndAdministerIssuePermission(issue); - call(issue.getKey(), MINOR); + call(issue.getKey(), MINOR, null); verifyNoInteractions(issueChangeEventService); } @Test - public void insert_entry_in_changelog_when_setting_severity() { - IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR)); + void insert_entry_in_changelog_when_setting_severity() { + IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity(MAJOR).setType(CODE_SMELL)); setUserWithBrowseAndAdministerIssuePermission(issueDto); - call(issueDto.getKey(), MINOR); + call(issueDto.getKey(), MINOR, null); List fieldDiffs = dbClient.issueChangeDao().selectChangelogByIssue(dbTester.getSession(), issueDto.getKey()); assertThat(fieldDiffs).hasSize(1); - assertThat(fieldDiffs.get(0).diffs()).hasSize(1); + assertThat(fieldDiffs.get(0).diffs()).hasSize(2); assertThat(fieldDiffs.get(0).diffs().get("severity").newValue()).isEqualTo(MINOR); assertThat(fieldDiffs.get(0).diffs().get("severity").oldValue()).isEqualTo(MAJOR); + assertThat(fieldDiffs.get(0).diffs().get("impactSeverity").oldValue()).isEqualTo("MAINTAINABILITY:HIGH"); + assertThat(fieldDiffs.get(0).diffs().get("impactSeverity").newValue()).isEqualTo("MAINTAINABILITY:LOW"); } @Test - public void fail_if_bad_severity() { + void fail_if_bad_severity() { IssueDto issueDto = issueDbTester.insertIssue(i -> i.setSeverity("unknown")); setUserWithBrowseAndAdministerIssuePermission(issueDto); - - assertThatThrownBy(() -> call(issueDto.getKey(), "unknown")) + String key = issueDto.getKey(); + assertThatThrownBy(() -> call(key, "unknown", null)) .isInstanceOf(IllegalArgumentException.class) .hasMessage("Value of parameter 'severity' (unknown) must be one of: [INFO, MINOR, MAJOR, CRITICAL, BLOCKER]"); } @Test - public void fail_NFE_if_hotspot() { + void fail_NFE_if_hotspot() { IssueDto hotspot = issueDbTester.insertHotspot(h -> h.setSeverity("CRITICAL")); setUserWithBrowseAndAdministerIssuePermission(hotspot); String hotspotKey = hotspot.getKey(); - assertThatThrownBy(() -> call(hotspotKey, "MAJOR")) + assertThatThrownBy(() -> call(hotspotKey, "MAJOR", null)) .isInstanceOf(NotFoundException.class) .hasMessage("Issue with key '%s' does not exist", hotspotKey); } @Test - public void fail_when_not_authenticated() { - assertThatThrownBy(() -> call("ABCD", MAJOR)) + void fail_when_not_authenticated() { + assertThatThrownBy(() -> call("ABCD", MAJOR, null)) .isInstanceOf(UnauthorizedException.class); } @Test - public void fail_when_missing_browse_permission() { + void fail_when_missing_browse_permission() { IssueDto issueDto = issueDbTester.insertIssue(); logInAndAddProjectPermission(issueDto, ISSUE_ADMIN); - - assertThatThrownBy(() -> call(issueDto.getKey(), MAJOR)) + String key = issueDto.getKey(); + assertThatThrownBy(() -> call(key, MAJOR, null)) .isInstanceOf(ForbiddenException.class); } @Test - public void fail_when_missing_administer_issue_permission() { + void fail_when_missing_administer_issue_permission() { IssueDto issueDto = issueDbTester.insertIssue(); logInAndAddProjectPermission(issueDto, USER); - - assertThatThrownBy(() -> call(issueDto.getKey(), MAJOR)) + String key = issueDto.getKey(); + assertThatThrownBy(() -> call(key, MAJOR, null)) .isInstanceOf(ForbiddenException.class); } @Test - public void test_definition() { + void test_definition() { WebService.Action action = tester.getDef(); assertThat(action.key()).isEqualTo("set_severity"); assertThat(action.isPost()).isTrue(); assertThat(action.isInternal()).isFalse(); - assertThat(action.params()).hasSize(2); + assertThat(action.params()).hasSize(3); assertThat(action.responseExample()).isNotNull(); } - private TestResponse call(@Nullable String issueKey, @Nullable String severity) { + private TestResponse call(@Nullable String issueKey, @Nullable String severity, @Nullable String impact) { TestRequest request = tester.newRequest(); ofNullable(issueKey).ifPresent(issue -> request.setParam("issue", issue)); ofNullable(severity).ifPresent(value -> request.setParam("severity", value)); + ofNullable(impact).ifPresent(value -> request.setParam("impact", value)); return request.execute(); } diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SetSeverityAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SetSeverityAction.java index 13bbde31fd2..f973d24a00c 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SetSeverityAction.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/issue/ws/SetSeverityAction.java @@ -21,6 +21,10 @@ package org.sonar.server.issue.ws; import com.google.common.io.Resources; import java.util.Date; +import java.util.Map; +import javax.annotation.Nullable; +import org.apache.commons.lang3.tuple.Pair; +import org.sonar.api.issue.impact.SoftwareQuality; import org.sonar.api.rule.Severity; import org.sonar.api.server.ws.Change; import org.sonar.api.server.ws.Request; @@ -32,16 +36,22 @@ import org.sonar.core.util.Uuids; import org.sonar.db.DbClient; import org.sonar.db.DbSession; import org.sonar.db.component.BranchDto; +import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; import org.sonar.server.issue.IssueFieldsSetter; import org.sonar.server.issue.IssueFinder; import org.sonar.server.pushapi.issues.IssueChangeEventService; import org.sonar.server.user.UserSession; +import static org.sonar.api.server.rule.internal.ImpactMapper.convertToDeprecatedSeverity; +import static org.sonar.api.server.rule.internal.ImpactMapper.convertToRuleType; +import static org.sonar.api.server.rule.internal.ImpactMapper.convertToSoftwareQuality; import static org.sonar.api.web.UserRole.ISSUE_ADMIN; import static org.sonar.core.issue.IssueChangeContext.issueChangeContextByUserBuilder; +import static org.sonar.core.rule.ImpactSeverityMapper.mapImpactSeverity; import static org.sonar.db.component.BranchType.BRANCH; import static org.sonarqube.ws.client.issue.IssuesWsParameters.ACTION_SET_SEVERITY; +import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_IMPACT; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_ISSUE; import static org.sonarqube.ws.client.issue.IssuesWsParameters.PARAM_SEVERITY; @@ -79,6 +89,9 @@ public class SetSeverityAction implements IssuesWsAction { "") .setSince("3.6") .setChangelog( + new Change("10.8", "Add 'impact' parameter to the request."), + new Change("10.8", "Parameter 'severity' is now optional."), + new Change("10.8", "This endpoint is not deprecated anymore."), new Change("10.4", "The response fields 'status' and 'resolution' are deprecated. Please use 'issueStatus' instead."), new Change("10.4", "Add 'issueStatus' field to the response."), new Change("10.2", "This endpoint is now deprecated."), @@ -88,7 +101,6 @@ public class SetSeverityAction implements IssuesWsAction { new Change("6.5", "the database ids of the components are removed from the response"), new Change("6.5", "the response field components.uuid is deprecated. Use components.key instead.")) .setHandler(this) - .setDeprecatedSince("10.2") .setResponseExample(Resources.getResource(this.getClass(), "set_severity-example.json")) .setPost(true); @@ -98,37 +110,121 @@ public class SetSeverityAction implements IssuesWsAction { .setExampleValue(Uuids.UUID_EXAMPLE_01); action.createParam(PARAM_SEVERITY) .setDescription("New severity") - .setRequired(true) + .setRequired(false) .setPossibleValues(Severity.ALL); + action.createParam(PARAM_IMPACT) + .setDescription("Override of impact severity for the rule. Cannot be used as the same time as 'severity'") + .setRequired(false) + .setExampleValue("impact=MAINTAINABILITY=HIGH"); } @Override public void handle(Request request, Response response) throws Exception { userSession.checkLoggedIn(); String issueKey = request.mandatoryParam(PARAM_ISSUE); - String severity = request.mandatoryParam(PARAM_SEVERITY); + String severity = request.param(PARAM_SEVERITY); + String impact = request.param(PARAM_IMPACT); + checkParams(severity, impact); + try (DbSession session = dbClient.openSession(false)) { - SearchResponseData preloadedSearchResponseData = setType(session, issueKey, severity); + SearchResponseData preloadedSearchResponseData = setSeverity(session, issueKey, impact, severity); responseWriter.write(issueKey, preloadedSearchResponseData, request, response, true); } } - private SearchResponseData setType(DbSession session, String issueKey, String severity) { + private SearchResponseData setSeverity(DbSession session, String issueKey, @Nullable String impact, @Nullable String severity) { IssueDto issueDto = issueFinder.getByKey(session, issueKey); DefaultIssue issue = issueDto.toDefaultIssue(); userSession.checkComponentUuidPermission(ISSUE_ADMIN, issue.projectUuid()); IssueChangeContext context = issueChangeContextByUserBuilder(new Date(), userSession.getUuid()).withRefreshMeasures().build(); - if (issueFieldsSetter.setManualSeverity(issue, severity, context)) { + SearchResponseData response; + if (severity != null) { + response = setManualSeverity(session, issue, issueDto, severity, context); + } else { + response = setManualImpact(session, issue, issueDto, impact, context); + } + return response; + } + + private SearchResponseData setManualImpact(DbSession session, DefaultIssue issue, IssueDto issueDto, String impact, + IssueChangeContext context) { + Map effectiveImpacts = issueDto.getEffectiveImpacts(); + Pair manualImpact = parseImpact(impact); + + org.sonar.api.issue.impact.Severity manualImpactSeverity = manualImpact.getValue(); + SoftwareQuality softwareQuality = manualImpact.getKey(); + + if (!effectiveImpacts.containsKey(softwareQuality)) { + throw new IllegalArgumentException("Issue does not support impact " + softwareQuality); + } + createImpactsIfMissing(issue, effectiveImpacts); + if (issueFieldsSetter.setImpactManualSeverity(issue, softwareQuality, manualImpactSeverity, context)) { + String manualSeverity = null; + boolean severityUpdated = false; + if (convertToRuleType(softwareQuality).equals(issue.type())) { + manualSeverity = convertToDeprecatedSeverity(manualImpactSeverity); + severityUpdated = issueFieldsSetter.setManualSeverity(issue, manualSeverity, context); + } BranchDto branch = issueUpdater.getBranch(session, issue); SearchResponseData response = issueUpdater.saveIssueAndPreloadSearchResponseData(session, issueDto, issue, context, branch); + if (branch.getBranchType().equals(BRANCH) && response.getComponentByUuid(issue.projectUuid()) != null) { + issueChangeEventService.distributeIssueChangeEvent(issue, severityUpdated ? manualSeverity : null, null, null, + branch, getProjectKey(issue, response)); + } + return response; + } + return new SearchResponseData(issueDto); + } + private SearchResponseData setManualSeverity(DbSession session, DefaultIssue issue, IssueDto issueDto, String severity, + IssueChangeContext context) { + if (issueFieldsSetter.setManualSeverity(issue, severity, context)) { + SoftwareQuality softwareQuality = convertToSoftwareQuality(issue.type()); + if (issueDto.getEffectiveImpacts().containsKey(softwareQuality)) { + createImpactsIfMissing(issue, issueDto.getEffectiveImpacts()); + issueFieldsSetter.setImpactManualSeverity(issue, softwareQuality, mapImpactSeverity(severity), context); + } + BranchDto branch = issueUpdater.getBranch(session, issue); + SearchResponseData response = issueUpdater.saveIssueAndPreloadSearchResponseData(session, issueDto, issue, context, branch); if (branch.getBranchType().equals(BRANCH) && response.getComponentByUuid(issue.projectUuid()) != null) { issueChangeEventService.distributeIssueChangeEvent(issue, severity, null, null, - branch, response.getComponentByUuid(issue.projectUuid()).getKey()); + branch, getProjectKey(issue, response)); } return response; } return new SearchResponseData(issueDto); } + + private static String getProjectKey(DefaultIssue issue, SearchResponseData response) { + ComponentDto componentByUuid = response.getComponentByUuid(issue.projectUuid()); + if (componentByUuid == null) { + throw new IllegalStateException("Component with uuid " + issue.projectUuid() + " not found"); + } + return componentByUuid.getKey(); + } + + private static void createImpactsIfMissing(DefaultIssue issue, Map effectiveImpacts) { + if(issue.impacts().isEmpty()){ + issue.replaceImpacts(effectiveImpacts); + issue.setChanged(true); + } + } + + private static void checkParams(@Nullable String severity, @Nullable String impact) { + if (severity != null && impact != null) { + throw new IllegalArgumentException("Parameters 'severity' and 'impact' cannot be used at the same time"); + } else if (severity == null && impact == null) { + throw new IllegalArgumentException("One of the parameters 'severity' or 'impact' must be provided"); + } + } + + private static Pair parseImpact(String impact) { + String[] parts = impact.split("="); + if (parts.length != 2) { + throw new IllegalArgumentException("Invalid impact format: " + impact); + } + return Pair.of(SoftwareQuality.valueOf(parts[0]), + org.sonar.api.issue.impact.Severity.valueOf(parts[1])); + } } diff --git a/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java b/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java index 4f8856d5c23..ff6eef18c2c 100644 --- a/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java +++ b/sonar-ws/src/main/java/org/sonarqube/ws/client/issue/IssuesWsParameters.java @@ -44,6 +44,7 @@ public class IssuesWsParameters { public static final String ACTION_ANTICIPATED_TRANSITIONS = "anticipated_transitions"; public static final String PARAM_ISSUE = "issue"; + public static final String PARAM_IMPACT = "impact"; public static final String PARAM_COMMENT = "comment"; public static final String PARAM_TEXT = "text"; public static final String PARAM_ASSIGNEE = "assignee"; -- 2.39.5