From: Simon Brandhof Date: Mon, 16 Jun 2014 21:31:28 +0000 (+0200) Subject: SONAR-5007 optimize changes of rule activation X-Git-Tag: 4.4-RC1~399 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=881a069d8d76cc23e6107dd7b9cb55ebad5b0f47;p=sonarqube.git SONAR-5007 optimize changes of rule activation --- diff --git a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java index 59650b8ecdd..f783170a5c2 100644 --- a/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java +++ b/sonar-server/src/main/java/org/sonar/server/qualityprofile/ActiveRuleChange.java @@ -19,6 +19,7 @@ */ package org.sonar.server.qualityprofile; +import com.google.common.base.Objects; import com.google.common.collect.Maps; import org.apache.commons.lang.StringUtils; import org.sonar.core.activity.ActivityLog; @@ -41,12 +42,9 @@ public class ActiveRuleChange implements ActivityLog { private ActiveRule.Inheritance inheritance = null; private Map parameters = Maps.newHashMap(); - private long start; - private ActiveRuleChange(Type type, ActiveRuleKey key) { this.type = type; this.key = key; - this.start = System.currentTimeMillis(); } public ActiveRuleKey getKey() { @@ -123,4 +121,15 @@ public class ActiveRuleChange implements ActivityLog { public static ActiveRuleChange createFor(Type type, ActiveRuleKey key) { return new ActiveRuleChange(type, key); } + + @Override + public String toString() { + return Objects.toStringHelper(this) + .add("type", type) + .add("key", key) + .add("severity", severity) + .add("inheritance", inheritance) + .add("parameters", parameters) + .toString(); + } } 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 e7c856962f5..e6c293cf88d 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 @@ -57,7 +57,7 @@ public class RuleActivation { /** * For internal use */ - boolean isReset() { + boolean useDefaults() { return severity == null && parameters.isEmpty(); } 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 a4fc4d8cfeb..318ed039670 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 @@ -124,15 +124,20 @@ public class RuleActivator implements ServerComponent { stopPropagation = true; } else { applySeverityAndParamToChange(activation, context, change); - if (!activation.isCascade() && context.parentProfile() != null) { + 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); } } + if (context.isSame(change)) { + change = null; + } } - changes.add(change); - persist(change, context, dbSession); + if (change != null) { + changes.add(change); + persist(change, context, dbSession); + } if (!stopPropagation) { changes.addAll(cascadeActivation(dbSession, activation)); @@ -339,7 +344,7 @@ public class RuleActivator implements ServerComponent { } catch (BadRequestException e) { // other exceptions stop the bulk activation result.incrementFailed(); - // TODO result.addMessage + result.getErrors().add(e.errors()); } } dbSession.commit(); 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 1ab2b1ef8c3..82d1d86827b 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 @@ -77,8 +77,8 @@ class RuleActivatorContext { } @CheckForNull - QualityProfileDto parentProfile() { - return parentProfile; + ActiveRuleDto parentActiveRule() { + return parentActiveRule; } RuleActivatorContext setParentProfile(@Nullable QualityProfileDto p) { @@ -171,7 +171,7 @@ class RuleActivatorContext { if (parentActiveRule == null) { return false; } - if (activation.isReset()) { + if (activation.useDefaults()) { return true; } if (StringUtils.equals(activation.getSeverity(), parentActiveRule.getSeverityString())) { @@ -180,6 +180,22 @@ class RuleActivatorContext { return false; } + boolean isSame(ActiveRuleChange change) { + if (change.getInheritance()!=null && !change.getInheritance().name().equals(activeRule.getInheritance())) { + return false; + } + if (change.getSeverity()!=null && !change.getSeverity().equals(activeRule.getSeverityString())) { + return false; + } + for (Map.Entry changeParam : change.getParameters().entrySet()) { + ActiveRuleParamDto param = activeRuleParams.get(changeParam.getKey()); + if (param != null && !StringUtils.equals(changeParam.getValue(), param.getValue())) { + return false; + } + } + return true; + } + void verifyForActivation() { if (RuleStatus.REMOVED == rule.getStatus()) { throw new BadRequestException("Rule was removed: " + rule.getKey()); 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 31c2ecb28da..15fc453695b 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 @@ -193,6 +193,22 @@ public class RuleActivatorMediumTest { verifyHasActiveRule(activeRuleKey, Severity.CRITICAL, null, ImmutableMap.of("max", "42")); } + @Test + public void ignore_activation_without_changes() throws Exception { + // initial activation + ActiveRuleKey activeRuleKey = ActiveRuleKey.of(XOO_PROFILE_KEY, XOO_RULE_1); + RuleActivation activation = new RuleActivation(activeRuleKey); + activation.setSeverity(Severity.BLOCKER); + ruleActivator.activate(activation); + dbSession.clearCache(); + + // update with exactly the same severity and params + RuleActivation update = new RuleActivation(ActiveRuleKey.of(XOO_PROFILE_KEY, XOO_RULE_1)); + update.setSeverity(Severity.BLOCKER); + List changes = ruleActivator.activate(update); + assertThat(changes).isEmpty(); + } + @Test public void revert_activation_to_default_severity_and_parameters() throws Exception { // initial activation @@ -396,7 +412,7 @@ public class RuleActivatorMediumTest { // INHERITANCE OF PROFILES @Test - public void activate_on_child_profile() throws Exception { + public void activate_on_child_profile_but_not_on_parent() throws Exception { createChildProfiles(); // activate on child profile, but not on root @@ -408,6 +424,15 @@ public class RuleActivatorMediumTest { verifyZeroActiveRules(XOO_PROFILE_KEY); verifyOneActiveRule(XOO_CHILD_PROFILE_KEY, XOO_RULE_1, Severity.BLOCKER, null, ImmutableMap.of("max", "7")); verifyOneActiveRule(XOO_GRAND_CHILD_PROFILE_KEY, XOO_RULE_1, Severity.BLOCKER, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); + + // update severity on child + activation = new RuleActivation(ActiveRuleKey.of(XOO_CHILD_PROFILE_KEY, XOO_RULE_1)); + activation.setSeverity(Severity.MINOR); + activation.setParameter("max", "77"); + ruleActivator.activate(activation); + verifyZeroActiveRules(XOO_PROFILE_KEY); + verifyOneActiveRule(XOO_CHILD_PROFILE_KEY, XOO_RULE_1, Severity.MINOR, null, ImmutableMap.of("max", "77")); + verifyOneActiveRule(XOO_GRAND_CHILD_PROFILE_KEY, XOO_RULE_1, Severity.MINOR, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "77")); } @Test @@ -418,8 +443,9 @@ public class RuleActivatorMediumTest { RuleActivation activation = new RuleActivation(ActiveRuleKey.of(XOO_PROFILE_KEY, XOO_RULE_1)); activation.setSeverity(Severity.BLOCKER); activation.setParameter("max", "7"); - ruleActivator.activate(activation); + List changes = ruleActivator.activate(activation); + assertThat(changes).hasSize(3); verifyOneActiveRule(XOO_PROFILE_KEY, XOO_RULE_1, Severity.BLOCKER, null, ImmutableMap.of("max", "7")); verifyOneActiveRule(XOO_CHILD_PROFILE_KEY, XOO_RULE_1, Severity.BLOCKER, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7")); verifyOneActiveRule(XOO_GRAND_CHILD_PROFILE_KEY, XOO_RULE_1, Severity.BLOCKER, ActiveRuleDto.INHERITED, ImmutableMap.of("max", "7"));