From: Stephane Gamard Date: Wed, 9 Apr 2014 14:08:38 +0000 (+0200) Subject: SONAR-5187 - Updated RuleRegistry and small modification for mapping rules in ES... X-Git-Tag: 4.3~77 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=4a8e16052825d18fdb11d188436c3f4364b6651d;p=sonarqube.git SONAR-5187 - Updated RuleRegistry and small modification for mapping rules in ES (indecued a minor modification of QProfileRuleLookup) --- diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleLookup.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleLookup.java index 11f99784244..84f700dbb5a 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleLookup.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileRuleLookup.java @@ -288,7 +288,7 @@ public class QProfileRuleLookup implements ServerExtension { if (StringUtils.isNotBlank(query.nameOrKey())) { result.must( queryFilter( - multiMatchQuery(query.nameOrKey().trim(), RuleDocument.FIELD_NAME + ".search", RuleDocument.FIELD_KEY) + multiMatchQuery(query.nameOrKey().trim(), RuleDocument.FIELD_NAME, RuleDocument.FIELD_NAME + ".search", RuleDocument.FIELD_KEY) .operator(Operator.AND))); } 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 5e32a3cd0bc..15b32ffad79 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,10 +20,23 @@ package org.sonar.server.rule; -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 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 org.apache.commons.lang.StringUtils; import org.apache.ibatis.session.SqlSession; import org.elasticsearch.ElasticSearchException; @@ -40,6 +53,7 @@ import org.elasticsearch.index.query.BoolFilterBuilder; import org.elasticsearch.index.query.FilterBuilder; import org.elasticsearch.index.query.FilterBuilders; 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; @@ -49,7 +63,11 @@ 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.*; +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.technicaldebt.db.CharacteristicDao; import org.sonar.core.technicaldebt.db.CharacteristicDto; import org.sonar.server.es.ESIndex; @@ -58,20 +76,10 @@ import org.sonar.server.paging.PagedResult; import org.sonar.server.paging.Paging; import org.sonar.server.paging.PagingResult; -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.sonar.api.rules.Rule.STATUS_REMOVED; +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; /** * Fill search index with rules @@ -352,12 +360,12 @@ public class RuleRegistry { } public PagedResult find(RuleQuery query) { - BoolFilterBuilder mainFilter = convertRuleQueryToFilterBuilder(query); + QueryBuilder mainQuery = convertRuleQueryToFilterBuilder(query); Builder rulesBuilder = ImmutableList.builder(); SearchRequestBuilder searchRequestBuilder = searchIndex.client().prepareSearch(INDEX_RULES).setTypes(TYPE_RULE) - .setPostFilter(mainFilter) + .setQuery(mainQuery) .addSort(RuleDocument.FIELD_NAME, SortOrder.ASC); if (RuleQuery.NO_PAGINATION == query.pageSize()) { @@ -394,26 +402,37 @@ public class RuleRegistry { } } - private static BoolFilterBuilder convertRuleQueryToFilterBuilder(RuleQuery query){ + private static QueryBuilder convertRuleQueryToFilterBuilder(RuleQuery query){ + + //Execute the main query (defaults to matchAll + QueryBuilder mainQuery = QueryBuilders.matchAllQuery(); + if (StringUtils.isNotBlank(query.searchQuery())) { + mainQuery= QueryBuilders.multiMatchQuery(query.searchQuery(), + RuleDocument.FIELD_KEY, + RuleDocument.FIELD_NAME, + RuleDocument.FIELD_NAME + ".search").operator(Operator.AND); + } + + //Execute all filters BoolFilterBuilder mainFilter = boolFilter().mustNot(termFilter(RuleDocument.FIELD_STATUS, STATUS_REMOVED)); - if (StringUtils.isNotBlank(query.searchQuery())) { - mainFilter.must(FilterBuilders.queryFilter( - QueryBuilders.multiMatchQuery(query.searchQuery(), RuleDocument.FIELD_NAME + ".search", RuleDocument.FIELD_KEY).operator(Operator.AND))); - } + addMustTermOrTerms(mainFilter, RuleDocument.FIELD_LANGUAGE, query.languages()); addMustTermOrTerms(mainFilter, RuleDocument.FIELD_REPOSITORY_KEY, query.repositories()); addMustTermOrTerms(mainFilter, RuleDocument.FIELD_SEVERITY, query.severities()); 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)) ); } + if (!query.debtCharacteristics().isEmpty()) { mainFilter.must(FilterBuilders.queryFilter( QueryBuilders.multiMatchQuery(query.debtCharacteristics(), RuleDocument.FIELD_CHARACTERISTIC_KEY, RuleDocument.FIELD_SUB_CHARACTERISTIC_KEY).operator(Operator.OR)) ); } + if (query.hasDebtCharacteristic() != null) { if (Boolean.TRUE.equals(query.hasDebtCharacteristic())) { mainFilter.must(FilterBuilders.existsFilter(RuleDocument.FIELD_CHARACTERISTIC_KEY)); @@ -421,7 +440,8 @@ public class RuleRegistry { mainFilter.mustNot(FilterBuilders.existsFilter(RuleDocument.FIELD_CHARACTERISTIC_KEY)); } } - return mainFilter; + + return QueryBuilders.filteredQuery(mainQuery, mainFilter); } private static void addMustTermOrTerms(BoolFilterBuilder filter, String field, Collection terms) { diff --git a/sonar-server/src/main/resources/org/sonar/server/es/config/elasticsearch.json b/sonar-server/src/main/resources/org/sonar/server/es/config/elasticsearch.json index 96d220e8174..64913338ee0 100644 --- a/sonar-server/src/main/resources/org/sonar/server/es/config/elasticsearch.json +++ b/sonar-server/src/main/resources/org/sonar/server/es/config/elasticsearch.json @@ -29,11 +29,11 @@ }, "rule_name": { "type": "custom", - "tokenizer": "rule_name_ngram", - "filter": "lowercase" + "tokenizer": "standard", + "filter": ["lowercase", "rule_name_ngram"] } }, - "tokenizer": { + "filter": { "rule_name_ngram": { "type": "nGram", "min_gram": 3, diff --git a/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json b/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json index 67fff5c5642..468059df614 100644 --- a/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json +++ b/sonar-server/src/main/resources/org/sonar/server/es/config/mappings/rule_mapping.json @@ -21,7 +21,7 @@ "fields": { "name": { "type": "string", "index": "analyzed" }, "raw": { "type": "string", "index": "analyzed", "analyzer": "sortable" }, - "search": { "type": "string", "index": "analyzed", "analyzer": "rule_name" } + "search": { "type": "string", "index": "analyzed", "index_analyzer": "rule_name", "search_analyzer": "standard" } } }, "description": { 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 30739141552..0b092ee9efc 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 @@ -364,6 +364,24 @@ public class RuleRegistryTest { assertThat(esSetup.exists("rules", "rule", "3")).isFalse(); } + @Test + public void filter_removed_rules() { + assertThat(registry.findIds(new HashMap())).containsOnly(1, 2); + } + + @Test + public void display_disabled_rule() { + assertThat(registry.findIds(ImmutableMap.of("status", "BETA|REMOVED"))).containsOnly(2, 3); + } + + @Test + public void find_rules_by_name_with_number_in_name() { + registry.reindex(new RuleDto().setId(10).setRepositoryKey("repo").setRuleKey("rule1").setName("MyRule 1").setSeverity(Severity.MINOR)); + registry.reindex(new RuleDto().setId(11).setRepositoryKey("repo").setRuleKey("rule2").setName("MyRule 2").setSeverity(Severity.MINOR)); + System.out.println("MY RESULT: " + registry.find(RuleQuery.builder().searchQuery("MyRule 1").build()).results()); + assertThat(registry.find(RuleQuery.builder().searchQuery("MyRule 1").build()).results()).hasSize(1); + } + @Test public void find_rule_by_key() { assertThat(registry.findByKey(RuleKey.of("unknown", "RuleWithParameters"))).isNull(); @@ -376,17 +394,7 @@ public class RuleRegistryTest { assertThat(rule.adminTags()).hasSize(1); assertThat(rule.systemTags()).hasSize(2); } - - @Test - public void filter_removed_rules() { - assertThat(registry.findIds(new HashMap())).containsOnly(1, 2); - } - - @Test - public void display_disabled_rule() { - assertThat(registry.findIds(ImmutableMap.of("status", "BETA|REMOVED"))).containsOnly(2, 3); - } - + @Test public void filter_on_name_or_key() throws Exception { assertThat(registry.findIds(ImmutableMap.of("nameOrKey", "parameters"))).containsOnly(1);