SONAR-16009 Extend response of `api/hotspots/search` to include secondary locations data

3 files changed, 148 insertions, 17 deletions

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 f16d346cebf..96053a46560 100644
--- 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
@@ -53,9 +53,11 @@ import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.SnapshotDto;
 import org.sonar.db.issue.IssueDto;
 import org.sonar.db.project.ProjectDto;
+import org.sonar.db.protobuf.DbIssues;
 import org.sonar.db.rule.RuleDefinitionDto;
 import org.sonar.server.es.SearchOptions;
 import org.sonar.server.exceptions.NotFoundException;
+import org.sonar.server.issue.TextRangeResponseFormatter;
 import org.sonar.server.issue.index.IssueIndex;
 import org.sonar.server.issue.index.IssueIndexSyncProgressChecker;
 import org.sonar.server.issue.index.IssueQuery;
@@ -119,16 +121,18 @@ public class SearchAction implements HotspotsWsAction {
   private final IssueIndex issueIndex;
   private final IssueIndexSyncProgressChecker issueIndexSyncProgressChecker;
   private final HotspotWsResponseFormatter responseFormatter;
+  private final TextRangeResponseFormatter textRangeFormatter;
   private final System2 system2;
 
   public SearchAction(DbClient dbClient, UserSession userSession, IssueIndex issueIndex,
     IssueIndexSyncProgressChecker issueIndexSyncProgressChecker,
-    HotspotWsResponseFormatter responseFormatter, System2 system2) {
+    HotspotWsResponseFormatter responseFormatter, TextRangeResponseFormatter textRangeFormatter, System2 system2) {
     this.dbClient = dbClient;
     this.userSession = userSession;
     this.issueIndex = issueIndex;
     this.issueIndexSyncProgressChecker = issueIndexSyncProgressChecker;
     this.responseFormatter = responseFormatter;
+    this.textRangeFormatter = textRangeFormatter;
     this.system2 = system2;
   }
 
@@ -257,10 +261,10 @@ public class SearchAction implements HotspotsWsAction {
       "A value must be provided for either parameter '%s' or parameter '%s'", PARAM_PROJECT_KEY, PARAM_HOTSPOTS);
 
     checkArgument(
-      !branch.isPresent() || projectKey.isPresent(),
+      branch.isEmpty() || projectKey.isPresent(),
       "Parameter '%s' must be used with parameter '%s'", PARAM_BRANCH, PARAM_PROJECT_KEY);
     checkArgument(
-      !pullRequest.isPresent() || projectKey.isPresent(),
+      pullRequest.isEmpty() || projectKey.isPresent(),
       "Parameter '%s' must be used with parameter '%s'", PARAM_PULL_REQUEST, PARAM_PROJECT_KEY);
     checkArgument(
       !(branch.isPresent() && pullRequest.isPresent()),
@@ -268,9 +272,9 @@ public class SearchAction implements HotspotsWsAction {
 
     Optional<String> status = wsRequest.getStatus();
     Optional<String> resolution = wsRequest.getResolution();
-    checkArgument(!status.isPresent() || hotspotKeys.isEmpty(),
+    checkArgument(status.isEmpty() || hotspotKeys.isEmpty(),
       "Parameter '%s' can't be used with parameter '%s'", PARAM_STATUS, PARAM_HOTSPOTS);
-    checkArgument(!resolution.isPresent() || hotspotKeys.isEmpty(),
+    checkArgument(resolution.isEmpty() || hotspotKeys.isEmpty(),
       "Parameter '%s' can't be used with parameter '%s'", PARAM_RESOLUTION, PARAM_HOTSPOTS);
 
     resolution.ifPresent(
@@ -456,14 +460,46 @@ public class SearchAction implements HotspotsWsAction {
   }
 
   private void loadComponents(DbSession dbSession, SearchResponseData searchResponseData) {
-    Set<String> componentKeys = searchResponseData.getOrderedHotspots().stream()
-      .flatMap(hotspot -> Stream.of(hotspot.getComponentKey(), hotspot.getProjectKey()))
+    Set<String> componentUuids = searchResponseData.getOrderedHotspots().stream()
+      .flatMap(hotspot -> Stream.of(hotspot.getComponentUuid(), hotspot.getProjectUuid()))
       .collect(Collectors.toSet());
-    if (!componentKeys.isEmpty()) {
-      searchResponseData.addComponents(dbClient.componentDao().selectByDbKeys(dbSession, componentKeys));
+
+    Set<String> locationComponentUuids = searchResponseData.getOrderedHotspots()
+      .stream()
+      .flatMap(hotspot -> getHotspotLocationComponentUuids(hotspot).stream())
+      .collect(Collectors.toSet());
+
+    Set<String> aggregatedComponentUuids = Stream.of(componentUuids, locationComponentUuids)
+      .flatMap(Collection::stream)
+      .collect(Collectors.toSet());
+
+    if (!aggregatedComponentUuids.isEmpty()) {
+      searchResponseData.addComponents(dbClient.componentDao().selectByUuids(dbSession, aggregatedComponentUuids));
     }
   }
 
+  private static Set<String> getHotspotLocationComponentUuids(IssueDto hotspot) {
+    Set<String> locationComponentUuids = new HashSet<>();
+    DbIssues.Locations locations = hotspot.parseLocations();
+
+    if (locations == null) {
+      return locationComponentUuids;
+    }
+
+    List<DbIssues.Flow> flows = locations.getFlowList();
+
+    for (DbIssues.Flow flow : flows) {
+      List<DbIssues.Location> flowLocations = flow.getLocationList();
+      for (DbIssues.Location location : flowLocations) {
+        if (location.hasComponentId()) {
+          locationComponentUuids.add(location.getComponentId());
+        }
+      }
+    }
+
+    return locationComponentUuids;
+  }
+
   private void loadRules(DbSession dbSession, SearchResponseData searchResponseData) {
     Set<RuleKey> ruleKeys = searchResponseData.getOrderedHotspots()
       .stream()
@@ -494,7 +530,7 @@ public class SearchAction implements HotspotsWsAction {
     responseBuilder.setPaging(pagingBuilder.build());
   }
 
-  private static void formatHotspots(SearchResponseData searchResponseData, SearchWsResponse.Builder responseBuilder) {
+  private void formatHotspots(SearchResponseData searchResponseData, SearchWsResponse.Builder responseBuilder) {
     List<IssueDto> orderedHotspots = searchResponseData.getOrderedHotspots();
     if (orderedHotspots.isEmpty()) {
       return;
@@ -522,13 +558,31 @@ public class SearchAction implements HotspotsWsAction {
       builder.setAuthor(nullToEmpty(hotspot.getAuthorLogin()));
       builder.setCreationDate(formatDateTime(hotspot.getIssueCreationDate()));
       builder.setUpdateDate(formatDateTime(hotspot.getIssueUpdateDate()));
-
+      completeHotspotLocations(hotspot, builder, searchResponseData);
       responseBuilder.addHotspots(builder.build());
     }
   }
 
+  private void completeHotspotLocations(IssueDto hotspot, SearchWsResponse.Hotspot.Builder hotspotBuilder, SearchResponseData data) {
+    DbIssues.Locations locations = hotspot.parseLocations();
+
+    if (locations == null) {
+      return;
+    }
+
+    textRangeFormatter.formatTextRange(locations, hotspotBuilder::setTextRange);
+
+    for (DbIssues.Flow flow : locations.getFlowList()) {
+      Common.Flow.Builder targetFlow = Common.Flow.newBuilder();
+      for (DbIssues.Location flowLocation : flow.getLocationList()) {
+        targetFlow.addLocations(textRangeFormatter.formatLocation(flowLocation, hotspotBuilder.getComponent(), data.getComponentsByUuid()));
+      }
+      hotspotBuilder.addFlows(targetFlow.build());
+    }
+  }
+
   private void formatComponents(SearchResponseData searchResponseData, SearchWsResponse.Builder responseBuilder) {
-    Set<ComponentDto> components = searchResponseData.getComponents();
+    Collection<ComponentDto> components = searchResponseData.getComponents();
     if (components.isEmpty()) {
       return;
     }
@@ -643,7 +697,7 @@ public class SearchAction implements HotspotsWsAction {
   private static final class SearchResponseData {
     private final Paging paging;
     private final List<IssueDto> orderedHotspots;
-    private final Set<ComponentDto> components = new HashSet<>();
+    private final Map<String, ComponentDto> componentsByUuid = new HashMap<>();
     private final Map<RuleKey, RuleDefinitionDto> rulesByRuleKey = new HashMap<>();
 
     private SearchResponseData(Paging paging, List<IssueDto> orderedHotspots) {
@@ -664,11 +718,17 @@ public class SearchAction implements HotspotsWsAction {
     }
 
     void addComponents(Collection<ComponentDto> components) {
-      this.components.addAll(components);
+      for (ComponentDto component : components) {
+        componentsByUuid.put(component.uuid(), component);
+      }
+    }
+
+    Collection<ComponentDto> getComponents() {
+      return componentsByUuid.values();
     }
 
-    Set<ComponentDto> getComponents() {
-      return components;
+    public Map<String, ComponentDto> getComponentsByUuid() {
+      return componentsByUuid;
     }
 
     void addRules(Collection<RuleDefinitionDto> rules) {
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 f8f88f35143..94d6aa45873 100644
--- 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
@@ -52,11 +52,14 @@ import org.sonar.db.component.ComponentDto;
 import org.sonar.db.component.ComponentTesting;
 import org.sonar.db.issue.IssueDto;
 import org.sonar.db.project.ProjectDto;
+import org.sonar.db.protobuf.DbCommons;
+import org.sonar.db.protobuf.DbIssues;
 import org.sonar.db.rule.RuleDefinitionDto;
 import org.sonar.db.rule.RuleTesting;
 import org.sonar.server.es.EsTester;
 import org.sonar.server.exceptions.ForbiddenException;
 import org.sonar.server.exceptions.NotFoundException;
+import org.sonar.server.issue.TextRangeResponseFormatter;
 import org.sonar.server.issue.index.AsyncIssueIndexing;
 import org.sonar.server.issue.index.IssueIndex;
 import org.sonar.server.issue.index.IssueIndexSyncProgressChecker;
@@ -70,6 +73,7 @@ import org.sonar.server.tester.UserSessionRule;
 import org.sonar.server.view.index.ViewIndexer;
 import org.sonar.server.ws.TestRequest;
 import org.sonar.server.ws.WsActionTester;
+import org.sonarqube.ws.Common;
 import org.sonarqube.ws.Hotspots;
 import org.sonarqube.ws.Hotspots.Component;
 import org.sonarqube.ws.Hotspots.SearchWsResponse;
@@ -82,6 +86,7 @@ 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.assertj.core.groups.Tuple.tuple;
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.ArgumentMatchers.eq;
 import static org.mockito.Mockito.mock;
@@ -124,7 +129,7 @@ public class SearchActionTest {
   private final HotspotWsResponseFormatter responseFormatter = new HotspotWsResponseFormatter();
   private final IssueIndexSyncProgressChecker issueIndexSyncProgressChecker = mock(IssueIndexSyncProgressChecker.class);
   private final SearchAction underTest = new SearchAction(dbClient, userSessionRule, issueIndex,
-    issueIndexSyncProgressChecker, responseFormatter, system2);
+    issueIndexSyncProgressChecker, responseFormatter, new TextRangeResponseFormatter(), system2);
   private final WsActionTester actionTester = new WsActionTester(underTest);
 
   @Test
@@ -1187,6 +1192,51 @@ public class SearchActionTest {
   }
 
   @Test
+  public void returns_hotspot_with_secondary_locations() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    indexPermissions();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    ComponentDto anotherFile = dbTester.components().insertComponent(newFileDto(project));
+
+    List<DbIssues.Location> hotspotLocations = Stream.of(
+        newHotspotLocation(file.uuid(), "security hotspot flow message 0", 1, 1, 0, 12),
+        newHotspotLocation(file.uuid(), "security hotspot flow message 1", 3, 3, 0, 10),
+        newHotspotLocation(anotherFile.uuid(), "security hotspot flow message 2", 5, 5, 0, 15),
+        newHotspotLocation(anotherFile.uuid(), "security hotspot flow message 3", 7, 7, 0, 18),
+        newHotspotLocation(null,"security hotspot flow message 4", 12, 12, 2, 8))
+      .collect(toList());
+
+    DbIssues.Locations.Builder locations = DbIssues.Locations.newBuilder().addFlow(DbIssues.Flow.newBuilder().addAllLocation(hotspotLocations));
+
+    RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+    dbTester.issues().insertHotspot(rule, project, file, h -> h.setLocations(locations.build()));
+
+    indexIssues();
+
+    SearchWsResponse response = newRequest(project)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsCount()).isOne();
+    assertThat(response.getHotspotsList().stream().findFirst().get().getFlowsCount()).isEqualTo(1);
+    assertThat(response.getHotspotsList().stream().findFirst().get().getFlowsList().stream().findFirst().get().getLocationsCount()).isEqualTo(5);
+    assertThat(response.getHotspotsList().stream().findFirst().get().getFlowsList().stream().findFirst().get().getLocationsList())
+      .extracting(
+        Common.Location::getComponent,
+        Common.Location::getMsg,
+        l -> l.getTextRange().getStartLine(),
+        l -> l.getTextRange().getEndLine(),
+        l -> l.getTextRange().getStartOffset(),
+        l -> l.getTextRange().getEndOffset())
+      .containsExactlyInAnyOrder(
+        tuple(file.getKey(), "security hotspot flow message 0", 1, 1, 0, 12),
+        tuple(file.getKey(), "security hotspot flow message 1", 3, 3, 0, 10),
+        tuple(anotherFile.getKey(), "security hotspot flow message 2", 5, 5, 0, 15),
+        tuple(anotherFile.getKey(), "security hotspot flow message 3", 7, 7, 0, 18),
+        tuple(file.getKey(),"security hotspot flow message 4", 12, 12, 2, 8));
+  }
+
+  @Test
   public void returns_first_page_with_100_results_by_default() {
     ComponentDto project = dbTester.components().insertPublicProject();
     userSessionRule.registerComponents(project);
@@ -1756,6 +1806,25 @@ public class SearchActionTest {
     return res.setType(SECURITY_HOTSPOT);
   }
 
+  private static DbIssues.Location newHotspotLocation(@Nullable String componentUuid, String message, int startLine, int endLine, int startOffset, int endOffset) {
+    DbIssues.Location.Builder builder = DbIssues.Location.newBuilder();
+
+    if (componentUuid != null) {
+      builder.setComponentId(componentUuid);
+    }
+
+    builder
+      .setMsg(message)
+      .setTextRange(DbCommons.TextRange.newBuilder()
+        .setStartLine(startLine)
+        .setEndLine(endLine)
+        .setStartOffset(startOffset)
+        .setEndOffset(endOffset)
+        .build());
+
+    return builder.build();
+  }
+
   private TestRequest newRequest(ComponentDto project) {
     return newRequest(project, null, null);
   }
diff --git a/sonar-ws/src/main/protobuf/ws-hotspots.proto b/sonar-ws/src/main/protobuf/ws-hotspots.proto
index 008a02ca159..4560baa7dcc 100644
--- a/sonar-ws/src/main/protobuf/ws-hotspots.proto
+++ b/sonar-ws/src/main/protobuf/ws-hotspots.proto
@@ -46,6 +46,8 @@ message SearchWsResponse {
     optional string author = 11;
     optional string creationDate = 12;
     optional string updateDate = 13;
+    optional sonarqube.ws.commons.TextRange textRange = 14;
+    repeated sonarqube.ws.commons.Flow flows = 15;
   }
 }