SONAR-12717 drop rule list from api/hotspots/search response

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

@@ -22,10 +22,12 @@ package org.sonar.server.hotspot.ws;
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
+import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
@@ -40,7 +42,6 @@ import org.sonar.api.server.ws.Response;
 import org.sonar.api.server.ws.WebService;
 import org.sonar.api.utils.Paging;
 import org.sonar.api.web.UserRole;
-import org.sonar.core.util.stream.MoreCollectors;
 import org.sonar.db.DbClient;
 import org.sonar.db.DbSession;
 import org.sonar.db.component.ComponentDto;
@@ -62,7 +63,9 @@ 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;
 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_PROJECT_EXAMPLE_001;
 import static org.sonar.server.ws.WsUtils.writeProtobuf;
 
@@ -130,7 +133,7 @@ public class SearchAction implements HotspotsWsAction {
     SearchResponse result = doIndexSearch(wsRequest, project);
     List<String> issueKeys = Arrays.stream(result.getHits().getHits())
       .map(SearchHit::getId)
-      .collect(MoreCollectors.toList(result.getHits().getHits().length));
+      .collect(toList(result.getHits().getHits().length));
 
     List<IssueDto> hotspots = toIssueDtos(dbSession, issueKeys);
 
@@ -190,7 +193,6 @@ public class SearchAction implements HotspotsWsAction {
     if (!searchResponseData.isEmpty()) {
       formatHotspots(searchResponseData, responseBuilder);
       formatComponents(searchResponseData, responseBuilder);
-      formatRules(searchResponseData, responseBuilder);
     }
     return responseBuilder.build();
   }
@@ -213,12 +215,18 @@ public class SearchAction implements HotspotsWsAction {
 
     Hotspots.Hotspot.Builder builder = Hotspots.Hotspot.newBuilder();
     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(
+          "Rule with key '%s' not found for Hotspot '%s'", hotspot.getRuleKey(), hotspot.getKey())));
+      SecurityStandards.SQCategory sqCategory = fromSecurityStandards(rule.getSecurityStandards()).getSqCategory();
       builder
         .clear()
         .setKey(hotspot.getKey())
         .setComponent(hotspot.getComponentKey())
         .setProject(hotspot.getProjectKey())
-        .setRule(hotspot.getRuleKey().toString());
+        .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);
@@ -257,27 +265,6 @@ public class SearchAction implements HotspotsWsAction {
     }
   }
 
-  private void formatRules(SearchResponseData searchResponseData, Hotspots.SearchWsResponse.Builder responseBuilder) {
-    Set<RuleDefinitionDto> rules = searchResponseData.getRules();
-    if (rules.isEmpty()) {
-      return;
-    }
-
-    Hotspots.Rule.Builder ruleBuilder = Hotspots.Rule.newBuilder();
-    for (RuleDefinitionDto rule : rules) {
-      SecurityStandards securityStandards = SecurityStandards.fromSecurityStandards(rule.getSecurityStandards());
-      SecurityStandards.SQCategory sqCategory = securityStandards.getSqCategory();
-      ruleBuilder
-        .clear()
-        .setKey(rule.getKey().toString())
-        .setName(nullToEmpty(rule.getName()))
-        .setSecurityCategory(sqCategory.getKey())
-        .setVulnerabilityProbability(sqCategory.getVulnerability().name());
-
-      responseBuilder.addRules(ruleBuilder.build());
-    }
-  }
-
   private static final class WsRequest {
     private final int page;
     private final int index;
@@ -306,7 +293,7 @@ public class SearchAction implements HotspotsWsAction {
     private final Paging paging;
     private final List<IssueDto> orderedHotspots;
     private final Set<ComponentDto> components = new HashSet<>();
-    private final Set<RuleDefinitionDto> rules = new HashSet<>();
+    private final Map<RuleKey, RuleDefinitionDto> rulesByRuleKey = new HashMap<>();
 
     private SearchResponseData(Paging paging, List<IssueDto> orderedHotspots) {
       this.paging = paging;
@@ -334,11 +321,11 @@ public class SearchAction implements HotspotsWsAction {
     }
 
     void addRules(Collection<RuleDefinitionDto> rules) {
-      this.rules.addAll(rules);
+      rules.forEach(t -> rulesByRuleKey.put(t.getKey(), t));
     }
 
-    Set<RuleDefinitionDto> getRules() {
-      return rules;
+    Optional<RuleDefinitionDto> getRule(RuleKey ruleKey) {
+      return ofNullable(rulesByRuleKey.get(ruleKey));
     }
 
   }

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

@@ -19,7 +19,11 @@
  */
 package org.sonar.server.hotspot.ws;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Ordering;
+import com.tngtech.java.junit.dataprovider.DataProvider;
+import com.tngtech.java.junit.dataprovider.DataProviderRunner;
+import com.tngtech.java.junit.dataprovider.UseDataProvider;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.Comparator;
@@ -33,6 +37,7 @@ import java.util.stream.IntStream;
 import java.util.stream.Stream;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.runner.RunWith;
 import org.sonar.api.issue.Issue;
 import org.sonar.api.rules.RuleType;
 import org.sonar.api.utils.System2;
@@ -64,6 +69,7 @@ import org.sonarqube.ws.Hotspots.Component;
 import org.sonarqube.ws.Hotspots.SearchWsResponse;
 
 import static java.util.Collections.singleton;
+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;
@@ -74,6 +80,7 @@ import static org.sonar.db.component.ComponentTesting.newDirectory;
 import static org.sonar.db.component.ComponentTesting.newFileDto;
 import static org.sonar.db.issue.IssueTesting.newIssue;
 
+@RunWith(DataProviderRunner.class)
 public class SearchActionTest {
   private static final Random RANDOM = new Random();
 
@@ -158,7 +165,6 @@ public class SearchActionTest {
 
     assertThat(response.getHotspotsList()).isEmpty();
     assertThat(response.getComponentsList()).isEmpty();
-    assertThat(response.getRulesList()).isEmpty();
   }
 
   @Test
@@ -172,7 +178,33 @@ public class SearchActionTest {
 
     assertThat(response.getHotspotsList()).isEmpty();
     assertThat(response.getComponentsList()).isEmpty();
-    assertThat(response.getRulesList()).isEmpty();
+  }
+
+  @Test
+  public void does_not_fail_if_rule_of_hotspot_does_not_exist_in_DB() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    indexPermissions();
+    IssueDto[] hotspots = IntStream.range(0, 1 + RANDOM.nextInt(10))
+      .mapToObj(i -> {
+        RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+        return dbTester.issues().insert(rule, project, file, t -> t.setType(SECURITY_HOTSPOT));
+      })
+      .toArray(IssueDto[]::new);
+    indexIssues();
+    IssueDto hotspotWithoutRule = hotspots[RANDOM.nextInt(hotspots.length)];
+    dbTester.executeUpdateSql("delete from rules where id=" + hotspotWithoutRule.getRuleId());
+
+    SearchWsResponse response = newRequest(project)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList())
+      .extracting(Hotspots.Hotspot::getKey)
+      .containsOnly(Arrays.stream(hotspots)
+        .filter(t -> !t.getKey().equals(hotspotWithoutRule.getKey()))
+        .map(IssueDto::getKey)
+        .toArray(String[]::new));
   }
 
   @Test
@@ -191,14 +223,10 @@ public class SearchActionTest {
 
     SearchWsResponse response = newRequest(project)
       .executeProtobuf(SearchWsResponse.class);
-
-    assertThat(response.getHotspotsList()).isEmpty();
-    assertThat(response.getComponentsList()).isEmpty();
-    assertThat(response.getRulesList()).isEmpty();
   }
 
   @Test
-  public void returns_hotspot_component_and_rule_when_project_has_hotspots() {
+  public void returns_hotspot_components_when_project_has_hotspots() {
     ComponentDto project = dbTester.components().insertPublicProject();
     userSessionRule.registerComponents(project);
     indexPermissions();
@@ -227,9 +255,30 @@ public class SearchActionTest {
     assertThat(response.getComponentsList())
       .extracting(Component::getKey)
       .containsOnly(project.getKey(), fileWithHotspot.getKey());
-    assertThat(response.getRulesList())
-      .extracting(Hotspots.Rule::getKey)
-      .containsOnly(Arrays.stream(hotspots).map(t -> t.getRuleKey().toString()).toArray(String[]::new));
+  }
+
+  @Test
+  public void returns_single_component_when_all_hotspots_are_on_project() {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    indexPermissions();
+    IssueDto[] hotspots = IntStream.range(0, 1 + RANDOM.nextInt(10))
+      .mapToObj(i -> {
+        RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
+        return dbTester.issues().insert(rule, project, project, t -> t.setType(SECURITY_HOTSPOT));
+      })
+      .toArray(IssueDto[]::new);
+    indexIssues();
+
+    SearchWsResponse response = newRequest(project)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList())
+      .extracting(Hotspots.Hotspot::getKey)
+      .containsOnly(Arrays.stream(hotspots).map(IssueDto::getKey).toArray(String[]::new));
+    assertThat(response.getComponentsList())
+      .extracting(Component::getKey)
+      .containsOnly(project.getKey());
   }
 
   @Test
@@ -258,9 +307,6 @@ public class SearchActionTest {
     assertThat(responseProject1.getComponentsList())
       .extracting(Component::getKey)
       .containsOnly(project1.getKey(), file1.getKey());
-    assertThat(responseProject1.getRulesList())
-      .extracting(Hotspots.Rule::getKey)
-      .containsOnly(Arrays.stream(hotspots2).map(t -> t.getRuleKey().toString()).toArray(String[]::new));
 
     SearchWsResponse responseProject2 = newRequest(project2)
       .executeProtobuf(SearchWsResponse.class);
@@ -271,9 +317,6 @@ public class SearchActionTest {
     assertThat(responseProject2.getComponentsList())
       .extracting(Component::getKey)
       .containsOnly(project2.getKey(), file2.getKey());
-    assertThat(responseProject2.getRulesList())
-      .extracting(Hotspots.Rule::getKey)
-      .containsOnly(Arrays.stream(hotspots2).map(t -> t.getRuleKey().toString()).toArray(String[]::new));
   }
 
   @Test
@@ -323,7 +366,6 @@ public class SearchActionTest {
     Hotspots.Hotspot actual = response.getHotspots(0);
     assertThat(actual.getComponent()).isEqualTo(file.getKey());
     assertThat(actual.getProject()).isEqualTo(project.getKey());
-    assertThat(actual.getRule()).isEqualTo(hotspot.getRuleKey().toString());
     assertThat(actual.getStatus()).isEqualTo(hotspot.getStatus());
     // FIXME resolution field will be added later
     // assertThat(actual.getResolution()).isEqualTo(hotspot.getResolution());
@@ -335,6 +377,40 @@ public class SearchActionTest {
     assertThat(actual.getUpdateDate()).isEqualTo(formatDateTime(hotspot.getIssueUpdateDate()));
   }
 
+  @Test
+  @UseDataProvider("allSQCategories")
+  public void returns_SQCategory_and_VulnerabilityProbability_of_rule(Set<String> securityStandards, SQCategory expected) {
+    ComponentDto project = dbTester.components().insertPublicProject();
+    userSessionRule.registerComponents(project);
+    indexPermissions();
+    ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
+    RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT, t -> t.setSecurityStandards(securityStandards));
+    IssueDto hotspot = dbTester.issues().insert(rule, project, file, t -> t.setType(SECURITY_HOTSPOT));
+    indexIssues();
+
+    SearchWsResponse response = newRequest(project)
+      .executeProtobuf(SearchWsResponse.class);
+
+    assertThat(response.getHotspotsList()).hasSize(1);
+    Hotspots.Hotspot actual = response.getHotspots(0);
+    assertThat(actual.getSecurityCategory()).isEqualTo(expected.getKey());
+    assertThat(actual.getVulnerabilityProbability()).isEqualTo(expected.getVulnerability().name());
+  }
+
+  @DataProvider
+  public static Object[][] allSQCategories() {
+    Stream<Object[]> allCategoriesButOTHERS = SecurityStandards.CWES_BY_SQ_CATEGORY.entrySet()
+      .stream()
+      .map(t -> new Object[] {
+        t.getValue().stream().map(c -> "cwe:" + c).collect(toSet()),
+        t.getKey()
+      });
+    Stream<Object[]> sqCategoryOTHERS = Stream.of(
+      new Object[] {Collections.emptySet(), SQCategory.OTHERS},
+      new Object[] {ImmutableSet.of("foo", "donut", "acme"), SQCategory.OTHERS});
+    return Stream.concat(allCategoriesButOTHERS, sqCategoryOTHERS).toArray(Object[][]::new);
+  }
+
   @Test
   public void does_not_fail_when_hotspot_has_none_of_the_nullable_fields() {
     ComponentDto project = dbTester.components().insertPublicProject();

sonar-ws/src/main/protobuf/ws-hotspots.proto

@@ -31,24 +31,23 @@ message SearchWsResponse {
   optional sonarqube.ws.commons.Paging paging = 1;
   repeated Hotspot hotspots = 2;
   repeated Component components = 3;
-  repeated Rule rules = 4;
 }
 
 message Hotspot {
   optional string key = 1;
   optional string component = 2;
   optional string project = 3;
-  optional string rule = 4;
-  optional string status = 5;
+  optional string securityCategory = 4;
+  optional string vulnerabilityProbability = 5;
+  optional string status = 6;
 //  FIXME resolution field will be added later
-//  optional string resolution = 6;
-  optional int32 line = 7;
-  optional string message = 8;
-  optional string assignee = 9;
-  // SCM login of the committer who introduced the issue
-  optional string author = 10;
-  optional string creationDate = 11;
-  optional string updateDate = 12;
+//  optional string resolution = 7;
+  optional int32 line = 8;
+  optional string message = 9;
+  optional string assignee = 10;
+  optional string author = 11;
+  optional string creationDate = 12;
+  optional string updateDate = 13;
 }
 
 message Component {
@@ -59,10 +58,3 @@ message Component {
   optional string longName = 5;
   optional string path = 6;
 }
-
-message Rule {
-  optional string key = 1;
-  optional string name = 2;
-  optional string securityCategory = 3;
-  optional string vulnerabilityProbability = 4;
-}