From 1da4af85829eba537175126e5648d3aa2945c906 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Fri, 8 Nov 2013 18:21:02 +0100 Subject: [PATCH] SONAR-4832 Add query to fetch IDs from search form criteria --- .../org/sonar/server/platform/Platform.java | 5 +- .../sonar/server/rule/RubyRuleService.java | 24 +++- .../org/sonar/server/rule/RuleRegistry.java | 32 ++++- .../org/sonar/server/search/SearchIndex.java | 29 ++++- .../org/sonar/server/search/SearchQuery.java | 59 ++++++--- .../server/rule/RubyRuleServiceTest.java | 4 +- .../sonar/server/rule/RuleRegistryTest.java | 112 ++++++++++++++++++ .../sonar/server/search/SearchIndexTest.java | 20 ++++ .../sonar/server/search/SearchQueryTest.java | 22 ---- .../rule/RuleRegistryTest/rules/rule1.json | 46 +++++++ .../rule/RuleRegistryTest/rules/rule2.json | 13 ++ 11 files changed, 316 insertions(+), 50 deletions(-) create mode 100644 sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java create mode 100644 sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule1.json create mode 100644 sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule2.json diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index c7e56a72e19..7b5549141e9 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -19,8 +19,6 @@ */ package org.sonar.server.platform; -import org.sonar.server.rule.RuleRegistry; - import org.apache.commons.configuration.BaseConfiguration; import org.slf4j.LoggerFactory; import org.sonar.api.config.EmailSettings; @@ -93,6 +91,7 @@ import org.sonar.server.permission.InternalPermissionService; import org.sonar.server.permission.InternalPermissionTemplateService; import org.sonar.server.plugins.*; import org.sonar.server.rule.RubyRuleService; +import org.sonar.server.rule.RuleRegistry; import org.sonar.server.rules.ProfilesConsole; import org.sonar.server.rules.RulesConsole; import org.sonar.server.search.SearchIndex; @@ -305,6 +304,7 @@ public final class Platform { servicesContainer.addSingleton(TransitionAction.class); // rules + servicesContainer.addSingleton(RuleRegistry.class); servicesContainer.addSingleton(RubyRuleService.class); // technical debt @@ -356,7 +356,6 @@ public final class Platform { startupContainer.addSingleton(LogServerId.class); startupContainer.addSingleton(RegisterServletFilters.class); startupContainer.addSingleton(CleanDryRunCache.class); - startupContainer.addSingleton(RuleRegistry.class); startupContainer.startComponents(); startupContainer.getComponentByType(ServerLifecycleNotifier.class).notifyStart(); 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 974e3865c1b..c00aac82553 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 @@ -19,6 +19,8 @@ */ package org.sonar.server.rule; +import com.google.common.collect.Maps; +import org.apache.commons.lang.StringUtils; import org.picocontainer.Startable; import org.sonar.api.ServerComponent; import org.sonar.api.rules.Rule; @@ -27,15 +29,19 @@ import org.sonar.server.user.UserSession; import javax.annotation.CheckForNull; +import java.util.Map; + /** * Used through ruby code 
Internal.rules
*/ public class RubyRuleService implements ServerComponent, Startable { private final RuleI18nManager i18n; + private final RuleRegistry ruleRegistry; - public RubyRuleService(RuleI18nManager i18n) { + public RubyRuleService(RuleI18nManager i18n, RuleRegistry ruleRegistry) { this.i18n = i18n; + this.ruleRegistry = ruleRegistry; } @CheckForNull @@ -55,6 +61,22 @@ public class RubyRuleService implements ServerComponent, Startable { return desc; } + public Integer[] findIds(Map options) { + Map 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, "searchtext", "nameOrKey"); + return ruleRegistry.findIds(params).toArray(new Integer[0]); + } + + private static void translateNonBlankKey(Map options, Map params, String optionKey, String paramKey) { + if(StringUtils.isNotBlank("" + options.get(optionKey))) { + params.put(paramKey, options.get(optionKey).toString()); + } + } + @Override public void start() { // used to force pico to instantiate the singleton at startup 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 e7c917bbdd5..c6e413ce5e2 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,7 +20,8 @@ package org.sonar.server.rule; -import org.elasticsearch.common.collect.Lists; +import com.google.common.collect.Lists; +import org.elasticsearch.common.collect.Maps; import org.elasticsearch.common.io.BytesStream; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; @@ -30,10 +31,12 @@ import org.sonar.api.rules.RuleParam; import org.sonar.core.i18n.RuleI18nManager; import org.sonar.jpa.session.DatabaseSessionFactory; import org.sonar.server.search.SearchIndex; +import org.sonar.server.search.SearchQuery; import java.io.IOException; import java.util.List; import java.util.Locale; +import java.util.Map; /** * Fill search index with rules @@ -69,7 +72,7 @@ public class RuleRegistry { XContentBuilder document = XContentFactory.jsonBuilder() .startObject() .field("id", rule.getId()) - .field("key", rule.ruleKey()) + .field("key", rule.getKey()) .field("language", rule.getLanguage()) .field("name", ruleI18nManager.getName(rule, Locale.getDefault())) .field("description", ruleI18nManager.getDescription(rule.getRepositoryKey(), rule.getKey(), Locale.getDefault())) @@ -98,4 +101,29 @@ public class RuleRegistry { throw new IllegalStateException("Unable to index rules", ioe); } } + + /** + * @param create + * @return + */ + public List findIds(Map query) { + Map params = Maps.newHashMap(query); + + 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"); + } + for(String key: params.keySet()) { + searchQuery.field(key, params.get(key)); + } + + List result = Lists.newArrayList(); + for(String docId: searchIndex.findDocumentIds(searchQuery)) { + result.add(Integer.parseInt(docId)); + } + return result; + } } 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 8935db2faba..91f96afd339 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 @@ -19,6 +19,7 @@ */ package org.sonar.server.search; +import com.google.common.collect.Lists; import org.apache.commons.io.IOUtils; import org.elasticsearch.ElasticSearchParseException; import org.elasticsearch.action.admin.indices.exists.indices.IndicesExistsRequest; @@ -26,14 +27,18 @@ import org.elasticsearch.action.bulk.BulkItemResponse; import org.elasticsearch.action.bulk.BulkRequestBuilder; import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.action.search.SearchResponse; +import org.elasticsearch.action.search.SearchType; import org.elasticsearch.client.Client; import org.elasticsearch.client.IndicesAdminClient; import org.elasticsearch.client.Requests; import org.elasticsearch.common.io.BytesStream; +import org.elasticsearch.common.unit.TimeValue; +import org.elasticsearch.search.SearchHit; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; +import java.util.List; import java.util.concurrent.ExecutionException; public class SearchIndex { @@ -111,7 +116,27 @@ public class SearchIndex { } } - public SearchResponse find(SearchQuery query) { - return query.toBuilder(client).execute().actionGet(); + public List findDocumentIds(SearchQuery searchQuery) { + List result = Lists.newArrayList(); + final int scrollTime = 100; + + SearchResponse scrollResp = searchQuery.toBuilder(client).addField("_id") + .setSearchType(SearchType.SCAN) + .setScroll(new TimeValue(scrollTime)) + .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; + } + } + + 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 a1e16114381..7a410aa1e87 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 @@ -24,14 +24,20 @@ import com.google.common.collect.Maps; 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; + /** - * This class can be used to build "AND" form queries, eventually with query facets, to be passed to {@link SearchIndex#find(SearchQuery)} + * This class can be used to build "AND" form queries, eventually with query facets, to be passed to {@link SearchIndex#findHits(SearchQuery)} * For instance the following code: *
SearchQuery.create("polop") @@ -45,6 +51,7 @@ import java.util.Map; * @since 4.1 */ public class SearchQuery { + private int scrollSize; private String searchString; private List indices; private List types; @@ -52,6 +59,7 @@ public class SearchQuery { private Map termFacets; private SearchQuery() { + scrollSize = 10; indices = Lists.newArrayList(); types = Lists.newArrayList(); fieldCriteria = Maps.newLinkedHashMap(); @@ -62,10 +70,18 @@ public class SearchQuery { return new SearchQuery(); } - public static SearchQuery create(String searchString) { - SearchQuery searchQuery = new SearchQuery(); - searchQuery.searchString = searchString; - return searchQuery; + public SearchQuery searchString(String searchString) { + this.searchString = searchString; + return this; + } + + public SearchQuery scrollSize(int scrollSize) { + this.scrollSize = scrollSize; + return this; + } + + int scrollSize() { + return scrollSize; } public SearchQuery index(String index) { @@ -73,6 +89,11 @@ public class SearchQuery { return this; } + public SearchQuery type(String type) { + types.add(type); + return this; + } + public SearchQuery field(String fieldName, String fieldValue) { fieldCriteria.put(fieldName, fieldValue); return this; @@ -90,25 +111,27 @@ public class SearchQuery { return builder; } - String getQueryString() { - List criteria = Lists.newArrayList(); + SearchRequestBuilder toBuilder(Client client) { + SearchRequestBuilder builder = client.prepareSearch(indices.toArray(new String[0])).setTypes(types.toArray(new String[0])); + List filters = Lists.newArrayList(); if (StringUtils.isNotBlank(searchString)) { - criteria.add(searchString); + filters.add(queryFilter(QueryBuilders.queryString(searchString))); } - for (String fieldName: fieldCriteria.keySet()) { - criteria.add(String.format("%s:%s", fieldName, fieldCriteria.get(fieldName))); + for (String field: fieldCriteria.keySet()) { + filters.add(termFilter(field, fieldCriteria.get(field))); } - return StringUtils.join(criteria, " AND "); - } - SearchRequestBuilder toBuilder(Client client) { - SearchRequestBuilder builder = client.prepareSearch(indices.toArray(new String[0])).setTypes(types.toArray(new String[0])); - String queryString = getQueryString(); - if (StringUtils.isBlank(queryString)) { - builder.setQuery(QueryBuilders.matchAllQuery()); + if (filters.isEmpty()) { + builder.setFilter(matchAllFilter()); + } else if(filters.size() == 1) { + builder.setFilter(filters.get(0)); } else { - builder.setQuery(queryString); + builder.setFilter(FilterBuilders.andFilter(filters.toArray(new FilterBuilder[0]))); } return addFacets(builder); } + + public String getQueryString() { + return ""; + } } 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 24a609ac38f..cb44da781a9 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 @@ -23,7 +23,6 @@ import org.junit.Test; import org.sonar.api.rules.Rule; import org.sonar.core.i18n.RuleI18nManager; import org.sonar.server.user.MockUserSession; -import org.sonar.server.user.UserSession; import java.util.Locale; @@ -34,7 +33,8 @@ import static org.mockito.Mockito.when; public class RubyRuleServiceTest { RuleI18nManager i18n = mock(RuleI18nManager.class); - RubyRuleService facade = new RubyRuleService(i18n); + RuleRegistry ruleRegistry = mock(RuleRegistry.class); + RubyRuleService facade = new RubyRuleService(i18n, ruleRegistry); @Test public void should_get_localized_rule_name() { 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 new file mode 100644 index 00000000000..9ab022a4dc4 --- /dev/null +++ b/sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java @@ -0,0 +1,112 @@ +/* + * 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.github.tlrx.elasticsearch.test.EsSetup; +import com.google.common.collect.ImmutableMap; +import org.apache.commons.io.IOUtils; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.sonar.core.i18n.RuleI18nManager; +import org.sonar.jpa.session.DatabaseSessionFactory; +import org.sonar.server.search.SearchIndex; +import org.sonar.server.search.SearchNode; +import org.sonar.test.TestUtils; + +import java.util.HashMap; + +import static org.fest.assertions.Assertions.assertThat; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +public class RuleRegistryTest { + + private EsSetup esSetup; + private SearchIndex searchIndex; + private DatabaseSessionFactory sessionFactory; + private RuleI18nManager ruleI18nManager; + RuleRegistry registry; + + @Before + public void setUp() throws Exception { + sessionFactory = mock(DatabaseSessionFactory.class); + ruleI18nManager = mock(RuleI18nManager.class); + + esSetup = new EsSetup(); + esSetup.execute(EsSetup.deleteAll()); + + SearchNode node = mock(SearchNode.class); + when(node.client()).thenReturn(esSetup.client()); + searchIndex = new SearchIndex(node); + searchIndex.start(); + + registry = new RuleRegistry(searchIndex, sessionFactory, ruleI18nManager); + registry.start(); + + String source1 = IOUtils.toString(TestUtils.getResource(getClass(), "rules/rule1.json").toURI()); + String source2 = IOUtils.toString(TestUtils.getResource(getClass(), "rules/rule2.json").toURI()); + + esSetup.execute( + EsSetup.index("rules", "rule", "1").withSource(source1), + EsSetup.index("rules", "rule", "2").withSource(source2) + ); + + } + + @After + public void tearDown() { + searchIndex.stop(); + esSetup.terminate(); + } + + @Test + public void should_register_mapping_at_startup() { + assertThat(esSetup.exists("rules")).isTrue(); + assertThat(esSetup.client().admin().indices().prepareTypesExists("rules").setTypes("rule").execute().actionGet().isExists()).isTrue(); + } + + @Test + public void should_find_all_rule_ids() { + + assertThat(registry.findIds(new HashMap())).containsOnly(1, 2); + } + + @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); + } + + @Test + public void should_filter_on_multiple_criteria() { + assertThat(registry.findIds(ImmutableMap.of("nameOrKey", "parameters", "key", "OneIssuePerLine"))).isEmpty(); + assertThat(registry.findIds(ImmutableMap.of("repositoryKey", "polop"))).isEmpty(); + + assertThat(registry.findIds(ImmutableMap.of("nameOrKey", "parameters", "repositoryKey", "xoo"))).containsOnly(1); + } +} diff --git a/sonar-server/src/test/java/org/sonar/server/search/SearchIndexTest.java b/sonar-server/src/test/java/org/sonar/server/search/SearchIndexTest.java index 0edd4146ff7..f0dc8f37e66 100644 --- a/sonar-server/src/test/java/org/sonar/server/search/SearchIndexTest.java +++ b/sonar-server/src/test/java/org/sonar/server/search/SearchIndexTest.java @@ -21,10 +21,14 @@ package org.sonar.server.search; import com.github.tlrx.elasticsearch.test.EsSetup; +import org.elasticsearch.common.io.BytesStream; +import org.elasticsearch.common.xcontent.XContentFactory; import org.junit.After; import org.junit.Before; import org.junit.Test; +import java.util.List; + import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.verify; @@ -104,4 +108,20 @@ public class SearchIndexTest { searchIndex.addMappingFromClasspath("unchecked", "unchecked", resourcePath); } + @Test + public void should_iterate_over_big_dataset() throws Exception { + final int numberOfDocuments = 10000; + + searchIndex.addMappingFromClasspath("index", "type1", "/org/sonar/server/search/SearchIndexTest/correct_mapping1.json"); + String[] ids = new String[numberOfDocuments]; + BytesStream[] sources = new BytesStream[numberOfDocuments]; + for (int i=0; i docIds = searchIndex.findDocumentIds(SearchQuery.create()); + assertThat(docIds).hasSize(numberOfDocuments); + } } diff --git a/sonar-server/src/test/java/org/sonar/server/search/SearchQueryTest.java b/sonar-server/src/test/java/org/sonar/server/search/SearchQueryTest.java index 57266eafb09..7bc2652e207 100644 --- a/sonar-server/src/test/java/org/sonar/server/search/SearchQueryTest.java +++ b/sonar-server/src/test/java/org/sonar/server/search/SearchQueryTest.java @@ -29,26 +29,4 @@ public class SearchQueryTest { public void should_return_empty_query() { assertThat(SearchQuery.create().getQueryString()).isEmpty(); } - - @Test - public void should_handle_custom_query() { - assertThat(SearchQuery.create("polop").getQueryString()).isEqualTo("polop"); - } - - @Test - public void should_add_fields() { - assertThat(SearchQuery.create() - .field("field1", "value1") - .field("field2", "value2") - .getQueryString()).isEqualTo("field1:value1 AND field2:value2"); - } - - @Test - public void should_add_fields_to_custom_query() { - assertThat(SearchQuery.create("polop") - .field("field1", "value1") - .field("field2", "value2") - .getQueryString()).isEqualTo("polop AND field1:value1 AND field2:value2"); - } - } diff --git a/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule1.json b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule1.json new file mode 100644 index 00000000000..6079ad65dc6 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule1.json @@ -0,0 +1,46 @@ +{ + "id": 1, + "key": "RuleWithParameters", + "language": "xoo", + "name": "Rule with parameters", + "description": "Rule containing parameter of different types : boolean, integer, etc. For information, no issue will be linked to this rule.", + "parentKey": null, + "repositoryKey": "xoo", + "severity": "MAJOR", + "status": "READY", + "createdAt": "2013-10-28T13:07:26.329Z", + "updatedAt": "2013-11-08T10:52:53.473Z", + "params": [ + { + "key": "string", + "type": "STRING", + "defaultValue": null, + "description": "" + }, + { + "key": "text", + "type": "TEXT", + "defaultValue": null, + "description": "" + }, + { + "key": "boolean", + "type": "BOOLEAN", + "defaultValue": null, + "description": "" + }, + { + "key": "integer", + "type": "INTEGER", + "defaultValue": null, + "description": "" + }, + { + "key": "float", + "type": "FLOAT", + "defaultValue": null, + "description": "" + } + ] + } +} 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 new file mode 100644 index 00000000000..4343a5a6ee5 --- /dev/null +++ b/sonar-server/src/test/resources/org/sonar/server/rule/RuleRegistryTest/rules/rule2.json @@ -0,0 +1,13 @@ +{ + "id": 2, + "key": "OneIssuePerLine", + "language": "xoo", + "name": "One Issue Per Line", + "description": "Generate an issue on each line of a file. It requires the metric \"lines\".", + "parentKey": null, + "repositoryKey": "xoo", + "severity": "MAJOR", + "status": "READY", + "createdAt": "2013-10-28T13:07:26.339Z", + "updatedAt": "2013-11-08T10:52:53.487Z" +} -- 2.39.5