From d3cb8e19f492f9fcf53f08d88d26b2b79c45d853 Mon Sep 17 00:00:00 2001 From: =?utf8?q?S=C3=A9bastien=20Lesaint?= Date: Fri, 20 Dec 2019 13:32:24 +0100 Subject: [PATCH] SONAR-12717 api/hotspots/search does not return closed hotspots --- .../sonar/server/hotspot/ws/SearchAction.java | 7 +- .../server/hotspot/ws/SearchActionTest.java | 114 ++++++++++++------ 2 files changed, 82 insertions(+), 39 deletions(-) diff --git a/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java b/server/sonar-webserver-webapi/src/main/java/org/sonar/server/hotspot/ws/SearchAction.java index d55096ccb7d..61f1baa53ec 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 @@ -19,6 +19,7 @@ */ package org.sonar.server.hotspot.ws; +import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import java.util.Arrays; import java.util.Collection; @@ -94,6 +95,7 @@ public class SearchAction implements HotspotsWsAction { private static final String PARAM_PULL_REQUEST = "pullRequest"; private static final String PARAM_SINCE_LEAK_PERIOD = "sinceLeakPeriod"; private static final String PARAM_ONLY_MINE = "onlyMine"; + private static final List STATUSES = ImmutableList.of(STATUS_TO_REVIEW, STATUS_REVIEWED); private final DbClient dbClient; private final UserSession userSession; @@ -138,7 +140,7 @@ public class SearchAction implements HotspotsWsAction { .setExampleValue(KEY_PROJECT_EXAMPLE_001); action.createParam(PARAM_STATUS) .setDescription("If '%s' is provided, only Security Hotspots with the specified status are returned.", PARAM_PROJECT_KEY) - .setPossibleValues(STATUS_TO_REVIEW, STATUS_REVIEWED) + .setPossibleValues(STATUSES) .setRequired(false); action.createParam(PARAM_RESOLUTION) .setDescription(format( @@ -272,7 +274,8 @@ public class SearchAction implements HotspotsWsAction { IssueQuery.Builder builder = IssueQuery.builder() .types(singleton(RuleType.SECURITY_HOTSPOT.name())) .sort(IssueQuery.SORT_HOTSPOTS) - .asc(true); + .asc(true) + .statuses(wsRequest.getStatus().map(Collections::singletonList).orElse(STATUSES));; project.ifPresent(p -> { builder.organizationUuid(p.getOrganizationUuid()); 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 e61ee0238fa..d591ea0bf03 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 @@ -79,6 +79,8 @@ 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.STATUSES; +import static org.sonar.api.issue.Issue.STATUS_CLOSED; 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; @@ -498,6 +500,32 @@ public class SearchActionTest { .containsOnly(project2.getKey(), file2.getKey()); } + @Test + public void returns_only_hotspots_to_review_or_reviewed_of_project() { + ComponentDto project = dbTester.components().insertPublicProject(); + userSessionRule.registerComponents(project); + indexPermissions(); + ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); + IssueDto[] hotspots = STATUSES.stream() + .map(status -> { + RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); + return insertHotspot(rule, project, file, t -> t.setStatus(status)); + }) + .toArray(IssueDto[]::new); + indexIssues(); + String[] expectedKeys = Arrays.stream(hotspots) + .filter(t -> STATUS_REVIEWED.equals(t.getStatus()) || STATUS_TO_REVIEW.equals(t.getStatus())) + .map(IssueDto::getKey) + .toArray(String[]::new); + + SearchWsResponse response = newRequest(project) + .executeProtobuf(SearchWsResponse.class); + + assertThat(response.getHotspotsList()) + .extracting(SearchWsResponse.Hotspot::getKey) + .containsOnly(expectedKeys); + } + @Test public void returns_hotspots_of_specified_application() { ComponentDto application1 = dbTester.components().insertPublicApplication(); @@ -644,8 +672,8 @@ public class SearchActionTest { IssueDto[] assigneeHotspots = IntStream.range(0, 1 + RANDOM.nextInt(10)) .mapToObj(i -> { RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); - insertHotspot(project1, file1, rule, randomAlphabetic(5)); - return insertHotspot(project1, file1, rule, assigneeUuid); + insertHotspot(rule, project1, file1, randomAlphabetic(5)); + return insertHotspot(rule, project1, file1, assigneeUuid); }) .toArray(IssueDto[]::new); @@ -794,13 +822,11 @@ public class SearchActionTest { indexPermissions(); ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); - IssueDto unresolvedHotspot = dbTester.issues().insert(rule, project, file, - t -> t.setType(SECURITY_HOTSPOT).setResolution(null)); - // unrealistic case since a resolution must be set, but shows a limit of current implementation - IssueDto badlyClosedHotspot = dbTester.issues().insert(rule, project, file, - t -> t.setType(SECURITY_HOTSPOT).setStatus(Issue.STATUS_CLOSED).setResolution(null)); - IssueDto resolvedHotspot = dbTester.issues().insert(rule, project, file, - t -> t.setType(SECURITY_HOTSPOT).setResolution(randomAlphabetic(5))); + IssueDto unresolvedHotspot = insertHotspot(rule, project, file, t -> t.setResolution(null)); + // unrealistic case since a resolution must be set, but shows a limit of current implementation (resolution is enough) + IssueDto badlyResolved = insertHotspot(rule, project, file, t -> t.setStatus(STATUS_TO_REVIEW).setResolution(randomAlphabetic(5))); + IssueDto badlyReviewed = insertHotspot(rule, project, file, t -> t.setStatus(STATUS_REVIEWED).setResolution(null)); + IssueDto badlyClosedHotspot = insertHotspot(rule, project, file, t -> t.setStatus(STATUS_CLOSED).setResolution(null)); indexIssues(); SearchWsResponse response = newRequest(project, STATUS_TO_REVIEW, null) @@ -808,7 +834,7 @@ public class SearchActionTest { assertThat(response.getHotspotsList()) .extracting(SearchWsResponse.Hotspot::getKey) - .containsOnly(unresolvedHotspot.getKey(), badlyClosedHotspot.getKey()); + .containsOnly(unresolvedHotspot.getKey()); } private Stream insertRandomNumberOfHotspotsOfAllSupportedStatusesAndResolutions(ComponentDto project, ComponentDto file) { @@ -838,8 +864,8 @@ public class SearchActionTest { indexPermissions(); ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); - IssueDto hotspot = dbTester.issues().insert(rule, project, file, - t -> t.setType(SECURITY_HOTSPOT) + IssueDto hotspot = insertHotspot(rule, project, file, + t -> t .setStatus(randomAlphabetic(11)) .setLine(RANDOM.nextInt(230)) .setMessage(randomAlphabetic(10)) @@ -920,10 +946,8 @@ public class SearchActionTest { indexPermissions(); ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); - dbTester.issues().insert(rule, project, file, - t -> t.setType(SECURITY_HOTSPOT) - .setStatus(null) - .setResolution(null) + insertHotspot(rule, project, file, + t -> t.setResolution(null) .setLine(null) .setMessage(null) .setAssigneeUuid(null) @@ -936,7 +960,6 @@ public class SearchActionTest { assertThat(response.getHotspotsList()) .hasSize(1); SearchWsResponse.Hotspot actual = response.getHotspots(0); - assertThat(actual.hasStatus()).isFalse(); assertThat(actual.hasResolution()).isFalse(); assertThat(actual.hasLine()).isFalse(); assertThat(actual.getMessage()).isEmpty(); @@ -1085,8 +1108,8 @@ public class SearchActionTest { 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)); + newHotspot(rule1, project, file).setKee(sqCategory + "_a"), + newHotspot(rule2, project, file).setKee(sqCategory + "_b")); }) .collect(toList()); String[] expectedHotspotKeys = hotspots.stream().map(IssueDto::getKey).toArray(String[]::new); @@ -1113,14 +1136,14 @@ public class SearchActionTest { 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)) + newHotspot(rule, project, file3).setLine(8), + newHotspot(rule, project, file3).setLine(10), + newHotspot(rule, project, file1).setLine(null), + newHotspot(rule, project, file1).setLine(9), + newHotspot(rule, project, file1).setLine(11).setKee("a"), + newHotspot(rule, project, file1).setLine(11).setKee("b"), + newHotspot(rule, project, file2).setLine(null), + newHotspot(rule, project, file2).setLine(2)) .collect(toList()); String[] expectedHotspotKeys = hotspots.stream().map(IssueDto::getKey).toArray(String[]::new); // insert hotspots in random order @@ -1145,7 +1168,7 @@ public class SearchActionTest { 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)) + .mapToObj(i -> newHotspot(rule, project, file).setLine(i)) .map(i -> dbTester.issues().insertIssue(i)) .collect(toList()); indexIssues(); @@ -1188,7 +1211,7 @@ public class SearchActionTest { 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)) + .mapToObj(i -> newHotspot(rule, project, file).setLine(i).setKee("issue_" + i)) .map(i -> dbTester.issues().insertIssue(i)) .collect(toList()); indexIssues(); @@ -1239,7 +1262,7 @@ public class SearchActionTest { ComponentDto file = dbTester.components().insertComponent(newFileDto(project)); RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); List hotspots = IntStream.range(0, 1 + RANDOM.nextInt(15)) - .mapToObj(i -> newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i)) + .mapToObj(i -> newHotspot(rule, project, file).setLine(i)) .map(i -> dbTester.issues().insertIssue(i)) .collect(toList()); indexIssues(); @@ -1259,7 +1282,7 @@ public class SearchActionTest { RuleDefinitionDto rule = newRule(SECURITY_HOTSPOT); int total = 1 + RANDOM.nextInt(20); List hotspots = IntStream.range(0, total) - .mapToObj(i -> newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i)) + .mapToObj(i -> newHotspot(rule, project, file).setLine(i)) .map(i -> dbTester.issues().insertIssue(i)) .collect(toList()); Collections.shuffle(hotspots); @@ -1287,19 +1310,19 @@ public class SearchActionTest { List hotspotsInLeakPeriod = IntStream.range(0, 1 + RANDOM.nextInt(20)) .mapToObj(i -> { long issueCreationDate = periodDate + ONE_MINUTE + (RANDOM.nextInt(300) * ONE_MINUTE); - return newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i).setIssueCreationTime(issueCreationDate); + return newHotspot(rule, project, file).setLine(i).setIssueCreationTime(issueCreationDate); }) .map(i -> dbTester.issues().insertIssue(i)) .collect(toList()); // included because List atLeakPeriod = IntStream.range(0, 1 + RANDOM.nextInt(20)) - .mapToObj(i -> newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i).setIssueCreationTime(periodDate)) + .mapToObj(i -> newHotspot(rule, project, file).setLine(i).setIssueCreationTime(periodDate)) .map(i -> dbTester.issues().insertIssue(i)) .collect(toList()); List hotspotsBefore = IntStream.range(0, 1 + RANDOM.nextInt(20)) .mapToObj(i -> { long issueCreationDate = periodDate - ONE_MINUTE - (RANDOM.nextInt(300) * ONE_MINUTE); - return newIssue(rule, project, file).setType(SECURITY_HOTSPOT).setLine(i).setIssueCreationTime(issueCreationDate); + return newHotspot(rule, project, file).setLine(i).setIssueCreationTime(issueCreationDate); }) .map(i -> dbTester.issues().insertIssue(i)) .collect(toList()); @@ -1330,11 +1353,28 @@ public class SearchActionTest { } private IssueDto insertHotspot(ComponentDto project, ComponentDto file, RuleDefinitionDto rule) { - return dbTester.issues().insert(rule, project, file, t -> t.setType(SECURITY_HOTSPOT)); + return insertHotspot(rule, project, file, t -> { + }); + } + + private IssueDto insertHotspot(RuleDefinitionDto rule, ComponentDto project, ComponentDto file, @Nullable String assigneeUuid) { + return insertHotspot(rule, project, file, t -> t.setAssigneeUuid(assigneeUuid)); } - private IssueDto insertHotspot(ComponentDto project, ComponentDto file, RuleDefinitionDto rule, String assigneeUuid) { - return dbTester.issues().insert(rule, project, file, t -> t.setType(SECURITY_HOTSPOT).setAssigneeUuid(assigneeUuid)); + private IssueDto insertHotspot(RuleDefinitionDto rule, ComponentDto project, ComponentDto file, Consumer consumer) { + return dbTester.issues().insertIssue(newHotspot(rule, project, file, consumer)); + } + + private static IssueDto newHotspot(RuleDefinitionDto rule, ComponentDto project, ComponentDto component) { + return newHotspot(rule, project, component, t -> { + }); + } + + private static IssueDto newHotspot(RuleDefinitionDto rule, ComponentDto project, ComponentDto component, Consumer consumer) { + IssueDto res = newIssue(rule, project, component) + .setStatus(STATUS_TO_REVIEW); + consumer.accept(res); + return res.setType(SECURITY_HOTSPOT); } private TestRequest newRequest(ComponentDto project) { -- 2.39.5