SONAR-10473 Keep severity up-to-date on built-in QP

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java
index 3df832ffd8ef6de5d7603fab33dd489815f69c97..19bd2d00664773529f2ead609206f7c99c906ecd 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java
@@ -169,62 +169,49 @@ public class RuleActivator {
   }
 
   /**
-   * Severity and parameter values are :
-   * 1. defined by end-user
-   * 2. else inherited from parent profile
-   * 3. else defined by rule defaults
-   * <p/>
-   * On custom rules, it's always rule parameters that are used
+   * Apply severity and parameter values
    */
   private void applySeverityAndParamToChange(RuleActivation request, RuleActivatorContext context, ActiveRuleChange change) {
+    // First apply severity
+    String severity;
     if (request.isReset()) {
-      // load severity and params from parent profile, else from default values
-      change.setSeverity(firstNonNull(
-        context.parentSeverity(), context.defaultSeverity()));
-      for (RuleParamDto ruleParamDto : context.ruleParams()) {
-        String paramKey = ruleParamDto.getName();
-        change.setParameter(paramKey, validateParam(ruleParamDto, firstNonNull(
-          context.parentParamValue(paramKey), context.defaultParamValue(paramKey))));
-      }
-
-    } else if (context.activeRule() != null) {
-      // already activated -> load severity and parameters from request, else keep existing ones, else from parent,
-      // else from default
-      change.setSeverity(firstNonNull(
-        request.getSeverity(),
-        context.currentSeverity(),
-        context.parentSeverity(),
-        context.defaultSeverity()));
-      for (RuleParamDto ruleParamDto : context.ruleParams()) {
-        String paramKey = ruleParamDto.getName();
-        String paramValue = context.hasRequestParamValue(request, paramKey) ?
-        // If the request contains the parameter then we're using either value from request, or parent value, or default value
+      // load severity from parent profile, else from default values
+      severity = firstNonNull(context.parentSeverity(), context.defaultSeverity());
+    } else if (context.getRulesProfile().isBuiltIn()) {
+      // for builtin quality profiles, the severity from profile, when null use the default severity of the rule
+      severity = firstNonNull(request.getSeverity(), context.defaultSeverity());
+    } else {
+      // load severity from request, else keep existing one (if already activated), else from parent, else from default
+      severity = firstNonNull(request.getSeverity(), context.currentSeverity(), context.parentSeverity(), context.defaultSeverity());
+    }
+    change.setSeverity(severity);
+
+    // Apply param values
+    for (RuleParamDto ruleParamDto : context.ruleParams()) {
+      String paramKey = ruleParamDto.getName();
+      String paramValue;
+      if (request.isReset()) {
+        // load params from parent profile, else from default values
+        paramValue = firstNonNull(context.parentParamValue(paramKey), context.defaultParamValue(paramKey));
+      } else if (context.getRulesProfile().isBuiltIn()) {
+        // use the value defined in the profile definition, else the rule default value
+        paramValue = firstNonNull(context.requestParamValue(request, paramKey), context.defaultParamValue(paramKey));
+      } else {
+        paramValue = context.hasRequestParamValue(request, paramKey) ?
+          // If the request contains the parameter then we're using either value from request, or parent value, or default value
           firstNonNull(
             context.requestParamValue(request, paramKey),
             context.parentParamValue(paramKey),
             context.defaultParamValue(paramKey))
+
           // If the request doesn't contain the parameter, then we're using either value in DB, or parent value, or default value
           : firstNonNull(
-            context.currentParamValue(paramKey),
-            context.parentParamValue(paramKey),
-            context.defaultParamValue(paramKey));
-        change.setParameter(paramKey, validateParam(ruleParamDto, paramValue));
+          context.currentParamValue(paramKey),
+          context.parentParamValue(paramKey),
+          context.defaultParamValue(paramKey));
       }
 
-    } else if (context.activeRule() == null) {
-      // not activated -> load severity and parameters from request, else from parent, else from defaults
-      change.setSeverity(firstNonNull(
-        request.getSeverity(),
-        context.parentSeverity(),
-        context.defaultSeverity()));
-      for (RuleParamDto ruleParamDto : context.ruleParams()) {
-        String paramKey = ruleParamDto.getName();
-        change.setParameter(paramKey, validateParam(ruleParamDto,
-          firstNonNull(
-            context.requestParamValue(request, paramKey),
-            context.parentParamValue(paramKey),
-            context.defaultParamValue(paramKey))));
-      }
+      change.setParameter(paramKey, validateParam(ruleParamDto, paramValue));
     }
   }
 

diff --git a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
index 9138d494b7fe7c1c93e897fac330e048cbe91864..88d3837123f4bb3788e799dc249002db632159d3 100644 (file)
--- a/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
+++ b/server/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java
@@ -95,8 +95,6 @@ class RuleActivatorContext {
     return this;
   }
 
-
-
   Date getInitDate() {
     return initDate;
   }

diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java
index ef48fb24dcf3dfc13247eb69608dbeee24362a8d..dc6b405548d881cd363b6a1bc7ef4fce6034c459 100644 (file)
--- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java
+++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorTest.java
@@ -983,6 +983,42 @@ public class RuleActivatorTest {
     assertThatRuleIsActivated(grandchildProfile, rule, changes, rule.getSeverityString(), INHERITED, emptyMap());
   }
 
+  // SONAR-10473
+  @Test
+  public void activateOnBuiltInRulesProfile_resets_severity_to_default_if_not_overridden() {
+    RuleDefinitionDto rule = db.rules().insert(r -> r.setSeverity(Severity.MAJOR).setLanguage("java"));
+    QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(),
+      p -> p.setLanguage(rule.getLanguage())
+        .setIsBuiltIn(true));
+    List<ActiveRuleChange> changes = underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile));
+    assertThatRuleIsActivated(profile, rule, changes, "MAJOR", null, emptyMap());
+
+    // emulate an upgrade of analyzer that changes the default severity of the rule
+    rule.setSeverity(Severity.MINOR);
+    db.rules().update(rule);
+
+    underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile));
+    assertThatRuleIsUpdated(profile, rule, "MINOR", null, emptyMap());
+  }
+
+  @Test
+  public void activateOnBuiltInRulesProfile_resets_params_to_default_if_not_overridden() {
+    RuleDefinitionDto rule = db.rules().insert(r -> r.setSeverity(Severity.MAJOR).setLanguage("java"));
+    RuleParamDto ruleParam = db.rules().insertRuleParam(rule, p -> p.setName("min").setDefaultValue("10"));
+    QProfileDto profile = db.qualityProfiles().insert(db.getDefaultOrganization(),
+      p -> p.setLanguage(rule.getLanguage())
+        .setIsBuiltIn(true));
+    List<ActiveRuleChange> changes = underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile));
+    assertThatRuleIsActivated(profile, rule, changes, "MAJOR", null, of("min", "20"));
+
+    // emulate an upgrade of analyzer that changes the default value of parameter min
+    ruleParam.setDefaultValue("20");
+    db.getDbClient().ruleDao().updateRuleParam(db.getSession(), rule, ruleParam);
+
+    underTest.activateOnBuiltInRulesProfile(db.getSession(), RuleActivation.create(rule.getKey()), RulesProfileDto.from(profile));
+    assertThatRuleIsUpdated(profile, rule, "MAJOR", null, of("min", "20"));
+  }
+
   @Test
   public void deactivateOnBuiltInProfile_throws_IAE_when_profile_is_not_built_in() {
     RuleDefinitionDto rule = createJavaRule();