From f471072173fb8477c956a48db8c94da580bf1a48 Mon Sep 17 00:00:00 2001 From: simonbrandhof Date: Thu, 9 Dec 2010 13:13:07 +0000 Subject: [PATCH] SONAR-1729 Violations decorator should test if the measure exist before saving something --- .../core/sensors/ViolationsDecorator.java | 101 ++++++++++-------- .../core/sensors/ViolationsDecoratorTest.java | 44 +++++--- 2 files changed, 85 insertions(+), 60 deletions(-) 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 79bbf2da1ad..baf826fbe12 100644 --- 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 rules = HashMultiset.create(); - private Multiset priorities = HashMultiset.create(); - private Map ruleToLevel = Maps.newHashMap(); + private Multiset severities = HashMultiset.create(); + private Map 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 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 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 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 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 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 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 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 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 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 6670ae97e41..40e9383557e 100644 --- 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. emptyList()); + when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.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.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. emptyList()); + when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.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. emptyList()); - when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections. emptyList()); + when(context.getViolations()).thenReturn(Collections.emptyList()); + when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.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. emptyList()); - when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections. emptyList()); + when(context.getViolations()).thenReturn(Collections.emptyList()); + when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.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. emptyList()); + when(context.getChildrenMeasures((MeasuresFilter) anyObject())).thenReturn(Collections.emptyList()); decorator.decorate(resource, context); @@ -134,7 +152,7 @@ public class ViolationsDecoratorTest { } private List createViolations() { - List violations = new ArrayList(); + List 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)); -- 2.39.5