From: Eric Hartmann Date: Mon, 12 Mar 2018 18:00:59 +0000 (+0100) Subject: SONAR-10473 Keep severity up-to-date on built-in QP X-Git-Tag: 7.5~1535 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=4df67277be75c9b41f5b434f03c2fbf6dbb8b426;p=sonarqube.git 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 1db7b05161c..3544ebbb7fe 100644 --- 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 @@ -158,70 +158,64 @@ 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 - *

- * 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)); } } diff --git a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java index 81c9a6717b3..9a414f0930a 100644 --- a/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java +++ b/server/sonar-server/src/test/java/org/sonar/server/qualityprofile/BuiltInQProfileUpdateImplTest.java @@ -21,6 +21,7 @@ package org.sonar.server.qualityprofile; 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; @@ -32,8 +33,10 @@ import org.sonar.api.utils.System2; 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; @@ -42,10 +45,12 @@ import org.sonar.server.util.TypeValidations; 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 { @@ -187,6 +192,56 @@ 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 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 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(); @@ -229,6 +284,12 @@ public class BuiltInQProfileUpdateImplTest { // 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 activeRules, RuleDefinitionDto rule, RulePriority severity) { ActiveRuleDto activeRule = findRule(activeRules, rule).get();