}
/**
- * 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
+ * Update severity and params
*/
private void applySeverityAndParamToChange(RuleActivation request, RuleActivationContext context, ActiveRuleChange change) {
RuleWrapper rule = context.getRule();
ActiveRuleWrapper activeRule = context.getActiveRule();
ActiveRuleWrapper parentActiveRule = context.getParentActiveRule();
+ // First apply severity
+ String severity;
if (request.isReset()) {
- // load severity and params from parent profile, else from default values
- change.setSeverity(firstNonNull(
+ // load severity from parent profile, else from default values
+ severity = firstNonNull(
parentActiveRule != null ? parentActiveRule.get().getSeverityString() : null,
- rule.get().getSeverityString()));
-
- for (RuleParamDto ruleParamDto : rule.getParams()) {
- String paramKey = ruleParamDto.getName();
- change.setParameter(paramKey, validateParam(ruleParamDto, firstNonNull(
- parentActiveRule != null ? parentActiveRule.getParamValue(paramKey) : null,
- rule.getParamDefaultValue(paramKey))));
- }
-
- } else if (activeRule != null) {
- // already activated -> load severity and parameters from request, else keep existing ones, else from parent,
- // else from default
- change.setSeverity(firstNonNull(
+ rule.get().getSeverityString());
+ } 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(), rule.get().getSeverityString());
+ } else {
+ // load severity from request, else keep existing one (if already activated), else from parent, else from default
+ severity = firstNonNull(
request.getSeverity(),
- activeRule.get().getSeverityString(),
+ activeRule == null ? null : activeRule.get().getSeverityString(),
parentActiveRule != null ? parentActiveRule.get().getSeverityString() : null,
- rule.get().getSeverityString()));
- for (RuleParamDto ruleParamDto : rule.getParams()) {
- String paramKey = ruleParamDto.getName();
+ rule.get().getSeverityString());
+ }
+ change.setSeverity(severity);
+
+ // Apply param values
+ for (RuleParamDto ruleParamDto : rule.getParams()) {
+ String paramKey = ruleParamDto.getName();
+ String paramValue;
+ if (request.isReset()) {
+ // load params from parent profile, else from default values
+ paramValue = firstNonNull(
+ parentActiveRule != null ? parentActiveRule.getParamValue(paramKey) : null,
+ rule.getParamDefaultValue(paramKey));
+ } else if (context.getRulesProfile().isBuiltIn()) {
+ // use the value defined in the profile definition, else the rule default value
+ paramValue = firstNonNull(
+ context.getRequestedParamValue(request, paramKey),
+ rule.getParamDefaultValue(paramKey));
+ } else {
String parentValue = parentActiveRule != null ? parentActiveRule.getParamValue(paramKey) : null;
- String paramValue = context.hasRequestedParamValue(request, paramKey) ?
- // If the request contains the parameter then we're using either value from request, or parent value, or default value
+ String activeRuleValue = activeRule == null ? null : activeRule.getParamValue(paramKey);
+ paramValue = context.hasRequestedParamValue(request, paramKey) ?
+ // If the request contains the parameter then we're using either value from request, or parent value, or default value
firstNonNull(
context.getRequestedParamValue(request, paramKey),
parentValue,
rule.getParamDefaultValue(paramKey))
// If the request doesn't contain the parameter, then we're using either value in DB, or parent value, or default value
: firstNonNull(
- activeRule.getParamValue(paramKey),
- parentValue,
- rule.getParamDefaultValue(paramKey));
- change.setParameter(paramKey, validateParam(ruleParamDto, paramValue));
+ activeRuleValue,
+ parentValue,
+ rule.getParamDefaultValue(paramKey));
}
- } else {
- // not activated -> load severity and parameters from request, else from parent, else from defaults
- change.setSeverity(firstNonNull(
- request.getSeverity(),
- parentActiveRule != null ? parentActiveRule.get().getSeverityString() : null,
- rule.get().getSeverityString()));
- for (RuleParamDto ruleParamDto : rule.getParams()) {
- String paramKey = ruleParamDto.getName();
- change.setParameter(paramKey, validateParam(ruleParamDto,
- firstNonNull(
- context.getRequestedParamValue(request, paramKey),
- parentActiveRule != null ? parentActiveRule.getParamValue(paramKey) : null,
- rule.getParamDefaultValue(paramKey))));
- }
+ change.setParameter(paramKey, validateParam(ruleParamDto, paramValue));
}
}
import java.util.List;
import java.util.Optional;
+import org.assertj.core.groups.Tuple;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.sonar.api.utils.internal.TestSystem2;
import org.sonar.db.DbTester;
import org.sonar.db.qualityprofile.ActiveRuleDto;
+import org.sonar.db.qualityprofile.ActiveRuleParamDto;
import org.sonar.db.qualityprofile.RulesProfileDto;
import org.sonar.db.rule.RuleDefinitionDto;
+import org.sonar.db.rule.RuleParamDto;
import org.sonar.server.qualityprofile.index.ActiveRuleIndexer;
import org.sonar.server.tester.UserSessionRule;
import org.sonar.server.util.IntegerTypeValidation;
import static java.util.Arrays.asList;
import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.groups.Tuple.tuple;
import static org.mockito.Mockito.mock;
import static org.sonar.api.rules.RulePriority.BLOCKER;
import static org.sonar.api.rules.RulePriority.CRITICAL;
import static org.sonar.api.rules.RulePriority.MAJOR;
+import static org.sonar.api.rules.RulePriority.MINOR;
import static org.sonar.db.qualityprofile.QualityProfileTesting.newRuleProfileDto;
public class BuiltInQProfileUpdateImplTest {
assertThatProfileIsMarkedAsUpdated(persistedProfile);
}
+ // SONAR-10473
+ @Test
+ public void activate_rule_on_built_in_profile_resets_severity_to_default_if_not_overridden() {
+ RuleDefinitionDto rule = db.rules().insert(r -> r.setSeverity(Severity.MAJOR).setLanguage("xoo"));
+
+ BuiltInQualityProfilesDefinition.Context context = new BuiltInQualityProfilesDefinition.Context();
+ NewBuiltInQualityProfile newQp = context.createBuiltInQualityProfile("Sonar way", "xoo");
+ newQp.activateRule(rule.getRepositoryKey(), rule.getRuleKey());
+ newQp.done();
+ BuiltInQProfile builtIn = builtInProfileRepository.create(context.profile("xoo", "Sonar way"), rule);
+ underTest.update(db.getSession(), builtIn, persistedProfile);
+
+ List<ActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile);
+ assertThatRuleIsNewlyActivated(activeRules, rule, MAJOR);
+
+ // emulate an upgrade of analyzer that changes the default severity of the rule
+ rule.setSeverity(Severity.MINOR);
+ db.rules().update(rule);
+
+ underTest.update(db.getSession(), builtIn, persistedProfile);
+ activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile);
+ assertThatRuleIsNewlyActivated(activeRules, rule, MINOR);
+ }
+
+ @Test
+ public void activate_rule_on_built_in_profile_resets_params_to_default_if_not_overridden() {
+ RuleDefinitionDto rule = db.rules().insert(r -> r.setLanguage("xoo"));
+ RuleParamDto ruleParam = db.rules().insertRuleParam(rule, p -> p.setName("min").setDefaultValue("10"));
+
+ BuiltInQualityProfilesDefinition.Context context = new BuiltInQualityProfilesDefinition.Context();
+ NewBuiltInQualityProfile newQp = context.createBuiltInQualityProfile("Sonar way", "xoo");
+ newQp.activateRule(rule.getRepositoryKey(), rule.getRuleKey());
+ newQp.done();
+ BuiltInQProfile builtIn = builtInProfileRepository.create(context.profile("xoo", "Sonar way"), rule);
+ underTest.update(db.getSession(), builtIn, persistedProfile);
+
+ List<ActiveRuleDto> activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile);
+ assertThat(activeRules).hasSize(1);
+ assertThatRuleHasParams(db, activeRules.get(0), tuple("min", "10"));
+
+ // 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.update(db.getSession(), builtIn, persistedProfile);
+ activeRules = db.getDbClient().activeRuleDao().selectByRuleProfile(db.getSession(), persistedProfile);
+ assertThat(activeRules).hasSize(1);
+ assertThatRuleHasParams(db, activeRules.get(0), tuple("min", "20"));
+ }
+
// @Test
// public void propagate_new_activation_to_profiles() {
// RuleDefinitionDto rule = createJavaRule();
// assertThatRuleIsNotPresent(grandchildProfile, rule);
// }
+ private static void assertThatRuleHasParams(DbTester db, ActiveRuleDto activeRule, Tuple... expectedParams) {
+ assertThat(db.getDbClient().activeRuleDao().selectParamsByActiveRuleId(db.getSession(), activeRule.getId()))
+ .extracting(ActiveRuleParamDto::getKey, ActiveRuleParamDto::getValue)
+ .containsExactlyInAnyOrder(expectedParams);
+ }
+
private static void assertThatRuleIsNewlyActivated(List<ActiveRuleDto> activeRules, RuleDefinitionDto rule, RulePriority severity) {
ActiveRuleDto activeRule = findRule(activeRules, rule).get();
projects / sonarqube.git / commitdiff