From b0d5f026d7b3430edd6f076a90fbde0b370e1605 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Sun, 29 Jun 2014 19:33:32 +0200 Subject: [PATCH] SONAR-5135 fix bulk activation and reset --- .../org/sonar/api/config/EmailSettings.java | 1 - .../qualityprofile/BulkChangeResult.java | 4 + .../server/qualityprofile/RuleActivation.java | 23 +++- .../server/qualityprofile/RuleActivator.java | 116 +++++++++++++----- .../qualityprofile/RuleActivatorContext.java | 80 +++++++----- .../ws/RuleActivationActions.java | 14 ++- .../RuleActivatorMediumTest.java | 112 +++++++++++++++-- .../ws/QProfilesWsMediumTest.java | 5 +- .../qualityprofile/ws/QProfilesWsTest.java | 2 +- 9 files changed, 275 insertions(+), 82 deletions(-) diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/config/EmailSettings.java b/sonar-plugin-api/src/main/java/org/sonar/api/config/EmailSettings.java index 39eb0e75701..5d955aa1ec9 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/config/EmailSettings.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/config/EmailSettings.java @@ -23,7 +23,6 @@ import com.google.common.base.Objects; import org.sonar.api.BatchComponent; import org.sonar.api.CoreProperties; import org.sonar.api.ServerComponent; -import org.sonar.api.config.Settings; /** * If batch extensions use this component, then batch must be executed with administrator rights (see properties sonar.login and sonar.password) diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java index 8eadf2c012c..b3f5f3684fd 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/BulkChangeResult.java @@ -54,4 +54,8 @@ public class BulkChangeResult { void addChanges(Collection c) { this.changes.addAll(c); } + + public List getChanges() { + return changes; + } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java index 2b69b6de659..b486a19c32c 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivation.java @@ -32,12 +32,22 @@ import java.util.Map; public class RuleActivation { private final RuleKey ruleKey; - private final Map parameters = Maps.newHashMap(); + private final Map parameters; private String severity = null; private boolean cascade = false; + private boolean reset = false; public RuleActivation(RuleKey ruleKey) { this.ruleKey = ruleKey; + this.parameters = Maps.newHashMap(); + } + + public RuleActivation(RuleActivation other) { + this.ruleKey = other.ruleKey; + this.parameters = Maps.newHashMap(other.parameters); + this.severity = other.severity; + this.reset = other.reset; + this.cascade = other.cascade; } /** @@ -50,7 +60,7 @@ public class RuleActivation { /** * For internal use */ - RuleActivation isCascade(boolean b) { + RuleActivation setCascade(boolean b) { this.cascade = b; return this; } @@ -101,4 +111,13 @@ public class RuleActivation { public Map getParameters() { return parameters; } + + public boolean isReset() { + return reset; + } + + public RuleActivation setReset(boolean b) { + this.reset = b; + return this; + } } diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java index aebd29c9341..6c53e8b856e 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivator.java @@ -39,14 +39,17 @@ import org.sonar.server.exceptions.BadRequestException; import org.sonar.server.qualityprofile.db.ActiveRuleDao; import org.sonar.server.rule.Rule; import org.sonar.server.rule.index.RuleIndex; +import org.sonar.server.rule.index.RuleNormalizer; import org.sonar.server.rule.index.RuleQuery; import org.sonar.server.search.IndexClient; import org.sonar.server.search.QueryOptions; import org.sonar.server.search.Result; import org.sonar.server.util.TypeValidations; +import javax.annotation.CheckForNull; import javax.annotation.Nullable; +import java.util.Arrays; import java.util.Iterator; import java.util.List; import java.util.Map; @@ -93,16 +96,18 @@ public class RuleActivator implements ServerComponent { boolean stopPropagation = false; if (context.activeRule() == null) { + if (activation.isReset()) { + // ignore reset when rule is not activated + return changes; + } // new activation change = ActiveRuleChange.createFor(ActiveRuleChange.Type.ACTIVATED, context.activeRuleKey()); - if (activation.isCascade() || context.isSameAsParent(activation)) { + applySeverityAndParamToChange(activation, context, change); + if (activation.isCascade() || context.isSameAsParent(change)) { change.setInheritance(ActiveRule.Inheritance.INHERITED); } - applySeverityAndParamToChange(activation, context, change); - } else { // already activated - if (activation.isCascade() && context.activeRule().doesOverride()) { // propagating to descendants, but child profile already overrides rule -> stop propagation return changes; @@ -111,14 +116,14 @@ public class RuleActivator implements ServerComponent { if (activation.isCascade() && context.activeRule().getInheritance() == null) { // activate on child, then on parent -> mark child as overriding parent change.setInheritance(ActiveRule.Inheritance.OVERRIDES); - change.setSeverity(context.activeRule().getSeverityString()); + change.setSeverity(context.currentSeverity()); change.setParameters(context.activeRuleParamsAsStringMap()); stopPropagation = true; } else { applySeverityAndParamToChange(activation, context, change); if (!activation.isCascade() && context.parentActiveRule() != null) { // override rule which is already declared on parents - change.setInheritance(context.isSameAsParent(activation) ? ActiveRule.Inheritance.INHERITED : ActiveRule.Inheritance.OVERRIDES); + change.setInheritance(context.isSameAsParent(change) ? ActiveRule.Inheritance.INHERITED : ActiveRule.Inheritance.OVERRIDES); } } if (context.isSame(change)) { @@ -155,18 +160,58 @@ public class RuleActivator implements ServerComponent { *

* On custom rules, it's always rule parameters that are used */ - private void applySeverityAndParamToChange(RuleActivation activation, RuleActivatorContext context, ActiveRuleChange change) { - change.setSeverity(StringUtils.defaultIfEmpty(activation.getSeverity(), context.defaultSeverity())); - for (RuleParamDto ruleParamDto : context.ruleParams()) { - String value = null; - if (context.rule().getTemplateId() == null) { - value = StringUtils.defaultIfEmpty( - activation.getParameters().get(ruleParamDto.getName()), - context.defaultParam(ruleParamDto.getName())); - verifyParam(ruleParamDto, value); + private void applySeverityAndParamToChange(RuleActivation request, RuleActivatorContext context, ActiveRuleChange change) { + if (request.isReset()) { + // load severity and params from parent profile, else from default values + change.setSeverity(firstNonEmpty( + context.parentSeverity(), context.defaultSeverity())); + for (RuleParamDto ruleParamDto : context.ruleParams()) { + String paramKey = ruleParamDto.getName(); + change.setParameter(paramKey, validateParam(ruleParamDto, firstNonEmpty( + 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(firstNonEmpty( + request.getSeverity(), + context.currentSeverity(), + context.parentSeverity(), + context.defaultSeverity())); + for (RuleParamDto ruleParamDto : context.ruleParams()) { + String paramKey = ruleParamDto.getName(); + change.setParameter(paramKey, validateParam(ruleParamDto, firstNonEmpty( + context.requestParamValue(request, paramKey), + 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(firstNonEmpty( + request.getSeverity(), + context.parentSeverity(), + context.defaultSeverity())); + for (RuleParamDto ruleParamDto : context.ruleParams()) { + String paramKey = ruleParamDto.getName(); + change.setParameter(paramKey, validateParam(ruleParamDto, firstNonEmpty( + context.requestParamValue(request, paramKey), + context.parentParamValue(paramKey), + context.defaultParamValue(paramKey)))); + } + } + } + + @CheckForNull + String firstNonEmpty(String... strings) { + for (String s : strings) { + if (StringUtils.isNotEmpty(s)) { + return s; } - change.setParameter(ruleParamDto.getName(), StringUtils.defaultIfEmpty(value, ruleParamDto.getDefaultValue())); } + return null; } private List cascadeActivation(DbSession session, RuleActivation activation, String profileKey) { @@ -175,10 +220,7 @@ public class RuleActivator implements ServerComponent { // get all inherited profiles List children = db.qualityProfileDao().findChildren(session, profileKey); for (QualityProfileDto child : children) { - RuleActivation childActivation = new RuleActivation(activation.getRuleKey()) - .isCascade(true) - .setParameters(activation.getParameters()) - .setSeverity(activation.getSeverity()); + RuleActivation childActivation = new RuleActivation(activation).setCascade(true); changes.addAll(activate(session, childActivation, child.getKey())); } return changes; @@ -299,7 +341,7 @@ public class RuleActivator implements ServerComponent { return changes; } if (!force && !isCascade && context.activeRule().getInheritance() != null) { - throw new IllegalStateException("Cannot deactivate inherited rule '" + key.ruleKey() + "'"); + throw new BadRequestException("Cannot deactivate inherited rule '" + key.ruleKey() + "'"); } change = ActiveRuleChange.createFor(ActiveRuleChange.Type.DEACTIVATED, key); changes.add(change); @@ -321,7 +363,8 @@ public class RuleActivator implements ServerComponent { return changes; } - private void verifyParam(RuleParamDto ruleParam, @Nullable String value) { + @CheckForNull + private String validateParam(RuleParamDto ruleParam, @Nullable String value) { if (value != null) { RuleParamType ruleParamType = RuleParamType.parse(ruleParam.getType()); if (ruleParamType.multiple()) { @@ -331,6 +374,7 @@ public class RuleActivator implements ServerComponent { typeValidations.validate(value, ruleParamType.type(), ruleParamType.values()); } } + return value; } BulkChangeResult bulkActivate(RuleQuery ruleQuery, String profileKey, @Nullable String severity) { @@ -338,7 +382,8 @@ public class RuleActivator implements ServerComponent { RuleIndex ruleIndex = index.get(RuleIndex.class); DbSession dbSession = db.openSession(false); try { - Result ruleSearchResult = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true)); + Result ruleSearchResult = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true) + .setFieldsToReturn(Arrays.asList(RuleNormalizer.RuleField.KEY.field()))); Iterator rules = ruleSearchResult.scroll(); while (rules.hasNext()) { Rule rule = rules.next(); @@ -347,7 +392,9 @@ public class RuleActivator implements ServerComponent { activation.setSeverity(severity); List changes = activate(dbSession, activation, profileKey); result.addChanges(changes); - result.incrementSucceeded(); + if (!changes.isEmpty()) { + result.incrementSucceeded(); + } } catch (BadRequestException e) { // other exceptions stop the bulk activation @@ -367,14 +414,23 @@ public class RuleActivator implements ServerComponent { try { RuleIndex ruleIndex = index.get(RuleIndex.class); BulkChangeResult result = new BulkChangeResult(); - Result ruleSearchResult = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true)); + Result ruleSearchResult = ruleIndex.search(ruleQuery, new QueryOptions().setScroll(true) + .setFieldsToReturn(Arrays.asList(RuleNormalizer.RuleField.KEY.field()))); Iterator rules = ruleSearchResult.scroll(); while (rules.hasNext()) { - Rule rule = rules.next(); - ActiveRuleKey key = ActiveRuleKey.of(profile, rule.key()); - List changes = deactivate(dbSession, key); - result.addChanges(changes); - result.incrementSucceeded(); + try { + Rule rule = rules.next(); + ActiveRuleKey key = ActiveRuleKey.of(profile, rule.key()); + List changes = deactivate(dbSession, key); + result.addChanges(changes); + if (!changes.isEmpty()) { + result.incrementSucceeded(); + } + } catch (BadRequestException e) { + // other exceptions stop the bulk activation + result.incrementFailed(); + result.getErrors().add(e.errors()); + } } dbSession.commit(); return result; diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java index 002551e16e5..1400427ab42 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/RuleActivatorContext.java @@ -111,6 +111,47 @@ class RuleActivatorContext { return this; } + @CheckForNull + String requestParamValue(RuleActivation request, String key) { + if (rule.getTemplateId() != null) { + return null; + } + return request.getParameters().get(key); + } + + @CheckForNull + String currentParamValue(String key) { + ActiveRuleParamDto param = activeRuleParams.get(key); + return param != null ? param.getValue() : null; + } + + @CheckForNull + String parentParamValue(String key) { + ActiveRuleParamDto param = parentActiveRuleParams.get(key); + return param != null ? param.getValue() : null; + } + + @CheckForNull + String defaultParamValue(String key) { + RuleParamDto param = ruleParams.get(key); + return param != null ? param.getDefaultValue() : null; + } + + @CheckForNull + String currentSeverity() { + return activeRule != null ? activeRule.getSeverityString() : null; + } + + @CheckForNull + String parentSeverity() { + return parentActiveRule != null ? parentActiveRule.getSeverityString() : null; + } + + @CheckForNull + String defaultSeverity() { + return rule.getSeverityString(); + } + @CheckForNull Map activeRuleParamsAsMap() { return activeRuleParams; @@ -124,14 +165,6 @@ class RuleActivatorContext { return params; } - Map parentActiveRuleParamsAsStringMap() { - Map params = Maps.newHashMap(); - for (Map.Entry param : parentActiveRuleParams.entrySet()) { - params.put(param.getKey(), param.getValue().getValue()); - } - return params; - } - RuleActivatorContext setActiveRuleParams(@Nullable Collection a) { activeRuleParams.clear(); if (a != null) { @@ -152,37 +185,22 @@ class RuleActivatorContext { return this; } - String defaultSeverity() { - return parentActiveRule != null ? parentActiveRule.getSeverityString() : rule.getSeverityString(); - } - - @CheckForNull - String defaultParam(String paramKey) { - ActiveRuleParamDto parentParam = parentActiveRuleParams.get(paramKey); - if (parentParam != null) { - return parentParam.getValue(); - } - RuleParamDto ruleParam = ruleParams.get(paramKey); - if (ruleParam == null) { - throw new BadRequestException(String.format("Rule parameter %s does not exist", paramKey)); - } - return ruleParam.getDefaultValue(); - } - /** * True if trying to override an inherited rule but with exactly the same values */ - boolean isSameAsParent(RuleActivation activation) { + boolean isSameAsParent(ActiveRuleChange change) { if (parentActiveRule == null) { return false; } - if (activation.useDefaults()) { - return true; + if (!StringUtils.equals(change.getSeverity(), parentActiveRule.getSeverityString())) { + return false; } - if (StringUtils.equals(activation.getSeverity(), parentActiveRule.getSeverityString())) { - return Maps.difference(activation.getParameters(), parentActiveRuleParamsAsStringMap()).areEqual(); + for (Map.Entry entry : change.getParameters().entrySet()) { + if (entry.getValue() != null && !entry.getValue().equals(parentParamValue(entry.getKey()))) { + return false; + } } - return false; + return true; } boolean isSame(ActiveRuleChange change) { diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java index 4845e716c09..b89ebbc5a3a 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ws/RuleActivationActions.java @@ -36,6 +36,7 @@ public class RuleActivationActions implements ServerComponent { public static final String PROFILE_KEY = "profile_key"; public static final String RULE_KEY = "rule_key"; public static final String SEVERITY = "severity"; + public static final String RESET = "reset"; public static final String PARAMS = "params"; public static final String ACTIVATE_ACTION = "activate_rule"; @@ -68,13 +69,17 @@ public class RuleActivationActions implements ServerComponent { defineActiveRuleKeyParameters(activate); activate.createParam(SEVERITY) - .setDescription("Severity. An empty value resets to the severity inherited from parent profile or to " + - "the default rule severity.") + .setDescription(String.format("Severity. Ignored if parameter %s is true.", RESET)) .setPossibleValues(Severity.ALL); activate.createParam(PARAMS) - .setDescription("Parameters as semi-colon list of =, for example 'params=key1=v1;key2=v2'. A parameter " + - "with empty value is reset to the value inherited from parent profile or to the parameter default value."); + .setDescription(String.format("Parameters as semi-colon list of =, for example " + + "'params=key1=v1;key2=v2'. Ignored if parameter %s is true.", RESET)); + + activate.createParam(RESET) + .setDescription("Reset severity and parameters of activated rule. Set the values defined on parent profile " + + "or from rule default values.") + .setBooleanPossibleValues(); } private void defineDeactivateAction(WebService.NewController controller) { @@ -112,6 +117,7 @@ public class RuleActivationActions implements ServerComponent { if (params != null) { activation.setParameters(KeyValueFormat.parse(params)); } + activation.setReset(request.paramAsBoolean(RESET) == Boolean.TRUE); service.activate(request.mandatoryParam(PROFILE_KEY), activation); } diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java index 8982bf9e1b8..3e5b5c5654b 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/RuleActivatorMediumTest.java @@ -20,6 +20,7 @@ package org.sonar.server.qualityprofile; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.Maps; import org.junit.After; import org.junit.Before; import org.junit.ClassRule; @@ -201,6 +202,23 @@ public class RuleActivatorMediumTest { assertThat(changes).isEmpty(); } + @Test + public void do_not_change_severity_and_params_if_unset_and_already_activated() throws Exception { + // initial activation + ActiveRuleKey activeRuleKey = ActiveRuleKey.of(XOO_P1_KEY, RuleTesting.XOO_X1); + RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1); + activation.setSeverity(Severity.BLOCKER); + activation.setParameter("max", "7"); + activate(activation, XOO_P1_KEY); + + // update without any severity or params => keep + RuleActivation update = new RuleActivation(RuleTesting.XOO_X1); + activate(update, XOO_P1_KEY); + + assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1); + verifyHasActiveRule(activeRuleKey, Severity.BLOCKER, null, ImmutableMap.of("max", "7")); + } + @Test public void revert_activation_to_default_severity_and_parameters() throws Exception { // initial activation @@ -211,7 +229,7 @@ public class RuleActivatorMediumTest { activate(activation, XOO_P1_KEY); // update without any severity or params = reset - RuleActivation update = new RuleActivation(RuleTesting.XOO_X1); + RuleActivation update = new RuleActivation(RuleTesting.XOO_X1).setReset(true); activate(update, XOO_P1_KEY); assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(1); @@ -487,7 +505,7 @@ public class RuleActivatorMediumTest { public void do_not_propagate_activation_update_on_child_overrides() throws Exception { createChildProfiles(); - // activate on root profile + // activate on root profile P1 RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1); activation.setSeverity(Severity.INFO); activation.setParameter("max", "7"); @@ -496,7 +514,7 @@ public class RuleActivatorMediumTest { verifyOneActiveRule(XOO_P2_KEY, RuleTesting.XOO_X1, Severity.INFO, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); verifyOneActiveRule(XOO_P3_KEY, RuleTesting.XOO_X1, Severity.INFO, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); - // override on child + // override on child P2 activation = new RuleActivation(RuleTesting.XOO_X1); activation.setSeverity(Severity.BLOCKER); activation.setParameter("max", "8"); @@ -515,7 +533,7 @@ public class RuleActivatorMediumTest { verifyOneActiveRule(XOO_P3_KEY, RuleTesting.XOO_X1, Severity.BLOCKER, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "8")); // reset on parent (use default severity and params) -> do not propagate on children because they're overriding values - activation = new RuleActivation(RuleTesting.XOO_X1); + activation = new RuleActivation(RuleTesting.XOO_X1).setReset(true); activate(activation, XOO_P1_KEY); verifyOneActiveRule(XOO_P1_KEY, RuleTesting.XOO_X1, Severity.MINOR, null, ImmutableMap.of("max", "10")); @@ -558,7 +576,7 @@ public class RuleActivatorMediumTest { verifyOneActiveRule(XOO_P1_KEY, RuleTesting.XOO_X1, Severity.INFO, null, ImmutableMap.of("max", "7")); verifyOneActiveRule(XOO_P2_KEY, RuleTesting.XOO_X1, Severity.INFO, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); - // override on child with same severity and params -> do nothing (still INHERITED but not OVERRIDDEN) + // override on child P2 with same severity and params -> do nothing (still INHERITED but not OVERRIDDEN) activation = new RuleActivation(RuleTesting.XOO_X1); activation.setSeverity(Severity.INFO); activation.setParameter("max", "7"); @@ -635,13 +653,13 @@ public class RuleActivatorMediumTest { try { ruleActivator.deactivate(ActiveRuleKey.of(XOO_P2_KEY, RuleTesting.XOO_X1)); fail(); - } catch (IllegalStateException e) { + } catch (BadRequestException e) { assertThat(e).hasMessage("Cannot deactivate inherited rule 'xoo:x1'"); } } @Test - public void reset_activation_on_child_profile() throws Exception { + public void reset_child_profile() throws Exception { createChildProfiles(); // activate on root profile @@ -663,7 +681,7 @@ public class RuleActivatorMediumTest { verifyOneActiveRule(XOO_P3_KEY, RuleTesting.XOO_X1, Severity.INFO, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "10")); // reset -> remove overridden values - activation = new RuleActivation(RuleTesting.XOO_X1); + activation = new RuleActivation(RuleTesting.XOO_X1).setReset(true); activate(activation, XOO_P2_KEY); verifyOneActiveRule(XOO_P1_KEY, RuleTesting.XOO_X1, Severity.BLOCKER, null, ImmutableMap.of("max", "7")); verifyOneActiveRule(XOO_P2_KEY, RuleTesting.XOO_X1, Severity.BLOCKER, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); @@ -671,7 +689,7 @@ public class RuleActivatorMediumTest { } @Test - public void activation_reset_does_not_propagate_to_child_overrides() throws Exception { + public void reset_is_not_propagated_to_child_overrides() throws Exception { createChildProfiles(); // activate on root profile @@ -701,14 +719,24 @@ public class RuleActivatorMediumTest { verifyOneActiveRule(XOO_P2_KEY, RuleTesting.XOO_X1, Severity.INFO, ActiveRuleDto.OVERRIDES, ImmutableMap.of("max", "10")); verifyOneActiveRule(XOO_P3_KEY, RuleTesting.XOO_X1, Severity.MINOR, ActiveRuleDto.OVERRIDES, ImmutableMap.of("max", "20")); - // reset child -> keep the overridden grand-child - activation = new RuleActivation(RuleTesting.XOO_X1); + // reset child P2 -> keep the overridden grand-child P3 + activation = new RuleActivation(RuleTesting.XOO_X1).setReset(true); activate(activation, XOO_P2_KEY); verifyOneActiveRule(XOO_P1_KEY, RuleTesting.XOO_X1, Severity.BLOCKER, null, ImmutableMap.of("max", "7")); verifyOneActiveRule(XOO_P2_KEY, RuleTesting.XOO_X1, Severity.BLOCKER, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); verifyOneActiveRule(XOO_P3_KEY, RuleTesting.XOO_X1, Severity.MINOR, ActiveRuleDto.OVERRIDES, ImmutableMap.of("max", "20")); } + @Test + public void ignore_reset_if_not_activated() throws Exception { + createChildProfiles(); + RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1).setReset(true); + activate(activation, XOO_P1_KEY); + + verifyZeroActiveRules(XOO_P1_KEY); + verifyZeroActiveRules(XOO_P2_KEY); + } + @Test public void bulk_activation() { // Generate more rules than the search's max limit @@ -780,6 +808,13 @@ public class RuleActivatorMediumTest { verifyHasActiveRule(ActiveRuleKey.of(XOO_P2_KEY, RuleTesting.XOO_X2), Severity.MAJOR, null, Collections.emptyMap()); } + @Test + public void unset_no_parent_does_not_fail() { + // P1 has no parent ! + ruleActivator.setParent(XOO_P1_KEY, null); + assertThat(db.qualityProfileDao().getByKey(dbSession, XOO_P1_KEY).getParentKee()).isNull(); + } + @Test public void fail_if_set_child_as_parent() { createChildProfiles(); @@ -849,6 +884,61 @@ public class RuleActivatorMediumTest { verifyHasActiveRule(ActiveRuleKey.of(XOO_P2_KEY, RuleTesting.XOO_X2), Severity.MAJOR, ActiveRuleDto.INHERITED, Collections.emptyMap()); } + @Test + public void bulk_deactivate() { + activate(new RuleActivation(RuleTesting.XOO_X1), XOO_P1_KEY); + activate(new RuleActivation(RuleTesting.XOO_X2), XOO_P1_KEY); + assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(2); + + BulkChangeResult result = ruleActivator.bulkDeactivate(new RuleQuery().setActivation(true).setQProfileKey(XOO_P1_KEY), XOO_P1_KEY); + + dbSession.clearCache(); + assertThat(countActiveRules(XOO_P1_KEY)).isEqualTo(0); + assertThat(result.countFailed()).isEqualTo(0); + assertThat(result.countSucceeded()).isEqualTo(2); + assertThat(result.getChanges()).hasSize(2); + } + + @Test + public void bulk_deactivation_ignores_errors() { + // activate on parent profile P1 + createChildProfiles(); + activate(new RuleActivation(RuleTesting.XOO_X1), XOO_P1_KEY); + assertThat(countActiveRules(XOO_P2_KEY)).isEqualTo(1); + + // bulk deactivate on child profile P2 -> not possible + BulkChangeResult result = ruleActivator.bulkDeactivate(new RuleQuery().setActivation(true).setQProfileKey(XOO_P2_KEY), XOO_P2_KEY); + + dbSession.clearCache(); + assertThat(countActiveRules(XOO_P2_KEY)).isEqualTo(1); + assertThat(result.countFailed()).isEqualTo(1); + assertThat(result.countSucceeded()).isEqualTo(0); + assertThat(result.getChanges()).hasSize(0); + } + + @Test + public void bulk_change_severity() throws Exception { + createChildProfiles(); + + // activate two rules on root profile P1 (propagated to P2 and P3) + RuleActivation activation = new RuleActivation(RuleTesting.XOO_X1).setSeverity(Severity.INFO).setParameter("max", "7"); + activate(activation, XOO_P1_KEY); + activation = new RuleActivation(RuleTesting.XOO_X2).setSeverity(Severity.INFO); + activate(activation, XOO_P1_KEY); + + // bulk change severity to BLOCKER. Parameters are not set. + RuleQuery query = new RuleQuery().setActivation(true).setQProfileKey(XOO_P1_KEY); + BulkChangeResult result = ruleActivator.bulkActivate(query, XOO_P1_KEY, "BLOCKER"); + assertThat(result.countSucceeded()).isEqualTo(2); + + verifyHasActiveRule(XOO_P1_KEY, RuleTesting.XOO_X1, Severity.BLOCKER, null, ImmutableMap.of("max", "7")); + verifyHasActiveRule(XOO_P1_KEY, RuleTesting.XOO_X2, Severity.BLOCKER, null, Collections.emptyMap()); + verifyHasActiveRule(XOO_P2_KEY, RuleTesting.XOO_X1, Severity.BLOCKER, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); + verifyHasActiveRule(XOO_P2_KEY, RuleTesting.XOO_X2, Severity.BLOCKER, ActiveRuleDto.INHERITED, Collections.emptyMap()); + verifyHasActiveRule(XOO_P3_KEY, RuleTesting.XOO_X1, Severity.BLOCKER, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); + verifyHasActiveRule(XOO_P3_KEY, RuleTesting.XOO_X2, Severity.BLOCKER, ActiveRuleDto.INHERITED, Collections.emptyMap()); + } + private int countActiveRules(String profileKey) { List activeRuleDtos = db.activeRuleDao().findByProfileKey(dbSession, profileKey); List activeRules = index.findByProfile(profileKey); diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java index 48982a2d9c6..b3ca675297d 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsMediumTest.java @@ -387,8 +387,9 @@ public class QProfilesWsMediumTest { // 1. reset child rule WsTester.TestRequest request = wsTester.newGetRequest(QProfilesWs.API_ENDPOINT, RuleActivationActions.ACTIVATE_ACTION); - request.setParam(RuleActivationActions.PROFILE_KEY, subProfile.getKey()); - request.setParam(RuleActivationActions.RULE_KEY, rule.getKey().toString()); + request.setParam("profile_key", subProfile.getKey()); + request.setParam("rule_key", rule.getKey().toString()); + request.setParam("reset", "true"); request.execute(); session.clearCache(); diff --git a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java index e54509a3de7..544ec58392d 100644 --- a/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java +++ b/sonar-server/src/test/java/org/sonar/server/qualityprofile/ws/QProfilesWsTest.java @@ -68,7 +68,7 @@ public class QProfilesWsTest { WebService.Action restoreProfiles = controller.action(RuleActivationActions.ACTIVATE_ACTION); assertThat(restoreProfiles).isNotNull(); assertThat(restoreProfiles.isPost()).isTrue(); - assertThat(restoreProfiles.params()).hasSize(4); + assertThat(restoreProfiles.params()).hasSize(5); } @Test -- 2.39.5