SONAR-5007 fix bug related to synchronization of rule parameters

* removal of rule parameter must be propagated to active rules
* addition of rule parameter with default value must be propagated to active rules

1 files changed, 90 insertions, 41 deletions

diff --git a/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java b/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java
index ad9ac91c5f1..349eb0689f7 100644
--- a/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/rule/RegisterRulesMediumTest.java
@@ -35,13 +35,16 @@ import org.sonar.api.server.rule.RulesDefinition;
 import org.sonar.api.utils.MessageException;
 import org.sonar.core.permission.GlobalPermissions;
 import org.sonar.core.persistence.DbSession;
+import org.sonar.core.qualityprofile.db.ActiveRuleKey;
 import org.sonar.core.rule.RuleDto;
 import org.sonar.core.rule.RuleParamDto;
 import org.sonar.server.db.DbClient;
 import org.sonar.server.platform.Platform;
+import org.sonar.server.qualityprofile.ActiveRule;
 import org.sonar.server.qualityprofile.QProfileService;
 import org.sonar.server.qualityprofile.QProfileTesting;
 import org.sonar.server.qualityprofile.RuleActivation;
+import org.sonar.server.qualityprofile.index.ActiveRuleIndex;
 import org.sonar.server.rule.index.RuleIndex;
 import org.sonar.server.rule.index.RuleQuery;
 import org.sonar.server.search.QueryOptions;
@@ -54,6 +57,7 @@ import javax.annotation.Nullable;
 
 import java.util.Date;
 import java.util.List;
+import java.util.Map;
 
 import static com.google.common.collect.Sets.newHashSet;
 import static org.fest.assertions.Assertions.assertThat;
@@ -66,7 +70,8 @@ public class RegisterRulesMediumTest {
   @ClassRule
   public static final ServerTester TESTER = new ServerTester().addComponents(RULE_DEFS);
 
-  RuleIndex index;
+  RuleIndex ruleIndex;
+  ActiveRuleIndex activeRuleIndex;
   DbClient db;
   DbSession dbSession;
 
@@ -76,7 +81,8 @@ public class RegisterRulesMediumTest {
     db = TESTER.get(DbClient.class);
     dbSession = TESTER.get(DbClient.class).openSession(false);
     dbSession.clearCache();
-    index = TESTER.get(RuleIndex.class);
+    ruleIndex = TESTER.get(RuleIndex.class);
+    activeRuleIndex = TESTER.get(ActiveRuleIndex.class);
   }
 
   @After
@@ -95,7 +101,7 @@ public class RegisterRulesMediumTest {
     db = TESTER.get(DbClient.class);
     dbSession = TESTER.get(DbClient.class).openSession(false);
     dbSession.clearCache();
-    index = TESTER.get(RuleIndex.class);
+    ruleIndex = TESTER.get(RuleIndex.class);
   }
 
   @Test
@@ -129,10 +135,10 @@ public class RegisterRulesMediumTest {
     assertThat(ruleParams).hasSize(2);
 
     // verify es
-    Result<Rule> searchResult = index.search(new RuleQuery(), new QueryOptions());
+    Result<Rule> searchResult = ruleIndex.search(new RuleQuery(), new QueryOptions());
     assertThat(searchResult.getTotal()).isEqualTo(1);
     assertThat(searchResult.getHits()).hasSize(1);
-    Rule rule = index.getByKey(RuleKey.of("xoo", "x1"));
+    Rule rule = ruleIndex.getByKey(RuleKey.of("xoo", "x1"));
     assertThat(rule.severity()).isEqualTo(Severity.MINOR);
     assertThat(rule.name()).isEqualTo("x1 name");
     assertThat(rule.htmlDescription()).isEqualTo("x1 desc");
@@ -175,8 +181,8 @@ public class RegisterRulesMediumTest {
     register(rules);
 
     // verify that rules are indexed
-    Result<Rule> searchResult = index.search(new RuleQuery(), new QueryOptions());
-    searchResult = index.search(new RuleQuery().setKey("xoo:x1"), new QueryOptions());
+    Result<Rule> searchResult = ruleIndex.search(new RuleQuery(), new QueryOptions());
+    searchResult = ruleIndex.search(new RuleQuery().setKey("xoo:x1"), new QueryOptions());
     assertThat(searchResult.getTotal()).isEqualTo(1);
     assertThat(searchResult.getHits()).hasSize(1);
     assertThat(searchResult.getHits().get(0).key()).isEqualTo(RuleKey.of("xoo", "x1"));
@@ -226,7 +232,7 @@ public class RegisterRulesMediumTest {
       }
     });
 
-    Rule rule = index.getByKey(RuleTesting.XOO_X1);
+    Rule rule = ruleIndex.getByKey(RuleTesting.XOO_X1);
     assertThat(rule.severity()).isEqualTo(Severity.INFO);
     assertThat(rule.name()).isEqualTo("Name2");
     assertThat(rule.htmlDescription()).isEqualTo("Desc2");
@@ -256,18 +262,18 @@ public class RegisterRulesMediumTest {
     register(rules);
 
     // Store updated at date
-    Date updatedAt = index.getByKey(RuleTesting.XOO_X1).updatedAt();
+    Date updatedAt = ruleIndex.getByKey(RuleTesting.XOO_X1).updatedAt();
 
     // Re-execute startup tasks
     register(rules);
 
     // Verify rule has not been updated
-    Rule customRuleReloaded = index.getByKey(RuleTesting.XOO_X1);
+    Rule customRuleReloaded = ruleIndex.getByKey(RuleTesting.XOO_X1);
     assertThat(DateUtils.isSameInstant(customRuleReloaded.updatedAt(), updatedAt));
   }
 
   @Test
-  public void uninstall_and_reinstall_rules() {
+  public void disable_then_enable_rules() {
     Rules rules = new Rules() {
       @Override
       public void init(RulesDefinition.NewRepository repository) {
@@ -280,14 +286,14 @@ public class RegisterRulesMediumTest {
     register(null);
     RuleDto rule = db.ruleDao().getByKey(dbSession, RuleTesting.XOO_X1);
     assertThat(rule.getStatus()).isEqualTo(RuleStatus.REMOVED);
-    Rule indexedRule = index.getByKey(RuleTesting.XOO_X1);
+    Rule indexedRule = ruleIndex.getByKey(RuleTesting.XOO_X1);
     assertThat(indexedRule.status()).isEqualTo(RuleStatus.REMOVED);
 
     // Re-install plugin
     register(rules);
     rule = db.ruleDao().getByKey(dbSession, RuleTesting.XOO_X1);
     assertThat(rule.getStatus()).isEqualTo(RuleStatus.READY);
-    indexedRule = index.getByKey(RuleTesting.XOO_X1);
+    indexedRule = ruleIndex.getByKey(RuleTesting.XOO_X1);
     assertThat(indexedRule.status()).isEqualTo(RuleStatus.READY);
   }
 
@@ -302,7 +308,6 @@ public class RegisterRulesMediumTest {
 
     // Create a profile and activate rule
     MockUserSession.set().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN).setLogin("me");
-
     db.qualityProfileDao().insert(dbSession, QProfileTesting.newXooP1());
     dbSession.commit();
     dbSession.clearCache();
@@ -316,13 +321,13 @@ public class RegisterRulesMediumTest {
         repository.createRule("x2").setName("x2 name").setHtmlDescription("x2 desc");
       }
     });
-    assertThat(db.ruleDao().getByKey(dbSession, RuleKey.of("xoo", "x1")).getStatus()).isEqualTo(RuleStatus.REMOVED);
-    assertThat(db.ruleDao().getByKey(dbSession, RuleKey.of("xoo", "x2")).getStatus()).isEqualTo(RuleStatus.READY);
-    assertThat(db.activeRuleDao().findByProfileKey(dbSession, QProfileTesting.XOO_P1_KEY)).hasSize(0);
+    assertThat(ruleIndex.getByKey(RuleKey.of("xoo", "x1")).status()).isEqualTo(RuleStatus.REMOVED);
+    assertThat(ruleIndex.getByKey(RuleKey.of("xoo", "x2")).status()).isEqualTo(RuleStatus.READY);
+    assertThat(activeRuleIndex.findByProfile(QProfileTesting.XOO_P1_KEY)).hasSize(0);
   }
 
   @Test
-  public void do_not_deactivate_removed_rules_if_repository_accidentaly_uninstalled() throws Exception {
+  public void do_not_deactivate_removed_rules_if_repository_accidentally_uninstalled() throws Exception {
     Rules rules = new Rules() {
       @Override
       public void init(RulesDefinition.NewRepository repository) {
@@ -341,13 +346,57 @@ public class RegisterRulesMediumTest {
 
     // Restart without xoo
     register(null);
-    assertThat(db.ruleDao().getByKey(dbSession, RuleTesting.XOO_X1).getStatus()).isEqualTo(RuleStatus.REMOVED);
-    assertThat(db.activeRuleDao().findByProfileKey(dbSession, QProfileTesting.XOO_P1_KEY)).hasSize(1);
+    assertThat(ruleIndex.getByKey(RuleTesting.XOO_X1).status()).isEqualTo(RuleStatus.REMOVED);
+    assertThat(activeRuleIndex.findByProfile(QProfileTesting.XOO_P1_KEY)).hasSize(1);
 
     // Re-install
     register(rules);
-    assertThat(db.ruleDao().getByKey(dbSession, RuleTesting.XOO_X1).getStatus()).isEqualTo(RuleStatus.READY);
-    assertThat(db.activeRuleDao().findByProfileKey(dbSession, QProfileTesting.XOO_P1_KEY)).hasSize(1);
+    assertThat(ruleIndex.getByKey(RuleTesting.XOO_X1).status()).isEqualTo(RuleStatus.READY);
+    assertThat(activeRuleIndex.findByProfile(QProfileTesting.XOO_P1_KEY)).hasSize(1);
+  }
+
+  @Test
+  public void update_active_rules_on_param_changes() throws Exception {
+    register(new Rules() {
+      @Override
+      public void init(RulesDefinition.NewRepository repository) {
+        RulesDefinition.NewRule x1Rule = repository.createRule("x1").setName("x1 name").setHtmlDescription("x1 desc");
+        // has default value
+        x1Rule.createParam("min").setType(RuleParamType.INTEGER).setDefaultValue("5");
+        // no default value
+        x1Rule.createParam("format").setType(RuleParamType.STRING);
+      }
+    });
+
+    // Create profile and activate rule
+    MockUserSession.set().setGlobalPermissions(GlobalPermissions.QUALITY_PROFILE_ADMIN).setLogin("me");
+    db.qualityProfileDao().insert(dbSession, QProfileTesting.newXooP1());
+    dbSession.commit();
+    dbSession.clearCache();
+    RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1);
+    activation.setParameter("format", "txt");
+    TESTER.get(QProfileService.class).activate(QProfileTesting.XOO_P1_KEY, activation);
+
+    // Default value of "min" is changed, "format" is removed, "format2" is added, "max" is added with a default value
+    register(new Rules() {
+      @Override
+      public void init(RulesDefinition.NewRepository repository) {
+        RulesDefinition.NewRule x1Rule = repository.createRule("x1").setName("x1 name").setHtmlDescription("x1 desc");
+        x1Rule.createParam("min").setType(RuleParamType.INTEGER).setDefaultValue("6");
+        x1Rule.createParam("format2").setType(RuleParamType.STRING);
+        x1Rule.createParam("max").setType(RuleParamType.INTEGER).setDefaultValue("10");
+      }
+    });
+
+    ActiveRule activeRule = activeRuleIndex.getByKey(ActiveRuleKey.of(QProfileTesting.XOO_P1_KEY, RuleTesting.XOO_X1));
+    Map<String, String> params = activeRule.params();
+    assertThat(params).hasSize(2);
+
+    // do not change default value on existing active rules -> keep min=5
+    assertThat(params.get("min")).isEqualTo("5");
+
+    // new param with default value
+    assertThat(params.get("max")).isEqualTo("10");
   }
 
   @Test
@@ -358,14 +407,14 @@ public class RegisterRulesMediumTest {
         repository.createRule("x1").setName("x1 name").setHtmlDescription("x1 desc").setTags("tag1");
       }
     });
-    Rule rule = index.getByKey(RuleTesting.XOO_X1);
+    Rule rule = ruleIndex.getByKey(RuleTesting.XOO_X1);
     assertThat(rule.systemTags()).containsOnly("tag1");
     assertThat(rule.tags()).isEmpty();
 
     // User adds tag
     TESTER.get(RuleUpdater.class).update(RuleUpdate.createForPluginRule(RuleTesting.XOO_X1).setTags(newHashSet("tag2")), UserSession.get());
     dbSession.clearCache();
-    rule = index.getByKey(RuleTesting.XOO_X1);
+    rule = ruleIndex.getByKey(RuleTesting.XOO_X1);
     assertThat(rule.systemTags()).containsOnly("tag1");
     assertThat(rule.tags()).containsOnly("tag2");
 
@@ -376,7 +425,7 @@ public class RegisterRulesMediumTest {
         repository.createRule("x1").setName("x1 name").setHtmlDescription("x1 desc").setTags("tag1", "tag2");
       }
     });
-    rule = index.getByKey(RuleTesting.XOO_X1);
+    rule = ruleIndex.getByKey(RuleTesting.XOO_X1);
     assertThat(rule.systemTags()).containsOnly("tag1", "tag2");
     assertThat(rule.tags()).isEmpty();
   }
@@ -415,7 +464,7 @@ public class RegisterRulesMediumTest {
           .setDescription("format parameter");
       }
     });
-    Rule template = index.getByKey(RuleKey.of("xoo", "T1"));
+    Rule template = ruleIndex.getByKey(RuleKey.of("xoo", "T1"));
 
     // Create custom rule
     RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", template.key())
@@ -444,7 +493,7 @@ public class RegisterRulesMediumTest {
     });
 
     // Verify custom rule has been restore from the template
-    Rule customRule = index.getByKey(customRuleKey);
+    Rule customRule = ruleIndex.getByKey(customRuleKey);
     assertThat(customRule.language()).isEqualTo("xoo");
     assertThat(customRule.internalKey()).isEqualTo("new_internal");
     assertThat(customRule.severity()).isEqualTo(Severity.BLOCKER);
@@ -471,7 +520,7 @@ public class RegisterRulesMediumTest {
       }
     };
     register(rules);
-    Rule template = index.getByKey(RuleKey.of("xoo", "T1"));
+    Rule template = ruleIndex.getByKey(RuleKey.of("xoo", "T1"));
 
     // Create custom rule
     RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", template.key())
@@ -481,12 +530,12 @@ public class RegisterRulesMediumTest {
       .setStatus(RuleStatus.READY)
       .setParameters(ImmutableMap.of("format", "txt")));
 
-    Date updatedAt = index.getByKey(customRuleKey).updatedAt();
+    Date updatedAt = ruleIndex.getByKey(customRuleKey).updatedAt();
 
     register(rules);
 
     // Verify custom rule has been restore from the template
-    Rule customRuleReloaded = index.getByKey(customRuleKey);
+    Rule customRuleReloaded = ruleIndex.getByKey(customRuleKey);
     assertThat(customRuleReloaded.updatedAt()).isEqualTo(updatedAt);
   }
 
@@ -506,7 +555,7 @@ public class RegisterRulesMediumTest {
           .setDescription("format parameter");
       }
     });
-    Rule templateRule = index.getByKey(RuleKey.of("xoo", "T1"));
+    Rule templateRule = ruleIndex.getByKey(RuleKey.of("xoo", "T1"));
 
     // Create custom rule
     RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", templateRule.key())
@@ -524,7 +573,7 @@ public class RegisterRulesMediumTest {
           .setHtmlDescription("template1 desc")
           .setSeverity(Severity.MAJOR)
           .setTemplate(true)
-            // "format" removed, "format2" added
+          // "format" removed, "format2" added
           .createParam("format2")
           .setDefaultValue("csv")
           .setType(RuleParamType.STRING)
@@ -533,7 +582,7 @@ public class RegisterRulesMediumTest {
     });
 
     // Verify custom rule param has not been changed!
-    Rule customRuleReloaded = index.getByKey(customRuleKey);
+    Rule customRuleReloaded = ruleIndex.getByKey(customRuleKey);
     assertThat(customRuleReloaded.params().get(0).key()).isEqualTo("format");
   }
 
@@ -554,7 +603,7 @@ public class RegisterRulesMediumTest {
       }
     };
     register(rules);
-    Rule templateRule = index.getByKey(RuleKey.of("xoo", "T1"));
+    Rule templateRule = ruleIndex.getByKey(RuleKey.of("xoo", "T1"));
 
     // Create custom rule
     RuleKey customRuleKey = TESTER.get(RuleCreator.class).create(NewRule.createForCustomRule("CUSTOM_RULE", templateRule.key())
@@ -563,19 +612,19 @@ public class RegisterRulesMediumTest {
       .setSeverity(Severity.MAJOR)
       .setStatus(RuleStatus.READY)
       .setParameters(ImmutableMap.of("format", "txt")));
-    assertThat(index.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.READY);
+    assertThat(ruleIndex.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.READY);
 
     // Restart without template
     register(null);
 
     // Verify custom rule is removed
-    assertThat(index.getByKey(templateRule.key()).status()).isEqualTo(RuleStatus.REMOVED);
-    assertThat(index.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.REMOVED);
+    assertThat(ruleIndex.getByKey(templateRule.key()).status()).isEqualTo(RuleStatus.REMOVED);
+    assertThat(ruleIndex.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.REMOVED);
 
     // Re-install template
     register(rules);
-    assertThat(index.getByKey(templateRule.key()).status()).isEqualTo(RuleStatus.READY);
-    assertThat(index.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.READY);
+    assertThat(ruleIndex.getByKey(templateRule.key()).status()).isEqualTo(RuleStatus.READY);
+    assertThat(ruleIndex.getByKey(customRuleKey).status()).isEqualTo(RuleStatus.READY);
   }
 
   @Test
@@ -586,13 +635,13 @@ public class RegisterRulesMediumTest {
       .setHtmlDescription("Some description"));
     dbSession.commit();
     dbSession.clearCache();
-    assertThat(index.getByKey(manualRuleKey).status()).isEqualTo(RuleStatus.READY);
+    assertThat(ruleIndex.getByKey(manualRuleKey).status()).isEqualTo(RuleStatus.READY);
 
     // Restart
     register(null);
 
     // Verify manual rule is still ready
-    assertThat(index.getByKey(manualRuleKey).status()).isEqualTo(RuleStatus.READY);
+    assertThat(ruleIndex.getByKey(manualRuleKey).status()).isEqualTo(RuleStatus.READY);
   }
 
   interface Rules {