Add checks for rules query parameters and paging

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 6776fcd8b7fd9191fef9f86252b76f08565baed6..0487470eb6f700c38b561aaf1d3664afac98d28e 100644 (file)
--- 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 3d7022c3a1eec0df137a4b53146009388bd67ff7..aa82b4ba56870ec0821c5b0a6e307ebabefd45d3 100644 (file)
--- 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 21158a0abff2b4a8dbf7f4d5ab361e252ba7f079..f964944868e7404a7dd136b335958e99dae16e25 100644 (file)
--- 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<QProfileRule> rules;
-  private final Paging paging;
+  private final PagingResult paging;
 
-  public QProfileRuleResult(List<QProfileRule> rules, Paging paging) {
+  public QProfileRuleResult(List<QProfileRule> 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 2d9b7c7052b839cd0b6d7ed53b53e0dd22ef1d4b..adaf03621b093eb0cec9ca4fc349f9aa43ce4934 100644 (file)
--- 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<String, Object> params) throws ValidationException {
-    List<String> validationMessages = Lists.newArrayList();
+  public static ProfileRuleQuery parse(Map<String, Object> 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<String, Object> params, List<String> validationMessages, String... paramNames) {
+  private static void validatePresenceOf(Map<String, Object> 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 87675123ef4658c9b712eb21fee80919faa6c95b..e5215173be31d1a9bad98107e87893a558cb1574 100644 (file)
--- 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<QProfileRule> 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 (file)
index 0000000..3577f31
--- /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<String, Object> 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<String, Object> 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<String, Object> 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 943f53464e40a8e73ce2dcbafd7e14bc1ff6331a..56d16e980ac16869c16666fddf14c5d87cb844a0 100644 (file)
--- 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 {