From f40c33478dc326f739726dc4f10a1a6aaaec3740 Mon Sep 17 00:00:00 2001 From: Eric Hartmann Date: Mon, 12 Mar 2018 19:00:59 +0100 Subject: [PATCH] SONAR-10473 Keep severity up-to-date on built-in QP --- .../server/qualityprofile/RuleActivator.java | 77 ++++++++----------- .../qualityprofile/RuleActivatorContext.java | 2 - .../qualityprofile/RuleActivatorTest.java | 36 +++++++++ 3 files changed, 68 insertions(+), 47 deletions(-) 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 3df832ffd8e..19bd2d00664 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 @@ -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 - *

- * 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 9138d494b7f..88d3837123f 100644 --- 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 ef48fb24dcf..dc6b405548d 100644 --- 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 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 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(); -- 2.39.5