From f66ba87a15895cccce22321e0f874056821bd419 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 20 Dec 2019 14:02:59 +0100 Subject: [PATCH] SONAR-12718 api/hotspots/* WS fail on closed hotspots --- .../server/hotspot/ws/HotspotWsSupport.java | 2 + .../hotspot/ws/AddCommentActionTest.java | 22 ++++++++-- .../server/hotspot/ws/AssignActionTest.java | 41 +++++++++++++------ .../hotspot/ws/ChangeStatusActionTest.java | 22 ++++++++++ .../server/hotspot/ws/ShowActionTest.java | 17 +++++++- 5 files changed, 87 insertions(+), 17 deletions(-) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java index 8420f32ceef..30ea1243303 100644 --- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java +++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/HotspotWsSupport.java @@ -20,6 +20,7 @@ package org.sonar.server.hotspot.ws; import java.util.Date; +import org.sonar.api.issue.Issue; import org.sonar.api.rules.RuleType; import org.sonar.api.utils.System2; import org.sonar.core.issue.IssueChangeContext; @@ -51,6 +52,7 @@ public class HotspotWsSupport { IssueDto loadHotspot(DbSession dbSession, String hotspotKey) { return dbClient.issueDao().selectByKey(dbSession, hotspotKey) .filter(t -> t.getType() == RuleType.SECURITY_HOTSPOT.getDbConstant()) + .filter(t -> !Issue.STATUS_CLOSED.equals(t.getStatus())) .orElseThrow(() -> new NotFoundException(format("Hotspot '%s' does not exist", hotspotKey))); } diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AddCommentActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AddCommentActionTest.java index 7f8d2eb8207..e22f345a62b 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AddCommentActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AddCommentActionTest.java @@ -42,7 +42,6 @@ import org.sonar.db.DbSession; import org.sonar.db.DbTester; import org.sonar.db.component.ComponentDto; import org.sonar.db.issue.IssueDto; -import org.sonar.db.issue.IssueTesting; import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.db.rule.RuleTesting; import org.sonar.server.exceptions.ForbiddenException; @@ -64,10 +63,12 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.RESOLUTION_SAFE; +import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.api.issue.Issue.STATUS_REVIEWED; import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; import static org.sonar.db.component.ComponentTesting.newFileDto; +import static org.sonar.db.issue.IssueTesting.newIssue; @RunWith(DataProviderRunner.class) public class AddCommentActionTest { @@ -143,7 +144,7 @@ public class AddCommentActionTest { ComponentDto project = dbTester.components().insertPublicProject(); ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); RuleDefinitionDto rule = newRule(ruleType); - IssueDto notAHotspot = dbTester.issues().insertIssue(IssueTesting.newIssue(rule, project, file).setType(ruleType)); + IssueDto notAHotspot = dbTester.issues().insertIssue(newIssue(rule, project, file).setType(ruleType)); userSessionRule.logIn(); TestRequest request = newRequest(notAHotspot, randomAlphabetic(12)); @@ -160,6 +161,21 @@ public class AddCommentActionTest { .toArray(Object[][]::new); } + @Test + public void fails_with_NotFoundException_if_hotspot_is_closed() { + ComponentDto project = dbTester.components().insertPublicProject(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto notAHotspot = dbTester.issues().insertIssue(newIssue(rule, project, file) + .setType(SECURITY_HOTSPOT).setStatus(STATUS_CLOSED)); + userSessionRule.logIn(); + TestRequest request = newRequest(notAHotspot, randomAlphabetic(12)); + + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class) + .hasMessage("Hotspot '%s' does not exist", notAHotspot.getKey()); + } + @Test public void fails_with_ForbiddenException_if_project_is_private_and_not_allowed() { ComponentDto project = dbTester.components().insertPrivateProject(); @@ -239,7 +255,7 @@ public class AddCommentActionTest { } private static IssueDto newHotspot(ComponentDto project, ComponentDto file, RuleDefinitionDto rule) { - return IssueTesting.newIssue(rule, project, file).setType(SECURITY_HOTSPOT); + return newIssue(rule, project, file).setType(SECURITY_HOTSPOT); } private TestRequest newRequest(IssueDto hotspot, String comment) { diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AssignActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AssignActionTest.java index 71d0d1e151c..5cde732f5ea 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AssignActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/AssignActionTest.java @@ -66,6 +66,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.STATUSES; +import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; import static org.sonar.db.component.ComponentTesting.newFileDto; @@ -246,7 +247,7 @@ public class AssignActionTest { } @Test - @UseDataProvider("allIssueStatusesExceptToReview") + @UseDataProvider("allIssueStatusesExceptToReviewAndClosed") public void fail_if_assign_user_to_hotspot_for_OTHER_STATUSES_for_public_project(String status) { ComponentDto project = dbTester.components().insertPublicProject(); @@ -263,7 +264,7 @@ public class AssignActionTest { } @Test - @UseDataProvider("allIssueStatusesExceptToReview") + @UseDataProvider("allIssueStatusesExceptToReviewAndClosed") public void fail_if_assign_user_to_hotspot_for_OTHER_STATUSES_for_private_project(String status) { ComponentDto project = dbTester.components().insertPrivateProject(); ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); @@ -278,6 +279,15 @@ public class AssignActionTest { .hasMessage("Assignee can only be changed on Security Hotspots with status 'TO_REVIEW'"); } + @DataProvider + public static Object[][] allIssueStatusesExceptToReviewAndClosed() { + return STATUSES.stream() + .filter(status -> !STATUS_TO_REVIEW.equals(status)) + .filter(status -> !STATUS_CLOSED.equals(status)) + .map(status -> new Object[] {status}) + .toArray(Object[][]::new); + } + @Test public void fail_if_not_authenticated() { ComponentDto project = dbTester.components().insertPublicProject(); @@ -328,13 +338,11 @@ public class AssignActionTest { public void fail_if_trying_to_assign_issue(RuleType ruleType, String status) { ComponentDto project = dbTester.components().insertPublicProject(); ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); - RuleDefinitionDto rule = newRule(ruleType); IssueDto issue = dbTester.issues().insertIssue( newIssue(rule, project, file) .setStatus(status) .setType(ruleType)); - UserDto me = insertUser(randomAlphanumeric(10)); userSessionRule.logIn().registerComponents(project); @@ -343,14 +351,6 @@ public class AssignActionTest { .hasMessage("Hotspot '%s' does not exist", issue.getKey()); } - @DataProvider - public static Object[][] allIssueStatusesExceptToReview() { - return STATUSES.stream() - .filter(status -> !STATUS_TO_REVIEW.equals(status)) - .map(status -> new Object[] {status}) - .toArray(Object[][]::new); - } - @DataProvider public static Object[][] allRuleTypesWithStatusesExceptHotspot() { Set ruleTypes = EnumSet.allOf(RuleType.class) @@ -367,6 +367,23 @@ public class AssignActionTest { .toArray(Object[][]::new); } + @Test + public void fail_with_NotFoundException_if_hotspot_is_closed() { + ComponentDto project = dbTester.components().insertPublicProject(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto issue = dbTester.issues().insertIssue( + newIssue(rule, project, file) + .setStatus(STATUS_CLOSED) + .setType(SECURITY_HOTSPOT)); + UserDto me = insertUser(randomAlphanumeric(10)); + userSessionRule.logIn().registerComponents(project); + + assertThatThrownBy(() -> executeRequest(issue, me.getLogin(), null)) + .isInstanceOf(NotFoundException.class) + .hasMessage("Hotspot '%s' does not exist", issue.getKey()); + } + private void verifyFieldSetters(UserDto assignee, @Nullable String comment) { ArgumentCaptor defaultIssueCaptor = ArgumentCaptor.forClass(DefaultIssue.class); short capturedArgsCount = 0; diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ChangeStatusActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ChangeStatusActionTest.java index eb613ae5bbb..5417c830535 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ChangeStatusActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ChangeStatusActionTest.java @@ -70,6 +70,7 @@ import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; import static org.sonar.api.issue.Issue.RESOLUTION_SAFE; +import static org.sonar.api.issue.Issue.STATUS_CLOSED; import static org.sonar.api.issue.Issue.STATUS_REVIEWED; import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; @@ -234,6 +235,27 @@ public class ChangeStatusActionTest { .hasMessage("Hotspot '%s' does not exist", key); } + @Test + @UseDataProvider("validStatusAndResolutions") + public void fails_with_NotFoundException_if_hotspot_is_closed(String status, @Nullable String resolution) { + ComponentDto project = dbTester.components().insertPublicProject(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto closedHotspot = dbTester.issues().insertIssue(IssueTesting.newIssue(rule, project, file) + .setType(SECURITY_HOTSPOT).setStatus(STATUS_CLOSED)); + userSessionRule.logIn(); + TestRequest request = actionTester.newRequest() + .setParam("hotspot", closedHotspot.getKey()) + .setParam("status", status); + if (resolution != null) { + request.setParam("resolution", resolution); + } + + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class) + .hasMessage("Hotspot '%s' does not exist", closedHotspot.getKey()); + } + @DataProvider public static Object[][] validStatusAndResolutions() { return new Object[][] { diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java index 1e73526a84d..78fbcd362ac 100644 --- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java +++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/ShowActionTest.java @@ -160,6 +160,20 @@ public class ShowActionTest { .toArray(Object[][]::new); } + @Test + public void fails_with_NotFoundException_if_issue_is_hotspot_is_called() { + ComponentDto project = dbTester.components().insertPublicProject(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + IssueDto notAHotspot = dbTester.issues().insertIssue(IssueTesting.newIssue(rule, project, file) + .setType(SECURITY_HOTSPOT).setStatus(Issue.STATUS_CLOSED)); + TestRequest request = newRequest(notAHotspot); + + assertThatThrownBy(request::execute) + .isInstanceOf(NotFoundException.class) + .hasMessage("Hotspot '%s' does not exist", notAHotspot.getKey()); + } + @Test public void fails_with_ForbiddenException_if_project_is_private_and_not_allowed() { ComponentDto project = dbTester.components().insertPrivateProject(); @@ -232,8 +246,7 @@ public class ShowActionTest { return new Object[][] { {Issue.STATUS_TO_REVIEW, null}, {Issue.STATUS_REVIEWED, Issue.RESOLUTION_FIXED}, - {Issue.STATUS_REVIEWED, Issue.RESOLUTION_SAFE}, - {Issue.STATUS_CLOSED, Issue.RESOLUTION_REMOVED} + {Issue.STATUS_REVIEWED, Issue.RESOLUTION_SAFE} }; } -- 2.39.5