Browse Source

SONAR-12797 add hotspots parameter to api/hotspots/search

tags/8.2.0.32929
Sébastien Lesaint 4 years ago
parent
commit
545e6dd086

+ 63
- 28
server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java View File

@@ -19,6 +19,7 @@
*/
package org.sonar.server.hotspot.ws;

import com.google.common.collect.ImmutableSet;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -43,6 +44,7 @@ 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;
@@ -81,6 +83,7 @@ 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 static final String PARAM_HOTSPOTS = "hotspots";

private final DbClient dbClient;
private final UserSession userSession;
@@ -106,17 +109,23 @@ public class SearchAction implements HotspotsWsAction {

action.addPagingParams(100);
action.createParam(PARAM_PROJECT_KEY)
.setDescription("Key of the project")
.setExampleValue(KEY_PROJECT_EXAMPLE_001)
.setRequired(true);
.setDescription(format(
"Key of the project. This parameter is required unless %s is provided.",
PARAM_HOTSPOTS))
.setExampleValue(KEY_PROJECT_EXAMPLE_001);
action.createParam(PARAM_HOTSPOTS)
.setDescription(format(
"Comma-separated list of Security Hotspot keys. This parameter is required unless %s is provided.",
PARAM_PROJECT_KEY))
.setExampleValue(KEY_PROJECT_EXAMPLE_001);
action.createParam(PARAM_STATUS)
.setDescription("If provided, only Security Hotspots with the specified status are returned.")
.setDescription("If '%s' is provided, only Security Hotspots with the specified status are returned.", PARAM_PROJECT_KEY)
.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))
"If '%s' is provided and if status is '%s', only Security Hotspots with the specified resolution are returned.",
PARAM_PROJECT_KEY, STATUS_REVIEWED))
.setPossibleValues(RESOLUTION_FIXED, RESOLUTION_SAFE)
.setRequired(false);
// FIXME add response example and test it
@@ -126,10 +135,11 @@ public class SearchAction implements HotspotsWsAction {
@Override
public void handle(Request request, Response response) throws Exception {
WsRequest wsRequest = toWsRequest(request);
validateParameters(wsRequest);
try (DbSession dbSession = dbClient.openSession(false)) {
ComponentDto project = validateRequest(dbSession, wsRequest);
Optional<ComponentDto> project = getAndValidateProject(dbSession, wsRequest);

SearchResponseData searchResponseData = searchHotspots(wsRequest, dbSession, project);
SearchResponseData searchResponseData = searchHotspots(wsRequest, dbSession, project, wsRequest.getHotspotKeys());
loadComponents(dbSession, searchResponseData);
loadRules(dbSession, searchResponseData);
writeProtobuf(formatResponse(searchResponseData), request, response);
@@ -137,30 +147,44 @@ public class SearchAction implements HotspotsWsAction {
}

private static WsRequest toWsRequest(Request request) {
List<String> hotspotKeysList = request.paramAsStrings(PARAM_HOTSPOTS);
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.mandatoryParam(PARAM_PROJECT_KEY),
request.param(PARAM_PROJECT_KEY), hotspotKeys,
request.param(PARAM_STATUS), request.param(PARAM_RESOLUTION));
}

private ComponentDto validateRequest(DbSession dbSession, WsRequest wsRequest) {
private static void validateParameters(WsRequest wsRequest) {
checkArgument(
wsRequest.getProjectKey().isPresent() || !wsRequest.getHotspotKeys().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(),
"Parameter '%s' can't be used with parameter '%s'", PARAM_STATUS, PARAM_HOTSPOTS);
checkArgument(!wsRequest.getResolution().isPresent() || wsRequest.getHotspotKeys().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(),
"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(format("Project '%s' not found", wsRequest.getProjectKey())));
userSession.checkComponentPermission(UserRole.USER, project);
return project;
resolution, PARAM_RESOLUTION, PARAM_STATUS, STATUS_REVIEWED));
}

private SearchResponseData searchHotspots(WsRequest wsRequest, DbSession dbSession, ComponentDto project) {
SearchResponse result = doIndexSearch(wsRequest, project);
private Optional<ComponentDto> getAndValidateProject(DbSession dbSession, WsRequest wsRequest) {
return wsRequest.getProjectKey().map(projectKey -> {
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(format("Project '%s' not found", projectKey)));
userSession.checkComponentPermission(UserRole.USER, project);
return project;
});
}

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())
.map(SearchHit::getId)
.collect(toList(result.getHits().getHits().length));
@@ -183,13 +207,18 @@ public class SearchAction implements HotspotsWsAction {
.collect(Collectors.toList());
}

private SearchResponse doIndexSearch(WsRequest wsRequest, ComponentDto project) {
private SearchResponse doIndexSearch(WsRequest wsRequest, Optional<ComponentDto> project, Set<String> hotspotKeys) {
IssueQuery.Builder builder = IssueQuery.builder()
.projectUuids(Collections.singletonList(project.uuid()))
.organizationUuid(project.getOrganizationUuid())
.types(singleton(RuleType.SECURITY_HOTSPOT.name()))
.sort(IssueQuery.SORT_HOTSPOTS)
.asc(true);
project.ifPresent(p -> {
builder.projectUuids(Collections.singletonList(p.uuid()));
builder.organizationUuid(p.getOrganizationUuid());
});
if (!hotspotKeys.isEmpty()) {
builder.issueKeys(hotspotKeys);
}
wsRequest.getStatus().ifPresent(status -> builder.resolved(STATUS_REVIEWED.equals(status)));
wsRequest.getResolution().ifPresent(resolution -> builder.resolutions(singleton(resolution)));

@@ -288,13 +317,15 @@ public class SearchAction implements HotspotsWsAction {
private final int page;
private final int index;
private final String projectKey;
private final Set<String> hotspotKeys;
private final String status;
private final String resolution;

private WsRequest(int page, int index, String projectKey, @Nullable String status, @Nullable String resolution) {
private WsRequest(int page, int index, @Nullable String projectKey, Set<String> hotspotKeys, @Nullable String status, @Nullable String resolution) {
this.page = page;
this.index = index;
this.projectKey = projectKey;
this.hotspotKeys = hotspotKeys;
this.status = status;
this.resolution = resolution;
}
@@ -307,8 +338,12 @@ public class SearchAction implements HotspotsWsAction {
return index;
}

String getProjectKey() {
return projectKey;
Optional<String> getProjectKey() {
return ofNullable(projectKey);
}

Set<String> getHotspotKeys() {
return hotspotKeys;
}

Optional<String> getStatus() {

+ 100
- 22
server/sonar-webserver-webapi/src/test/java/org/sonar/server/hotspot/ws/SearchActionTest.java View File

@@ -25,6 +25,7 @@ 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.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
@@ -32,7 +33,6 @@ 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 javax.annotation.Nullable;
@@ -70,6 +70,8 @@ import org.sonarqube.ws.Hotspots.Component;
import org.sonarqube.ws.Hotspots.SearchWsResponse;

import static java.util.Collections.singleton;
import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
import static org.apache.commons.lang.RandomStringUtils.randomAlphabetic;
import static org.assertj.core.api.Assertions.assertThat;
@@ -113,12 +115,12 @@ public class SearchActionTest {
}

@Test
public void fails_with_IAE_if_parameter_projectKey_is_missing() {
public void fails_with_IAE_if_parameters_projectKey_and_hotspots_are_missing() {
TestRequest request = actionTester.newRequest();

assertThatThrownBy(request::execute)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("The 'projectKey' parameter is missing");
.hasMessage("A value must be provided for either parameter 'projectKey' or parameter 'hotspots'");
}

@Test
@@ -144,6 +146,18 @@ public class SearchActionTest {
.toArray(Object[][]::new);
}

@Test
@UseDataProvider("validStatusesAndResolutions")
public void fail_with_IAE_if_parameter_status_is_specified_with_hotspots_parameter(String status, @Nullable String notUsed) {
TestRequest request = actionTester.newRequest()
.setParam("hotspots", randomAlphabetic(12))
.setParam("status", status);

assertThatThrownBy(request::execute)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Parameter 'status' can't be used with parameter 'hotspots'");
}

@Test
@UseDataProvider("badResolutions")
public void fails_with_IAE_if_resolution_parameter_is_neither_FIXED_nor_SAFE(String badResolution) {
@@ -183,6 +197,18 @@ public class SearchActionTest {
.hasMessage("Value '" + resolution + "' of parameter 'resolution' can only be provided if value of parameter 'status' is 'REVIEWED'");
}

@Test
@UseDataProvider("fixedOrSafeResolution")
public void fails_with_IAE_if_resolution_is_provided_with_hotspots_parameter(String resolution) {
TestRequest request = actionTester.newRequest()
.setParam("hotspots", randomAlphabetic(13))
.setParam("resolution", resolution);

assertThatThrownBy(request::execute)
.isInstanceOf(IllegalArgumentException.class)
.hasMessage("Parameter 'resolution' can't be used with parameter 'hotspots'");
}

@DataProvider
public static Object[][] fixedOrSafeResolution() {
return new Object[][] {
@@ -265,7 +291,7 @@ public class SearchActionTest {
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));
return insertHotspot(project, file, rule);
})
.toArray(IssueDto[]::new);
indexIssues();
@@ -317,7 +343,7 @@ public class SearchActionTest {
IssueDto[] hotspots = IntStream.range(0, 1 + RANDOM.nextInt(10))
.mapToObj(i -> {
RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
return dbTester.issues().insert(rule, project, fileWithHotspot, t -> t.setType(SECURITY_HOTSPOT));
return insertHotspot(project, fileWithHotspot, rule);
})
.toArray(IssueDto[]::new);
indexIssues();
@@ -341,7 +367,7 @@ public class SearchActionTest {
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));
return insertHotspot(project, project, rule);
})
.toArray(IssueDto[]::new);
indexIssues();
@@ -368,8 +394,8 @@ public class SearchActionTest {
IssueDto[] hotspots2 = IntStream.range(0, 1 + RANDOM.nextInt(10))
.mapToObj(i -> {
RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
dbTester.issues().insert(rule, project1, file1, t -> t.setType(SECURITY_HOTSPOT));
return dbTester.issues().insert(rule, project2, file2, t -> t.setType(SECURITY_HOTSPOT));
insertHotspot(project1, file1, rule);
return insertHotspot(project2, file2, rule);
})
.toArray(IssueDto[]::new);
indexIssues();
@@ -379,7 +405,7 @@ public class SearchActionTest {

assertThat(responseProject1.getHotspotsList())
.extracting(SearchWsResponse.Hotspot::getKey)
.doesNotContainAnyElementsOf(Arrays.stream(hotspots2).map(IssueDto::getKey).collect(Collectors.toList()));
.doesNotContainAnyElementsOf(Arrays.stream(hotspots2).map(IssueDto::getKey).collect(toList()));
assertThat(responseProject1.getComponentsList())
.extracting(Component::getKey)
.containsOnly(project1.getKey(), file1.getKey());
@@ -402,7 +428,7 @@ public class SearchActionTest {
indexPermissions();
ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
List<IssueDto> hotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file)
.collect(Collectors.toList());
.collect(toList());
indexIssues();

SearchWsResponse response = newRequest(project, null, null)
@@ -421,7 +447,7 @@ public class SearchActionTest {
ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
List<IssueDto> reviewedHotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file)
.filter(t -> STATUS_REVIEWED.equals(t.getStatus()))
.collect(Collectors.toList());
.collect(toList());
indexIssues();

SearchWsResponse response = newRequest(project, STATUS_REVIEWED, null)
@@ -440,7 +466,7 @@ public class SearchActionTest {
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());
.collect(toList());
indexIssues();

SearchWsResponse response = newRequest(project, STATUS_REVIEWED, RESOLUTION_SAFE)
@@ -459,7 +485,7 @@ public class SearchActionTest {
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());
.collect(toList());
indexIssues();

SearchWsResponse response = newRequest(project, STATUS_REVIEWED, RESOLUTION_FIXED)
@@ -507,7 +533,7 @@ public class SearchActionTest {
.setStatus(status)
.setResolution(resolution));
})
.collect(Collectors.toList());
.collect(toList());
Collections.shuffle(hotspots);
hotspots.forEach(t -> dbTester.issues().insertIssue(t));
return hotspots.stream();
@@ -570,7 +596,7 @@ public class SearchActionTest {
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));
IssueDto hotspot = insertHotspot(project, file, rule);
indexIssues();

SearchWsResponse response = newRequest(project)
@@ -637,9 +663,9 @@ public class SearchActionTest {
ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
ComponentDto file2 = dbTester.components().insertComponent(newFileDto(project));
RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
IssueDto fileHotspot = dbTester.issues().insert(rule, project, file, t -> t.setType(SECURITY_HOTSPOT));
IssueDto dirHotspot = dbTester.issues().insert(rule, project, directory, t -> t.setType(SECURITY_HOTSPOT));
IssueDto projectHotspot = dbTester.issues().insert(rule, project, project, t -> t.setType(SECURITY_HOTSPOT));
IssueDto fileHotspot = insertHotspot(project, file, rule);
IssueDto dirHotspot = insertHotspot(project, directory, rule);
IssueDto projectHotspot = insertHotspot(project, project, rule);
indexIssues();

SearchWsResponse response = newRequest(project)
@@ -695,7 +721,7 @@ public class SearchActionTest {
newIssue(rule1, project, file).setKee(sqCategory + "_a").setType(SECURITY_HOTSPOT),
newIssue(rule2, project, file).setKee(sqCategory + "_b").setType(SECURITY_HOTSPOT));
})
.collect(Collectors.toList());
.collect(toList());
String[] expectedHotspotKeys = hotspots.stream().map(IssueDto::getKey).toArray(String[]::new);
// insert hotspots in random order
Collections.shuffle(hotspots);
@@ -728,7 +754,7 @@ public class SearchActionTest {
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());
.collect(toList());
String[] expectedHotspotKeys = hotspots.stream().map(IssueDto::getKey).toArray(String[]::new);
// insert hotspots in random order
Collections.shuffle(hotspots);
@@ -754,7 +780,7 @@ public class SearchActionTest {
List<IssueDto> 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());
.collect(toList());
indexIssues();

TestRequest request = newRequest(project);
@@ -797,7 +823,7 @@ public class SearchActionTest {
List<IssueDto> 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());
.collect(toList());
indexIssues();

SearchWsResponse response = newRequest(project)
@@ -838,6 +864,53 @@ public class SearchActionTest {
assertThat(response.getPaging().getPageSize()).isEqualTo(pageSize);
}

@Test
public void returns_empty_if_none_of_hotspot_keys_exist() {
ComponentDto project = dbTester.components().insertPublicProject();
userSessionRule.registerComponents(project);
indexPermissions();
ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
List<IssueDto> hotspots = IntStream.range(0, 1 + RANDOM.nextInt(15))
.mapToObj(i -> newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i))
.map(i -> dbTester.issues().insertIssue(i))
.collect(toList());
indexIssues();

SearchWsResponse response = newRequest(IntStream.range(0, 1 + RANDOM.nextInt(30)).mapToObj(i -> "key_" + i).collect(toList()))
.executeProtobuf(SearchWsResponse.class);

assertThat(response.getHotspotsList()).isEmpty();
}

@Test
public void returns_specified_hotspots() {
ComponentDto project = dbTester.components().insertPublicProject();
userSessionRule.registerComponents(project);
indexPermissions();
ComponentDto file = dbTester.components().insertComponent(newFileDto(project));
RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT);
int total = 1 + RANDOM.nextInt(20);
List<IssueDto> hotspots = IntStream.range(0, total)
.mapToObj(i -> newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i))
.map(i -> dbTester.issues().insertIssue(i))
.collect(toList());
Collections.shuffle(hotspots);
List<IssueDto> selectedHotspots = hotspots.stream().limit(total == 1 ? 1 : 1 + RANDOM.nextInt(total - 1)).collect(toList());
indexIssues();

SearchWsResponse response = newRequest(selectedHotspots.stream().map(IssueDto::getKey).collect(toList()))
.executeProtobuf(SearchWsResponse.class);

assertThat(response.getHotspotsList())
.extracting(SearchWsResponse.Hotspot::getKey)
.containsExactlyInAnyOrder(selectedHotspots.stream().map(IssueDto::getKey).toArray(String[]::new));
}

private IssueDto insertHotspot(ComponentDto project, ComponentDto file, RuleDefinitionDto rule) {
return dbTester.issues().insert(rule, project, file, t -> t.setType(SECURITY_HOTSPOT));
}

private TestRequest newRequest(ComponentDto project) {
return newRequest(project, null, null);
}
@@ -854,6 +927,11 @@ public class SearchActionTest {
return res;
}

private TestRequest newRequest(Collection<String> hotspotKeys) {
return actionTester.newRequest()
.setParam("hotspots", hotspotKeys.stream().collect(joining(",")));
}

private void indexPermissions() {
permissionIndexer.indexOnStartup(permissionIndexer.getIndexTypes());
}

Loading…
Cancel
Save