From 67b7694f63106219f9c0c15d91aeb3eb8c0f626d Mon Sep 17 00:00:00 2001 From: Jacek Date: Tue, 9 Jun 2020 10:12:46 +0200 Subject: [PATCH] SONAR-13398 fail with 503 api/hotspots/search WS if needIssueSync is set to true --- .../sonar/server/hotspot/ws/SearchAction.java | 24 ++++++++++++-- .../server/hotspot/ws/SearchActionTest.java | 31 +++++++++++++------ 2 files changed, 43 insertions(+), 12 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 31658fea6f7..58f1cadc05d 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 @@ -56,6 +56,7 @@ import org.sonar.db.rule.RuleDefinitionDto; import org.sonar.server.es.SearchOptions; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.issue.index.IssueIndex; +import org.sonar.server.issue.index.IssueIndexSyncProgressChecker; import org.sonar.server.issue.index.IssueQuery; import org.sonar.server.security.SecurityStandards; import org.sonar.server.user.UserSession; @@ -102,13 +103,17 @@ public class SearchAction implements HotspotsWsAction { private final DbClient dbClient; private final UserSession userSession; private final IssueIndex issueIndex; + private final IssueIndexSyncProgressChecker issueIndexSyncProgressChecker; private final HotspotWsResponseFormatter responseFormatter; private System2 system2; - public SearchAction(DbClient dbClient, UserSession userSession, IssueIndex issueIndex, HotspotWsResponseFormatter responseFormatter, System2 system2) { + public SearchAction(DbClient dbClient, UserSession userSession, IssueIndex issueIndex, + IssueIndexSyncProgressChecker issueIndexSyncProgressChecker, + HotspotWsResponseFormatter responseFormatter, System2 system2) { this.dbClient = dbClient; this.userSession = userSession; this.issueIndex = issueIndex; + this.issueIndexSyncProgressChecker = issueIndexSyncProgressChecker; this.responseFormatter = responseFormatter; this.system2 = system2; } @@ -118,7 +123,8 @@ public class SearchAction implements HotspotsWsAction { WebService.NewAction action = controller .createAction("search") .setHandler(this) - .setDescription("Search for Security Hotpots.") + .setDescription("Search for Security Hotpots." + + "
When issue indexation is in progress returns 503 service unavailable HTTP code.") .setSince("8.1") .setInternal(true); @@ -166,8 +172,8 @@ public class SearchAction implements HotspotsWsAction { WsRequest wsRequest = toWsRequest(request); validateParameters(wsRequest); try (DbSession dbSession = dbClient.openSession(false)) { + checkIfNeedIssueSync(dbSession, wsRequest); Optional project = getAndValidateProjectOrApplication(dbSession, wsRequest); - SearchResponseData searchResponseData = searchHotspots(wsRequest, dbSession, project, wsRequest.getHotspotKeys()); loadComponents(dbSession, searchResponseData); loadRules(dbSession, searchResponseData); @@ -175,6 +181,18 @@ public class SearchAction implements HotspotsWsAction { } } + private void checkIfNeedIssueSync(DbSession dbSession, WsRequest wsRequest) { + Optional projectKey = wsRequest.getProjectKey(); + if (projectKey.isPresent()) { + String branch = wsRequest.getBranch().orElse(null); + String pullRequest = wsRequest.getPullRequest().orElse(null); + issueIndexSyncProgressChecker.checkIfAnyComponentsIssueSyncInProgress(dbSession, singletonList(projectKey.get()), branch, pullRequest); + } else { + // component keys not provided - asking for global + issueIndexSyncProgressChecker.checkIfIssueSyncInProgress(dbSession); + } + } + private static WsRequest toWsRequest(Request request) { List hotspotKeysList = request.paramAsStrings(PARAM_HOTSPOTS); Set hotspotKeys = hotspotKeysList == null ? ImmutableSet.of() : hotspotKeysList.stream().collect(MoreCollectors.toSet(hotspotKeysList.size())); 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 db792d87a90..21160473bcb 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 @@ -57,6 +57,7 @@ import org.sonar.server.es.EsTester; import org.sonar.server.exceptions.ForbiddenException; import org.sonar.server.exceptions.NotFoundException; import org.sonar.server.issue.index.IssueIndex; +import org.sonar.server.issue.index.IssueIndexSyncProgressChecker; import org.sonar.server.issue.index.IssueIndexer; import org.sonar.server.issue.index.IssueIteratorFactory; import org.sonar.server.organization.TestDefaultOrganizationProvider; @@ -79,6 +80,12 @@ 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.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.argThat; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.ArgumentMatchers.isNull; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; 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; @@ -113,8 +120,9 @@ public class SearchActionTest { private ViewIndexer viewIndexer = new ViewIndexer(dbClient, es.client()); private PermissionIndexer permissionIndexer = new PermissionIndexer(dbClient, es.client(), issueIndexer); private HotspotWsResponseFormatter responseFormatter = new HotspotWsResponseFormatter(defaultOrganizationProvider); - - private SearchAction underTest = new SearchAction(dbClient, userSessionRule, issueIndex, responseFormatter, system2); + private IssueIndexSyncProgressChecker issueIndexSyncProgressChecker = mock(IssueIndexSyncProgressChecker.class); + private SearchAction underTest = new SearchAction(dbClient, userSessionRule, issueIndex, + issueIndexSyncProgressChecker, responseFormatter, system2); private WsActionTester actionTester = new WsActionTester(underTest); @Test @@ -662,6 +670,10 @@ public class SearchActionTest { assertThat(responsePR.getHotspotsList()) .extracting(SearchWsResponse.Hotspot::getKey) .containsExactlyInAnyOrder(Arrays.stream(hotspotPR).map(IssueDto::getKey).toArray(String[]::new)); + + verify(issueIndexSyncProgressChecker).checkIfAnyComponentsIssueSyncInProgress(any(), argThat(arg -> arg.contains(project.getKey())), isNull(), isNull()); + verify(issueIndexSyncProgressChecker).checkIfAnyComponentsIssueSyncInProgress(any(), argThat(arg -> arg.contains(project.getKey())), eq(branch.getBranch()), isNull()); + verify(issueIndexSyncProgressChecker).checkIfAnyComponentsIssueSyncInProgress(any(), argThat(arg -> arg.contains(project.getKey())), isNull(), eq(branch.getPullRequest())); } @Test @@ -725,8 +737,8 @@ public class SearchActionTest { .setParam("hotspots", IntStream.range(2, 10).mapToObj(String::valueOf).collect(joining(","))) .setParam("onlyMine", "true") .execute()) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Parameter 'onlyMine' can be used with parameter 'projectKey' only"); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Parameter 'onlyMine' can be used with parameter 'projectKey' only"); } @Test @@ -739,8 +751,8 @@ public class SearchActionTest { .setParam("projectKey", project.getKey()) .setParam("onlyMine", "true") .execute()) - .isInstanceOf(IllegalArgumentException.class) - .hasMessage("Parameter 'onlyMine' requires user to be logged in"); + .isInstanceOf(IllegalArgumentException.class) + .hasMessage("Parameter 'onlyMine' requires user to be logged in"); } @Test @@ -1270,6 +1282,7 @@ public class SearchActionTest { SearchWsResponse response = newRequest(IntStream.range(0, 1 + RANDOM.nextInt(30)).mapToObj(i -> "key_" + i).collect(toList())) .executeProtobuf(SearchWsResponse.class); + verify(issueIndexSyncProgressChecker).checkIfIssueSyncInProgress(any()); assertThat(response.getHotspotsList()).isEmpty(); } @@ -1341,7 +1354,7 @@ public class SearchActionTest { SearchWsResponse responseOnLeak = newRequest(project, t -> t.setParam("sinceLeakPeriod", "true")) - .executeProtobuf(SearchWsResponse.class); + .executeProtobuf(SearchWsResponse.class); assertThat(responseOnLeak.getHotspotsList()) .extracting(SearchWsResponse.Hotspot::getKey) .containsExactlyInAnyOrder(Stream.concat( @@ -1378,7 +1391,7 @@ public class SearchActionTest { SearchWsResponse responseOnLeak = newRequest(project, t -> t.setParam("sinceLeakPeriod", "true")) - .executeProtobuf(SearchWsResponse.class); + .executeProtobuf(SearchWsResponse.class); assertThat(responseOnLeak.getHotspotsList()).isEmpty(); } @@ -1410,7 +1423,7 @@ public class SearchActionTest { SearchWsResponse responseOnLeak = newRequest(project, t -> t.setParam("sinceLeakPeriod", "true").setParam("pullRequest", "pr")) - .executeProtobuf(SearchWsResponse.class); + .executeProtobuf(SearchWsResponse.class); assertThat(responseOnLeak.getHotspotsList()).hasSize(3); } -- 2.39.5