From: Julien Lancelot Date: Fri, 18 Apr 2014 09:27:11 +0000 (+0200) Subject: Fix issue when searching by tags and add documentation on /api/rules/list X-Git-Tag: 4.3~11 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=a0ae48c8075e7c8c578c8fddf6ff9b6ac2c1d269;p=sonarqube.git Fix issue when searching by tags and add documentation on /api/rules/list --- diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java index 2c228636ec1..36ac539e5b9 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java @@ -20,23 +20,10 @@ package org.sonar.server.rule; -import static com.google.common.collect.Lists.newArrayList; -import static com.google.common.collect.Maps.newHashMap; -import static com.google.common.collect.Sets.newHashSet; -import static org.elasticsearch.index.query.FilterBuilders.boolFilter; -import static org.elasticsearch.index.query.FilterBuilders.termFilter; -import static org.elasticsearch.index.query.FilterBuilders.termsFilter; -import static org.sonar.api.rules.Rule.STATUS_REMOVED; - -import java.io.IOException; -import java.util.Arrays; -import java.util.Collection; -import java.util.List; -import java.util.Map; - -import javax.annotation.CheckForNull; -import javax.annotation.Nullable; - +import com.google.common.collect.ArrayListMultimap; +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableList.Builder; +import com.google.common.collect.Multimap; import org.apache.commons.lang.StringUtils; import org.apache.ibatis.session.SqlSession; import org.elasticsearch.ElasticSearchException; @@ -49,12 +36,8 @@ import org.elasticsearch.common.io.BytesStream; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; -import org.elasticsearch.index.query.BoolFilterBuilder; -import org.elasticsearch.index.query.FilterBuilder; -import org.elasticsearch.index.query.FilterBuilders; +import org.elasticsearch.index.query.*; import org.elasticsearch.index.query.MatchQueryBuilder.Operator; -import org.elasticsearch.index.query.QueryBuilder; -import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.SearchHits; import org.elasticsearch.search.sort.SortOrder; @@ -63,11 +46,7 @@ import org.slf4j.LoggerFactory; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.TimeProfiler; import org.sonar.core.persistence.MyBatis; -import org.sonar.core.rule.RuleDao; -import org.sonar.core.rule.RuleDto; -import org.sonar.core.rule.RuleParamDto; -import org.sonar.core.rule.RuleRuleTagDto; -import org.sonar.core.rule.RuleTagType; +import org.sonar.core.rule.*; import org.sonar.core.technicaldebt.db.CharacteristicDao; import org.sonar.core.technicaldebt.db.CharacteristicDto; import org.sonar.server.es.ESIndex; @@ -76,10 +55,21 @@ import org.sonar.server.paging.PagedResult; import org.sonar.server.paging.Paging; import org.sonar.server.paging.PagingResult; -import com.google.common.collect.ArrayListMultimap; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableList.Builder; -import com.google.common.collect.Multimap; +import javax.annotation.CheckForNull; +import javax.annotation.Nullable; + +import java.io.IOException; +import java.util.Arrays; +import java.util.Collection; +import java.util.List; +import java.util.Map; + +import static com.google.common.collect.Lists.newArrayList; +import static com.google.common.collect.Maps.newHashMap; +import static com.google.common.collect.Sets.newHashSet; +import static org.elasticsearch.index.query.FilterBuilders.*; +import static org.elasticsearch.index.query.QueryBuilders.multiMatchQuery; +import static org.sonar.api.rules.Rule.STATUS_REMOVED; /** * Fill search index with rules @@ -420,15 +410,14 @@ public class RuleRegistry { addMustTermOrTerms(mainFilter, RuleDocument.FIELD_STATUS, query.statuses()); if (!query.tags().isEmpty()) { - mainFilter.must(FilterBuilders.queryFilter( - QueryBuilders.multiMatchQuery(query.tags(), RuleDocument.FIELD_ADMIN_TAGS, RuleDocument.FIELD_SYSTEM_TAGS).operator(Operator.OR)) - ); + for (String tag: query.tags()) { + mainFilter.must(queryFilter(multiMatchQuery(tag, RuleDocument.FIELD_ADMIN_TAGS, RuleDocument.FIELD_SYSTEM_TAGS))); + } } if (!query.debtCharacteristics().isEmpty()) { mainFilter.must(FilterBuilders.queryFilter( - QueryBuilders.multiMatchQuery(query.debtCharacteristics(), RuleDocument.FIELD_CHARACTERISTIC_KEY, RuleDocument.FIELD_SUB_CHARACTERISTIC_KEY).operator(Operator.OR)) - ); + QueryBuilders.multiMatchQuery(query.debtCharacteristics(), RuleDocument.FIELD_CHARACTERISTIC_KEY, RuleDocument.FIELD_SUB_CHARACTERISTIC_KEY).operator(Operator.OR))); } if (query.hasDebtCharacteristic() != null) { diff --git a/sonar-server/src/main/java/org/sonar/server/rule/ws/RulesWs.java b/sonar-server/src/main/java/org/sonar/server/rule/ws/RulesWs.java index 33187d35072..20ad416fd0f 100644 --- a/sonar-server/src/main/java/org/sonar/server/rule/ws/RulesWs.java +++ b/sonar-server/src/main/java/org/sonar/server/rule/ws/RulesWs.java @@ -44,17 +44,17 @@ public class RulesWs implements WebService { .setDescription("List rules that match the given criteria") .setSince("4.3") .setHandler(searchHandler) - .createParam("s", "An optional query that will be matched against rule titles.") + .createParam("s", "To return rules whose title contains this string.") .createParam("k", "An optional query that will be matched exactly agains rule keys.") - .createParam("languages", "An optional query that will be matched against rule languages.") - .createParam("repositories", "An optional query that will be matched against rule repositories.") - .createParam("severities", "An optional query that will be matched against rule severities.") - .createParam("statuses", "An optional query that will be matched against rule statuses.") - .createParam("tags", "An optional query that will be matched against rule tags.") - .createParam("debtCharacteristics", "An optional query that will be matched against rule debt characteristics or sub-characteristics.") - .createParam("hasDebtCharacteristic", "Determine if returned rules should be linked to a debt characteristics or not.") - .createParam("ps", "Optional page size (default is 25).") - .createParam("p", "Optional page number (default is 0)."); + .createParam("languages", "Comma-separated list of language keys") + .createParam("repositories", "Comma-separated list of repositories") + .createParam("severities", "Comma-separated list of severities. Possible values: INFO | MINOR | MAJOR | CRITICAL | BLOCKER.") + .createParam("statuses", "Comma-separated list of statuses. Possible values: READY | BETA | DEPRECATED.") + .createParam("tags", "Comma-separated list of tags. The rule is returned if it matches at least of one these tags.") + .createParam("debtCharacteristics", "Comma-separated list of characteristics / sub-characteristics.") + .createParam("hasDebtCharacteristic", "Determine if returned rules should be linked to a debt characteristic or not. Possible values: true | false") + .createParam("ps", "Page size (default is 25).") + .createParam("p", "Page number (default is 1)."); controller.createAction("show") .setDescription("Detail of rule") diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java index 0b092ee9efc..6120390982a 100644 --- a/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java +++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java @@ -97,8 +97,8 @@ public class RuleRegistryTest { esSetup.execute( EsSetup.index("rules", "rule", "1").withSource(testFileAsString("shared/rule1.json")), EsSetup.index("rules", "rule", "2").withSource(testFileAsString("shared/rule2.json")), - // rule 3 is removed - EsSetup.index("rules", "rule", "3").withSource(testFileAsString("shared/removed_rule.json")) + EsSetup.index("rules", "rule", "3").withSource(testFileAsString("shared/rule3.json")), + EsSetup.index("rules", "rule", "50").withSource(testFileAsString("shared/removed_rule.json")) ); esSetup.client().admin().cluster().prepareHealth(RuleRegistry.INDEX_RULES).setWaitForGreenStatus().execute().actionGet(); } @@ -366,12 +366,12 @@ public class RuleRegistryTest { @Test public void filter_removed_rules() { - assertThat(registry.findIds(new HashMap())).containsOnly(1, 2); + assertThat(registry.findIds(new HashMap())).containsOnly(1, 2, 3); } @Test public void display_disabled_rule() { - assertThat(registry.findIds(ImmutableMap.of("status", "BETA|REMOVED"))).containsOnly(2, 3); + assertThat(registry.findIds(ImmutableMap.of("status", "BETA|REMOVED"))).containsOnly(2, 50); } @Test @@ -460,23 +460,24 @@ public class RuleRegistryTest { @Test public void find_rules_by_severities() { - assertThat(registry.find(RuleQuery.builder().severities(newArrayList("MAJOR")).build()).results()).hasSize(1); - assertThat(registry.find(RuleQuery.builder().severities(newArrayList("MAJOR", "MINOR")).build()).results()).hasSize(2); + assertThat(registry.find(RuleQuery.builder().severities(newArrayList("MAJOR")).build()).results()).hasSize(2); + assertThat(registry.find(RuleQuery.builder().severities(newArrayList("MAJOR", "MINOR")).build()).results()).hasSize(3); assertThat(registry.find(RuleQuery.builder().severities(newArrayList("unknown")).build()).results()).isEmpty(); } @Test public void find_rules_by_statuses() { - assertThat(registry.find(RuleQuery.builder().statuses(newArrayList("READY")).build()).results()).hasSize(1); - assertThat(registry.find(RuleQuery.builder().statuses(newArrayList("READY", "BETA")).build()).results()).hasSize(2); + assertThat(registry.find(RuleQuery.builder().statuses(newArrayList("READY")).build()).results()).hasSize(2); + assertThat(registry.find(RuleQuery.builder().statuses(newArrayList("READY", "BETA")).build()).results()).hasSize(3); assertThat(registry.find(RuleQuery.builder().statuses(newArrayList("unknown")).build()).results()).isEmpty(); } @Test public void find_rules_by_tags() { - assertThat(registry.find(RuleQuery.builder().tags(newArrayList("has-params")).build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().tags(newArrayList("has-params")).build()).results()).hasSize(2); assertThat(registry.find(RuleQuery.builder().tags(newArrayList("keep-enabled")).build()).results()).hasSize(1); assertThat(registry.find(RuleQuery.builder().tags(newArrayList("has-params", "keep-enabled")).build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().tags(newArrayList("has-params", "integration-tests")).build()).results()).hasSize(1); assertThat(registry.find(RuleQuery.builder().tags(newArrayList("unknown")).build()).results()).isEmpty(); } @@ -484,20 +485,22 @@ public class RuleRegistryTest { public void find_rules_by_characteristics() { assertThat(registry.find(RuleQuery.builder().debtCharacteristics(newArrayList("MODULARITY")).build()).results()).hasSize(1); assertThat(registry.find(RuleQuery.builder().debtCharacteristics(newArrayList("REUSABILITY")).build()).results()).hasSize(1); + // FIXME query has to be updated +// assertThat(registry.find(RuleQuery.builder().debtCharacteristics(newArrayList("MODULARITY", "PORTABILITY")).build()).results()).hasSize(2); assertThat(registry.find(RuleQuery.builder().debtCharacteristics(newArrayList("MODULARITY", "REUSABILITY")).build()).results()).hasSize(1); assertThat(registry.find(RuleQuery.builder().debtCharacteristics(newArrayList("unknown")).build()).results()).isEmpty(); } @Test public void find_rules_by_has_debt_characteristic() { - assertThat(registry.find(RuleQuery.builder().hasDebtCharacteristic(null).build()).results()).hasSize(2); - assertThat(registry.find(RuleQuery.builder().hasDebtCharacteristic(true).build()).results()).hasSize(1); + assertThat(registry.find(RuleQuery.builder().hasDebtCharacteristic(null).build()).results()).hasSize(3); + assertThat(registry.find(RuleQuery.builder().hasDebtCharacteristic(true).build()).results()).hasSize(2); assertThat(registry.find(RuleQuery.builder().hasDebtCharacteristic(false).build()).results()).hasSize(1); } @Test public void find_with_no_pagination() { - assertThat(registry.find(RuleQuery.builder().pageSize(-1).build()).results()).hasSize(2); + assertThat(registry.find(RuleQuery.builder().pageSize(-1).build()).results()).hasSize(3); } private String testFileAsString(String testFile) throws Exception { diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/removed_rule.json b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/removed_rule.json index 534591d038b..f610e422dfe 100644 --- a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/removed_rule.json +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/removed_rule.json @@ -1,5 +1,5 @@ { - "id": 1, + "id": 50, "key": "RemobedRule", "language": "xoo", "name": "Removed rule", diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule1.json b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule1.json index 134ddd71a08..c7f72a794fd 100644 --- a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule1.json +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule1.json @@ -52,10 +52,8 @@ ], "characteristicId": 1, "characteristicKey": "REUSABILITY", - "characteristicName": "Reusability", "subCharacteristicId": 2, "subCharacteristicKey": "MODULARITY", - "subCharacteristicName": "Modularity", "remediationFunction": "LINEAR_OFFSET", "remediationCoefficient": "1h", "remediationOffset": "15min" diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule2.json b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule2.json index a29bfa5fde3..accc2788551 100644 --- a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule2.json +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule2.json @@ -9,5 +9,6 @@ "severity": "MINOR", "status": "BETA", "createdAt": "2013-10-28T13:07:26.339Z", - "updatedAt": "2013-11-08T10:52:53.487Z" + "updatedAt": "2013-11-08T10:52:53.487Z", + "systemTags": [ "integration-tests" ] } diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule3.json b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule3.json new file mode 100644 index 00000000000..63f8d56f6df --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/shared/rule3.json @@ -0,0 +1,20 @@ +{ + "id": 3, + "key": "S00100", + "language": "java", + "name": "Method names should comply with a naming convention", + "description": "Method names should comply with a naming convention", + "parentKey": null, + "repositoryKey": "squid", + "severity": "MAJOR", + "status": "READY", + "systemTags": [ "has-params" ], + "createdAt": "2013-10-28T13:07:26.339Z", + "updatedAt": "2013-11-08T10:52:53.487Z", + "characteristicId": 3, + "characteristicKey": "PORTABILITY", + "subCharacteristicId": 4, + "subCharacteristicKey": "COMPILER", + "remediationFunction": "LINEAR", + "remediationCoefficient": "2h" +}