SONAR-12751 add status and resolution params to api/hotspots/search

also added resolution field to the response

diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java
index 250278316d80312b2afbd5892e2dc34e2b7b9b6f..884560065690bce6da4e78679f9251e8fcb97687 100644 (file)
--- a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java
+++ b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java
@@ -31,6 +31,7 @@ import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
+import javax.annotation.Nullable;
 import org.elasticsearch.action.search.SearchResponse;
 import org.elasticsearch.search.SearchHit;
 import org.sonar.api.resources.Qualifiers;
@@ -57,7 +58,14 @@ import org.sonarqube.ws.Common;
 import org.sonarqube.ws.Hotspots;
 import org.sonarqube.ws.Hotspots.SearchWsResponse;
 
+import static com.google.common.base.Preconditions.checkArgument;
+import static java.lang.String.format;
+import static java.util.Collections.singleton;
 import static java.util.Optional.ofNullable;
+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_REVIEWED;
+import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW;
 import static org.sonar.api.server.ws.WebService.Param.PAGE;
 import static org.sonar.api.server.ws.WebService.Param.PAGE_SIZE;
 import static org.sonar.api.utils.DateUtils.formatDateTime;
@@ -71,6 +79,8 @@ import static org.sonarqube.ws.WsUtils.nullToEmpty;
 
 public class SearchAction implements HotspotsWsAction {
   private static final String PARAM_PROJECT_KEY = "projectKey";
+  private static final String PARAM_STATUS = "status";
+  private static final String PARAM_RESOLUTION = "resolution";
 
   private final DbClient dbClient;
   private final UserSession userSession;
@@ -99,6 +109,16 @@ public class SearchAction implements HotspotsWsAction {
       .setDescription("Key of the project")
       .setExampleValue(KEY_PROJECT_EXAMPLE_001)
       .setRequired(true);
+    action.createParam(PARAM_STATUS)
+      .setDescription("If provided, only Security Hotspots with the specified status are returned.")
+      .setPossibleValues(STATUS_TO_REVIEW, STATUS_REVIEWED)
+      .setRequired(false);
+    action.createParam(PARAM_RESOLUTION)
+      .setDescription(format(
+        "If provided and if status is '%s', only Security Hotspots with the specified resolution are returned.",
+        STATUS_REVIEWED))
+      .setPossibleValues(RESOLUTION_FIXED, RESOLUTION_SAFE)
+      .setRequired(false);
     // FIXME add response example and test it
     // action.setResponseExample()
   }
@@ -117,15 +137,24 @@ public class SearchAction implements HotspotsWsAction {
   }
 
   private static WsRequest toWsRequest(Request request) {
-    return new WsRequest(request.mandatoryParamAsInt(PAGE), request.mandatoryParamAsInt(PAGE_SIZE), request.mandatoryParam(PARAM_PROJECT_KEY));
+    return new WsRequest(
+      request.mandatoryParamAsInt(PAGE), request.mandatoryParamAsInt(PAGE_SIZE),
+      request.mandatoryParam(PARAM_PROJECT_KEY),
+      request.param(PARAM_STATUS), request.param(PARAM_RESOLUTION));
   }
 
   private ComponentDto validateRequest(DbSession dbSession, WsRequest wsRequest) {
+    wsRequest.getResolution().ifPresent(
+      resolution -> checkArgument(wsRequest.getStatus().filter(STATUS_REVIEWED::equals).isPresent(),
+        "Value '%s' of parameter '%s' can only be provided if value of parameter '%s' is '%s'",
+        resolution, PARAM_RESOLUTION, PARAM_STATUS, STATUS_REVIEWED)
+    );
+
     ComponentDto project = dbClient.componentDao().selectByKey(dbSession, wsRequest.getProjectKey())
       .filter(t -> Scopes.PROJECT.equals(t.scope()) && Qualifiers.PROJECT.equals(t.qualifier()))
       .filter(ComponentDto::isEnabled)
       .filter(t -> t.getMainBranchProjectUuid() == null)
-      .orElseThrow(() -> new NotFoundException(String.format("Project '%s' not found", wsRequest.getProjectKey())));
+      .orElseThrow(() -> new NotFoundException(format("Project '%s' not found", wsRequest.getProjectKey())));
     userSession.checkComponentPermission(UserRole.USER, project);
     return project;
   }
@@ -158,10 +187,12 @@ public class SearchAction implements HotspotsWsAction {
     IssueQuery.Builder builder = IssueQuery.builder()
       .projectUuids(Collections.singletonList(project.uuid()))
       .organizationUuid(project.getOrganizationUuid())
-      .types(Collections.singleton(RuleType.SECURITY_HOTSPOT.name()))
-      .resolved(false)
+      .types(singleton(RuleType.SECURITY_HOTSPOT.name()))
       .sort(IssueQuery.SORT_HOTSPOTS)
       .asc(true);
+    wsRequest.getStatus().ifPresent(status -> builder.resolved(STATUS_REVIEWED.equals(status)));
+    wsRequest.getResolution().ifPresent(resolution -> builder.resolutions(singleton(resolution)));
+
     IssueQuery query = builder.build();
     SearchOptions searchOptions = new SearchOptions()
       .setPage(wsRequest.page, wsRequest.index);
@@ -218,7 +249,7 @@ public class SearchAction implements HotspotsWsAction {
     for (IssueDto hotspot : orderedHotspots) {
       RuleDefinitionDto rule = searchResponseData.getRule(hotspot.getRuleKey())
         // due to join with table Rule when retrieving data from Issues, this can't happen
-        .orElseThrow(() -> new IllegalStateException(String.format(
+        .orElseThrow(() -> new IllegalStateException(format(
           "Rule with key '%s' not found for Hotspot '%s'", hotspot.getRuleKey(), hotspot.getKey())));
       SecurityStandards.SQCategory sqCategory = fromSecurityStandards(rule.getSecurityStandards()).getSqCategory();
       builder
@@ -229,15 +260,11 @@ public class SearchAction implements HotspotsWsAction {
         .setSecurityCategory(sqCategory.getKey())
         .setVulnerabilityProbability(sqCategory.getVulnerability().name());
       ofNullable(hotspot.getStatus()).ifPresent(builder::setStatus);
-      // FIXME resolution field will be added later
-      // ofNullable(hotspot.getResolution()).ifPresent(builder::setResolution);
+      ofNullable(hotspot.getResolution()).ifPresent(builder::setResolution);
       ofNullable(hotspot.getLine()).ifPresent(builder::setLine);
       builder.setMessage(nullToEmpty(hotspot.getMessage()));
       ofNullable(hotspot.getAssigneeUuid()).ifPresent(builder::setAssignee);
-      // FIXME Filter author only if user is member of the organization (as done in issues/search WS)
-      // if (data.getUserOrganizationUuids().contains(component.getOrganizationUuid())) {
       builder.setAuthor(nullToEmpty(hotspot.getAuthorLogin()));
-      // }
       builder.setCreationDate(formatDateTime(hotspot.getIssueCreationDate()));
       builder.setUpdateDate(formatDateTime(hotspot.getIssueUpdateDate()));
 
@@ -261,11 +288,15 @@ public class SearchAction implements HotspotsWsAction {
     private final int page;
     private final int index;
     private final String projectKey;
+    private final String status;
+    private final String resolution;
 
-    private WsRequest(int page, int index, String projectKey) {
+    private WsRequest(int page, int index, String projectKey, @Nullable String status, @Nullable String resolution) {
       this.page = page;
       this.index = index;
       this.projectKey = projectKey;
+      this.status = status;
+      this.resolution = resolution;
     }
 
     int getPage() {
@@ -279,6 +310,14 @@ public class SearchAction implements HotspotsWsAction {
     String getProjectKey() {
       return projectKey;
     }
+
+    Optional<String> getStatus() {
+      return ofNullable(status);
+    }
+
+    Optional<String> getResolution() {
+      return ofNullable(resolution);
+    }
   }
 
   private static final class SearchResponseData {

diff --git a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java
index d1a1fc45ef9de7f94861684a9806ee6c4e30ff64..e0cf3bd3611bb18f92b4b70d15b3723946314d52 100644 (file)
--- a/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java
+++ b/server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java
@@ -35,6 +35,7 @@ import java.util.function.Consumer;
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 import java.util.stream.Stream;
+import javax.annotation.Nullable;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -73,6 +74,10 @@ import static java.util.stream.Collectors.toSet;
 import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
+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_REVIEWED;
+import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW;
 import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT;
 import static org.sonar.api.utils.DateUtils.formatDateTime;
 import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex;
@@ -116,6 +121,76 @@ public class SearchActionTest {
       .hasMessage("The 'projectKey' parameter is missing");
   }
 
+  @Test
+  @UseDataProvider("badStatuses")
+  public void fails_with_IAE_if_status_parameter_is_neither_TO_REVIEW_or_REVIEWED(String badStatus) {
+    TestRequest request = actionTester.newRequest()
+      .setParam("projectKey", randomAlphabetic(13))
+      .setParam("status", badStatus);
+
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Value of parameter 'status' (" + badStatus + ") must be one of: [TO_REVIEW, REVIEWED]");
+  }
+
+  @DataProvider
+  public static Object[][] badStatuses() {
+    return Stream.concat(
+      Issue.STATUSES.stream(),
+      Stream.of(randomAlphabetic(3)))
+      .filter(t -> !STATUS_REVIEWED.equals(t))
+      .filter(t -> !STATUS_TO_REVIEW.equals(t))
+      .map(t -> new Object[] {t})
+      .toArray(Object[][]::new);
+  }
+
+  @Test
+  @UseDataProvider("badResolutions")
+  public void fails_with_IAE_if_resolution_parameter_is_neither_FIXED_nor_SAFE(String badResolution) {
+    TestRequest request = actionTester.newRequest()
+      .setParam("projectKey", randomAlphabetic(13))
+      .setParam("status", STATUS_TO_REVIEW)
+      .setParam("resolution", badResolution);
+
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Value of parameter 'resolution' (" + badResolution + ") must be one of: [FIXED, SAFE]");
+  }
+
+  @DataProvider
+  public static Object[][] badResolutions() {
+    return Stream.of(
+      Issue.RESOLUTIONS.stream(),
+      Issue.SECURITY_HOTSPOT_RESOLUTIONS.stream(),
+      Stream.of(randomAlphabetic(4)))
+      .flatMap(t -> t)
+      .filter(t -> !RESOLUTION_FIXED.equals(t))
+      .filter(t -> !RESOLUTION_SAFE.equals(t))
+      .map(t -> new Object[] {t})
+      .toArray(Object[][]::new);
+  }
+
+  @Test
+  @UseDataProvider("fixedOrSafeResolution")
+  public void fails_with_IAE_if_resolution_is_provided_with_status_TO_REVIEW(String resolution) {
+    TestRequest request = actionTester.newRequest()
+      .setParam("projectKey", randomAlphabetic(13))
+      .setParam("status", STATUS_TO_REVIEW)
+      .setParam("resolution", resolution);
+
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Value '" + resolution + "' of parameter 'resolution' can only be provided if value of parameter 'status' is 'REVIEWED'");
+  }
+
+  @DataProvider
+  public static Object[][] fixedOrSafeResolution() {
+    return new Object[][] {
+      {RESOLUTION_SAFE},
+      {RESOLUTION_FIXED}
+    };
+  }
+
   @Test
   public void fails_with_NotFoundException_if_project_does_not_exist() {
     String key = randomAlphabetic(12);
@@ -321,7 +396,82 @@ public class SearchActionTest {
   }
 
   @Test
-  public void returns_only_unresolved_hotspots() {
+  public void returns_hotpots_with_any_status_if_no_status_nor_resolution_parameter() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    indexPermissions();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    List<IssueDto> hotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file)
+      .collect(Collectors.toList());
+    indexIssues();
+
+    SearchWsResponse response = newRequest(project, null, null)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsExactlyInAnyOrder(hotspots.stream().map(IssueDto::getKey).toArray(String[]::new));
+  }
+
+  @Test
+  public void returns_hotpots_reviewed_as_safe_and_fixed_if_status_is_REVIEWED_and_resolution_is_not_set() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    indexPermissions();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    List<IssueDto> reviewedHotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file)
+      .filter(t -> STATUS_REVIEWED.equals(t.getStatus()))
+      .collect(Collectors.toList());
+    indexIssues();
+
+    SearchWsResponse response = newRequest(project, STATUS_REVIEWED, null)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsExactlyInAnyOrder(reviewedHotspots.stream().map(IssueDto::getKey).toArray(String[]::new));
+  }
+
+  @Test
+  public void returns_hotpots_reviewed_as_safe_if_status_is_REVIEWED_and_resolution_is_SAFE() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    indexPermissions();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    List<IssueDto> safeHotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file)
+      .filter(t -> STATUS_REVIEWED.equals(t.getStatus()) && RESOLUTION_SAFE.equals(t.getResolution()))
+      .collect(Collectors.toList());
+    indexIssues();
+
+    SearchWsResponse response = newRequest(project, STATUS_REVIEWED, RESOLUTION_SAFE)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsExactlyInAnyOrder(safeHotspots.stream().map(IssueDto::getKey).toArray(String[]::new));
+  }
+
+  @Test
+  public void returns_hotpots_reviewed_as_fixed_if_status_is_REVIEWED_and_resolution_is_FIXED() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    indexPermissions();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    List<IssueDto> fixedHotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file)
+      .filter(t -> STATUS_REVIEWED.equals(t.getStatus()) && RESOLUTION_FIXED.equals(t.getResolution()))
+      .collect(Collectors.toList());
+    indexIssues();
+
+    SearchWsResponse response = newRequest(project, STATUS_REVIEWED, RESOLUTION_FIXED)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsExactlyInAnyOrder(fixedHotspots.stream().map(IssueDto::getKey).toArray(String[]::new));
+  }
+
+  @Test
+  public void returns_only_unresolved_hotspots_when_status_is_TO_REVIEW() {
     ComponentDto project = dbTester.components().insertPublicProject();
     userSessionRule.registerComponents(project);
     indexPermissions();
@@ -336,7 +486,7 @@ public class SearchActionTest {
       t -> t.setType(SECURITY_HOTSPOT).setResolution(randomAlphabetic(5)));
     indexIssues();
 
-    SearchWsResponse response = newRequest(project)
+    SearchWsResponse response = newRequest(project, STATUS_TO_REVIEW, null)
       .executeProtobuf(SearchWsResponse.class);
 
     assertThat(response.getHotspotsList())
@@ -344,8 +494,28 @@ public class SearchActionTest {
       .containsOnly(unresolvedHotspot.getKey(), badlyClosedHotspot.getKey());
   }
 
+  private Stream<IssueDto> insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(ComponentDto project, ComponentDto file) {
+    RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+    List<IssueDto> hotspots = Arrays.stream(validStatusesAndResolutions())
+      .flatMap(objects -> {
+        String status = (String) objects[0];
+        String resolution = (String) objects[1];
+        return IntStream.range(0, 1 + RANDOM.nextInt(15))
+          .mapToObj(i -> newIssue(rule, project, file)
+            .setKee("hotspot_" + status + "_" + resolution + "_" + i)
+            .setType(SECURITY_HOTSPOT)
+            .setStatus(status)
+            .setResolution(resolution));
+      })
+      .collect(Collectors.toList());
+    Collections.shuffle(hotspots);
+    hotspots.forEach(t -> dbTester.issues().insertIssue(t));
+    return hotspots.stream();
+  }
+
   @Test
-  public void returns_fields_of_hotspot() {
+  @UseDataProvider("validStatusesAndResolutions")
+  public void returns_fields_of_hotspot(String status, @Nullable String resolution) {
     ComponentDto project = dbTester.components().insertPublicProject();
     userSessionRule.registerComponents(project);
     indexPermissions();
@@ -357,7 +527,9 @@ public class SearchActionTest {
         .setLine(RANDOM.nextInt(230))
         .setMessage(randomAlphabetic(10))
         .setAssigneeUuid(randomAlphabetic(9))
-        .setAuthorLogin(randomAlphabetic(8)));
+        .setAuthorLogin(randomAlphabetic(8))
+        .setStatus(status)
+        .setResolution(resolution));
     indexIssues();
 
     SearchWsResponse response = newRequest(project)
@@ -367,9 +539,12 @@ public class SearchActionTest {
     SearchWsResponse.Hotspot actual = response.getHotspots(0);
     assertThat(actual.getComponent()).isEqualTo(file.getKey());
     assertThat(actual.getProject()).isEqualTo(project.getKey());
-    assertThat(actual.getStatus()).isEqualTo(hotspot.getStatus());
-    // FIXME resolution field will be added later
-    // assertThat(actual.getResolution()).isEqualTo(hotspot.getResolution());
+    assertThat(actual.getStatus()).isEqualTo(status);
+    if (resolution == null) {
+      assertThat(actual.hasResolution()).isFalse();
+    } else {
+      assertThat(actual.getResolution()).isEqualTo(resolution);
+    }
     assertThat(actual.getLine()).isEqualTo(hotspot.getLine());
     assertThat(actual.getMessage()).isEqualTo(hotspot.getMessage());
     assertThat(actual.getAssignee()).isEqualTo(hotspot.getAssigneeUuid());
@@ -378,6 +553,15 @@ public class SearchActionTest {
     assertThat(actual.getUpdateDate()).isEqualTo(formatDateTime(hotspot.getIssueUpdateDate()));
   }
 
+  @DataProvider
+  public static Object[][] validStatusesAndResolutions() {
+    return new Object[][] {
+      {STATUS_TO_REVIEW, null},
+      {STATUS_REVIEWED, RESOLUTION_FIXED},
+      {STATUS_REVIEWED, RESOLUTION_SAFE},
+    };
+  }
+
   @Test
   @UseDataProvider("allSQCategories")
   public void returns_SQCategory_and_VulnerabilityProbability_of_rule(Set<String> securityStandards, SQCategory expected) {
@@ -422,8 +606,7 @@ public class SearchActionTest {
     dbTester.issues().insert(rule, project, file,
       t -> t.setType(SECURITY_HOTSPOT)
         .setStatus(null)
-        // FIXME resolution field will be added later
-        // .setResolution(null)
+        .setResolution(null)
         .setLine(null)
         .setMessage(null)
         .setAssigneeUuid(null)
@@ -437,8 +620,7 @@ public class SearchActionTest {
       .hasSize(1);
     SearchWsResponse.Hotspot actual = response.getHotspots(0);
     assertThat(actual.hasStatus()).isFalse();
-    // FIXME resolution field will be added later
-    // assertThat(actual.hasResolution()).isFalse();
+    assertThat(actual.hasResolution()).isFalse();
     assertThat(actual.hasLine()).isFalse();
     assertThat(actual.getMessage()).isEmpty();
     assertThat(actual.hasAssignee()).isFalse();
@@ -657,8 +839,19 @@ public class SearchActionTest {
   }
 
   private TestRequest newRequest(ComponentDto project) {
-    return actionTester.newRequest()
+    return newRequest(project, null, null);
+  }
+
+  private TestRequest newRequest(ComponentDto project, @Nullable String status, @Nullable String resolution) {
+    TestRequest res = actionTester.newRequest()
       .setParam("projectKey", project.getKey());
+    if (status != null) {
+      res.setParam("status", status);
+    }
+    if (resolution != null) {
+      res.setParam("resolution", resolution);
+    }
+    return res;
   }
 
   private void indexPermissions() {

diff --git a/sonar-ws/src/main/protobuf/ws-hotspots.proto b/sonar-ws/src/main/protobuf/ws-hotspots.proto
index d67bc02ef6d58e20cc5967412bf3bedf2a1c4f79..4d4f4efce302118c8e127e4900d83f5788ce8b42 100644 (file)
--- a/sonar-ws/src/main/protobuf/ws-hotspots.proto
+++ b/sonar-ws/src/main/protobuf/ws-hotspots.proto
@@ -39,8 +39,7 @@ message SearchWsResponse {
     optional string securityCategory = 4;
     optional string vulnerabilityProbability = 5;
     optional string status = 6;
-  //  FIXME resolution field will be added later
-  //  optional string resolution = 7;
+    optional string resolution = 7;
     optional int32 line = 8;
     optional string message = 9;
     optional string assignee = 10;