SONAR-4833 Link rule search form to RuleRegistry

9 files changed, 112 insertions, 68 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/rule/RubyRuleService.java b/sonar-server/src/main/java/org/sonar/server/rule/RubyRuleService.java
index c00aac82553..688cba4803a 100644
--- a/sonar-server/src/main/java/org/sonar/server/rule/RubyRuleService.java
+++ b/sonar-server/src/main/java/org/sonar/server/rule/RubyRuleService.java
@@ -39,6 +39,9 @@ public class RubyRuleService implements ServerComponent, Startable {
   private final RuleI18nManager i18n;
   private final RuleRegistry ruleRegistry;
 
+  private static final String OPTIONS_STATUS = "status";
+  private static final String OPTIONS_LANGUAGE = "language";
+
   public RubyRuleService(RuleI18nManager i18n, RuleRegistry ruleRegistry) {
     this.i18n = i18n;
     this.ruleRegistry = ruleRegistry;
@@ -61,18 +64,17 @@ public class RubyRuleService implements ServerComponent, Startable {
     return desc;
   }
 
-  public Integer[] findIds(Map options) {
+  public Integer[] findIds(Map<String, String> options) {
     Map<String, String> params = Maps.newHashMap();
-    translateNonBlankKey(options, params, "status", "status");
-    //translateNonNullKey(options, params, "plugins", "repositoryKey");
-    //translateNonNullKey(options, params, "repositories", "repositoryKey");
-    translateNonBlankKey(options, params, "language", "language");
+    translateNonBlankKey(options, params, OPTIONS_STATUS, OPTIONS_STATUS);
+    translateNonBlankKey(options, params, "repositories", "repositoryKey");
+    translateNonBlankKey(options, params, OPTIONS_LANGUAGE, OPTIONS_LANGUAGE);
     translateNonBlankKey(options, params, "searchtext", "nameOrKey");
     return ruleRegistry.findIds(params).toArray(new Integer[0]);
   }
 
-  private static void translateNonBlankKey(Map options, Map<String, String> params, String optionKey, String paramKey) {
-    if(StringUtils.isNotBlank("" + options.get(optionKey))) {
+  private static void translateNonBlankKey(Map<String, String> options, Map<String, String> params, String optionKey, String paramKey) {
+    if(options.get(optionKey) != null && StringUtils.isNotBlank(options.get(optionKey).toString())) {
       params.put(paramKey, options.get(optionKey).toString());
     }
   }
@@ -84,5 +86,6 @@ public class RubyRuleService implements ServerComponent, Startable {
 
   @Override
   public void stop() {
+    // implement startable
   }
 }
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 c6e413ce5e2..5f3ed7b680e 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
@@ -44,6 +44,10 @@ import java.util.Map;
  */
 public class RuleRegistry {
 
+  /**
+   *
+   */
+  private static final String PARAM_NAMEORKEY = "nameOrKey";
   private static final String INDEX_RULES = "rules";
   private static final String TYPE_RULE = "rule";
 
@@ -103,7 +107,14 @@ public class RuleRegistry {
   }
 
   /**
-   * @param create
+   * <p>Find rule IDs matching the given criteria.</p>
+   * @param query <p>A collection of (optional) criteria with the following meaning:
+   * <ul>
+   *  <li><em>nameOrKey</em>: will be used as a query string over the "name" field</li>
+   *  <li><em>&lt;anyField&gt;</em>: will be used to match the given field against the passed value(s);
+   *  mutiple values must be separated by the '<code>|</code>' (vertical bar) character</li>
+   * </ul>
+   * </p>
    * @return
    */
   public List<Integer> findIds(Map<String, String> query) {
@@ -112,12 +123,12 @@ public class RuleRegistry {
     SearchQuery searchQuery = SearchQuery.create();
     searchQuery.index(INDEX_RULES).type(TYPE_RULE).scrollSize(500);
 
-    if (params.containsKey("nameOrKey")) {
-      searchQuery.searchString(query.get("nameOrKey"));
-      params.remove("nameOrKey");
+    if (params.containsKey(PARAM_NAMEORKEY)) {
+      searchQuery.searchString(params.remove(PARAM_NAMEORKEY));
     }
-    for(String key: params.keySet()) {
-      searchQuery.field(key, params.get(key));
+
+    for(Map.Entry<String, String> param: params.entrySet()) {
+      searchQuery.field(param.getKey(), param.getValue().split("\\|"));
     }
 
     List<Integer> result = Lists.newArrayList();
diff --git a/sonar-server/src/main/java/org/sonar/server/search/SearchIndex.java b/sonar-server/src/main/java/org/sonar/server/search/SearchIndex.java
index 91f96afd339..f3e6484c83e 100644
--- a/sonar-server/src/main/java/org/sonar/server/search/SearchIndex.java
+++ b/sonar-server/src/main/java/org/sonar/server/search/SearchIndex.java
@@ -38,6 +38,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.io.IOException;
+import java.net.URL;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
 
@@ -91,9 +92,11 @@ public class SearchIndex {
 
   public void addMappingFromClasspath(String index, String type, String resourcePath) {
     try {
-      addMapping(index, type, IOUtils.toString(getClass().getResource(resourcePath)));
-    } catch(NullPointerException nonExisting) {
-      throw new IllegalArgumentException("Could not load unexisting file at " + resourcePath, nonExisting);
+      URL resource = getClass().getResource(resourcePath);
+      if (resource == null) {
+        throw new IllegalArgumentException("Could not load unexisting file at " + resourcePath);
+      }
+      addMapping(index, type, IOUtils.toString(resource));
     } catch(IOException ioException) {
       throw new IllegalArgumentException("Problem loading file at " + resourcePath, ioException);
     }
@@ -126,14 +129,14 @@ public class SearchIndex {
             .setSize(searchQuery.scrollSize()).execute().actionGet();
     //Scroll until no hits are returned
     while (true) {
-        scrollResp = client.prepareSearchScroll(scrollResp.getScrollId()).setScroll(new TimeValue(scrollTime)).execute().actionGet();
-        for (SearchHit hit : scrollResp.getHits()) {
-            result.add(hit.getId());
-        }
-        //Break condition: No hits are returned
-        if (scrollResp.getHits().getHits().length == 0) {
-            break;
-        }
+      scrollResp = client.prepareSearchScroll(scrollResp.getScrollId()).setScroll(new TimeValue(scrollTime)).execute().actionGet();
+      for (SearchHit hit : scrollResp.getHits()) {
+        result.add(hit.getId());
+      }
+      //Break condition: No hits are returned
+      if (scrollResp.getHits().getHits().length == 0) {
+        break;
+      }
     }
 
     return result;
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 7a410aa1e87..4d4d69068b8 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
@@ -19,22 +19,21 @@
  */
 package org.sonar.server.search;
 
+import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
 import org.apache.commons.lang.StringUtils;
 import org.elasticsearch.action.search.SearchRequestBuilder;
 import org.elasticsearch.client.Client;
 import org.elasticsearch.index.query.FilterBuilder;
-import org.elasticsearch.index.query.FilterBuilders;
 import org.elasticsearch.index.query.QueryBuilders;
 import org.elasticsearch.search.facet.FacetBuilders;
 
 import java.util.List;
 import java.util.Map;
 
-import static org.elasticsearch.index.query.FilterBuilders.matchAllFilter;
-import static org.elasticsearch.index.query.FilterBuilders.queryFilter;
-import static org.elasticsearch.index.query.FilterBuilders.termFilter;
+import static org.elasticsearch.index.query.FilterBuilders.*;
 
 /**
  * This class can be used to build "AND" form queries, eventually with query facets, to be passed to {@link SearchIndex#findHits(SearchQuery)}
@@ -55,14 +54,14 @@ public class SearchQuery {
   private String searchString;
   private List<String> indices;
   private List<String> types;
-  private Map<String, String> fieldCriteria;
+  private Multimap<String, String> fieldCriteria;
   private Map<String, String> termFacets;
 
   private SearchQuery() {
     scrollSize = 10;
     indices = Lists.newArrayList();
     types = Lists.newArrayList();
-    fieldCriteria = Maps.newLinkedHashMap();
+    fieldCriteria = ArrayListMultimap.create();
     termFacets = Maps.newHashMap();
   }
 
@@ -94,13 +93,13 @@ public class SearchQuery {
     return this;
   }
 
-  public SearchQuery field(String fieldName, String fieldValue) {
-    fieldCriteria.put(fieldName, fieldValue);
+  public SearchQuery field(String fieldName, String... fieldValues) {
+    fieldCriteria.putAll(fieldName, Lists.newArrayList(fieldValues));
     return this;
   }
 
   public SearchQuery facet(String facetName, String fieldName) {
-    fieldCriteria.put(facetName, fieldName);
+    termFacets.put(facetName, fieldName);
     return this;
   }
 
@@ -118,7 +117,11 @@ public class SearchQuery {
       filters.add(queryFilter(QueryBuilders.queryString(searchString)));
     }
     for (String field: fieldCriteria.keySet()) {
-      filters.add(termFilter(field, fieldCriteria.get(field)));
+      if (fieldCriteria.get(field).size() > 1) {
+        filters.add(termsFilter(field, fieldCriteria.get(field)));
+      } else {
+        filters.add(termFilter(field, fieldCriteria.get(field)));
+      }
     }
 
     if (filters.isEmpty()) {
@@ -126,7 +129,7 @@ public class SearchQuery {
     } else if(filters.size() == 1) {
       builder.setFilter(filters.get(0));
     } else {
-      builder.setFilter(FilterBuilders.andFilter(filters.toArray(new FilterBuilder[0])));
+      builder.setFilter(andFilter(filters.toArray(new FilterBuilder[0])));
     }
     return addFacets(builder);
   }
diff --git a/sonar-server/src/main/resources/com/sonar/search/rule_mapping.json b/sonar-server/src/main/resources/com/sonar/search/rule_mapping.json
index eccec14f8dd..2ccc2058f33 100644
--- a/sonar-server/src/main/resources/com/sonar/search/rule_mapping.json
+++ b/sonar-server/src/main/resources/com/sonar/search/rule_mapping.json
@@ -20,7 +20,8 @@
         "type": "string"
       },
       "description": {
-        "type": "string"
+        "type": "string",
+        "index": "no"
       },
       "parentKey": {
         "type": "string",
diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb
index 76bb20df4ea..7dc2567b4d3 100644
--- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb
+++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/rules_configuration_controller.rb
@@ -48,18 +48,18 @@ class RulesConfigurationController < ApplicationController
                       [message('rules.status.ready'), Rule::STATUS_READY]]
     @select_sort_by = [[message('rules_configuration.rule_name'), Rule::SORT_BY_RULE_NAME], [message('rules_configuration.creation_date'), Rule::SORT_BY_CREATION_DATE]]
 
-    @rules = Rule.search(java_facade, {
+    @rules = Rule.search({
         :profile => @profile, :activation => @activation, :priorities => @priorities, :inheritance => @inheritance, :status => @status,
         :repositories => @repositories, :searchtext => @searchtext, :include_parameters_and_notes => true, :language => @profile.language, :sort_by => @sort_by})
 
     unless @searchtext.blank?
       if @activation==STATUS_ACTIVE
-        @hidden_inactives = Rule.search(java_facade, {
+        @hidden_inactives = Rule.search({
             :profile => @profile, :activation => STATUS_INACTIVE, :priorities => @priorities, :status => @status,
             :repositories => @repositories, :language => @profile.language, :searchtext => @searchtext, :include_parameters_and_notes => false}).size
 
       elsif @activation==STATUS_INACTIVE
-        @hidden_actives = Rule.search(java_facade, {
+        @hidden_actives = Rule.search({
             :profile => @profile, :activation => STATUS_ACTIVE, :priorities => @priorities, :status => @status,
             :repositories => @repositories, :language => @profile.language, :searchtext => @searchtext, :include_parameters_and_notes => false}).size
       end
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 4d302fdf10f..5f33425cc90 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
@@ -39,6 +39,16 @@ class Rule < ActiveRecord::Base
   has_one :rule_note, :inverse_of => :rule
   alias_attribute :note, :rule_note
 
+=begin TODO Uncomment these lines to make rule read-only when Rule facade is complete
+  def read_only?
+    true
+  end
+
+  def before_destroy
+    raise ActiveRecord::ReadOnlyRecord
+  end
+=end
+
   def repository_key
     plugin_name
   end
@@ -269,15 +279,20 @@ class Rule < ActiveRecord::Base
 
 
   # options :language => nil, :repositories => [], :searchtext => '', :profile => nil, :priorities => [], :activation => '', :status => [], :sort_by => nil
-  def self.search(java_facade, options={})
+  def self.search(options={})
     conditions = []
     values = {}
 
-    # For retro compatibility for the Sqale plugin
-    unless options[:plugins].blank?
-      options[:repositories] = options[:plugins]
+    # First, perform search with rule-specific criteria
+    java_hash = {}
+    options.each do |key, value|
+      java_hash[key.to_s] = Array(value).join("|")
     end
+    params = java.util.HashMap.new(java_hash)
+    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']
@@ -292,31 +307,7 @@ class Rule < ActiveRecord::Base
       conditions << "status IN (:status)"
       values[:status] = options[:status]
     end
-
-    unless options[:language].blank?
-      conditions << ['language = :language']
-      values[:language] = options[:language]
-    end
-
-    if remove_blank(options[:repositories])
-      conditions << "plugin_name IN (:plugin_names)"
-      values[:plugin_names] = options[:repositories]
-    end
-
-    unless options[:searchtext].blank?
-      searchtext = options[:searchtext].to_s.strip
-      search_text_conditions='(UPPER(rules.name) LIKE :searchtext OR plugin_rule_key = :key'
-
-      additional_keys=java_facade.searchRuleName(I18n.locale, searchtext)
-      additional_keys.each do |java_rule_key|
-        search_text_conditions<<" OR (plugin_name='#{java_rule_key.getRepositoryKey()}' AND plugin_rule_key='#{java_rule_key.getKey()}')"
-      end
-
-      search_text_conditions<<')'
-      conditions << search_text_conditions
-      values[:searchtext] = "%" << searchtext.clone.upcase << "%"
-      values[:key] = searchtext
-    end
+=end
 
     includes=(options[:include_parameters_and_notes] ? [:rules_parameters, :rule_note] : nil)
     rules = Rule.all(:include => includes, :conditions => [conditions.join(" AND "), values])
diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RubyRuleServiceTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RubyRuleServiceTest.java
index cb44da781a9..74305e3656a 100644
--- a/sonar-server/src/test/java/org/sonar/server/rule/RubyRuleServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/RubyRuleServiceTest.java
@@ -19,15 +19,19 @@
  */
 package org.sonar.server.rule;
 
+import com.google.common.collect.Maps;
 import org.junit.Test;
+import org.mockito.ArgumentCaptor;
 import org.sonar.api.rules.Rule;
 import org.sonar.core.i18n.RuleI18nManager;
 import org.sonar.server.user.MockUserSession;
 
 import java.util.Locale;
+import java.util.Map;
 
 import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
 
 public class RubyRuleServiceTest {
@@ -77,6 +81,29 @@ public class RubyRuleServiceTest {
   }
 
   @Test
+  @SuppressWarnings({ "unchecked", "rawtypes" })
+  public void should_translate_arguments_ind_find_ids() {
+    Map<String, String> options = Maps.newHashMap();
+    String status = " ";
+    String repositories = "repo1|repo2";
+    String searchText = "search text";
+
+    options.put("status", status);
+    options.put("repositories", repositories);
+    // language not specified to cover blank option case
+    options.put("searchtext", searchText );
+
+    facade.findIds(options);
+    ArgumentCaptor<Map> captor = ArgumentCaptor.forClass(Map.class);
+    verify(ruleRegistry).findIds(captor.capture());
+    Map<String, String> params = (Map<String, String>) captor.getValue();
+    assertThat(params.get("status")).isNull();
+    assertThat(params.get("repositoryKey")).isEqualTo(repositories);
+    assertThat(params.get("language")).isNull();
+    assertThat(params.get("nameOrKey")).isEqualTo(searchText);
+  }
+
+  @Test
   public void just_for_fun_and_coverage() throws Exception {
     facade.start();
     facade.stop();
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 9ab022a4dc4..65bf4c6250a 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
@@ -109,4 +109,9 @@ public class RuleRegistryTest {
 
     assertThat(registry.findIds(ImmutableMap.of("nameOrKey", "parameters", "repositoryKey", "xoo"))).containsOnly(1);
   }
+
+  @Test
+  public void should_filter_on_multiple_values() {
+    assertThat(registry.findIds(ImmutableMap.of("key", "RuleWithParameters|OneIssuePerLine"))).hasSize(2);
+  }
 }