]> source.dussan.org Git - sonarqube.git/commitdiff
Have newly activated rules refresh the ES index
authorJean-Baptiste Lievremont <jean-baptiste.lievremont@sonarsource.com>
Thu, 19 Dec 2013 14:44:26 +0000 (15:44 +0100)
committerJean-Baptiste Lievremont <jean-baptiste.lievremont@sonarsource.com>
Thu, 19 Dec 2013 14:44:47 +0000 (15:44 +0100)
sonar-server/src/main/java/org/sonar/server/qualityprofile/QProfileOperations.java
sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivationResult.java
sonar-server/src/main/java/org/sonar/server/rule/ProfileRules.java
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/webapp/WEB-INF/app/controllers/new_rules_configuration_controller.rb
sonar-server/src/test/java/org/sonar/server/qualityprofile/QProfileOperationsTest.java
sonar-server/src/test/java/org/sonar/server/rule/ProfileRulesTest.java
sonar-server/src/test/java/org/sonar/server/rule/RuleRegistryTest.java

index c7474c973c15cda23f0fc89714987ebdae19a8f3..f4afc5c6382759c1c12481bfd784449d7cbaa8d3 100644 (file)
@@ -45,6 +45,7 @@ import org.sonar.core.rule.RuleDao;
 import org.sonar.core.rule.RuleParamDto;
 import org.sonar.server.configuration.ProfilesManager;
 import org.sonar.server.exceptions.BadRequestException;
+import org.sonar.server.rule.ProfileRules;
 import org.sonar.server.rule.RuleRegistry;
 import org.sonar.server.user.UserSession;
 
@@ -69,6 +70,7 @@ public class QProfileOperations implements ServerComponent {
   private final List<ProfileImporter> importers;
   private final PreviewCache dryRunCache;
   private final RuleRegistry ruleRegistry;
+  private final ProfileRules profileRules;
 
   // Should not be used as it still uses Hibernate
   private final ProfilesManager profilesManager;
@@ -77,14 +79,14 @@ public class QProfileOperations implements ServerComponent {
    * Used by pico when no plugin provide profile exporter / importer
    */
   public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, RuleDao ruleDao, PropertiesDao propertiesDao,
-                            PreviewCache dryRunCache, RuleRegistry ruleRegistry, ProfilesManager profilesManager) {
+                            PreviewCache dryRunCache, RuleRegistry ruleRegistry, ProfilesManager profilesManager, ProfileRules profileRules) {
     this(myBatis, dao, activeRuleDao, ruleDao, propertiesDao, Lists.<ProfileExporter>newArrayList(), Lists.<ProfileImporter>newArrayList(), dryRunCache, ruleRegistry,
-      profilesManager);
+      profilesManager, profileRules);
   }
 
   public QProfileOperations(MyBatis myBatis, QualityProfileDao dao, ActiveRuleDao activeRuleDao, RuleDao ruleDao, PropertiesDao propertiesDao,
                             List<ProfileExporter> exporters, List<ProfileImporter> importers, PreviewCache dryRunCache, RuleRegistry ruleRegistry,
-                            ProfilesManager profilesManager) {
+                            ProfilesManager profilesManager, ProfileRules profileRules) {
     this.myBatis = myBatis;
     this.dao = dao;
     this.activeRuleDao = activeRuleDao;
@@ -95,6 +97,7 @@ public class QProfileOperations implements ServerComponent {
     this.dryRunCache = dryRunCache;
     this.ruleRegistry = ruleRegistry;
     this.profilesManager = profilesManager;
+    this.profileRules = profileRules;
   }
 
   public NewProfileResult newProfile(String name, String language, Map<String, String> xmlProfilesByPlugin, UserSession userSession) {
@@ -141,7 +144,7 @@ public class QProfileOperations implements ServerComponent {
       } else {
         updateSeverity(activeRule, severity, userSession, session);
       }
-      return new RuleActivationResult(QProfile.from(qualityProfile), rule, activeRule);
+      return new RuleActivationResult(QProfile.from(qualityProfile), profileRules.getFromActiveRuleId(activeRule.getId()));
     } finally {
       MyBatis.closeQuietly(session);
     }
@@ -155,16 +158,20 @@ public class QProfileOperations implements ServerComponent {
     activeRuleDao.insert(activeRuleDto, session);
 
     List<RuleParamDto> ruleParams = ruleDao.selectParameters(rule.getId().longValue(), session);
+    List<ActiveRuleParamDto> activeRuleParams = Lists.newArrayList();
     for (RuleParamDto ruleParam : ruleParams) {
       ActiveRuleParamDto activeRuleParam = new ActiveRuleParamDto()
         .setActiveRuleId(activeRuleDto.getId())
         .setRulesParameterId(ruleParam.getId())
         .setKey(ruleParam.getName())
         .setValue(ruleParam.getDefaultValue());
+      activeRuleParams.add(activeRuleParam);
       activeRuleDao.insert(activeRuleParam, session);
     }
     session.commit();
 
+    ruleRegistry.save(activeRuleDto, activeRuleParams);
+
     profilesManager.activated(qualityProfile.getId(), activeRuleDto.getId(), userSession.name());
     return activeRuleDto;
   }
@@ -191,7 +198,7 @@ public class QProfileOperations implements ServerComponent {
       activeRuleDao.deleteParameters(activeRule.getId(), session);
       session.commit();
 
-      return new RuleActivationResult(QProfile.from(qualityProfile), rule, null);
+      return new RuleActivationResult(QProfile.from(qualityProfile), profileRules.getFromRuleId(rule.getId()));
     } finally {
       MyBatis.closeQuietly(session);
     }
index ee8366370a60676e7e9e6be2b48933247474e1ac..a787804ec57331342373a8e024f0488a7c35c52b 100644 (file)
 
 package org.sonar.server.qualityprofile;
 
-import org.sonar.api.rules.Rule;
-import org.sonar.core.qualityprofile.db.ActiveRuleDto;
-
-import javax.annotation.CheckForNull;
-import javax.annotation.Nullable;
 
 public class RuleActivationResult {
 
   private QProfile profile;
-  private Rule rule;
-  private ActiveRuleDto activeRule;
+  private QProfileRule rule;
 
-  public RuleActivationResult(QProfile profile, Rule rule, @Nullable ActiveRuleDto activeRule) {
+  public RuleActivationResult(QProfile profile, QProfileRule rule) {
     this.profile = profile;
     this.rule = rule;
-    this.activeRule = activeRule;
   }
 
   public QProfile profile() {
     return profile;
   }
 
-  public Rule rule() {
+  public QProfileRule rule() {
     return rule;
   }
-
-  @CheckForNull
-  public ActiveRuleDto activeRule() {
-    return activeRule;
-  }
 }
index 1125d7dff11bcf4f3dac933e4cf212a4966ff956..d69faba4a4a91aef7a4e540f15c313c6a4b78eb2 100644 (file)
@@ -20,6 +20,7 @@
 package org.sonar.server.rule;
 
 import org.apache.commons.lang.StringUtils;
+import org.elasticsearch.action.get.GetResponse;
 import org.elasticsearch.action.get.MultiGetItemResponse;
 import org.elasticsearch.action.get.MultiGetRequestBuilder;
 import org.elasticsearch.action.search.SearchRequestBuilder;
@@ -59,6 +60,22 @@ public class ProfileRules implements ServerExtension {
     this.index = index;
   }
 
+  public QProfileRule getFromActiveRuleId(int activeRuleId) {
+    GetResponse activeRuleResponse = index.client().prepareGet(INDEX_RULES, TYPE_ACTIVE_RULE, Integer.toString(activeRuleId))
+      .setFields(FIELD_SOURCE, FIELD_PARENT)
+      .execute().actionGet();
+    Map<String, Object> activeRuleSource = activeRuleResponse.getSourceAsMap();
+    Map<String, Object> ruleSource = index.client().prepareGet(INDEX_RULES, TYPE_RULE, (String) activeRuleResponse.getField(FIELD_PARENT).getValue())
+      .execute().actionGet().getSourceAsMap();
+    return new QProfileRule(ruleSource, activeRuleSource);
+  }
+
+  public QProfileRule getFromRuleId(Integer ruleId) {
+    Map<String, Object> ruleSource = index.client().prepareGet(INDEX_RULES, TYPE_RULE, Integer.toString(ruleId))
+      .execute().actionGet().getSourceAsMap();
+    return new QProfileRule(ruleSource);
+  }
+
   public QProfileRuleResult searchActiveRules(ProfileRuleQuery query, Paging paging) {
     BoolFilterBuilder filter = activeRuleFilter(query);
 
index 718d0845eeefc2d4756d50eb7ac7ee449edc6ab1..ebb5a7b2874728ed6b2630fe0733e270e6d40240 100644 (file)
@@ -165,6 +165,14 @@ public class RuleRegistry {
     }
   }
 
+  public void save(ActiveRuleDto activeRule, Collection<ActiveRuleParamDto> params) {
+    try {
+      searchIndex.putSynchronous(INDEX_RULES, TYPE_ACTIVE_RULE, Long.toString(activeRule.getId()), activeRuleDocument(activeRule, params), activeRule.getRulId().toString());
+    } catch (IOException ioexception) {
+      throw new IllegalStateException("Unable to index active rule with id=" + activeRule.getId(), ioexception);
+    }
+  }
+
   private void bulkIndex(List<RuleDto> rules, Multimap<Long, RuleParamDto> paramsByRule) throws IOException {
     String[] ids = new String[rules.size()];
     BytesStream[] docs = new BytesStream[rules.size()];
index 86466b325669e1d7e18ac120767247a96e6beb82..7be47572bc2d2050ce29c075282b1ce5d3e7d3ee 100644 (file)
@@ -112,6 +112,10 @@ public class SearchIndex {
     internalPut(index, type, id, source, false, parent);
   }
 
+  public void putSynchronous(String index, String type, String id, BytesStream source, String parent) {
+    internalPut(index, type, id, source, true, 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) {
index 4e9cd23eadd20386237fd2cb1525130f66b8c09f..5e3c8a987fc465ae86a84a2f9ee956928d7683ce 100644 (file)
@@ -125,13 +125,11 @@ class NewRulesConfigurationController < ApplicationController
       end
     end
 
-    # TODO replace it by QProfileRule
-    profile = Profile.find(params[:id].to_i)
-    rule = Rule.first(:conditions => ["id = ? and status <> ?", params[:rule_id].to_i, Rule::STATUS_REMOVED]) if profile
-    active_rule = ActiveRule.first(:conditions => ['id = ?', result.activeRule().getId()]) if result && result.activeRule()
+    profile = result.profile
+    rule = result.rule
 
     render :update do |page|
-      page.replace_html("rule_#{rule.id}", :partial => 'rule', :object => rule, :locals => {:profile => profile, :rule => rule, :active_rule => active_rule})
+      page.replace_html("rule_#{rule.id}", :partial => 'rule', :object => rule, :locals => {:profile => profile, :rule => rule})
       page.assign('localModifications', true)
     end
   end
index b1c436474c9db3e4e897be82bf6f15486f3aeaee..6a22f8d7e33708f9f91d74bc36fabde07dfed222 100644 (file)
@@ -50,10 +50,12 @@ import org.sonar.core.rule.RuleParamDto;
 import org.sonar.server.configuration.ProfilesManager;
 import org.sonar.server.exceptions.BadRequestException;
 import org.sonar.server.exceptions.ForbiddenException;
+import org.sonar.server.rule.ProfileRules;
 import org.sonar.server.rule.RuleRegistry;
 import org.sonar.server.user.MockUserSession;
 
 import java.io.Reader;
+import java.util.Collection;
 import java.util.List;
 import java.util.Map;
 
@@ -61,6 +63,7 @@ import static com.google.common.collect.Lists.newArrayList;
 import static com.google.common.collect.Maps.newHashMap;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.fest.assertions.Fail.fail;
+import static org.mockito.Matchers.*;
 import static org.mockito.Mockito.*;
 
 @RunWith(MockitoJUnitRunner.class)
@@ -90,6 +93,9 @@ public class QProfileOperationsTest {
   @Mock
   RuleRegistry ruleRegistry;
 
+  @Mock
+  ProfileRules profileRules;
+
   @Mock
   ProfilesManager profilesManager;
 
@@ -115,7 +121,8 @@ public class QProfileOperationsTest {
       }
     }).when(activeRuleDao).insert(any(ActiveRuleDto.class), any(SqlSession.class));
 
-    operations = new QProfileOperations(myBatis, qualityProfileDao, activeRuleDao, ruleDao, propertiesDao, exporters, importers, dryRunCache, ruleRegistry, profilesManager);
+    operations = new QProfileOperations(myBatis, qualityProfileDao, activeRuleDao, ruleDao, propertiesDao, exporters, importers, dryRunCache, ruleRegistry, profilesManager,
+      profileRules);
   }
 
   @Test
@@ -239,11 +246,12 @@ public class QProfileOperationsTest {
     Rule rule = Rule.create().setRepositoryKey("squid").setKey("AvoidCycle");
     rule.setId(10);
     when(ruleDao.selectParameters(eq(10L), eq(session))).thenReturn(newArrayList(new RuleParamDto().setId(20).setName("max").setDefaultValue("10")));
+    when(profileRules.getFromActiveRuleId(anyInt())).thenReturn(mock(QProfileRule.class));
 
     RuleActivationResult result = operations.activateRule(qualityProfile, rule, Severity.CRITICAL, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN));
     assertThat(result.profile()).isNotNull();
     assertThat(result.rule()).isNotNull();
-    assertThat(result.activeRule()).isNotNull();
+    assertThat(result.rule().activeRuleId()).isNotNull();
 
     ArgumentCaptor<ActiveRuleDto> activeRuleArgument = ArgumentCaptor.forClass(ActiveRuleDto.class);
     verify(activeRuleDao).insert(activeRuleArgument.capture(), eq(session));
@@ -256,6 +264,8 @@ public class QProfileOperationsTest {
     assertThat(activeRuleParamArgument.getValue().getValue()).isEqualTo("10");
 
     verify(session).commit();
+    verify(ruleRegistry).save(activeRuleArgument.capture(), (Collection<ActiveRuleParamDto>) anyCollection());
+    verify(profileRules).getFromActiveRuleId(anyInt());
     verify(profilesManager).activated(eq(1), anyInt(), eq("nicolas"));
   }
 
@@ -266,15 +276,16 @@ public class QProfileOperationsTest {
     rule.setId(10);
     ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
     when(activeRuleDao.selectByProfileAndRule(1, 10)).thenReturn(activeRule);
+    when(profileRules.getFromActiveRuleId(anyInt())).thenReturn(mock(QProfileRule.class));
 
     RuleActivationResult result = operations.activateRule(qualityProfile, rule, Severity.MAJOR, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN));
     assertThat(result.profile()).isNotNull();
     assertThat(result.rule()).isNotNull();
-    assertThat(result.activeRule()).isNotNull();
+    assertThat(result.rule().activeRuleId()).isNotNull();
 
     verify(activeRuleDao).update(eq(activeRule), eq(session));
-
     verify(session).commit();
+    verify(profileRules).getFromActiveRuleId(anyInt());
     verify(profilesManager).ruleSeverityChanged(eq(1), eq(5), eq(RulePriority.MINOR), eq(RulePriority.MAJOR), eq("nicolas"));
   }
 
@@ -285,15 +296,16 @@ public class QProfileOperationsTest {
     rule.setId(10);
     ActiveRuleDto activeRule = new ActiveRuleDto().setId(5).setProfileId(1).setRuleId(10).setSeverity(1);
     when(activeRuleDao.selectByProfileAndRule(1, 10)).thenReturn(activeRule);
+    when(profileRules.getFromRuleId(anyInt())).thenReturn(mock(QProfileRule.class));
 
     RuleActivationResult result = operations.deactivateRule(qualityProfile, rule, MockUserSession.create().setName("nicolas").setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN));
     assertThat(result.profile()).isNotNull();
     assertThat(result.rule()).isNotNull();
-    assertThat(result.activeRule()).isNull();
 
     verify(activeRuleDao).delete(eq(5), eq(session));
     verify(activeRuleDao).deleteParameters(eq(5), eq(session));
     verify(session).commit();
+    verify(profileRules).getFromRuleId(anyInt());
     verify(profilesManager).deactivated(eq(1), anyInt(), eq("nicolas"));
   }
 
index d8523c7b557e334f3ec1bfd9b79c3da2993fc351..b6b22b9510a401701c520c7cc5e5763a859342c2 100644 (file)
@@ -46,7 +46,7 @@ public class ProfileRulesTest {
   private EsSetup esSetup;
 
   @Before
-  public void setUp() {
+  public void setUp() throws Exception {
     esSetup = new EsSetup();
     esSetup.execute(EsSetup.deleteAll());
 
@@ -60,6 +60,16 @@ public class ProfileRulesTest {
     RuleRegistry registry = new RuleRegistry(index, null, null);
     registry.start();
     profileRules = new ProfileRules(index);
+
+    esSetup.client().prepareBulk()
+    .add(Requests.indexRequest().index("rules").type("rule").source(testFileAsString("should_find_active_rules/rule25.json")))
+    .add(Requests.indexRequest().index("rules").type("rule").source(testFileAsString("should_find_active_rules/rule759.json")))
+    .add(Requests.indexRequest().index("rules").type("rule").source(testFileAsString("should_find_active_rules/rule1482.json")))
+    .add(Requests.indexRequest().index("rules").type("active_rule").parent("25").source(testFileAsString("should_find_active_rules/active_rule25.json")))
+    .add(Requests.indexRequest().index("rules").type("active_rule").parent("759").source(testFileAsString("should_find_active_rules/active_rule391.json")))
+    .add(Requests.indexRequest().index("rules").type("active_rule").parent("759").source(testFileAsString("should_find_active_rules/active_rule523.json")))
+    .add(Requests.indexRequest().index("rules").type("active_rule").parent("1482").source(testFileAsString("should_find_active_rules/active_rule2702.json")))
+    .setRefresh(true).execute().actionGet();
   }
 
   @After
@@ -69,15 +79,6 @@ public class ProfileRulesTest {
 
   @Test
   public void should_find_active_rules() throws Exception {
-    esSetup.client().prepareBulk()
-      .add(Requests.indexRequest().index("rules").type("rule").source(testFileAsString("should_find_active_rules/rule25.json")))
-      .add(Requests.indexRequest().index("rules").type("rule").source(testFileAsString("should_find_active_rules/rule759.json")))
-      .add(Requests.indexRequest().index("rules").type("rule").source(testFileAsString("should_find_active_rules/rule1482.json")))
-      .add(Requests.indexRequest().index("rules").type("active_rule").parent("25").source(testFileAsString("should_find_active_rules/active_rule25.json")))
-      .add(Requests.indexRequest().index("rules").type("active_rule").parent("759").source(testFileAsString("should_find_active_rules/active_rule391.json")))
-      .add(Requests.indexRequest().index("rules").type("active_rule").parent("759").source(testFileAsString("should_find_active_rules/active_rule523.json")))
-      .add(Requests.indexRequest().index("rules").type("active_rule").parent("1482").source(testFileAsString("should_find_active_rules/active_rule2702.json")))
-      .setRefresh(true).execute().actionGet();
 
     Paging paging = Paging.create(10, 1);
 
@@ -114,6 +115,16 @@ public class ProfileRulesTest {
     assertThat(rulesWParam.get(0).params()).hasSize(2);
   }
 
+  @Test
+  public void should_get_from_active_rule() {
+    assertThat(profileRules.getFromActiveRuleId(391)).isNotNull();
+  }
+
+  @Test
+  public void should_get_from_rule() {
+    assertThat(profileRules.getFromActiveRuleId(25)).isNotNull();
+  }
+
   private String testFileAsString(String testFile) throws Exception {
     return IOUtils.toString(TestUtils.getResource(getClass(), testFile).toURI());
   }
index 5d38bbd74b657892bdb64cc8816b4d5bb5ca7574..31550c401cd98ab36d627214812df8e03e4180c7 100644 (file)
@@ -48,6 +48,7 @@ import org.sonar.server.search.SearchNode;
 import org.sonar.test.TestUtils;
 
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.HashMap;
 import java.util.List;
 
@@ -289,4 +290,26 @@ public class RuleRegistryTest {
     assertThat(childHit).hasSize(1);
     assertThat(childHit[0].getId()).isEqualTo("1");
   }
+
+  @Test
+  public void save_active_rule() throws IOException {
+    ActiveRuleDto activeRule = new ActiveRuleDto().setId(1).setProfileId(10).setRuleId(1).setSeverity(2);
+    ArrayList<ActiveRuleParamDto> params = newArrayList(new ActiveRuleParamDto().setId(1).setActiveRuleId(1).setRulesParameterId(1).setKey("key").setValue("RuleWithParameters"));
+
+    registry.save(activeRule, params);
+    assertThat(esSetup.exists("rules", "active_rule", "1"));
+
+    SearchHit[] parentHit = esSetup.client().prepareSearch("rules").setFilter(
+      hasChildFilter("active_rule", termFilter("profileId", 10))
+    ).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");
+  }
+
 }