From: Jean-Baptiste Lievremont Date: Wed, 20 Nov 2013 10:20:15 +0000 (+0100) Subject: SONAR-4832 Fix issue with status search criterion (by default, exclude REMOVED rules) X-Git-Tag: 4.1-RC1~270 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=6d22f4969cf3dd26fab34eb730dcd623542db31e;p=sonarqube.git SONAR-4832 Fix issue with status search criterion (by default, exclude REMOVED rules) --- 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 3d920768a39..facc7cad1a5 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 @@ -49,6 +49,7 @@ public class RuleRegistry { * */ private static final String PARAM_NAMEORKEY = "nameOrKey"; + private static final String PARAM_STATUS = "status"; private static final String INDEX_RULES = "rules"; private static final String TYPE_RULE = "rule"; @@ -102,6 +103,9 @@ public class RuleRegistry { if (params.containsKey(PARAM_NAMEORKEY)) { searchQuery.searchString(params.remove(PARAM_NAMEORKEY)); } + if (! params.containsKey(PARAM_STATUS)) { + searchQuery.notField(PARAM_STATUS, Rule.STATUS_REMOVED); + } for(Map.Entry param: params.entrySet()) { searchQuery.field(param.getKey(), param.getValue().split("\\|")); diff --git a/sonar-server/src/main/java/org/sonar/server/search/SearchQuery.java b/sonar-server/src/main/java/org/sonar/server/search/SearchQuery.java index 4d4d69068b8..7b8dc124c51 100644 --- a/sonar-server/src/main/java/org/sonar/server/search/SearchQuery.java +++ b/sonar-server/src/main/java/org/sonar/server/search/SearchQuery.java @@ -55,6 +55,7 @@ public class SearchQuery { private List indices; private List types; private Multimap fieldCriteria; + private Multimap notFieldCriteria; private Map termFacets; private SearchQuery() { @@ -62,6 +63,7 @@ public class SearchQuery { indices = Lists.newArrayList(); types = Lists.newArrayList(); fieldCriteria = ArrayListMultimap.create(); + notFieldCriteria = ArrayListMultimap.create(); termFacets = Maps.newHashMap(); } @@ -98,6 +100,11 @@ public class SearchQuery { return this; } + public SearchQuery notField(String fieldName, String... fieldValues) { + notFieldCriteria.putAll(fieldName, Lists.newArrayList(fieldValues)); + return this; + } + public SearchQuery facet(String facetName, String fieldName) { termFacets.put(facetName, fieldName); return this; @@ -116,6 +123,7 @@ public class SearchQuery { if (StringUtils.isNotBlank(searchString)) { filters.add(queryFilter(QueryBuilders.queryString(searchString))); } + for (String field: fieldCriteria.keySet()) { if (fieldCriteria.get(field).size() > 1) { filters.add(termsFilter(field, fieldCriteria.get(field))); @@ -124,6 +132,14 @@ public class SearchQuery { } } + for (String field: notFieldCriteria.keySet()) { + if (notFieldCriteria.get(field).size() > 1) { + filters.add(notFilter(termsFilter(field, notFieldCriteria.get(field)))); + } else { + filters.add(notFilter(termFilter(field, notFieldCriteria.get(field)))); + } + } + if (filters.isEmpty()) { builder.setFilter(matchAllFilter()); } else if(filters.size() == 1) { diff --git a/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb b/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb index ca364eb9fe9..e12043ab117 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/models/rule.rb @@ -293,23 +293,6 @@ class Rule < ActiveRecord::Base conditions << "id IN (:ids)" values[:ids] = Array(Internal.rules.findIds(params)) -=begin TODO Correctly handle status != STATUS_REMOVED - status = options[:status] - if status.blank? - conditions << ['status <> :status'] - values[:status] = STATUS_REMOVED - elsif status == 'ACTIVE' || status == 'INACTIVE' - # For retro compatibility for the Sqale plugin - # To be removed when SonarQube version will no more be compatible with SQALE version < 1.10 - options[:activation] = status - conditions << ['status <> :status'] - values[:status] = STATUS_REMOVED - else - conditions << "status IN (:status)" - values[:status] = options[:status] - end -=end - includes=(options[:include_parameters_and_notes] ? [:rules_parameters, :rule_note] : nil) rules = Rule.all(:include => includes, :conditions => [conditions.join(" AND "), values]) rules = Rule.sort_by(rules, options[:sort_by]) 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 104369f9734..9e8cc563b1a 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 @@ -68,10 +68,12 @@ public class RuleRegistryTest { String source1 = IOUtils.toString(TestUtils.getResource(getClass(), "rules/rule1.json").toURI()); String source2 = IOUtils.toString(TestUtils.getResource(getClass(), "rules/rule2.json").toURI()); + String source3 = IOUtils.toString(TestUtils.getResource(getClass(), "rules/rule3.json").toURI()); esSetup.execute( EsSetup.index("rules", "rule", "1").withSource(source1), - EsSetup.index("rules", "rule", "2").withSource(source2) + EsSetup.index("rules", "rule", "2").withSource(source2), + EsSetup.index("rules", "rule", "3").withSource(source3) ); } @@ -89,20 +91,22 @@ public class RuleRegistryTest { } @Test - public void should_find_all_rule_ids() { - + public void should_filter_removed_rules() { assertThat(registry.findIds(new HashMap())).containsOnly(1, 2); } @Test - public void should_filter_on_name_or_key() throws Exception { + public void should_display_disabled_rule() { + assertThat(registry.findIds(ImmutableMap.of("status", "BETA|REMOVED"))).containsOnly(2, 3); + } + @Test + public void should_filter_on_name_or_key() throws Exception { assertThat(registry.findIds(ImmutableMap.of("nameOrKey", "parameters"))).containsOnly(1); } @Test public void should_filter_on_key() throws Exception { - assertThat(registry.findIds(ImmutableMap.of("key", "OneIssuePerLine"))).containsOnly(2); } diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule2.json b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule2.json index 4343a5a6ee5..f1adc5ecc65 100644 --- a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule2.json +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule2.json @@ -7,7 +7,7 @@ "parentKey": null, "repositoryKey": "xoo", "severity": "MAJOR", - "status": "READY", + "status": "BETA", "createdAt": "2013-10-28T13:07:26.339Z", "updatedAt": "2013-11-08T10:52:53.487Z" } diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule3.json b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule3.json new file mode 100644 index 00000000000..83ec573d96e --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule3.json @@ -0,0 +1,13 @@ +{ + "id": 1, + "key": "RemobedRule", + "language": "xoo", + "name": "Removed rule", + "description": "Removed rule that should be filtered out in default query", + "parentKey": null, + "repositoryKey": "xoo", + "severity": "CRITICAL", + "status": "REMOVED", + "createdAt": "2013-10-28T13:07:26.329Z", + "updatedAt": "2013-11-08T10:52:53.473Z" +}