]> source.dussan.org Git - sonarqube.git/commitdiff
Put active rules into ES index at startup
authorJean-Baptiste Lievremont <jean-baptiste.lievremont@sonarsource.com>
Wed, 11 Dec 2013 16:38:04 +0000 (17:38 +0100)
committerJean-Baptiste Lievremont <jean-baptiste.lievremont@sonarsource.com>
Wed, 11 Dec 2013 16:39:01 +0000 (17:39 +0100)
sonar-server/src/main/java/org/sonar/server/rule/RuleRegistry.java
sonar-server/src/main/java/org/sonar/server/search/SearchIndex.java
sonar-server/src/main/java/org/sonar/server/startup/RegisterNewProfiles.java
sonar-server/src/main/resources/com/sonar/search/active_rule_mapping.json [new file with mode: 0644]
sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java
sonar-server/src/test/java/org/sonar/server/search/SearchIndexTest.java

index 5a20ad93b13eee23a1b8a38468f0893d5526ec10..773e2d5025c3b55b93b36fe60688ec2652d9ecae 100644 (file)
@@ -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<ActiveRule> rules = session.getResults(ActiveRule.class);
+    profiler.stop();
+
+    try {
+      bulkIndex(rules);
+    } catch (IOException ioe) {
+      throw new IllegalStateException("Unable to index active rules", ioe);
+    }
+  }
+
   /**
    * <p>Find rule IDs matching the given criteria.</p>
    * @param query <p>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<ActiveRule> 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<String> 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<RuleParamDto> 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;
+  }
 }
index 11de7b748c4bc12215ee89119a179365429ee575..9c1a3325f7857e9b9251eca901421149003326ab 100644 (file)
@@ -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<ids.length; i++) {
+      builder.add(client.prepareIndex(index, type, ids[i]).setParent(parentIds[i]).setSource(sources[i].bytes()));
+    }
+    StopWatch watch = createWatch();
+    try {
+      doBulkOperation(builder);
+    } finally {
+      watch.stop("bulk index of %d child documents with type '%s' into index '%s'", ids.length, type, index);
+    }
+  }
+
+
   public void addMappingFromClasspath(String index, String type, String resourcePath) {
     try {
       URL resource = getClass().getResource(resourcePath);
@@ -207,22 +215,27 @@ public class SearchIndex {
       builder.add(client.prepareDelete(index, type, ids[i]));
     }
     StopWatch watch = createWatch();
+    try {
+      doBulkOperation(builder);
+    } finally {
+      watch.stop("bulk delete of %d documents with type '%s' from index '%s'", ids.length, type, index);
+    }
+  }
+
+  private void doBulkOperation(BulkRequestBuilder builder) {
     try {
       BulkResponse bulkResponse = client.bulk(builder.setRefresh(true).request()).get();
       if (bulkResponse.hasFailures()) {
         for (BulkItemResponse bulkItemResponse : bulkResponse.getItems()) {
           if(bulkItemResponse.isFailed()) {
-            int itemId = bulkItemResponse.getItemId();
-            client.prepareDelete(index, type, ids[itemId]).execute().actionGet();
+            throw new IllegalStateException("Bulk operation partially executed");
           }
         }
       }
     } catch (InterruptedException e) {
-      LOG.error(BULK_INTERRUPTED, e);
+      throw new IllegalStateException(BULK_INTERRUPTED, e);
     } catch (ExecutionException e) {
-      LOG.error(BULK_EXECUTE_FAILED, e);
-    } finally {
-      watch.stop("bulk delete of %d documents with type '%s' from index '%s'", ids.length, type, index);
+      throw new IllegalStateException(BULK_EXECUTE_FAILED, e);
     }
   }
 
index 7be10cb78282288079546c73b20ec0d37ba27638..9d67ecce3ec4c7cbba6e90f940b6069657ba8d4a 100644 (file)
@@ -38,6 +38,7 @@ import org.sonar.core.template.LoadedTemplateDao;
 import org.sonar.core.template.LoadedTemplateDto;
 import org.sonar.jpa.session.DatabaseSessionFactory;
 import org.sonar.server.platform.PersistentSettings;
+import org.sonar.server.rule.RuleRegistry;
 
 import java.util.*;
 
@@ -49,6 +50,7 @@ public class RegisterNewProfiles {
   private final List<ProfileDefinition> 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<ProfileDefinition> 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.<ProfileDefinition>emptyList(), settings, ruleFinder, loadedTemplateDao, sessionFactory, registerRulesBefore);
+    this(Collections.<ProfileDefinition>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<RulesProfile> 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 (file)
index 0000000..33badb9
--- /dev/null
@@ -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"
+          }
+        }
+      }
+    }
+  }
+}
index 36308d9af0cc0f3a984108455d071f2e1639665b..6689af02771bf4e31521f9901f6d88109064c690 100644 (file)
@@ -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<ActiveRuleParam> params = ImmutableList.of(param);
+    when(rule.getActiveRuleParams()).thenReturn(params );
+    List<ActiveRule> 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");
+
+  }
 }
index 3a505730825943b943b3d361273e5becb8c58d25..7ee79bb5d6871a781a7491cb070513c8fe7dcb61 100644 (file)
@@ -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()
     );
   }