From 26f7ca3d5d0c9f47fc3393ad342f71bc44720ba2 Mon Sep 17 00:00:00 2001 From: Jean-Baptiste Lievremont Date: Wed, 11 Dec 2013 17:38:04 +0100 Subject: [PATCH] Put active rules into ES index at startup --- .../org/sonar/server/rule/RuleRegistry.java | 72 ++++++++++++++++++- .../org/sonar/server/search/SearchIndex.java | 59 +++++++++------ .../server/startup/RegisterNewProfiles.java | 9 ++- .../com/sonar/search/active_rule_mapping.json | 40 +++++++++++ .../sonar/server/rule/RuleRegistryTest.java | 53 +++++++++++++- .../sonar/server/search/SearchIndexTest.java | 7 +- 6 files changed, 209 insertions(+), 31 deletions(-) create mode 100644 sonar-server/src/main/resources/com/sonar/search/active_rule_mapping.json 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 5a20ad93b13..773e2d5025c 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 @@ -28,6 +28,9 @@ import org.elasticsearch.common.collect.Maps; import org.elasticsearch.common.io.BytesStream; import org.elasticsearch.common.xcontent.XContentBuilder; import org.elasticsearch.common.xcontent.XContentFactory; +import org.sonar.api.database.DatabaseSession; +import org.sonar.api.rules.ActiveRule; +import org.sonar.api.rules.ActiveRuleParam; import org.sonar.api.rules.Rule; import org.sonar.api.utils.TimeProfiler; import org.sonar.core.rule.RuleDao; @@ -52,17 +55,21 @@ public class RuleRegistry { private static final String PARAM_STATUS = "status"; private static final String INDEX_RULES = "rules"; private static final String TYPE_RULE = "rule"; + private static final String TYPE_ACTIVE_RULE = "active_rule"; private SearchIndex searchIndex; private RuleDao ruleDao; + private DatabaseSession session; - public RuleRegistry(SearchIndex searchIndex, RuleDao ruleDao) { + public RuleRegistry(SearchIndex searchIndex, RuleDao ruleDao, DatabaseSession session) { this.searchIndex = searchIndex; this.ruleDao = ruleDao; + this.session = session; } public void start() { searchIndex.addMappingFromClasspath(INDEX_RULES, TYPE_RULE, "/com/sonar/search/rule_mapping.json"); + searchIndex.addMappingFromClasspath(INDEX_RULES, TYPE_ACTIVE_RULE, "/com/sonar/search/active_rule_mapping.json"); } public void bulkRegisterRules() { @@ -85,6 +92,19 @@ public class RuleRegistry { } } + public void bulkRegisterActiveRules() { + TimeProfiler profiler = new TimeProfiler(); + profiler.start("Rebuilding active rules index - query"); + List rules = session.getResults(ActiveRule.class); + profiler.stop(); + + try { + bulkIndex(rules); + } catch (IOException ioe) { + throw new IllegalStateException("Unable to index active rules", ioe); + } + } + /** *

Find rule IDs matching the given criteria.

* @param query

A collection of (optional) criteria with the following meaning: @@ -159,9 +179,38 @@ public class RuleRegistry { if (! indexIds.isEmpty()) { profiler.start("Remove deleted rule documents"); searchIndex.bulkDelete(INDEX_RULES, TYPE_RULE, indexIds.toArray(new String[0])); + profiler.stop(); + } + } + + private void bulkIndex(List rules) throws IOException { + String[] ids = new String[rules.size()]; + BytesStream[] docs = new BytesStream[rules.size()]; + String[] parentIds = new String[rules.size()]; + int index = 0; + TimeProfiler profiler = new TimeProfiler(); + profiler.start("Build active rules documents"); + for (ActiveRule rule: rules) { + ids[index] = rule.getId().toString(); + docs[index] = activeRuleDocument(rule); + parentIds[index] = rule.getRule().getId().toString(); + index ++; + } + profiler.stop(); + profiler.start("Index active rules"); + searchIndex.bulkIndex(INDEX_RULES, TYPE_ACTIVE_RULE, ids, docs, parentIds); + profiler.stop(); + + List indexIds = searchIndex.findDocumentIds(SearchQuery.create().index(INDEX_RULES).type(TYPE_ACTIVE_RULE)); + indexIds.removeAll(Arrays.asList(ids)); + if (! indexIds.isEmpty()) { + profiler.start("Remove deleted active rule documents"); + searchIndex.bulkDelete(INDEX_RULES, TYPE_ACTIVE_RULE, indexIds.toArray(new String[0])); + profiler.stop(); } } + private XContentBuilder ruleDocument(RuleDto rule, Collection params) throws IOException { XContentBuilder document = XContentFactory.jsonBuilder() .startObject() @@ -191,4 +240,25 @@ public class RuleRegistry { document.endObject(); return document; } + + private XContentBuilder activeRuleDocument(ActiveRule rule) throws IOException { + XContentBuilder document = XContentFactory.jsonBuilder() + .startObject() + .field("id", rule.getId()) + .field("severity", rule.getSeverity()) + .field("profileId", rule.getRulesProfile().getId()) + .field("inheritance", rule.getInheritance()); + if(!rule.getActiveRuleParams().isEmpty()) { + document.startArray("params"); + for (ActiveRuleParam param: rule.getActiveRuleParams()) { + document.startObject() + .field("key", param.getKey()) + .field("value", param.getValue()) + .endObject(); + } + document.endArray(); + } + document.endObject(); + return document; + } } 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 11de7b748c4..9c1a3325f78 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 @@ -87,15 +87,22 @@ public class SearchIndex { } public void put(String index, String type, String id, BytesStream source) { - internalPut(index, type, id, source, false); + internalPut(index, type, id, source, false, null); } public void putSynchronous(String index, String type, String id, BytesStream source) { - internalPut(index, type, id, source, true); + internalPut(index, type, id, source, true, null); } - private void internalPut(String index, String type, String id, BytesStream source, boolean refresh) { + public void put(String index, String type, String id, BytesStream source, String parent) { + internalPut(index, type, id, source, false, parent); + } + + private void internalPut(String index, String type, String id, BytesStream source, boolean refresh, String parent) { IndexRequestBuilder builder = client.prepareIndex(index, type, id).setSource(source.bytes()).setRefresh(refresh); + if (parent != null) { + builder.setParent(parent); + } StopWatch watch = createWatch(); builder.execute().actionGet(); watch.stop("put document with id '%s' with type '%s' into index '%s'", id, type, index); @@ -108,25 +115,26 @@ public class SearchIndex { } StopWatch watch = createWatch(); try { - BulkResponse bulkResponse = client.bulk(builder.setRefresh(true).request()).get(); - if (bulkResponse.hasFailures()) { - // Retry once per failed doc -- ugly - for (BulkItemResponse bulkItemResponse : bulkResponse.getItems()) { - if(bulkItemResponse.isFailed()) { - int itemId = bulkItemResponse.getItemId(); - put(index, type, ids[itemId], sources[itemId]); - } - } - } - } catch (InterruptedException e) { - LOG.error(BULK_INTERRUPTED, e); - } catch (ExecutionException e) { - LOG.error(BULK_EXECUTE_FAILED, e); + doBulkOperation(builder); } finally { watch.stop("bulk index of %d documents with type '%s' into index '%s'", ids.length, type, index); } } + public void bulkIndex(String index, String type, String[] ids, BytesStream[] sources, String[] parentIds) { + BulkRequestBuilder builder = new BulkRequestBuilder(client); + for (int i=0; i definitions; private final LoadedTemplateDao loadedTemplateDao; private final RuleFinder ruleFinder; + private final RuleRegistry ruleRegistry; private final DatabaseSessionFactory sessionFactory; private final PersistentSettings settings; private DatabaseSession session = null; @@ -56,11 +58,13 @@ public class RegisterNewProfiles { public RegisterNewProfiles(List definitions, PersistentSettings settings, RuleFinder ruleFinder, + RuleRegistry ruleRegistry, LoadedTemplateDao loadedTemplateDao, DatabaseSessionFactory sessionFactory, RegisterRules registerRulesBefore) { this.settings = settings; this.ruleFinder = ruleFinder; + this.ruleRegistry = ruleRegistry; this.definitions = definitions; this.loadedTemplateDao = loadedTemplateDao; this.sessionFactory = sessionFactory; @@ -68,10 +72,11 @@ public class RegisterNewProfiles { public RegisterNewProfiles(PersistentSettings settings, RuleFinder ruleFinder, + RuleRegistry ruleRegistry, LoadedTemplateDao loadedTemplateDao, DatabaseSessionFactory sessionFactory, RegisterRules registerRulesBefore) { - this(Collections.emptyList(), settings, ruleFinder, loadedTemplateDao, sessionFactory, registerRulesBefore); + this(Collections.emptyList(), settings, ruleFinder, ruleRegistry, loadedTemplateDao, sessionFactory, registerRulesBefore); } public void start() { @@ -93,6 +98,8 @@ public class RegisterNewProfiles { } session.commit(); profiler.stop(); + + ruleRegistry.bulkRegisterActiveRules(); } private void setDefault(String language, List profiles) { diff --git a/sonar-server/src/main/resources/com/sonar/search/active_rule_mapping.json b/sonar-server/src/main/resources/com/sonar/search/active_rule_mapping.json new file mode 100644 index 00000000000..33badb93f2a --- /dev/null +++ b/sonar-server/src/main/resources/com/sonar/search/active_rule_mapping.json @@ -0,0 +1,40 @@ +{ + "active_rule": { + "_id": { + "path": "id" + }, + "_parent": { + "type": "rule" + }, + "properties": { + "id": { + "type": "integer", + "index": "not_analyzed" + }, + "severity": { + "type": "string", + "index": "not_analyzed" + }, + "profileId": { + "type": "integer", + "index": "not_analyzed" + }, + "inheritance": { + "type": "string", + "index": "not_analyzed" + }, + "params": { + "properties": { + "key": { + "type": "string", + "index": "no" + }, + "value": { + "type": "string", + "index": "no" + } + } + } + } + } +} 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 36308d9af0c..6689af02771 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 @@ -24,10 +24,16 @@ import com.github.tlrx.elasticsearch.test.EsSetup; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import org.apache.commons.io.IOUtils; +import org.elasticsearch.search.SearchHit; import org.junit.After; import org.junit.Before; import org.junit.Test; import org.sonar.api.config.Settings; +import org.sonar.api.database.DatabaseSession; +import org.sonar.api.profiles.RulesProfile; +import org.sonar.api.rules.ActiveRule; +import org.sonar.api.rules.ActiveRuleParam; +import org.sonar.api.rules.Rule; import org.sonar.core.profiling.Profiling; import org.sonar.core.rule.RuleDao; import org.sonar.core.rule.RuleDto; @@ -39,6 +45,9 @@ import org.sonar.test.TestUtils; import java.util.HashMap; import java.util.List; +import static org.elasticsearch.index.query.FilterBuilders.hasChildFilter; +import static org.elasticsearch.index.query.FilterBuilders.hasParentFilter; +import static org.elasticsearch.index.query.FilterBuilders.termFilter; import static org.fest.assertions.Assertions.assertThat; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -48,6 +57,7 @@ public class RuleRegistryTest { private EsSetup esSetup; private SearchIndex searchIndex; private RuleDao ruleDao; + private DatabaseSession session; RuleRegistry registry; @Before @@ -65,7 +75,9 @@ public class RuleRegistryTest { searchIndex = new SearchIndex(node, profiling); searchIndex.start(); - registry = new RuleRegistry(searchIndex, ruleDao); + session = mock(DatabaseSession.class); + + registry = new RuleRegistry(searchIndex, ruleDao, session); registry.start(); String source1 = IOUtils.toString(TestUtils.getResource(getClass(), "rules/rule1.json").toURI()); @@ -89,7 +101,7 @@ public class RuleRegistryTest { @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(); + assertThat(esSetup.client().admin().indices().prepareTypesExists("rules").setTypes("rule", "active_rule").execute().actionGet().isExists()).isTrue(); } @Test @@ -202,4 +214,41 @@ public class RuleRegistryTest { .containsOnly((int) ruleId1, (int) ruleId2); assertThat(esSetup.exists("rules", "rule", "3")).isFalse(); } + + @Test + public void should_index_all_active_rules() { + int id = 1; + int profileId = 42; + ActiveRule rule = mock(ActiveRule.class); + when(rule.getId()).thenReturn(id); + Rule refRule = Rule.create(); + refRule.setId(1); + when(rule.getRule()).thenReturn(refRule ); + RulesProfile profile = mock(RulesProfile.class); + when(profile.getId()).thenReturn(profileId); + when(rule.getRulesProfile()).thenReturn(profile ); + ActiveRuleParam param = mock(ActiveRuleParam.class); + when(param.getKey()).thenReturn("string"); + when(param.getValue()).thenReturn("polop"); + List params = ImmutableList.of(param); + when(rule.getActiveRuleParams()).thenReturn(params ); + List rules = ImmutableList.of(rule); + + when(session.getResults(ActiveRule.class)).thenReturn(rules); + registry.bulkRegisterActiveRules(); + assertThat(esSetup.exists("rules", "active_rule", "1")); + + SearchHit[] parentHit = esSetup.client().prepareSearch("rules").setFilter( + hasChildFilter("active_rule", termFilter("profileId", profileId)) + ).execute().actionGet().getHits().getHits(); + assertThat(parentHit).hasSize(1); + assertThat(parentHit[0].getId()).isEqualTo("1"); + + SearchHit[] childHit = esSetup.client().prepareSearch("rules").setFilter( + hasParentFilter("rule", termFilter("key", "RuleWithParameters")) + ).execute().actionGet().getHits().getHits(); + assertThat(childHit).hasSize(1); + assertThat(childHit[0].getId()).isEqualTo("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 3a505730825..7ee79bb5d68 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 @@ -20,9 +20,6 @@ package org.sonar.server.search; -import org.sonar.api.config.Settings; - -import org.sonar.core.profiling.Profiling; import com.github.tlrx.elasticsearch.test.EsSetup; import org.elasticsearch.common.io.BytesStream; import org.elasticsearch.common.xcontent.XContentFactory; @@ -30,6 +27,8 @@ import org.elasticsearch.index.mapper.StrictDynamicMappingException; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.sonar.api.config.Settings; +import org.sonar.core.profiling.Profiling; import java.util.List; @@ -134,7 +133,7 @@ public class SearchIndexTest { @Test(expected = StrictDynamicMappingException.class) public void should_forbid_dynamic_mapping() throws Exception { searchIndex.addMappingFromClasspath("index", "type1", "/org/sonar/server/search/SearchIndexTest/correct_mapping1.json"); - searchIndex.put("index", "type1", "666", + searchIndex.putSynchronous("index", "type1", "666", XContentFactory.jsonBuilder().startObject().field("unknown", "plouf").endObject() ); } -- 2.39.5