From 671130c3a2b7aa4fe8d715386d9fc0e62ef0812b Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Fri, 13 Dec 2013 17:51:43 +0100 Subject: [PATCH] Add checks for rules query parameters and paging --- .../exceptions/BadRequestException.java | 5 ++ .../server/qualityprofile/PagingResult.java | 14 ++-- .../qualityprofile/QProfileRuleResult.java | 8 +-- .../sonar/server/rule/ProfileRuleQuery.java | 17 +++-- .../org/sonar/server/rule/ProfileRules.java | 6 +- .../server/rule/ProfileRuleQueryTest.java | 72 +++++++++++++++++++ .../sonar/server/rule/ProfileRulesTest.java | 14 ++-- 7 files changed, 106 insertions(+), 30 deletions(-) create mode 100644 sonar-server/src/test/java/org/sonar/server/rule/ProfileRuleQueryTest.java diff --git a/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java b/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java index 6776fcd8b7f..0487470eb6f 100644 --- a/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java +++ b/sonar-server/src/main/java/org/sonar/server/exceptions/BadRequestException.java @@ -141,4 +141,9 @@ public class BadRequestException extends ServerException { } } + public void checkMessages() { + if (! errors.isEmpty()) { + throw this; + } + } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/PagingResult.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/PagingResult.java index 3d7022c3a1e..aa82b4ba568 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/PagingResult.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/PagingResult.java @@ -26,9 +26,9 @@ package org.sonar.server.qualityprofile; */ public class PagingResult extends Paging { - private final int total; + private final long total; - private PagingResult(int pageSize, int pageIndex, int total) { + private PagingResult(int pageSize, int pageIndex, long total) { super(pageSize, pageIndex); this.total = total; } @@ -36,8 +36,8 @@ public class PagingResult extends Paging { /** * Number of pages. It is greater than or equal 0. */ - public int pages() { - int p = total / pageSize(); + public long pages() { + long p = total / pageSize(); if (total % pageSize() > 0) { p++; } @@ -51,11 +51,11 @@ public class PagingResult extends Paging { /** * Total number of items. It is greater than or equal 0. */ - public int total() { + public long total() { return total; } - public static PagingResult create(int pageSize, int pageIndex, int totalItems) { + public static PagingResult create(int pageSize, int pageIndex, long totalItems) { checkPageSize(pageSize); checkPageIndex(pageIndex); checkTotalItems(totalItems); @@ -63,7 +63,7 @@ public class PagingResult extends Paging { return new PagingResult(pageSize, pageIndex, totalItems); } - protected static void checkTotalItems(int totalItems) { + protected static void checkTotalItems(long totalItems) { if (totalItems<0) { throw new IllegalArgumentException("Total items must be positive. Got " + totalItems); } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleResult.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleResult.java index 21158a0abff..f964944868e 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleResult.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleResult.java @@ -19,17 +19,15 @@ */ package org.sonar.server.qualityprofile; -import org.sonar.api.utils.Paging; - import java.util.Collection; import java.util.List; public class QProfileRuleResult { private final List rules; - private final Paging paging; + private final PagingResult paging; - public QProfileRuleResult(List rules, Paging paging) { + public QProfileRuleResult(List rules, PagingResult paging) { this.rules = rules; this.paging = paging; } @@ -38,7 +36,7 @@ public class QProfileRuleResult { return rules; } - public Paging paging() { + public PagingResult paging() { return paging; } } diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ProfileRuleQuery.java b/sonar-server/src/main/java/org/sonar/server/rule/ProfileRuleQuery.java index 2d9b7c7052b..adaf03621b0 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/ProfileRuleQuery.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/ProfileRuleQuery.java @@ -23,6 +23,7 @@ package org.sonar.server.rule; import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import org.apache.commons.lang.StringUtils; +import org.sonar.server.exceptions.BadRequestException; import javax.annotation.CheckForNull; @@ -48,30 +49,28 @@ public class ProfileRuleQuery { statuses = Lists.newArrayList(); } - public static ProfileRuleQuery parse(Map params) throws ValidationException { - List validationMessages = Lists.newArrayList(); + public static ProfileRuleQuery parse(Map params) { + BadRequestException validationException = BadRequestException.of("Incorrect rule search parameters"); - validatePresenceOf(params, validationMessages, "profileId"); + validatePresenceOf(params, validationException, "profileId"); ProfileRuleQuery result = new ProfileRuleQuery(); try { result.profileId = Integer.parseInt((String) params.get("profileId")); } catch (Exception badProfileId) { - validationMessages.add("profileId could not be parsed"); + validationException.addError("profileId could not be parsed"); } - if (! validationMessages.isEmpty()) { - throw new ValidationException(validationMessages); - } + validationException.checkMessages(); return result; } - private static void validatePresenceOf(Map params, List validationMessages, String... paramNames) { + private static void validatePresenceOf(Map params, BadRequestException validationException, String... paramNames) { for (String param: paramNames) { if (params.get(param) == null) { - validationMessages.add("Missing parameter "+param); + validationException.addError("Missing parameter "+param); } } } diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java b/sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java index 87675123ef4..e5215173be3 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java @@ -30,7 +30,9 @@ import org.elasticsearch.index.query.MatchQueryBuilder.Operator; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHits; import org.sonar.server.qualityprofile.Paging; +import org.sonar.server.qualityprofile.PagingResult; import org.sonar.server.qualityprofile.QProfileRule; +import org.sonar.server.qualityprofile.QProfileRuleResult; import org.sonar.server.search.SearchIndex; import java.util.Collection; @@ -47,7 +49,7 @@ public class ProfileRules { this.index = index; } - public List searchActiveRules(ProfileRuleQuery query, Paging paging) { + public QProfileRuleResult searchActiveRules(ProfileRuleQuery query, Paging paging) { BoolFilterBuilder filter = boolFilter().must( termFilter("profileId", query.profileId()), hasParentFilter("rule", parentRuleFilter(query)) @@ -81,7 +83,7 @@ public class ProfileRules { } } - return result; + return new QProfileRuleResult(result, PagingResult.create(paging.pageSize(), paging.pageIndex(), hits.getTotalHits())); } private FilterBuilder parentRuleFilter(ProfileRuleQuery query) { diff --git a/sonar-server/src/test/java/org/sonar/server/rule/ProfileRuleQueryTest.java b/sonar-server/src/test/java/org/sonar/server/rule/ProfileRuleQueryTest.java new file mode 100644 index 00000000000..3577f311899 --- /dev/null +++ b/sonar-server/src/test/java/org/sonar/server/rule/ProfileRuleQueryTest.java @@ -0,0 +1,72 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * SonarQube is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ + +package org.sonar.server.rule; + +import com.google.common.collect.ImmutableMap; +import org.fest.assertions.Fail; +import org.junit.Test; +import org.sonar.server.exceptions.BadRequestException; + +import java.util.Map; + +import static org.fest.assertions.Assertions.assertThat; + +public class ProfileRuleQueryTest { + + @Test + public void should_create_basic_query() { + final int profileId = 42; + ProfileRuleQuery query = ProfileRuleQuery.create(profileId); + assertThat(query.profileId()).isEqualTo(profileId); + assertThat(query.hasParentRuleCriteria()).isFalse(); + } + + @Test + public void should_parse_nominal_request() { + final int profileId = 42; + Map params = ImmutableMap.of("profileId", (Object) Integer.toString(profileId)); + ProfileRuleQuery query = ProfileRuleQuery.parse(params); + assertThat(query.profileId()).isEqualTo(profileId); + assertThat(query.hasParentRuleCriteria()).isFalse(); + } + + @Test + public void should_fail_on_missing_profileId() { + Map params = ImmutableMap.of(); + try { + ProfileRuleQuery.parse(params); + Fail.fail("Expected BadRequestException"); + } catch(BadRequestException bre) { + assertThat(bre.errors().get(0).text()).isEqualTo("Missing parameter profileId"); + } + } + + @Test + public void should_fail_on_incorrect_profileId() { + Map params = ImmutableMap.of("profileId", (Object) "not an integer"); + try { + ProfileRuleQuery.parse(params); + Fail.fail("Expected BadRequestException"); + } catch(BadRequestException bre) { + assertThat(bre.errors().get(0).text()).isEqualTo("profileId could not be parsed"); + } + } +} diff --git a/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java b/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java index 943f53464e4..56d16e980ac 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java @@ -76,25 +76,25 @@ public class ProfileRulesTest { Paging paging = Paging.create(10, 1); // All rules for profile 1 - assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1), paging)).hasSize(2); + assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1), paging).rules()).hasSize(2); // All rules for profile 2 - assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(2), paging)).hasSize(1); + assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(2), paging).rules()).hasSize(1); // Inexistent profile - assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(3), paging)).hasSize(0); + assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(3), paging).rules()).hasSize(0); // Inexistent name/key - assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1).setNameOrKey("polop"), paging)).hasSize(0); + assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1).setNameOrKey("polop"), paging).rules()).hasSize(0); // Match on key - assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1).setNameOrKey("DM_CONVERT_CASE"), paging)).hasSize(1); + assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1).setNameOrKey("DM_CONVERT_CASE"), paging).rules()).hasSize(1); // Match on name - assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1).setNameOrKey("Unused Check"), paging)).hasSize(1); + assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1).setNameOrKey("Unused Check"), paging).rules()).hasSize(1); // Match on repositoryKey - assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1).addRepositoryKeys("findbugs"), paging)).hasSize(1); + assertThat(profileRules.searchActiveRules(ProfileRuleQuery.create(1).addRepositoryKeys("findbugs"), paging).rules()).hasSize(1); } private String testFileAsString(String testFile) throws Exception { -- 2.39.5