Refactoring: WeightedViolationsDecorator should use Settings API instead of deprecated Commons Configuration.

3 files changed, 87 insertions, 35 deletions

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
index 637bd74254c..8fa1ac51be7 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/CorePlugin.java
@@ -98,14 +98,6 @@ import java.util.List;
         global = false,
         category = CoreProperties.CATEGORY_GENERAL),
     @Property(
-        key = CoreProperties.CORE_RULE_WEIGHTS_PROPERTY,
-        defaultValue = CoreProperties.CORE_RULE_WEIGHTS_DEFAULT_VALUE,
-        name = "Rules weight",
-        description = "A weight is associated to each priority to calculate the Rules Compliance Index.",
-        project = false,
-        global = true,
-        category = CoreProperties.CATEGORY_GENERAL),
-    @Property(
         key = CoreProperties.CORE_FORCE_AUTHENTICATION_PROPERTY,
         defaultValue = "" + CoreProperties.CORE_FORCE_AUTHENTICATION_DEFAULT_VALUE,
         name = "Force user authentication",
diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/WeightedViolationsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/WeightedViolationsDecorator.java
index c6818f15912..45093c059a7 100644
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/WeightedViolationsDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/WeightedViolationsDecorator.java
@@ -21,10 +21,14 @@ package org.sonar.plugins.core.sensors;
 
 import com.google.common.collect.Multiset;
 import com.google.common.collect.TreeMultiset;
+import org.sonar.api.CoreProperties;
+import org.sonar.api.Properties;
+import org.sonar.api.Property;
 import org.sonar.api.batch.Decorator;
 import org.sonar.api.batch.DecoratorContext;
 import org.sonar.api.batch.DependedUpon;
 import org.sonar.api.batch.DependsUpon;
+import org.sonar.api.config.Settings;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.measures.Measure;
 import org.sonar.api.measures.MeasureUtils;
@@ -33,19 +37,35 @@ import org.sonar.api.resources.Project;
 import org.sonar.api.resources.Resource;
 import org.sonar.api.rules.RulePriority;
 import org.sonar.api.rules.RuleUtils;
-import org.sonar.api.rules.Violation;
 import org.sonar.api.utils.KeyValueFormat;
 
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
 
+@Properties(
+    @Property(
+        key = CoreProperties.CORE_RULE_WEIGHTS_PROPERTY,
+        defaultValue = CoreProperties.CORE_RULE_WEIGHTS_DEFAULT_VALUE,
+        name = "Rules weight",
+        description = "A weight is associated to each severity to calculate the Rules Compliance Index.",
+        project = false,
+        global = true,
+        category = CoreProperties.CATEGORY_GENERAL)
+)
 public class WeightedViolationsDecorator implements Decorator {
 
+  private Settings settings;
+  private Map<RulePriority,Integer> weightsBySeverity;
+
+  public WeightedViolationsDecorator(Settings settings) {
+    this.settings = settings;
+  }
+
   @DependsUpon
   public List<Metric> dependsUponViolations() {
     return Arrays.asList(CoreMetrics.BLOCKER_VIOLATIONS, CoreMetrics.CRITICAL_VIOLATIONS,
-      CoreMetrics.MAJOR_VIOLATIONS, CoreMetrics.MINOR_VIOLATIONS, CoreMetrics.INFO_VIOLATIONS);
+        CoreMetrics.MAJOR_VIOLATIONS, CoreMetrics.MINOR_VIOLATIONS, CoreMetrics.INFO_VIOLATIONS);
   }
 
   @DependedUpon
@@ -57,11 +77,33 @@ public class WeightedViolationsDecorator implements Decorator {
     return true;
   }
 
+  public void start() {
+    weightsBySeverity = getWeights(settings);
+  }
+
+  Map<RulePriority, Integer> getWeightsBySeverity() {
+    return weightsBySeverity;
+  }
+
+  static Map<RulePriority, Integer> getWeights(final Settings settings) {
+    String value = settings.getString(CoreProperties.CORE_RULE_WEIGHTS_PROPERTY);
+
+    Map<RulePriority, Integer> weights = KeyValueFormat.parse(value, KeyValueFormat.newPriorityConverter(), KeyValueFormat.newIntegerConverter());
+
+    for (RulePriority priority : RulePriority.values()) {
+      if (!weights.containsKey(priority)) {
+        weights.put(priority, 1);
+      }
+    }
+    return weights;
+  }
+
+
   public void decorate(Resource resource, DecoratorContext context) {
-    decorate(context, RuleUtils.getPriorityWeights(context.getProject().getConfiguration()));
+    decorate(context);
   }
 
-  void decorate(DecoratorContext context, Map<RulePriority, Integer> weights) {
+  void decorate(DecoratorContext context) {
     double debt = 0.0;
     Multiset<RulePriority> distribution = TreeMultiset.create();
 
@@ -69,7 +111,7 @@ public class WeightedViolationsDecorator implements Decorator {
       Measure measure = context.getMeasure(SeverityUtils.severityToViolationMetric(severity));
       if (measure != null && MeasureUtils.hasValue(measure)) {
         distribution.add(severity, measure.getIntValue());
-        double add = weights.get(severity) * measure.getIntValue();
+        double add = weightsBySeverity.get(severity) * measure.getIntValue();
         debt += add;
       }
     }
diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/WeightedViolationsDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/WeightedViolationsDecoratorTest.java
index 60048f084f7..ad77f3ab77d 100644
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/WeightedViolationsDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/WeightedViolationsDecoratorTest.java
@@ -19,44 +19,35 @@
  */
 package org.sonar.plugins.core.sensors;
 
-import com.google.common.collect.Maps;
+import org.hamcrest.core.Is;
 import org.junit.Test;
+import org.sonar.api.CoreProperties;
 import org.sonar.api.batch.DecoratorContext;
+import org.sonar.api.config.Settings;
 import org.sonar.api.measures.CoreMetrics;
 import org.sonar.api.measures.Measure;
-import org.sonar.api.measures.MeasuresFilter;
-import org.sonar.api.measures.Metric;
 import org.sonar.api.rules.RulePriority;
 import org.sonar.api.test.IsMeasure;
 
-import java.util.Arrays;
-import java.util.Collections;
 import java.util.Map;
 
+import static org.junit.Assert.assertThat;
 import static org.mockito.Mockito.*;
 
 public class WeightedViolationsDecoratorTest {
 
-  private Map<RulePriority, Integer> createWeights() {
-    Map<RulePriority, Integer> weights = Maps.newHashMap();
-    weights.put(RulePriority.BLOCKER, 10);
-    weights.put(RulePriority.CRITICAL, 5);
-    weights.put(RulePriority.MAJOR, 2);
-    weights.put(RulePriority.MINOR, 1);
-    weights.put(RulePriority.INFO, 0);
-    return weights;
-  }
-
   @Test
-  public void weightedViolations() {
-    Map<RulePriority, Integer> weights = createWeights();
-    WeightedViolationsDecorator decorator = new WeightedViolationsDecorator();
+  public void testWeightedViolations() {
+    Settings settings = new Settings();
+    settings.setProperty(CoreProperties.CORE_RULE_WEIGHTS_PROPERTY, "BLOCKER=10;CRITICAL=5;MAJOR=2;MINOR=1;INFO=0");
+    WeightedViolationsDecorator decorator = new WeightedViolationsDecorator(settings);
     DecoratorContext context = mock(DecoratorContext.class);
     when(context.getMeasure(CoreMetrics.INFO_VIOLATIONS)).thenReturn(new Measure(CoreMetrics.INFO_VIOLATIONS, 50.0));
     when(context.getMeasure(CoreMetrics.CRITICAL_VIOLATIONS)).thenReturn(new Measure(CoreMetrics.CRITICAL_VIOLATIONS, 80.0));
     when(context.getMeasure(CoreMetrics.BLOCKER_VIOLATIONS)).thenReturn(new Measure(CoreMetrics.BLOCKER_VIOLATIONS, 100.0));
 
-    decorator.decorate(context, weights);
+    decorator.start();
+    decorator.decorate(context);
 
     verify(context).saveMeasure(argThat(new IsMeasure(CoreMetrics.WEIGHTED_VIOLATIONS, (double) (100 * 10 + 80 * 5 + 50 * 0))));
     verify(context).saveMeasure(argThat(new IsMeasure(CoreMetrics.WEIGHTED_VIOLATIONS, "INFO=50;CRITICAL=80;BLOCKER=100")));
@@ -64,11 +55,38 @@ public class WeightedViolationsDecoratorTest {
 
   @Test
   public void doNotSaveZero() {
-    Map<RulePriority, Integer> weights = createWeights();
-    WeightedViolationsDecorator decorator = new WeightedViolationsDecorator();
+    Settings settings = new Settings();
+    settings.setProperty(CoreProperties.CORE_RULE_WEIGHTS_PROPERTY, "BLOCKER=10;CRITICAL=5;MAJOR=2;MINOR=1;INFO=0");
     DecoratorContext context = mock(DecoratorContext.class);
-    decorator.decorate(context, weights);
+
+    WeightedViolationsDecorator decorator = new WeightedViolationsDecorator(settings);
+    decorator.start();
+    decorator.decorate(context);
 
     verify(context, never()).saveMeasure((Measure) anyObject());
   }
+
+  @Test
+  public void shouldLoadSeverityWeightsAtStartup() {
+    Settings settings = new Settings();
+    settings.setProperty(CoreProperties.CORE_RULE_WEIGHTS_PROPERTY, "BLOCKER=2;CRITICAL=1;MAJOR=0;MINOR=0;INFO=0");
+    
+    WeightedViolationsDecorator decorator = new WeightedViolationsDecorator(settings);
+    decorator.start();
+
+    assertThat(decorator.getWeightsBySeverity().get(RulePriority.BLOCKER), Is.is(2));
+    assertThat(decorator.getWeightsBySeverity().get(RulePriority.CRITICAL), Is.is(1));
+    assertThat(decorator.getWeightsBySeverity().get(RulePriority.MAJOR), Is.is(0));
+  }
+
+  @Test
+  public void weightsSettingShouldBeOptional() {
+    Settings settings = new Settings();
+    settings.setProperty(CoreProperties.CORE_RULE_WEIGHTS_PROPERTY, "BLOCKER=2");
+
+    WeightedViolationsDecorator decorator = new WeightedViolationsDecorator(settings);
+    decorator.start();
+
+    assertThat(decorator.getWeightsBySeverity().get(RulePriority.MAJOR), Is.is(1));
+  }
 }
\ No newline at end of file