From 5aeeedbda63c17fefc7bba99dbf8c718c7bbbd17 Mon Sep 17 00:00:00 2001 From: Teryk Bellahsene Date: Mon, 7 Dec 2015 15:03:34 +0100 Subject: [PATCH] SONAR-6820 WS api/rules/search has a page size limit of 500 --- .../sonar/server/rule/ws/SearchAction.java | 9 +- .../rule/ws/SearchActionMediumTest.java | 183 ++++++++++-------- .../org/sonar/api/server/ws/WebService.java | 18 ++ 3 files changed, 123 insertions(+), 87 deletions(-) diff --git a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/SearchAction.java b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/SearchAction.java index 00439eb0f9b..7fed268d5ad 100644 --- a/server/sonar-server/src/main/java/org/sonar/server/rule/ws/SearchAction.java +++ b/server/sonar-server/src/main/java/org/sonar/server/rule/ws/SearchAction.java @@ -56,6 +56,7 @@ import org.sonar.server.user.UserSession; import org.sonarqube.ws.Common; import org.sonarqube.ws.Rules.SearchResponse; +import static org.sonar.server.ws.WsUtils.checkRequest; import static org.sonar.server.ws.WsUtils.writeProtobuf; /** @@ -81,6 +82,8 @@ public class SearchAction implements RulesWsAction { public static final String PARAM_TEMPLATE_KEY = "template_key"; private static final Collection DEFAULT_FACETS = ImmutableSet.of(PARAM_LANGUAGES, PARAM_REPOSITORIES, "tags"); + private static final int MAX_PAGE_SIZE = 500; + private static final String MSG_MAX_PAGE_SIZE_ERROR = "Page size must be less than " + MAX_PAGE_SIZE; private final RuleService ruleService; private final ActiveRuleCompleter activeRuleCompleter; @@ -97,7 +100,7 @@ public class SearchAction implements RulesWsAction { @Override public void define(WebService.NewController controller) { WebService.NewAction action = controller.createAction(ACTION) - .addPagingParams(100) + .addPagingParams(100, MAX_PAGE_SIZE) .setHandler(this); Collection possibleFacets = possibleFacets(); @@ -167,8 +170,7 @@ public class SearchAction implements RulesWsAction { RuleIndex.FACET_SEVERITIES, RuleIndex.FACET_ACTIVE_SEVERITIES, RuleIndex.FACET_STATUSES, - RuleIndex.FACET_OLD_DEFAULT - ); + RuleIndex.FACET_OLD_DEFAULT); } /** @@ -330,6 +332,7 @@ public class SearchAction implements RulesWsAction { private QueryContext loadCommonContext(Request request) { int pageSize = request.mandatoryParamAsInt(Param.PAGE_SIZE); + checkRequest(pageSize <= MAX_PAGE_SIZE, MSG_MAX_PAGE_SIZE_ERROR); QueryContext context = new QueryContext(userSession).addFieldsToReturn(request.paramAsStrings(Param.FIELDS)); List facets = request.paramAsStrings(Param.FACETS); if (facets != null) { diff --git a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionMediumTest.java b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionMediumTest.java index 46124819e38..2323e7b6b8e 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionMediumTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/rule/ws/SearchActionMediumTest.java @@ -20,11 +20,15 @@ package org.sonar.server.rule.ws; import com.google.common.collect.ImmutableSet; +import java.util.Calendar; +import java.util.Collections; +import java.util.Date; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; +import org.junit.rules.ExpectedException; import org.sonar.api.rule.RuleKey; import org.sonar.api.rule.RuleStatus; import org.sonar.api.rule.Severity; @@ -42,6 +46,7 @@ import org.sonar.db.rule.RuleParamDto; import org.sonar.db.rule.RuleTesting; import org.sonar.server.db.DbClient; import org.sonar.server.debt.DebtTesting; +import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.qualityprofile.QProfileTesting; import org.sonar.server.qualityprofile.db.ActiveRuleDao; import org.sonar.server.rule.db.RuleDao; @@ -50,10 +55,6 @@ import org.sonar.server.tester.ServerTester; import org.sonar.server.tester.UserSessionRule; import org.sonar.server.ws.WsTester; -import java.util.Calendar; -import java.util.Collections; -import java.util.Date; - import static org.assertj.core.api.Assertions.assertThat; public class SearchActionMediumTest { @@ -62,6 +63,8 @@ public class SearchActionMediumTest { public static ServerTester tester = new ServerTester().addXoo(); @Rule public UserSessionRule userSessionRule = UserSessionRule.forServerTester(tester); + @Rule + public ExpectedException expectedException = ExpectedException.none(); private static final String API_ENDPOINT = "api/rules"; private static final String API_SEARCH_METHOD = "search"; @@ -185,16 +188,16 @@ public class SearchActionMediumTest { insertDebtCharacteristics(dbSession); ruleDao.insert(dbSession, RuleTesting.newXooX1() - .setDefaultSubCharacteristicId(hardReliabilityId) - .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setDefaultRemediationCoefficient("1h") - .setDefaultRemediationOffset("15min") - - .setSubCharacteristicId(null) - .setRemediationFunction(null) - .setRemediationCoefficient(null) - .setRemediationOffset(null) - ); + .setDefaultSubCharacteristicId(hardReliabilityId) + .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setDefaultRemediationCoefficient("1h") + .setDefaultRemediationOffset("15min") + + .setSubCharacteristicId(null) + .setRemediationFunction(null) + .setRemediationCoefficient(null) + .setRemediationOffset(null) + ); dbSession.commit(); WsTester.TestRequest request = tester.wsTester().newGetRequest(API_ENDPOINT, API_SEARCH_METHOD); @@ -209,16 +212,16 @@ public class SearchActionMediumTest { insertDebtCharacteristics(dbSession); ruleDao.insert(dbSession, RuleTesting.newXooX1() - .setDefaultSubCharacteristicId(hardReliabilityId) - .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setDefaultRemediationCoefficient("1h") - .setDefaultRemediationOffset("15min") - - .setSubCharacteristicId(softReliabilityId) - .setRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setRemediationCoefficient("2h") - .setRemediationOffset("25min") - ); + .setDefaultSubCharacteristicId(hardReliabilityId) + .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setDefaultRemediationCoefficient("1h") + .setDefaultRemediationOffset("15min") + + .setSubCharacteristicId(softReliabilityId) + .setRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setRemediationCoefficient("2h") + .setRemediationOffset("25min") + ); dbSession.commit(); WsTester.TestRequest request = tester.wsTester().newGetRequest(API_ENDPOINT, API_SEARCH_METHOD); @@ -232,16 +235,16 @@ public class SearchActionMediumTest { insertDebtCharacteristics(dbSession); ruleDao.insert(dbSession, RuleTesting.newXooX1() - .setDefaultSubCharacteristicId(hardReliabilityId) - .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setDefaultRemediationCoefficient("1h") - .setDefaultRemediationOffset("15min") - - .setSubCharacteristicId(softReliabilityId) - .setRemediationFunction(DebtRemediationFunction.Type.CONSTANT_ISSUE.name()) - .setRemediationCoefficient(null) - .setRemediationOffset("5min") - ); + .setDefaultSubCharacteristicId(hardReliabilityId) + .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setDefaultRemediationCoefficient("1h") + .setDefaultRemediationOffset("15min") + + .setSubCharacteristicId(softReliabilityId) + .setRemediationFunction(DebtRemediationFunction.Type.CONSTANT_ISSUE.name()) + .setRemediationCoefficient(null) + .setRemediationOffset("5min") + ); dbSession.commit(); WsTester.TestRequest request = tester.wsTester().newGetRequest(API_ENDPOINT, API_SEARCH_METHOD); @@ -255,16 +258,16 @@ public class SearchActionMediumTest { insertDebtCharacteristics(dbSession); ruleDao.insert(dbSession, RuleTesting.newXooX1() - .setDefaultSubCharacteristicId(hardReliabilityId) - .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setDefaultRemediationCoefficient("1h") - .setDefaultRemediationOffset("15min") - - .setSubCharacteristicId(softReliabilityId) - .setRemediationFunction(DebtRemediationFunction.Type.LINEAR.name()) - .setRemediationCoefficient("1h") - .setRemediationOffset(null) - ); + .setDefaultSubCharacteristicId(hardReliabilityId) + .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setDefaultRemediationCoefficient("1h") + .setDefaultRemediationOffset("15min") + + .setSubCharacteristicId(softReliabilityId) + .setRemediationFunction(DebtRemediationFunction.Type.LINEAR.name()) + .setRemediationCoefficient("1h") + .setRemediationOffset(null) + ); dbSession.commit(); WsTester.TestRequest request = tester.wsTester().newGetRequest(API_ENDPOINT, API_SEARCH_METHOD); @@ -278,49 +281,49 @@ public class SearchActionMediumTest { insertDebtCharacteristics(dbSession); ruleDao.insert(dbSession, RuleTesting.newXooX1() - .setDefaultSubCharacteristicId(hardReliabilityId) - .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setDefaultRemediationCoefficient("1h") - .setDefaultRemediationOffset("15min") - - .setSubCharacteristicId(null) - .setRemediationFunction(null) - .setRemediationCoefficient(null) - .setRemediationOffset(null) - ); + .setDefaultSubCharacteristicId(hardReliabilityId) + .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setDefaultRemediationCoefficient("1h") + .setDefaultRemediationOffset("15min") + + .setSubCharacteristicId(null) + .setRemediationFunction(null) + .setRemediationCoefficient(null) + .setRemediationOffset(null) + ); ruleDao.insert(dbSession, RuleTesting.newXooX2() - .setDefaultSubCharacteristicId(hardReliabilityId) - .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setDefaultRemediationCoefficient("1h") - .setDefaultRemediationOffset("15min") - - .setSubCharacteristicId(softReliabilityId) - .setRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setRemediationCoefficient("30min") - .setRemediationOffset("5min") - ); + .setDefaultSubCharacteristicId(hardReliabilityId) + .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setDefaultRemediationCoefficient("1h") + .setDefaultRemediationOffset("15min") + + .setSubCharacteristicId(softReliabilityId) + .setRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setRemediationCoefficient("30min") + .setRemediationOffset("5min") + ); ruleDao.insert(dbSession, RuleTesting.newXooX3() - .setDefaultSubCharacteristicId(null) - .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setDefaultRemediationCoefficient("2min") - .setDefaultRemediationOffset("1min") - - .setSubCharacteristicId(null) - .setRemediationFunction(null) - .setRemediationCoefficient(null) - .setRemediationOffset(null) - ); + .setDefaultSubCharacteristicId(null) + .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setDefaultRemediationCoefficient("2min") + .setDefaultRemediationOffset("1min") + + .setSubCharacteristicId(null) + .setRemediationFunction(null) + .setRemediationCoefficient(null) + .setRemediationOffset(null) + ); ruleDao.insert(dbSession, RuleTesting.newDto(RuleKey.of("xoo", "x4")).setLanguage("xoo") - .setDefaultSubCharacteristicId(softReliabilityId) - .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) - .setDefaultRemediationCoefficient("2min") - .setDefaultRemediationOffset("1min") - - .setSubCharacteristicId(-1) - .setRemediationFunction(null) - .setRemediationCoefficient(null) - .setRemediationOffset(null) - ); + .setDefaultSubCharacteristicId(softReliabilityId) + .setDefaultRemediationFunction(DebtRemediationFunction.Type.LINEAR_OFFSET.name()) + .setDefaultRemediationCoefficient("2min") + .setDefaultRemediationOffset("1min") + + .setSubCharacteristicId(-1) + .setRemediationFunction(null) + .setRemediationCoefficient(null) + .setRemediationOffset(null) + ); dbSession.commit(); WsTester.Result result = tester.wsTester().newGetRequest(API_ENDPOINT, API_SEARCH_METHOD) @@ -637,6 +640,18 @@ public class SearchActionMediumTest { result.assertJson("{\"total\":0,\"p\":1,\"ps\":100,\"rules\":[]}"); } + @Test + public void fail_when_page_size_greater_than_500() throws Exception { + expectedException.expect(BadRequestException.class); + expectedException.expectMessage("Page size must be less than 500"); + + WsTester.TestRequest request = tester.wsTester().newGetRequest(API_ENDPOINT, API_SEARCH_METHOD) + .setParam(WebService.Param.PAGE_SIZE, String.valueOf(501)); + request.setParam(WebService.Param.FIELDS, "actives"); + + request.execute(); + } + private ActiveRuleDto newActiveRule(QualityProfileDto profile, RuleDto rule) { return ActiveRuleDto.createFor(profile, rule) .setInheritance(null) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java index 6141bed8d8f..c75c80daded 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/server/ws/WebService.java @@ -363,6 +363,24 @@ public interface WebService extends Definable { return this; } + /** + * Add predefined parameters related to pagination of results. + */ + public NewAction addPagingParams(int defaultPageSize, int maxSize) { + createParam(Param.PAGE) + .setDescription("1-based page number") + .setExampleValue("42") + .setDeprecatedKey("pageIndex") + .setDefaultValue("1"); + + createParam(Param.PAGE_SIZE) + .setDescription("Page size. Must be greater than 0 and less than " + maxSize) + .setExampleValue("20") + .setDeprecatedKey("pageSize") + .setDefaultValue(String.valueOf(defaultPageSize)); + return this; + } + /** * Creates the parameter {@link org.sonar.api.server.ws.WebService.Param#FIELDS}, which is * used to restrict the number of fields returned in JSON response. -- 2.39.5