SONAR-1729 Violations decorator should test if the measure exist before saving something

diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ViolationsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ViolationsDecorator.java
index 79bbf2da1adcb2c08bbc5b257e4d76c351017946..baf826fbe12f6c8e192da33dd6bae0ad14159f1b 100644 (file)
--- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ViolationsDecorator.java
+++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ViolationsDecorator.java
@@ -31,15 +31,18 @@ import org.sonar.api.rules.Rule;
 import org.sonar.api.rules.RulePriority;
 import org.sonar.api.rules.Violation;
 
-import java.util.*;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Map;
 
 @DependsUpon(value = DecoratorBarriers.END_OF_VIOLATIONS_GENERATION)
 public class ViolationsDecorator implements Decorator {
 
   // temporary data for current resource
   private Multiset<Rule> rules = HashMultiset.create();
-  private Multiset<RulePriority> priorities = HashMultiset.create();
-  private Map<Rule, RulePriority> ruleToLevel = Maps.newHashMap();
+  private Multiset<RulePriority> severities = HashMultiset.create();
+  private Map<Rule, RulePriority> ruleToSeverity = Maps.newHashMap();
   private int total = 0;
 
   public boolean shouldExecuteOnProject(Project project) {
@@ -53,85 +56,89 @@ public class ViolationsDecorator implements Decorator {
   }
 
   public void decorate(Resource resource, DecoratorContext context) {
-    if (shouldDecorateResource(resource, context)) {
+    if (shouldDecorateResource(resource)) {
+      resetCounters();
       countViolations(context);
+      saveTotalViolations(context);
+      saveViolationsBySeverity(context);
+      saveViolationsByRule(context);
     }
   }
 
-  private boolean shouldDecorateResource(Resource resource, DecoratorContext context) {
-    return !ResourceUtils.isUnitTestClass(resource) && context.getMeasure(CoreMetrics.VIOLATIONS) == null;
+  private boolean shouldDecorateResource(Resource resource) {
+    return !ResourceUtils.isUnitTestClass(resource);
   }
 
   private void resetCounters() {
     rules.clear();
-    priorities.clear();
-    ruleToLevel.clear();
+    severities.clear();
+    ruleToSeverity.clear();
     total = 0;
   }
 
-  private void countViolations(DecoratorContext context) {
-    resetCounters();
-    countCurrentResourceViolations(context);
-    saveTotalViolations(context);
-    saveViolationsByPriority(context);
-    saveViolationsByRule(context);
-  }
-
-  private void saveViolationsByPriority(DecoratorContext context) {
-    for (RulePriority priority : RulePriority.values()) {
-      Metric metric = getMetricForPriority(priority);
-      Collection<Measure> children = context.getChildrenMeasures(MeasuresFilters.metric(metric));
-      double sum = MeasureUtils.sum(true, children) + priorities.count(priority);
-      context.saveMeasure(new Measure(metric, sum));
+  private void saveViolationsBySeverity(DecoratorContext context) {
+    for (RulePriority severity : RulePriority.values()) {
+      Metric metric = getMetricForSeverity(severity);
+      if (context.getMeasure(metric) == null) {
+        Collection<Measure> children = context.getChildrenMeasures(MeasuresFilters.metric(metric));
+        double sum = MeasureUtils.sum(true, children) + severities.count(severity);
+        context.saveMeasure(new Measure(metric, sum));
+      }
     }
   }
 
-  private Metric getMetricForPriority(RulePriority priority) {
+  private Metric getMetricForSeverity(RulePriority severity) {
     Metric metric = null;
-    if (priority.equals(RulePriority.BLOCKER)) {
+    if (severity.equals(RulePriority.BLOCKER)) {
       metric = CoreMetrics.BLOCKER_VIOLATIONS;
-    } else if (priority.equals(RulePriority.CRITICAL)) {
+    } else if (severity.equals(RulePriority.CRITICAL)) {
       metric = CoreMetrics.CRITICAL_VIOLATIONS;
-    } else if (priority.equals(RulePriority.MAJOR)) {
+    } else if (severity.equals(RulePriority.MAJOR)) {
       metric = CoreMetrics.MAJOR_VIOLATIONS;
-    } else if (priority.equals(RulePriority.MINOR)) {
+    } else if (severity.equals(RulePriority.MINOR)) {
       metric = CoreMetrics.MINOR_VIOLATIONS;
-    } else if (priority.equals(RulePriority.INFO)) {
+    } else if (severity.equals(RulePriority.INFO)) {
       metric = CoreMetrics.INFO_VIOLATIONS;
     }
     return metric;
   }
 
   private void saveViolationsByRule(DecoratorContext context) {
-    Collection<Measure> children = context.getChildrenMeasures(MeasuresFilters.rules(CoreMetrics.VIOLATIONS));
-    for (Measure childMeasure : children) {
-      RuleMeasure childRuleMeasure = (RuleMeasure) childMeasure;
-      Rule rule = childRuleMeasure.getRule();
-      if (rule != null && MeasureUtils.hasValue(childRuleMeasure)) {
-        rules.add(rule, childRuleMeasure.getValue().intValue());
-        ruleToLevel.put(childRuleMeasure.getRule(), childRuleMeasure.getRulePriority());
+    // See SONAR-1729
+    // Extrapolation : assume that the measure with key [metric "violations", rule] does not exist when the measure "violations" does not exist as well.
+    if (context.getMeasure(CoreMetrics.VIOLATIONS) == null) {
+      Collection<Measure> children = context.getChildrenMeasures(MeasuresFilters.rules(CoreMetrics.VIOLATIONS));
+      for (Measure childMeasure : children) {
+        RuleMeasure childRuleMeasure = (RuleMeasure) childMeasure;
+        Rule rule = childRuleMeasure.getRule();
+        if (rule != null && MeasureUtils.hasValue(childRuleMeasure)) {
+          rules.add(rule, childRuleMeasure.getValue().intValue());
+          ruleToSeverity.put(childRuleMeasure.getRule(), childRuleMeasure.getRulePriority());
+        }
+      }
+      for (Multiset.Entry<Rule> entry : rules.entrySet()) {
+        Rule rule = entry.getElement();
+        RuleMeasure measure = RuleMeasure.createForRule(CoreMetrics.VIOLATIONS, rule, (double) entry.getCount());
+        measure.setRulePriority(ruleToSeverity.get(rule));
+        context.saveMeasure(measure);
       }
-    }
-    for (Multiset.Entry<Rule> entry : rules.entrySet()) {
-      Rule rule = entry.getElement();
-      RuleMeasure measure = RuleMeasure.createForRule(CoreMetrics.VIOLATIONS, rule, (double) entry.getCount());
-      measure.setRulePriority(ruleToLevel.get(rule));
-      context.saveMeasure(measure);
     }
   }
 
   private void saveTotalViolations(DecoratorContext context) {
-    Collection<Measure> childrenViolations = context.getChildrenMeasures(CoreMetrics.VIOLATIONS);
-    Double sum = MeasureUtils.sum(true, childrenViolations) + total;
-    context.saveMeasure(new Measure(CoreMetrics.VIOLATIONS, sum));
+    if (context.getMeasure(CoreMetrics.VIOLATIONS) == null) {
+      Collection<Measure> childrenViolations = context.getChildrenMeasures(CoreMetrics.VIOLATIONS);
+      Double sum = MeasureUtils.sum(true, childrenViolations) + total;
+      context.saveMeasure(new Measure(CoreMetrics.VIOLATIONS, sum));
+    }
   }
 
-  private void countCurrentResourceViolations(DecoratorContext context) {
+  private void countViolations(DecoratorContext context) {
     List<Violation> violations = context.getViolations();
     for (Violation violation : violations) {
       rules.add(violation.getRule());
-      priorities.add(violation.getPriority());
-      ruleToLevel.put(violation.getRule(), violation.getPriority());
+      severities.add(violation.getSeverity());
+      ruleToSeverity.put(violation.getRule(), violation.getSeverity());
     }
     total = violations.size();
   }

diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ViolationsDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ViolationsDecoratorTest.java
index 6670ae97e41c144b422f15eeade3d6ba86c74503..40e9383557e6e5c8425ddb72db46ab1dd089f123 100644 (file)
--- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ViolationsDecoratorTest.java
+++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ViolationsDecoratorTest.java
@@ -19,6 +19,7 @@
  */
 package org.sonar.plugins.core.sensors;
 
+import com.google.common.collect.Lists;
 import org.junit.Before;
 import org.junit.Test;
 import org.sonar.api.batch.DecoratorContext;
@@ -32,7 +33,6 @@ import org.sonar.api.rules.Violation;
 import org.sonar.api.test.IsMeasure;
 import org.sonar.api.test.IsRuleMeasure;
 
-import java.util.ArrayList;
 import java.util.Collections;
 import java.util.List;
 
@@ -61,21 +61,39 @@ public class ViolationsDecoratorTest {
   }
 
   @Test
-  public void countViolations() throws Exception {
+  public void countViolations() {
     when(resource.getScope()).thenReturn(Resource.SCOPE_SET);
     when(context.getViolations()).thenReturn(createViolations());
-    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure> emptyList());
+    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure>emptyList());
 
     decorator.decorate(resource, context);
 
     verify(context).saveMeasure(argThat(new IsMeasure(CoreMetrics.VIOLATIONS, 4.0)));
   }
 
+  /**
+   * See http://jira.codehaus.org/browse/SONAR-1729
+   */
+  @Test
+  public void shouldNotCountViolationsIfMeasureAlreadyExists() {
+    when(resource.getScope()).thenReturn(Resource.SCOPE_SET);
+    when(context.getViolations()).thenReturn(createViolations());
+    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure>emptyList());
+    when(context.getMeasure(CoreMetrics.VIOLATIONS)).thenReturn(new Measure(CoreMetrics.VIOLATIONS, 3000.0));
+    when(context.getMeasure(CoreMetrics.MAJOR_VIOLATIONS)).thenReturn(new Measure(CoreMetrics.MAJOR_VIOLATIONS, 500.0));
+
+    decorator.decorate(resource, context);
+
+    verify(context, never()).saveMeasure(argThat(new IsMeasure(CoreMetrics.VIOLATIONS)));// not changed
+    verify(context, never()).saveMeasure(argThat(new IsMeasure(CoreMetrics.MAJOR_VIOLATIONS)));// not changed
+    verify(context, times(1)).saveMeasure(argThat(new IsMeasure(CoreMetrics.CRITICAL_VIOLATIONS)));// did not exist
+  }
+
   @Test
   public void resetCountersAfterExecution() {
     when(resource.getScope()).thenReturn(Resource.SCOPE_SET);
     when(context.getViolations()).thenReturn(createViolations());
-    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure> emptyList());
+    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure>emptyList());
 
     decorator.decorate(resource, context);
     decorator.decorate(resource, context);
@@ -86,10 +104,10 @@ public class ViolationsDecoratorTest {
   }
 
   @Test
-  public void saveZeroOnProjects() throws Exception {
+  public void saveZeroOnProjects() {
     when(resource.getScope()).thenReturn(Resource.SCOPE_SET);
-    when(context.getViolations()).thenReturn(Collections.<Violation> emptyList());
-    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure> emptyList());
+    when(context.getViolations()).thenReturn(Collections.<Violation>emptyList());
+    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure>emptyList());
 
     decorator.decorate(resource, context);
 
@@ -97,10 +115,10 @@ public class ViolationsDecoratorTest {
   }
 
   @Test
-  public void saveZeroOnDirectories() throws Exception {
+  public void saveZeroOnDirectories() {
     when(resource.getScope()).thenReturn(Resource.SCOPE_SPACE);
-    when(context.getViolations()).thenReturn(Collections.<Violation> emptyList());
-    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure> emptyList());
+    when(context.getViolations()).thenReturn(Collections.<Violation>emptyList());
+    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure>emptyList());
 
     decorator.decorate(resource, context);
 
@@ -108,10 +126,10 @@ public class ViolationsDecoratorTest {
   }
 
   @Test
-  public void priorityViolations() throws Exception {
+  public void shouldCountViolationsBySeverity() {
     when(resource.getScope()).thenReturn(Resource.SCOPE_SET);
     when(context.getViolations()).thenReturn(createViolations());
-    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure> emptyList());
+    when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.<Measure>emptyList());
 
     decorator.decorate(resource, context);
 
@@ -134,7 +152,7 @@ public class ViolationsDecoratorTest {
   }
 
   private List<Violation> createViolations() {
-    List<Violation> violations = new ArrayList<Violation>();
+    List<Violation> violations = Lists.newArrayList();
     violations.add(Violation.create(ruleA1, resource).setSeverity(RulePriority.CRITICAL));
     violations.add(Violation.create(ruleA1, resource).setSeverity(RulePriority.CRITICAL));
     violations.add(Violation.create(ruleA2, resource).setSeverity(RulePriority.MAJOR));