SONAR-12721 add pullRequest and branch parameters api/hotspots/search

server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java

@@ -22,7 +22,6 @@ package org.sonar.server.hotspot.ws;
 import com.google.common.collect.ImmutableSet;
 import java.util.Arrays;
 import java.util.Collection;
-import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -63,6 +62,7 @@ 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.Collections.singletonList;
 import static java.util.Optional.ofNullable;
 import static org.sonar.api.issue.Issue.RESOLUTION_FIXED;
 import static org.sonar.api.issue.Issue.RESOLUTION_SAFE;
@@ -75,7 +75,9 @@ import static org.sonar.api.utils.Paging.forPageIndex;
 import static org.sonar.core.util.stream.MoreCollectors.toList;
 import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex;
 import static org.sonar.server.security.SecurityStandards.fromSecurityStandards;
+import static org.sonar.server.ws.KeyExamples.KEY_BRANCH_EXAMPLE_001;
 import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001;
+import static org.sonar.server.ws.KeyExamples.KEY_PULL_REQUEST_EXAMPLE_001;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
 import static org.sonarqube.ws.WsUtils.nullToEmpty;
 
@@ -84,6 +86,8 @@ public class SearchAction implements HotspotsWsAction {
   private static final String PARAM_STATUS = "status";
   private static final String PARAM_RESOLUTION = "resolution";
   private static final String PARAM_HOTSPOTS = "hotspots";
+  private static final String PARAM_BRANCH = "branch";
+  private static final String PARAM_PULL_REQUEST = "pullRequest";
 
   private final DbClient dbClient;
   private final UserSession userSession;
@@ -113,6 +117,14 @@ public class SearchAction implements HotspotsWsAction {
         "Key of the project. This parameter is required unless %s is provided.",
         PARAM_HOTSPOTS))
       .setExampleValue(KEY_PROJECT_EXAMPLE_001);
+    action.createParam(PARAM_BRANCH)
+      .setDescription("Branch key")
+      .setExampleValue(KEY_BRANCH_EXAMPLE_001)
+      .setInternal(true);
+    action.createParam(PARAM_PULL_REQUEST)
+      .setDescription("Pull request id")
+      .setExampleValue(KEY_PULL_REQUEST_EXAMPLE_001)
+      .setInternal(true);
     action.createParam(PARAM_HOTSPOTS)
       .setDescription(format(
         "Comma-separated list of Security Hotspot keys. This parameter is required unless %s is provided.",
@@ -151,38 +163,64 @@ public class SearchAction implements HotspotsWsAction {
     Set<String> hotspotKeys = hotspotKeysList == null ? ImmutableSet.of() : hotspotKeysList.stream().collect(MoreCollectors.toSet(hotspotKeysList.size()));
     return new WsRequest(
       request.mandatoryParamAsInt(PAGE), request.mandatoryParamAsInt(PAGE_SIZE),
-      request.param(PARAM_PROJECT_KEY), hotspotKeys,
+      request.param(PARAM_PROJECT_KEY), request.param(PARAM_BRANCH), request.param(PARAM_PULL_REQUEST),
+      hotspotKeys,
       request.param(PARAM_STATUS), request.param(PARAM_RESOLUTION));
   }
 
   private static void validateParameters(WsRequest wsRequest) {
+    Optional<String> projectKey = wsRequest.getProjectKey();
+    Optional<String> branch = wsRequest.getBranch();
+    Optional<String> pullRequest = wsRequest.getPullRequest();
+    Set<String> hotspotKeys = wsRequest.getHotspotKeys();
+
     checkArgument(
-      wsRequest.getProjectKey().isPresent() || !wsRequest.getHotspotKeys().isEmpty(),
+      projectKey.isPresent() || !hotspotKeys.isEmpty(),
       "A value must be provided for either parameter '%s' or parameter '%s'", PARAM_PROJECT_KEY, PARAM_HOTSPOTS);
 
-    checkArgument(!wsRequest.getStatus().isPresent() || wsRequest.getHotspotKeys().isEmpty(),
+    checkArgument(
+      !branch.isPresent() || projectKey.isPresent(),
+      "Parameter '%s' must be used with parameter '%s'", PARAM_BRANCH, PARAM_PROJECT_KEY);
+    checkArgument(
+      !pullRequest.isPresent() || projectKey.isPresent(),
+      "Parameter '%s' must be used with parameter '%s'", PARAM_PULL_REQUEST, PARAM_PROJECT_KEY);
+    checkArgument(
+      !(branch.isPresent() && pullRequest.isPresent()),
+      "Only one of parameters '%s' and '%s' can be provided", PARAM_BRANCH, PARAM_PULL_REQUEST);
+
+    Optional<String> status = wsRequest.getStatus();
+    Optional<String> resolution = wsRequest.getResolution();
+    checkArgument(!status.isPresent() || hotspotKeys.isEmpty(),
       "Parameter '%s' can't be used with parameter '%s'", PARAM_STATUS, PARAM_HOTSPOTS);
-    checkArgument(!wsRequest.getResolution().isPresent() || wsRequest.getHotspotKeys().isEmpty(),
+    checkArgument(!resolution.isPresent() || hotspotKeys.isEmpty(),
       "Parameter '%s' can't be used with parameter '%s'", PARAM_RESOLUTION, PARAM_HOTSPOTS);
 
-    wsRequest.getResolution().ifPresent(
-      resolution -> checkArgument(wsRequest.getStatus().filter(STATUS_REVIEWED::equals).isPresent(),
+    resolution.ifPresent(
+      r -> checkArgument(status.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));
+        r, PARAM_RESOLUTION, PARAM_STATUS, STATUS_REVIEWED));
   }
 
   private Optional<ComponentDto> getAndValidateProject(DbSession dbSession, WsRequest wsRequest) {
     return wsRequest.getProjectKey().map(projectKey -> {
-      ComponentDto project = dbClient.componentDao().selectByKey(dbSession, projectKey)
+      ComponentDto project = getProject(dbSession, projectKey, wsRequest.getBranch(), wsRequest.getPullRequest())
         .filter(t -> Scopes.PROJECT.equals(t.scope()) && Qualifiers.PROJECT.equals(t.qualifier()))
         .filter(ComponentDto::isEnabled)
-        .filter(t -> t.getMainBranchProjectUuid() == null)
         .orElseThrow(() -> new NotFoundException(format("Project '%s' not found", projectKey)));
       userSession.checkComponentPermission(UserRole.USER, project);
       return project;
     });
   }
 
+  private Optional<ComponentDto> getProject(DbSession dbSession, String projectKey, Optional<String> branch, Optional<String> pullRequest) {
+    if (branch.isPresent()) {
+      return dbClient.componentDao().selectByKeyAndBranch(dbSession, projectKey, branch.get());
+    } else if (pullRequest.isPresent()) {
+      return dbClient.componentDao().selectByKeyAndPullRequest(dbSession, projectKey, pullRequest.get());
+    }
+    return dbClient.componentDao().selectByKey(dbSession, projectKey);
+  }
+
   private SearchResponseData searchHotspots(WsRequest wsRequest, DbSession dbSession, Optional<ComponentDto> project, Set<String> hotspotKeys) {
     SearchResponse result = doIndexSearch(wsRequest, project, hotspotKeys);
     List<String> issueKeys = Arrays.stream(result.getHits().getHits())
@@ -213,8 +251,15 @@ public class SearchAction implements HotspotsWsAction {
       .sort(IssueQuery.SORT_HOTSPOTS)
       .asc(true);
     project.ifPresent(p -> {
-      builder.projectUuids(Collections.singletonList(p.uuid()));
       builder.organizationUuid(p.getOrganizationUuid());
+
+      if (p.getMainBranchProjectUuid() == null) {
+        builder.mainBranch(true);
+        builder.projectUuids(singletonList(p.uuid()));
+      } else {
+        builder.branchUuid(p.projectUuid());
+        builder.mainBranch(false);
+      }
     });
     if (!hotspotKeys.isEmpty()) {
       builder.issueKeys(hotspotKeys);
@@ -317,14 +362,21 @@ public class SearchAction implements HotspotsWsAction {
     private final int page;
     private final int index;
     private final String projectKey;
+    private final String branch;
+    private final String pullRequest;
     private final Set<String> hotspotKeys;
     private final String status;
     private final String resolution;
 
-    private WsRequest(int page, int index, @Nullable String projectKey, Set<String> hotspotKeys, @Nullable String status, @Nullable String resolution) {
+    private WsRequest(int page, int index,
+      @Nullable String projectKey, @Nullable String branch, @Nullable String pullRequest,
+      Set<String> hotspotKeys,
+      @Nullable String status, @Nullable String resolution) {
       this.page = page;
       this.index = index;
       this.projectKey = projectKey;
+      this.branch = branch;
+      this.pullRequest = pullRequest;
       this.hotspotKeys = hotspotKeys;
       this.status = status;
       this.resolution = resolution;
@@ -342,6 +394,14 @@ public class SearchAction implements HotspotsWsAction {
       return ofNullable(projectKey);
     }
 
+    Optional<String> getBranch() {
+      return ofNullable(branch);
+    }
+
+    Optional<String> getPullRequest() {
+      return ofNullable(pullRequest);
+    }
+
     Set<String> getHotspotKeys() {
       return hotspotKeys;
     }

server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java

@@ -45,6 +45,7 @@ import org.sonar.api.utils.System2;
 import org.sonar.api.web.UserRole;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbTester;
+import org.sonar.db.component.BranchType;
 import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.ComponentTesting;
 import org.sonar.db.issue.IssueDto;
@@ -123,6 +124,40 @@ public class SearchActionTest {
       .hasMessage("A value must be provided for either parameter 'projectKey' or parameter 'hotspots'");
   }
 
+  @Test
+  public void fail_with_IAE_if_parameter_branch_is_used_without_parameter_projectKey() {
+    TestRequest request = actionTester.newRequest()
+      .setParam("hotspots", randomAlphabetic(2))
+      .setParam("branch", randomAlphabetic(1));
+
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Parameter 'branch' must be used with parameter 'projectKey'");
+  }
+
+  @Test
+  public void fail_with_IAE_if_parameter_pullRequest_is_used_without_parameter_projectKey() {
+    TestRequest request = actionTester.newRequest()
+      .setParam("hotspots", randomAlphabetic(2))
+      .setParam("pullRequest", randomAlphabetic(1));
+
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Parameter 'pullRequest' must be used with parameter 'projectKey'");
+  }
+
+  @Test
+  public void fail_with_IAE_if_both_parameters_pullRequest_and_branch_are_provided() {
+    TestRequest request = actionTester.newRequest()
+      .setParam("projectKey", randomAlphabetic(2))
+      .setParam("branch", randomAlphabetic(1))
+      .setParam("pullRequest", randomAlphabetic(1));
+
+    assertThatThrownBy(request::execute)
+      .isInstanceOf(IllegalArgumentException.class)
+      .hasMessage("Only one of parameters 'branch' and 'pullRequest' can be provided");
+  }
+
   @Test
   @UseDataProvider("badStatuses")
   public void fails_with_IAE_if_status_parameter_is_neither_TO_REVIEW_or_REVIEWED(String badStatus) {
@@ -325,6 +360,8 @@ public class SearchActionTest {
 
     SearchWsResponse response = newRequest(project)
       .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList()).isEmpty();
   }
 
   @Test
@@ -421,6 +458,54 @@ public class SearchActionTest {
       .containsOnly(project2.getKey(), file2.getKey());
   }
 
+  @Test
+  public void returns_hotspot_of_branch_or_pullRequest() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    indexPermissions();
+    ComponentDto branch = dbTester.components().insertProjectBranch(project);
+    ComponentDto pullRequest = dbTester.components().insertProjectBranch(project, t -> t.setBranchType(BranchType.PULL_REQUEST));
+    ComponentDto fileProject = dbTester.components().insertComponent(newFileDto(project));
+    ComponentDto fileBranch = dbTester.components().insertComponent(newFileDto(branch));
+    ComponentDto filePR = dbTester.components().insertComponent(newFileDto(pullRequest));
+    IssueDto[] hotspotProject = IntStream.range(0, 1 + RANDOM.nextInt(10))
+      .mapToObj(i -> {
+        RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+        return insertHotspot(project, fileProject, rule);
+      })
+      .toArray(IssueDto[]::new);
+    IssueDto[] hotspotBranch = IntStream.range(0, 1 + RANDOM.nextInt(10))
+      .mapToObj(i -> {
+        RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+        return insertHotspot(branch, fileBranch, rule);
+      })
+      .toArray(IssueDto[]::new);
+    IssueDto[] hotspotPR = IntStream.range(0, 1 + RANDOM.nextInt(10))
+      .mapToObj(i -> {
+        RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+        return insertHotspot(pullRequest, filePR, rule);
+      })
+      .toArray(IssueDto[]::new);
+    indexIssues();
+
+    SearchWsResponse responseProject = newRequest(project)
+      .executeProtobuf(SearchWsResponse.class);
+    SearchWsResponse responseBranch = newRequest(branch)
+      .executeProtobuf(SearchWsResponse.class);
+    SearchWsResponse responsePR = newRequest(pullRequest)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(responseProject.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsExactlyInAnyOrder(Arrays.stream(hotspotProject).map(IssueDto::getKey).toArray(String[]::new));
+    assertThat(responseBranch.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsExactlyInAnyOrder(Arrays.stream(hotspotBranch).map(IssueDto::getKey).toArray(String[]::new));
+    assertThat(responsePR.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsExactlyInAnyOrder(Arrays.stream(hotspotPR).map(IssueDto::getKey).toArray(String[]::new));
+  }
+
   @Test
   public void returns_hotpots_with_any_status_if_no_status_nor_resolution_parameter() {
     ComponentDto project = dbTester.components().insertPublicProject();
@@ -687,16 +772,92 @@ public class SearchActionTest {
     assertThat(actualProject.getName()).isEqualTo(project.name());
     assertThat(actualProject.getLongName()).isEqualTo(project.longName());
     assertThat(actualProject.hasPath()).isFalse();
+    assertThat(actualProject.hasBranch()).isFalse();
+    assertThat(actualProject.hasPullRequest()).isFalse();
     Component actualDirectory = componentByKey.get(directory.getKey());
     assertThat(actualDirectory.getQualifier()).isEqualTo(directory.qualifier());
     assertThat(actualDirectory.getName()).isEqualTo(directory.name());
     assertThat(actualDirectory.getLongName()).isEqualTo(directory.longName());
     assertThat(actualDirectory.getPath()).isEqualTo(directory.path());
+    assertThat(actualDirectory.hasBranch()).isFalse();
+    assertThat(actualDirectory.hasPullRequest()).isFalse();
     Component actualFile = componentByKey.get(file.getKey());
     assertThat(actualFile.getQualifier()).isEqualTo(file.qualifier());
     assertThat(actualFile.getName()).isEqualTo(file.name());
     assertThat(actualFile.getLongName()).isEqualTo(file.longName());
     assertThat(actualFile.getPath()).isEqualTo(file.path());
+    assertThat(actualFile.hasBranch()).isFalse();
+    assertThat(actualFile.hasPullRequest()).isFalse();
+  }
+
+  @Test
+  public void returns_branch_field_of_components_of_branch() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    ComponentDto branch = dbTester.components().insertProjectBranch(project);
+    userSessionRule.registerComponents(project, branch);
+    indexPermissions();
+    ComponentDto directory = dbTester.components().insertComponent(newDirectory(branch, "donut/acme"));
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(branch));
+    RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+    IssueDto fileHotspot = insertHotspot(branch, file, rule);
+    IssueDto dirHotspot = insertHotspot(branch, directory, rule);
+    IssueDto projectHotspot = insertHotspot(branch, branch, rule);
+    indexIssues();
+
+    SearchWsResponse response = newRequest(branch)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsOnly(fileHotspot.getKey(), dirHotspot.getKey(), projectHotspot.getKey());
+    assertThat(response.getComponentsList())
+      .extracting(Component::getKey)
+      .containsOnly(project.getKey(), directory.getKey(), file.getKey());
+    Map<String, Component> componentByKey = response.getComponentsList().stream().collect(uniqueIndex(Component::getKey));
+    Component actualProject = componentByKey.get(project.getKey());
+    assertThat(actualProject.getBranch()).isEqualTo(branch.getBranch());
+    assertThat(actualProject.hasPullRequest()).isFalse();
+    Component actualDirectory = componentByKey.get(directory.getKey());
+    assertThat(actualDirectory.getBranch()).isEqualTo(branch.getBranch());
+    assertThat(actualDirectory.hasPullRequest()).isFalse();
+    Component actualFile = componentByKey.get(file.getKey());
+    assertThat(actualFile.getBranch()).isEqualTo(branch.getBranch());
+    assertThat(actualFile.hasPullRequest()).isFalse();
+  }
+
+  @Test
+  public void returns_pullRequest_field_of_components_of_pullRequest() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    ComponentDto pullRequest = dbTester.components().insertProjectBranch(project, t -> t.setBranchType(BranchType.PULL_REQUEST));
+    userSessionRule.registerComponents(project, pullRequest);
+    indexPermissions();
+    ComponentDto directory = dbTester.components().insertComponent(newDirectory(pullRequest, "donut/acme"));
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(pullRequest));
+    RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+    IssueDto fileHotspot = insertHotspot(pullRequest, file, rule);
+    IssueDto dirHotspot = insertHotspot(pullRequest, directory, rule);
+    IssueDto projectHotspot = insertHotspot(pullRequest, pullRequest, rule);
+    indexIssues();
+
+    SearchWsResponse response = newRequest(pullRequest)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList())
+      .extracting(SearchWsResponse.Hotspot::getKey)
+      .containsOnly(fileHotspot.getKey(), dirHotspot.getKey(), projectHotspot.getKey());
+    assertThat(response.getComponentsList())
+      .extracting(Component::getKey)
+      .containsOnly(project.getKey(), directory.getKey(), file.getKey());
+    Map<String, Component> componentByKey = response.getComponentsList().stream().collect(uniqueIndex(Component::getKey));
+    Component actualProject = componentByKey.get(project.getKey());
+    assertThat(actualProject.hasBranch()).isFalse();
+    assertThat(actualProject.getPullRequest()).isEqualTo(pullRequest.getPullRequest());
+    Component actualDirectory = componentByKey.get(directory.getKey());
+    assertThat(actualDirectory.hasBranch()).isFalse();
+    assertThat(actualDirectory.getPullRequest()).isEqualTo(pullRequest.getPullRequest());
+    Component actualFile = componentByKey.get(file.getKey());
+    assertThat(actualFile.hasBranch()).isFalse();
+    assertThat(actualFile.getPullRequest()).isEqualTo(pullRequest.getPullRequest());
   }
 
   @Test
@@ -918,6 +1079,14 @@ public class SearchActionTest {
   private TestRequest newRequest(ComponentDto project, @Nullable String status, @Nullable String resolution) {
     TestRequest res = actionTester.newRequest()
       .setParam("projectKey", project.getKey());
+    String branch = project.getBranch();
+    if (branch != null) {
+      res.setParam("branch", branch);
+    }
+    String pullRequest = project.getPullRequest();
+    if (pullRequest != null) {
+      res.setParam("pullRequest", pullRequest);
+    }
     if (status != null) {
       res.setParam("status", status);
     }