From e13456fdb410c9938402b36a50a5462bc6597a42 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 29 Nov 2019 16:16:53 +0100 Subject: [PATCH] SONAR-12717 add sort and paging to api/hotspots/search --- .../sonar/server/issue/index/IssueDoc.java | 16 +- .../issue/index/IssueIndexDefinition.java | 6 +- .../index/IssueIteratorForSingleChunk.java | 4 +- .../server/security/SecurityStandards.java | 26 ++- .../server/issue/index/IssueIndexerTest.java | 2 + .../sonar/server/issue/index/IssueIndex.java | 16 +- .../sonar/server/issue/index/IssueQuery.java | 6 +- .../sonar/server/hotspot/ws/SearchAction.java | 110 +++++++-- .../server/hotspot/ws/SearchActionTest.java | 213 ++++++++++++++++-- 9 files changed, 339 insertions(+), 60 deletions(-) diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueDoc.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueDoc.java index 88e8254667a..292139ef1a7 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueDoc.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueDoc.java @@ -33,6 +33,7 @@ import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.server.es.BaseDoc; import org.sonar.server.permission.index.AuthorizationDoc; import org.sonar.server.security.SecurityStandards; +import org.sonar.server.security.SecurityStandards.VulnerabilityProbability; import static org.sonar.server.issue.index.IssueIndexDefinition.TYPE_ISSUE; @@ -330,12 +331,23 @@ public class IssueDoc extends BaseDoc { @CheckForNull public SecurityStandards.SQCategory getSonarSourceSecurityCategory() { - String key = getNullableField(IssueIndexDefinition.FIELD_ISSUE_SONARSOURCE_SECURITY); + String key = getNullableField(IssueIndexDefinition.FIELD_ISSUE_SQ_SECURITY_CATEGORY); return SecurityStandards.SQCategory.fromKey(key).orElse(null); } public IssueDoc setSonarSourceSecurityCategory(@Nullable SecurityStandards.SQCategory c) { - setField(IssueIndexDefinition.FIELD_ISSUE_SONARSOURCE_SECURITY, c == null ? null : c.getKey()); + setField(IssueIndexDefinition.FIELD_ISSUE_SQ_SECURITY_CATEGORY, c == null ? null : c.getKey()); + return this; + } + + @CheckForNull + public VulnerabilityProbability getVulnerabilityProbability() { + Integer score = getNullableField(IssueIndexDefinition.FIELD_ISSUE_VULNERABILITY_PROBABILITY); + return VulnerabilityProbability.byScore(score).orElse(null); + } + + public IssueDoc setVulnerabilityProbability(@Nullable VulnerabilityProbability v) { + setField(IssueIndexDefinition.FIELD_ISSUE_VULNERABILITY_PROBABILITY, v == null ? null : v.getScore()); return this; } } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java index 3e4114d1af1..f02f7536cfc 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIndexDefinition.java @@ -99,7 +99,8 @@ public class IssueIndexDefinition implements IndexDefinition { public static final String FIELD_ISSUE_OWASP_TOP_10 = "owaspTop10"; public static final String FIELD_ISSUE_SANS_TOP_25 = "sansTop25"; public static final String FIELD_ISSUE_CWE = "cwe"; - public static final String FIELD_ISSUE_SONARSOURCE_SECURITY = "sonarsourceSecurity"; + public static final String FIELD_ISSUE_SQ_SECURITY_CATEGORY = "sonarsourceSecurity"; + public static final String FIELD_ISSUE_VULNERABILITY_PROBABILITY = "vulnerabilityProbability"; private final Configuration config; private final boolean enableSource; @@ -160,6 +161,7 @@ public class IssueIndexDefinition implements IndexDefinition { mapping.keywordFieldBuilder(FIELD_ISSUE_OWASP_TOP_10).disableNorms().build(); mapping.keywordFieldBuilder(FIELD_ISSUE_SANS_TOP_25).disableNorms().build(); mapping.keywordFieldBuilder(FIELD_ISSUE_CWE).disableNorms().build(); - mapping.keywordFieldBuilder(FIELD_ISSUE_SONARSOURCE_SECURITY).disableNorms().build(); + mapping.keywordFieldBuilder(FIELD_ISSUE_SQ_SECURITY_CATEGORY).disableNorms().build(); + mapping.keywordFieldBuilder(FIELD_ISSUE_VULNERABILITY_PROBABILITY).disableNorms().build(); } } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java index 0095f0c050d..d3ffeb07751 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/issue/index/IssueIteratorForSingleChunk.java @@ -230,10 +230,12 @@ class IssueIteratorForSingleChunk implements IssueIterator { doc.setType(RuleType.valueOf(rs.getInt(22))); SecurityStandards securityStandards = fromSecurityStandards(deserializeSecurityStandardsString(rs.getString(23))); + SecurityStandards.SQCategory sqCategory = securityStandards.getSqCategory(); doc.setOwaspTop10(securityStandards.getOwaspTop10()); doc.setCwe(securityStandards.getCwe()); doc.setSansTop25(securityStandards.getSansTop25()); - doc.setSonarSourceSecurityCategory(securityStandards.getSqCategory()); + doc.setSonarSourceSecurityCategory(sqCategory); + doc.setVulnerabilityProbability(sqCategory.getVulnerability()); return doc; } diff --git a/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java b/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java index b954d454339..89bb8ab155b 100644 --- a/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java +++ b/server/sonar-server-common/src/main/java/org/sonar/server/security/SecurityStandards.java @@ -22,6 +22,7 @@ package org.sonar.server.security; import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Ordering; +import java.util.Arrays; import java.util.Collection; import java.util.HashSet; import java.util.List; @@ -64,9 +65,28 @@ public final class SecurityStandards { SANS_TOP_25_POROUS_DEFENSES, POROUS_CWE); public enum VulnerabilityProbability { - HIGH, - MEDIUM, - LOW + HIGH(3), + MEDIUM(2), + LOW(1); + + private final int score; + + VulnerabilityProbability(int index) { + this.score = index; + } + + public int getScore() { + return score; + } + + public static Optional byScore(@Nullable Integer score) { + if (score == null) { + return Optional.empty(); + } + return Arrays.stream(values()) + .filter(t -> t.score == score) + .findFirst(); + } } public enum SQCategory { diff --git a/server/sonar-server-common/src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java b/server/sonar-server-common/src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java index abaefc7c63a..c33af919d42 100644 --- a/server/sonar-server-common/src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java +++ b/server/sonar-server-common/src/test/java/org/sonar/server/issue/index/IssueIndexerTest.java @@ -50,6 +50,7 @@ import org.sonar.server.permission.index.AuthorizationScope; import org.sonar.server.permission.index.IndexPermissions; import org.sonar.server.security.SecurityStandards; import org.sonar.server.security.SecurityStandards.SQCategory; +import org.sonar.server.security.SecurityStandards.VulnerabilityProbability; import static java.util.Arrays.asList; import static java.util.Collections.emptyList; @@ -140,6 +141,7 @@ public class IssueIndexerTest { assertThat(doc.getOwaspTop10()).isEmpty(); assertThat(doc.getSansTop25()).isEmpty(); assertThat(doc.getSonarSourceSecurityCategory()).isEqualTo(SQCategory.OTHERS); + assertThat(doc.getVulnerabilityProbability()).isEqualTo(VulnerabilityProbability.LOW); } @Test diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java index 9a2e700f341..431850a80ac 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueIndex.java @@ -149,10 +149,11 @@ import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_RULE import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_SANS_TOP_25; import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_SEVERITY; import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_SEVERITY_VALUE; -import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_SONARSOURCE_SECURITY; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_SQ_SECURITY_CATEGORY; import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_STATUS; import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_TAGS; import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_TYPE; +import static org.sonar.server.issue.index.IssueIndexDefinition.FIELD_ISSUE_VULNERABILITY_PROBABILITY; import static org.sonar.server.issue.index.IssueIndexDefinition.TYPE_ISSUE; import static org.sonar.server.security.SecurityStandards.SANS_TOP_25_INSECURE_INTERACTION; import static org.sonar.server.security.SecurityStandards.SANS_TOP_25_POROUS_DEFENSES; @@ -232,7 +233,7 @@ public class IssueIndex { SANS_TOP_25(PARAM_SANS_TOP_25, FIELD_ISSUE_SANS_TOP_25, DEFAULT_FACET_SIZE), CWE(PARAM_CWE, FIELD_ISSUE_CWE, DEFAULT_FACET_SIZE), CREATED_AT(PARAM_CREATED_AT, FIELD_ISSUE_FUNC_CREATED_AT, DEFAULT_FACET_SIZE), - SONARSOURCE_SECURITY(PARAM_SONARSOURCE_SECURITY, FIELD_ISSUE_SONARSOURCE_SECURITY, DEFAULT_FACET_SIZE); + SONARSOURCE_SECURITY(PARAM_SONARSOURCE_SECURITY, FIELD_ISSUE_SQ_SECURITY_CATEGORY, DEFAULT_FACET_SIZE); private final String name; private final String fieldName; @@ -300,6 +301,13 @@ public class IssueIndex { this.sorting.add(IssueQuery.SORT_BY_FILE_LINE, FIELD_ISSUE_LINE); this.sorting.add(IssueQuery.SORT_BY_FILE_LINE, FIELD_ISSUE_SEVERITY_VALUE).reverse(); this.sorting.add(IssueQuery.SORT_BY_FILE_LINE, FIELD_ISSUE_KEY); + this.sorting.add(IssueQuery.SORT_HOTSPOTS, FIELD_ISSUE_VULNERABILITY_PROBABILITY).reverse(); + this.sorting.add(IssueQuery.SORT_HOTSPOTS, FIELD_ISSUE_SQ_SECURITY_CATEGORY); + this.sorting.add(IssueQuery.SORT_HOTSPOTS, FIELD_ISSUE_RULE_ID); + this.sorting.add(IssueQuery.SORT_HOTSPOTS, FIELD_ISSUE_PROJECT_UUID); + this.sorting.add(IssueQuery.SORT_HOTSPOTS, FIELD_ISSUE_FILE_PATH); + this.sorting.add(IssueQuery.SORT_HOTSPOTS, FIELD_ISSUE_LINE); + this.sorting.add(IssueQuery.SORT_HOTSPOTS, FIELD_ISSUE_KEY); // by default order by created date, project, file, line and issue key (in order to be deterministic when same ms) this.sorting.addDefault(FIELD_ISSUE_FUNC_CREATED_AT).reverse(); @@ -391,7 +399,7 @@ public class IssueIndex { filters.put(FIELD_ISSUE_SANS_TOP_25, createTermsFilter(FIELD_ISSUE_SANS_TOP_25, query.sansTop25())); filters.put(FIELD_ISSUE_CWE, createTermsFilter(FIELD_ISSUE_CWE, query.cwe())); addSeverityFilter(query, filters); - filters.put(FIELD_ISSUE_SONARSOURCE_SECURITY, createTermsFilter(FIELD_ISSUE_SONARSOURCE_SECURITY, query.sonarsourceSecurity())); + filters.put(FIELD_ISSUE_SQ_SECURITY_CATEGORY, createTermsFilter(FIELD_ISSUE_SQ_SECURITY_CATEGORY, query.sonarsourceSecurity())); addComponentRelatedFilters(query, filters); addDatesFilter(filters, query); @@ -885,7 +893,7 @@ public class IssueIndex { Arrays.stream(SQCategory.values()) .forEach(sonarsourceCategory -> request.addAggregation( newSecurityReportSubAggregations( - AggregationBuilders.filter(sonarsourceCategory.getKey(), boolQuery().filter(termQuery(FIELD_ISSUE_SONARSOURCE_SECURITY, sonarsourceCategory.getKey()))), + AggregationBuilders.filter(sonarsourceCategory.getKey(), boolQuery().filter(termQuery(FIELD_ISSUE_SQ_SECURITY_CATEGORY, sonarsourceCategory.getKey()))), includeCwe, Optional.ofNullable(SecurityStandards.CWES_BY_SQ_CATEGORY.get(sonarsourceCategory))))); return processSecurityReportSearchResults(request, includeCwe); diff --git a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java index 1aa7db69925..577dc6dcaec 100644 --- a/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java +++ b/server/sonar-webserver-es/src/main/java/org/sonar/server/issue/index/IssueQuery.java @@ -53,9 +53,13 @@ public class IssueQuery { * Sort by project, file path then line id */ public static final String SORT_BY_FILE_LINE = "FILE_LINE"; + /** + * Sort hotspots by vulnerabilityProbability, sqSecurityCategory, project, file path then line id + */ + public static final String SORT_HOTSPOTS = "HOTSPOTS"; public static final Set SORTS = ImmutableSet.of(SORT_BY_CREATION_DATE, SORT_BY_UPDATE_DATE, SORT_BY_CLOSE_DATE, SORT_BY_ASSIGNEE, SORT_BY_SEVERITY, - SORT_BY_STATUS, SORT_BY_FILE_LINE); + SORT_BY_STATUS, SORT_BY_FILE_LINE, SORT_HOTSPOTS); private final Collection issueKeys; private final Collection severities; 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 f93d5101fb8..3e38031cafc 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 @@ -38,6 +38,7 @@ import org.sonar.api.rules.RuleType; import org.sonar.api.server.ws.Request; 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; @@ -52,6 +53,7 @@ import org.sonar.server.issue.index.IssueQuery; import org.sonar.server.organization.DefaultOrganizationProvider; import org.sonar.server.security.SecurityStandards; import org.sonar.server.user.UserSession; +import org.sonarqube.ws.Common; import org.sonarqube.ws.Hotspots; import static com.google.common.base.Strings.nullToEmpty; @@ -59,6 +61,7 @@ import static java.util.Optional.ofNullable; 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.uniqueIndex; import static org.sonar.server.ws.KeyExamples.KEY_PROJECT_EXAMPLE_001; import static org.sonar.server.ws.WsUtils.writeProtobuf; @@ -80,7 +83,6 @@ public class SearchAction implements HotspotsWsAction { @Override public void define(WebService.NewController controller) { - WebService.NewAction action = controller .createAction("search") .setHandler(this) @@ -99,50 +101,67 @@ public class SearchAction implements HotspotsWsAction { @Override public void handle(Request request, Response response) throws Exception { + WsRequest wsRequest = toWsRequest(request); try (DbSession dbSession = dbClient.openSession(false)) { - String projectKey = request.mandatoryParam(PARAM_PROJECT_KEY); - ComponentDto project = dbClient.componentDao().selectByKey(dbSession, projectKey) - .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", projectKey))); - userSession.checkComponentPermission(UserRole.USER, project); - - List orderedIssues = searchHotspots(request, dbSession, project); - SearchResponseData searchResponseData = new SearchResponseData(orderedIssues); + ComponentDto project = validateRequest(dbSession, wsRequest); + + SearchResponseData searchResponseData = searchHotspots(wsRequest, dbSession, project); loadComponents(dbSession, searchResponseData); loadRules(dbSession, searchResponseData); writeProtobuf(formatResponse(searchResponseData), request, response); } } - private List searchHotspots(Request request, DbSession dbSession, ComponentDto project) { - List issueKeys = searchHotspots(request, project); + private static WsRequest toWsRequest(Request request) { + return new WsRequest(request.mandatoryParamAsInt(PAGE), request.mandatoryParamAsInt(PAGE_SIZE), request.mandatoryParam(PARAM_PROJECT_KEY)); + } + + private ComponentDto validateRequest(DbSession dbSession, WsRequest wsRequest) { + 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()))); + userSession.checkComponentPermission(UserRole.USER, project); + return project; + } + + private SearchResponseData searchHotspots(WsRequest wsRequest, DbSession dbSession, ComponentDto project) { + SearchResponse result = doIndexSearch(wsRequest, project); + List issueKeys = Arrays.stream(result.getHits().getHits()) + .map(SearchHit::getId) + .collect(MoreCollectors.toList(result.getHits().getHits().length)); + + List hotspots = toIssueDtos(dbSession, issueKeys); + + Paging paging = forPageIndex(wsRequest.getPage()).withPageSize(wsRequest.getIndex()).andTotal((int) result.getHits().getTotalHits()); + return new SearchResponseData(paging, hotspots); + } - List unorderedIssues = dbClient.issueDao().selectByKeys(dbSession, issueKeys); - Map unorderedIssuesMap = unorderedIssues + private List toIssueDtos(DbSession dbSession, List issueKeys) { + List unorderedHotspots = dbClient.issueDao().selectByKeys(dbSession, issueKeys); + Map hotspotsByKey = unorderedHotspots .stream() - .collect(uniqueIndex(IssueDto::getKey, unorderedIssues.size())); + .collect(uniqueIndex(IssueDto::getKey, unorderedHotspots.size())); return issueKeys.stream() - .map(unorderedIssuesMap::get) + .map(hotspotsByKey::get) .filter(Objects::nonNull) .collect(Collectors.toList()); } - private List searchHotspots(Request request, ComponentDto project) { + private SearchResponse doIndexSearch(WsRequest wsRequest, ComponentDto project) { IssueQuery.Builder builder = IssueQuery.builder() .projectUuids(Collections.singletonList(project.uuid())) .organizationUuid(project.getOrganizationUuid()) .types(Collections.singleton(RuleType.SECURITY_HOTSPOT.name())) - .resolved(false); + .resolved(false) + .sort(IssueQuery.SORT_HOTSPOTS) + .asc(true); IssueQuery query = builder.build(); SearchOptions searchOptions = new SearchOptions() - .setPage(request.mandatoryParamAsInt(PAGE), request.mandatoryParamAsInt(PAGE_SIZE)); - SearchResponse result = issueIndex.search(query, searchOptions); - return Arrays.stream(result.getHits().getHits()) - .map(SearchHit::getId) - .collect(MoreCollectors.toList(result.getHits().getHits().length)); + .setPage(wsRequest.page, wsRequest.index); + return issueIndex.search(query, searchOptions); } private void loadComponents(DbSession dbSession, SearchResponseData searchResponseData) { @@ -167,6 +186,7 @@ public class SearchAction implements HotspotsWsAction { private Hotspots.SearchWsResponse formatResponse(SearchResponseData searchResponseData) { Hotspots.SearchWsResponse.Builder responseBuilder = Hotspots.SearchWsResponse.newBuilder(); + formatPaging(searchResponseData, responseBuilder); if (!searchResponseData.isEmpty()) { formatHotspots(searchResponseData, responseBuilder); formatComponents(searchResponseData, responseBuilder); @@ -175,6 +195,16 @@ public class SearchAction implements HotspotsWsAction { return responseBuilder.build(); } + private void formatPaging(SearchResponseData searchResponseData, Hotspots.SearchWsResponse.Builder responseBuilder) { + Paging paging = searchResponseData.getPaging(); + Common.Paging.Builder pagingBuilder = Common.Paging.newBuilder() + .setPageIndex(paging.pageIndex()) + .setPageSize(paging.pageSize()) + .setTotal(paging.total()); + + responseBuilder.setPaging(pagingBuilder.build()); + } + private static void formatHotspots(SearchResponseData searchResponseData, Hotspots.SearchWsResponse.Builder responseBuilder) { List orderedHotspots = searchResponseData.getOrderedHotspots(); if (orderedHotspots.isEmpty()) { @@ -248,12 +278,38 @@ public class SearchAction implements HotspotsWsAction { } } + private static final class WsRequest { + private final int page; + private final int index; + private final String projectKey; + + private WsRequest(int page, int index, String projectKey) { + this.page = page; + this.index = index; + this.projectKey = projectKey; + } + + int getPage() { + return page; + } + + int getIndex() { + return index; + } + + String getProjectKey() { + return projectKey; + } + } + private static final class SearchResponseData { + private final Paging paging; private final List orderedHotspots; private final Set components = new HashSet<>(); private final Set rules = new HashSet<>(); - private SearchResponseData(List orderedHotspots) { + private SearchResponseData(Paging paging, List orderedHotspots) { + this.paging = paging; this.orderedHotspots = orderedHotspots; } @@ -261,6 +317,10 @@ public class SearchAction implements HotspotsWsAction { return orderedHotspots.isEmpty(); } + public Paging getPaging() { + return paging; + } + List getOrderedHotspots() { return orderedHotspots; } 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 5f955e5e4e5..082e4a801da 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 @@ -19,12 +19,18 @@ */ package org.sonar.server.hotspot.ws; +import com.google.common.collect.Ordering; import java.util.Arrays; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; import java.util.Map; import java.util.Random; +import java.util.Set; import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.IntStream; +import java.util.stream.Stream; import org.junit.Rule; import org.junit.Test; import org.sonar.api.issue.Issue; @@ -48,6 +54,8 @@ import org.sonar.server.issue.index.IssueIteratorFactory; import org.sonar.server.organization.TestDefaultOrganizationProvider; import org.sonar.server.permission.index.PermissionIndexer; import org.sonar.server.permission.index.WebAuthorizationTypeSupport; +import org.sonar.server.security.SecurityStandards; +import org.sonar.server.security.SecurityStandards.SQCategory; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.TestRequest; import org.sonar.server.ws.WsActionTester; @@ -55,6 +63,7 @@ import org.sonarqube.ws.Hotspots; import org.sonarqube.ws.Hotspots.Component; import org.sonarqube.ws.Hotspots.SearchWsResponse; +import static java.util.Collections.singleton; import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -63,6 +72,7 @@ import static org.sonar.api.utils.DateUtils.formatDateTime; import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; import static org.sonar.db.component.ComponentTesting.newDirectory; import static org.sonar.db.component.ComponentTesting.newFileDto; +import static org.sonar.db.issue.IssueTesting.newIssue; public class SearchActionTest { private static final Random RANDOM = new Random(); @@ -131,8 +141,7 @@ public class SearchActionTest { public void fails_with_ForbiddenException_if_project_is_private_and_not_allowed() { ComponentDto project = dbTester.components().insertPrivateProject(); userSessionRule.registerComponents(project); - TestRequest request = actionTester.newRequest() - .setParam("projectKey", project.getKey()); + TestRequest request = newRequest(project); assertThatThrownBy(request::execute) .isInstanceOf(ForbiddenException.class) @@ -144,8 +153,7 @@ public class SearchActionTest { ComponentDto project = dbTester.components().insertPublicProject(); userSessionRule.registerComponents(project); - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) + SearchWsResponse response = newRequest(project) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()).isEmpty(); @@ -159,8 +167,7 @@ public class SearchActionTest { userSessionRule.registerComponents(project); userSessionRule.logIn().addProjectPermission(UserRole.USER, project); - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) + SearchWsResponse response = newRequest(project) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()).isEmpty(); @@ -182,8 +189,7 @@ public class SearchActionTest { }); indexIssues(); - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) + SearchWsResponse response = newRequest(project) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()).isEmpty(); @@ -212,8 +218,7 @@ public class SearchActionTest { .toArray(IssueDto[]::new); indexIssues(); - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) + SearchWsResponse response = newRequest(project) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) @@ -244,8 +249,7 @@ public class SearchActionTest { .toArray(IssueDto[]::new); indexIssues(); - SearchWsResponse responseProject1 = actionTester.newRequest() - .setParam("projectKey", project1.getKey()) + SearchWsResponse responseProject1 = newRequest(project1) .executeProtobuf(SearchWsResponse.class); assertThat(responseProject1.getHotspotsList()) @@ -258,8 +262,7 @@ public class SearchActionTest { .extracting(Hotspots.Rule::getKey) .containsOnly(Arrays.stream(hotspots2).map(t -> t.getRuleKey().toString()).toArray(String[]::new)); - SearchWsResponse responseProject2 = actionTester.newRequest() - .setParam("projectKey", project2.getKey()) + SearchWsResponse responseProject2 = newRequest(project2) .executeProtobuf(SearchWsResponse.class); assertThat(responseProject2.getHotspotsList()) @@ -289,8 +292,7 @@ public class SearchActionTest { t -> t.setType(SECURITY_HOTSPOT).setResolution(randomAlphabetic(5))); indexIssues(); - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) + SearchWsResponse response = newRequest(project) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) @@ -314,8 +316,7 @@ public class SearchActionTest { .setAuthorLogin(randomAlphabetic(8))); indexIssues(); - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) + SearchWsResponse response = newRequest(project) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()).hasSize(1); @@ -352,8 +353,7 @@ public class SearchActionTest { .setAuthorLogin(null)); indexIssues(); - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) + SearchWsResponse response = newRequest(project) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) @@ -383,8 +383,7 @@ public class SearchActionTest { IssueDto projectHotspot = dbTester.issues().insert(rule, project, project, t -> t.setType(SECURITY_HOTSPOT)); indexIssues(); - SearchWsResponse response = actionTester.newRequest() - .setParam("projectKey", project.getKey()) + SearchWsResponse response = newRequest(project) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) @@ -415,6 +414,176 @@ public class SearchActionTest { assertThat(actualFile.getPath()).isEqualTo(file.path()); } + @Test + public void returns_hotspots_ordered_by_vulnerabilityProbability_score_then_rule_id() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + List hotspots = Arrays.stream(SQCategory.values()) + .sorted(Ordering.from(Comparator.comparingInt(t1 -> t1.getVulnerability().getScore()).reversed()) + .thenComparing(SQCategory::getKey)) + .flatMap(sqCategory -> { + Set cwes = SecurityStandards.CWES_BY_SQ_CATEGORY.get(sqCategory); + Set securityStandards = singleton("cwe:" + (cwes == null ? "unknown" : cwes.iterator().next())); + RuleDefinitionDto rule1 = newRule( + SECURITY_HOTSPOT, + t -> t.setName("rule_" + sqCategory.name() + "_a").setSecurityStandards(securityStandards)); + RuleDefinitionDto rule2 = newRule( + SECURITY_HOTSPOT, + t -> t.setName("rule_" + sqCategory.name() + "_b").setSecurityStandards(securityStandards)); + return Stream.of( + newIssue(rule1, project, file).setKee(sqCategory + "_a").setType(SECURITY_HOTSPOT), + newIssue(rule2, project, file).setKee(sqCategory + "_b").setType(SECURITY_HOTSPOT)); + }) + .collect(Collectors.toList()); + String[] expectedHotspotKeys = hotspots.stream().map(IssueDto::getKey).toArray(String[]::new); + // insert hotspots in random order + Collections.shuffle(hotspots); + hotspots.forEach(dbTester.issues()::insertIssue); + indexIssues(); + + SearchWsResponse response = newRequest(project) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(Hotspots.Hotspot::getKey) + .containsExactly(expectedHotspotKeys); + } + + @Test + public void returns_hotspots_ordered_by_file_path_then_line_then_key() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file1 = dbTester.components().insertComponent(newFileDto(project).setPath("b/c/a")); + ComponentDto file2 = dbTester.components().insertComponent(newFileDto(project).setPath("b/c/b")); + ComponentDto file3 = dbTester.components().insertComponent(newFileDto(project).setPath("a/a/d")); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + List hotspots = Stream.of( + newIssue(rule, project, file3).setType(SECURITY_HOTSPOT).setLine(8), + newIssue(rule, project, file3).setType(SECURITY_HOTSPOT).setLine(10), + newIssue(rule, project, file1).setType(SECURITY_HOTSPOT).setLine(null), + newIssue(rule, project, file1).setType(SECURITY_HOTSPOT).setLine(9), + newIssue(rule, project, file1).setType(SECURITY_HOTSPOT).setLine(11).setKee("a"), + newIssue(rule, project, file1).setType(SECURITY_HOTSPOT).setLine(11).setKee("b"), + newIssue(rule, project, file2).setType(SECURITY_HOTSPOT).setLine(null), + newIssue(rule, project, file2).setType(SECURITY_HOTSPOT).setLine(2)) + .collect(Collectors.toList()); + String[] expectedHotspotKeys = hotspots.stream().map(IssueDto::getKey).toArray(String[]::new); + // insert hotspots in random order + Collections.shuffle(hotspots); + hotspots.forEach(dbTester.issues()::insertIssue); + indexIssues(); + + SearchWsResponse response = newRequest(project) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(Hotspots.Hotspot::getKey) + .containsExactly(expectedHotspotKeys); + } + + @Test + public void returns_first_page_with_100_results_by_default() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + int total = 436; + List hotspots = IntStream.range(0, total) + .mapToObj(i -> newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i)) + .map(i -> dbTester.issues().insertIssue(i)) + .collect(Collectors.toList()); + indexIssues(); + + TestRequest request = newRequest(project); + + SearchWsResponse response = request.executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(Hotspots.Hotspot::getKey) + .containsExactly(hotspots.stream().limit(100).map(IssueDto::getKey).toArray(String[]::new)); + assertThat(response.getPaging().getTotal()).isEqualTo(hotspots.size()); + assertThat(response.getPaging().getPageIndex()).isEqualTo(1); + assertThat(response.getPaging().getPageSize()).isEqualTo(100); + } + + @Test + public void returns_specified_page_with_100_results_by_default() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + + verifyPaging(project, file, rule, 336, 100); + } + + @Test + public void returns_specified_page_with_specified_number_of_results() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + int total = 336; + int pageSize = 1 + new Random().nextInt(100); + + verifyPaging(project, file, rule, total, pageSize); + } + + private void verifyPaging(ComponentDto project, ComponentDto file, RuleDefinitionDto rule, int total, int pageSize) { + List hotspots = IntStream.range(0, total) + .mapToObj(i -> newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i).setKee("issue_" + i)) + .map(i -> dbTester.issues().insertIssue(i)) + .collect(Collectors.toList()); + indexIssues(); + + SearchWsResponse response = newRequest(project) + .setParam("p", "3") + .setParam("ps", String.valueOf(pageSize)) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(Hotspots.Hotspot::getKey) + .containsExactly(hotspots.stream().skip(2 * pageSize).limit(pageSize).map(IssueDto::getKey).toArray(String[]::new)); + assertThat(response.getPaging().getTotal()).isEqualTo(hotspots.size()); + assertThat(response.getPaging().getPageIndex()).isEqualTo(3); + assertThat(response.getPaging().getPageSize()).isEqualTo(pageSize); + + response = newRequest(project) + .setParam("p", "4") + .setParam("ps", String.valueOf(pageSize)) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(Hotspots.Hotspot::getKey) + .containsExactly(hotspots.stream().skip(3 * pageSize).limit(pageSize).map(IssueDto::getKey).toArray(String[]::new)); + assertThat(response.getPaging().getTotal()).isEqualTo(hotspots.size()); + assertThat(response.getPaging().getPageIndex()).isEqualTo(4); + assertThat(response.getPaging().getPageSize()).isEqualTo(pageSize); + + int emptyPage = (hotspots.size() / pageSize) + 10; + response = newRequest(project) + .setParam("p", String.valueOf(emptyPage)) + .setParam("ps", String.valueOf(pageSize)) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(Hotspots.Hotspot::getKey) + .isEmpty(); + assertThat(response.getPaging().getTotal()).isEqualTo(hotspots.size()); + assertThat(response.getPaging().getPageIndex()).isEqualTo(emptyPage); + assertThat(response.getPaging().getPageSize()).isEqualTo(pageSize); + } + + private TestRequest newRequest(ComponentDto project) { + return actionTester.newRequest() + .setParam("projectKey", project.getKey()); + } + private void indexPermissions() { permissionIndexer.indexOnStartup(permissionIndexer.getIndexTypes()); } -- 2.39.5