From: Sébastien Lesaint Date: Wed, 11 Dec 2019 10:22:58 +0000 (+0100) Subject: SONAR-12751 add status and resolution params to api/hotspots/search X-Git-Tag: 8.2.0.32929~197 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=fc60b0a3b35ebab8ac47a3547ccd231537f4765a;p=sonarqube.git SONAR-12751 add status and resolution params to api/hotspots/search also added resolution field to the response --- 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 250278316d8..88456006569 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 @@ -31,6 +31,7 @@ import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import javax.annotation.Nullable; import org.elasticsearch.action.search.SearchResponse; import org.elasticsearch.search.SearchHit; import org.sonar.api.resources.Qualifiers; @@ -57,7 +58,14 @@ import org.sonarqube.ws.Common; import org.sonarqube.ws.Hotspots; 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.Optional.ofNullable; +import static org.sonar.api.issue.Issue.RESOLUTION_FIXED; +import static org.sonar.api.issue.Issue.RESOLUTION_SAFE; +import static org.sonar.api.issue.Issue.STATUS_REVIEWED; +import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; 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; @@ -71,6 +79,8 @@ import static org.sonarqube.ws.WsUtils.nullToEmpty; 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 final DbClient dbClient; private final UserSession userSession; @@ -99,6 +109,16 @@ public class SearchAction implements HotspotsWsAction { .setDescription("Key of the project") .setExampleValue(KEY_PROJECT_EXAMPLE_001) .setRequired(true); + action.createParam(PARAM_STATUS) + .setDescription("If provided, only Security Hotspots with the specified status are returned.") + .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)) + .setPossibleValues(RESOLUTION_FIXED, RESOLUTION_SAFE) + .setRequired(false); // FIXME add response example and test it // action.setResponseExample() } @@ -117,15 +137,24 @@ public class SearchAction implements HotspotsWsAction { } private static WsRequest toWsRequest(Request request) { - return new WsRequest(request.mandatoryParamAsInt(PAGE), request.mandatoryParamAsInt(PAGE_SIZE), request.mandatoryParam(PARAM_PROJECT_KEY)); + return new WsRequest( + request.mandatoryParamAsInt(PAGE), request.mandatoryParamAsInt(PAGE_SIZE), + request.mandatoryParam(PARAM_PROJECT_KEY), + request.param(PARAM_STATUS), request.param(PARAM_RESOLUTION)); } private ComponentDto validateRequest(DbSession dbSession, WsRequest wsRequest) { + 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(String.format("Project '%s' not found", wsRequest.getProjectKey()))); + .orElseThrow(() -> new NotFoundException(format("Project '%s' not found", wsRequest.getProjectKey()))); userSession.checkComponentPermission(UserRole.USER, project); return project; } @@ -158,10 +187,12 @@ public class SearchAction implements HotspotsWsAction { IssueQuery.Builder builder = IssueQuery.builder() .projectUuids(Collections.singletonList(project.uuid())) .organizationUuid(project.getOrganizationUuid()) - .types(Collections.singleton(RuleType.SECURITY_HOTSPOT.name())) - .resolved(false) + .types(singleton(RuleType.SECURITY_HOTSPOT.name())) .sort(IssueQuery.SORT_HOTSPOTS) .asc(true); + wsRequest.getStatus().ifPresent(status -> builder.resolved(STATUS_REVIEWED.equals(status))); + wsRequest.getResolution().ifPresent(resolution -> builder.resolutions(singleton(resolution))); + IssueQuery query = builder.build(); SearchOptions searchOptions = new SearchOptions() .setPage(wsRequest.page, wsRequest.index); @@ -218,7 +249,7 @@ public class SearchAction implements HotspotsWsAction { 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( + .orElseThrow(() -> new IllegalStateException(format( "Rule with key '%s' not found for Hotspot '%s'", hotspot.getRuleKey(), hotspot.getKey()))); SecurityStandards.SQCategory sqCategory = fromSecurityStandards(rule.getSecurityStandards()).getSqCategory(); builder @@ -229,15 +260,11 @@ public class SearchAction implements HotspotsWsAction { .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); + ofNullable(hotspot.getResolution()).ifPresent(builder::setResolution); ofNullable(hotspot.getLine()).ifPresent(builder::setLine); builder.setMessage(nullToEmpty(hotspot.getMessage())); ofNullable(hotspot.getAssigneeUuid()).ifPresent(builder::setAssignee); - // FIXME Filter author only if user is member of the organization (as done in issues/search WS) - // if (data.getUserOrganizationUuids().contains(component.getOrganizationUuid())) { builder.setAuthor(nullToEmpty(hotspot.getAuthorLogin())); - // } builder.setCreationDate(formatDateTime(hotspot.getIssueCreationDate())); builder.setUpdateDate(formatDateTime(hotspot.getIssueUpdateDate())); @@ -261,11 +288,15 @@ public class SearchAction implements HotspotsWsAction { private final int page; private final int index; private final String projectKey; + private final String status; + private final String resolution; - private WsRequest(int page, int index, String projectKey) { + private WsRequest(int page, int index, String projectKey, @Nullable String status, @Nullable String resolution) { this.page = page; this.index = index; this.projectKey = projectKey; + this.status = status; + this.resolution = resolution; } int getPage() { @@ -279,6 +310,14 @@ public class SearchAction implements HotspotsWsAction { String getProjectKey() { return projectKey; } + + Optional getStatus() { + return ofNullable(status); + } + + Optional getResolution() { + return ofNullable(resolution); + } } private static final class SearchResponseData { 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 d1a1fc45ef9..e0cf3bd3611 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 @@ -35,6 +35,7 @@ import java.util.function.Consumer; import java.util.stream.Collectors; import java.util.stream.IntStream; import java.util.stream.Stream; +import javax.annotation.Nullable; import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; @@ -73,6 +74,10 @@ 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.sonar.api.issue.Issue.RESOLUTION_FIXED; +import static org.sonar.api.issue.Issue.RESOLUTION_SAFE; +import static org.sonar.api.issue.Issue.STATUS_REVIEWED; +import static org.sonar.api.issue.Issue.STATUS_TO_REVIEW; import static org.sonar.api.rules.RuleType.SECURITY_HOTSPOT; import static org.sonar.api.utils.DateUtils.formatDateTime; import static org.sonar.core.util.stream.MoreCollectors.uniqueIndex; @@ -116,6 +121,76 @@ public class SearchActionTest { .hasMessage("The 'projectKey' parameter is missing"); } + @Test + @UseDataProvider("badStatuses") + public void fails_with_IAE_if_status_parameter_is_neither_TO_REVIEW_or_REVIEWED(String badStatus) { + TestRequest request = actionTester.newRequest() + .setParam("projectKey", randomAlphabetic(13)) + .setParam("status", badStatus); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Value of parameter 'status' (" + badStatus + ") must be one of: [TO_REVIEW, REVIEWED]"); + } + + @DataProvider + public static Object[][] badStatuses() { + return Stream.concat( + Issue.STATUSES.stream(), + Stream.of(randomAlphabetic(3))) + .filter(t -> !STATUS_REVIEWED.equals(t)) + .filter(t -> !STATUS_TO_REVIEW.equals(t)) + .map(t -> new Object[] {t}) + .toArray(Object[][]::new); + } + + @Test + @UseDataProvider("badResolutions") + public void fails_with_IAE_if_resolution_parameter_is_neither_FIXED_nor_SAFE(String badResolution) { + TestRequest request = actionTester.newRequest() + .setParam("projectKey", randomAlphabetic(13)) + .setParam("status", STATUS_TO_REVIEW) + .setParam("resolution", badResolution); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Value of parameter 'resolution' (" + badResolution + ") must be one of: [FIXED, SAFE]"); + } + + @DataProvider + public static Object[][] badResolutions() { + return Stream.of( + Issue.RESOLUTIONS.stream(), + Issue.SECURITY_HOTSPOT_RESOLUTIONS.stream(), + Stream.of(randomAlphabetic(4))) + .flatMap(t -> t) + .filter(t -> !RESOLUTION_FIXED.equals(t)) + .filter(t -> !RESOLUTION_SAFE.equals(t)) + .map(t -> new Object[] {t}) + .toArray(Object[][]::new); + } + + @Test + @UseDataProvider("fixedOrSafeResolution") + public void fails_with_IAE_if_resolution_is_provided_with_status_TO_REVIEW(String resolution) { + TestRequest request = actionTester.newRequest() + .setParam("projectKey", randomAlphabetic(13)) + .setParam("status", STATUS_TO_REVIEW) + .setParam("resolution", resolution); + + assertThatThrownBy(request::execute) + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Value '" + resolution + "' of parameter 'resolution' can only be provided if value of parameter 'status' is 'REVIEWED'"); + } + + @DataProvider + public static Object[][] fixedOrSafeResolution() { + return new Object[][] { + {RESOLUTION_SAFE}, + {RESOLUTION_FIXED} + }; + } + @Test public void fails_with_NotFoundException_if_project_does_not_exist() { String key = randomAlphabetic(12); @@ -321,7 +396,82 @@ public class SearchActionTest { } @Test - public void returns_only_unresolved_hotspots() { + public void returns_hotpots_with_any_status_if_no_status_nor_resolution_parameter() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + List hotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file) + .collect(Collectors.toList()); + indexIssues(); + + SearchWsResponse response = newRequest(project, null, null) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .containsExactlyInAnyOrder(hotspots.stream().map(IssueDto::getKey).toArray(String[]::new)); + } + + @Test + public void returns_hotpots_reviewed_as_safe_and_fixed_if_status_is_REVIEWED_and_resolution_is_not_set() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + List reviewedHotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file) + .filter(t -> STATUS_REVIEWED.equals(t.getStatus())) + .collect(Collectors.toList()); + indexIssues(); + + SearchWsResponse response = newRequest(project, STATUS_REVIEWED, null) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .containsExactlyInAnyOrder(reviewedHotspots.stream().map(IssueDto::getKey).toArray(String[]::new)); + } + + @Test + public void returns_hotpots_reviewed_as_safe_if_status_is_REVIEWED_and_resolution_is_SAFE() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + List safeHotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file) + .filter(t -> STATUS_REVIEWED.equals(t.getStatus()) && RESOLUTION_SAFE.equals(t.getResolution())) + .collect(Collectors.toList()); + indexIssues(); + + SearchWsResponse response = newRequest(project, STATUS_REVIEWED, RESOLUTION_SAFE) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .containsExactlyInAnyOrder(safeHotspots.stream().map(IssueDto::getKey).toArray(String[]::new)); + } + + @Test + public void returns_hotpots_reviewed_as_fixed_if_status_is_REVIEWED_and_resolution_is_FIXED() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + List fixedHotspots = insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(project, file) + .filter(t -> STATUS_REVIEWED.equals(t.getStatus()) && RESOLUTION_FIXED.equals(t.getResolution())) + .collect(Collectors.toList()); + indexIssues(); + + SearchWsResponse response = newRequest(project, STATUS_REVIEWED, RESOLUTION_FIXED) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .containsExactlyInAnyOrder(fixedHotspots.stream().map(IssueDto::getKey).toArray(String[]::new)); + } + + @Test + public void returns_only_unresolved_hotspots_when_status_is_TO_REVIEW() { ComponentDto project = dbTester.components().insertPublicProject(); userSessionRule.registerComponents(project); indexPermissions(); @@ -336,7 +486,7 @@ public class SearchActionTest { t -> t.setType(SECURITY_HOTSPOT).setResolution(randomAlphabetic(5))); indexIssues(); - SearchWsResponse response = newRequest(project) + SearchWsResponse response = newRequest(project, STATUS_TO_REVIEW, null) .executeProtobuf(SearchWsResponse.class); assertThat(response.getHotspotsList()) @@ -344,8 +494,28 @@ public class SearchActionTest { .containsOnly(unresolvedHotspot.getKey(), badlyClosedHotspot.getKey()); } + private Stream insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(ComponentDto project, ComponentDto file) { + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + List hotspots = Arrays.stream(validStatusesAndResolutions()) + .flatMap(objects -> { + String status = (String) objects[0]; + String resolution = (String) objects[1]; + return IntStream.range(0, 1 + RANDOM.nextInt(15)) + .mapToObj(i -> newIssue(rule, project, file) + .setKee("hotspot_" + status + "_" + resolution + "_" + i) + .setType(SECURITY_HOTSPOT) + .setStatus(status) + .setResolution(resolution)); + }) + .collect(Collectors.toList()); + Collections.shuffle(hotspots); + hotspots.forEach(t -> dbTester.issues().insertIssue(t)); + return hotspots.stream(); + } + @Test - public void returns_fields_of_hotspot() { + @UseDataProvider("validStatusesAndResolutions") + public void returns_fields_of_hotspot(String status, @Nullable String resolution) { ComponentDto project = dbTester.components().insertPublicProject(); userSessionRule.registerComponents(project); indexPermissions(); @@ -357,7 +527,9 @@ public class SearchActionTest { .setLine(RANDOM.nextInt(230)) .setMessage(randomAlphabetic(10)) .setAssigneeUuid(randomAlphabetic(9)) - .setAuthorLogin(randomAlphabetic(8))); + .setAuthorLogin(randomAlphabetic(8)) + .setStatus(status) + .setResolution(resolution)); indexIssues(); SearchWsResponse response = newRequest(project) @@ -367,9 +539,12 @@ public class SearchActionTest { SearchWsResponse.Hotspot actual = response.getHotspots(0); assertThat(actual.getComponent()).isEqualTo(file.getKey()); assertThat(actual.getProject()).isEqualTo(project.getKey()); - assertThat(actual.getStatus()).isEqualTo(hotspot.getStatus()); - // FIXME resolution field will be added later - // assertThat(actual.getResolution()).isEqualTo(hotspot.getResolution()); + assertThat(actual.getStatus()).isEqualTo(status); + if (resolution == null) { + assertThat(actual.hasResolution()).isFalse(); + } else { + assertThat(actual.getResolution()).isEqualTo(resolution); + } assertThat(actual.getLine()).isEqualTo(hotspot.getLine()); assertThat(actual.getMessage()).isEqualTo(hotspot.getMessage()); assertThat(actual.getAssignee()).isEqualTo(hotspot.getAssigneeUuid()); @@ -378,6 +553,15 @@ public class SearchActionTest { assertThat(actual.getUpdateDate()).isEqualTo(formatDateTime(hotspot.getIssueUpdateDate())); } + @DataProvider + public static Object[][] validStatusesAndResolutions() { + return new Object[][] { + {STATUS_TO_REVIEW, null}, + {STATUS_REVIEWED, RESOLUTION_FIXED}, + {STATUS_REVIEWED, RESOLUTION_SAFE}, + }; + } + @Test @UseDataProvider("allSQCategories") public void returns_SQCategory_and_VulnerabilityProbability_of_rule(Set securityStandards, SQCategory expected) { @@ -422,8 +606,7 @@ public class SearchActionTest { dbTester.issues().insert(rule, project, file, t -> t.setType(SECURITY_HOTSPOT) .setStatus(null) - // FIXME resolution field will be added later - // .setResolution(null) + .setResolution(null) .setLine(null) .setMessage(null) .setAssigneeUuid(null) @@ -437,8 +620,7 @@ public class SearchActionTest { .hasSize(1); SearchWsResponse.Hotspot actual = response.getHotspots(0); assertThat(actual.hasStatus()).isFalse(); - // FIXME resolution field will be added later - // assertThat(actual.hasResolution()).isFalse(); + assertThat(actual.hasResolution()).isFalse(); assertThat(actual.hasLine()).isFalse(); assertThat(actual.getMessage()).isEmpty(); assertThat(actual.hasAssignee()).isFalse(); @@ -657,8 +839,19 @@ public class SearchActionTest { } private TestRequest newRequest(ComponentDto project) { - return actionTester.newRequest() + return newRequest(project, null, null); + } + + private TestRequest newRequest(ComponentDto project, @Nullable String status, @Nullable String resolution) { + TestRequest res = actionTester.newRequest() .setParam("projectKey", project.getKey()); + if (status != null) { + res.setParam("status", status); + } + if (resolution != null) { + res.setParam("resolution", resolution); + } + return res; } private void indexPermissions() { diff --git a/sonar-ws/src/main/protobuf/ws-hotspots.proto b/sonar-ws/src/main/protobuf/ws-hotspots.proto index d67bc02ef6d..4d4f4efce30 100644 --- a/sonar-ws/src/main/protobuf/ws-hotspots.proto +++ b/sonar-ws/src/main/protobuf/ws-hotspots.proto @@ -39,8 +39,7 @@ message SearchWsResponse { optional string securityCategory = 4; optional string vulnerabilityProbability = 5; optional string status = 6; - // FIXME resolution field will be added later - // optional string resolution = 7; + optional string resolution = 7; optional int32 line = 8; optional string message = 9; optional string assignee = 10;