SONAR-12718 api/hotspots/* WS fail on closed hotspots

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)));
   }
 

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) {

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<RuleType> 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<DefaultIssue> defaultIssueCaptor = ArgumentCaptor.forClass(DefaultIssue.class);
     short capturedArgsCount = 0;

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[][] {

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}
     };
   }