SONAR-2344 Improve the algorithm in charge to define/update the creation date of each violation

This commit is contained in:
Freddy Mallet 2011-04-22 22:53:26 +02:00
parent 4b39fc7320
commit db1f420129
2 changed files with 196 additions and 152 deletions

View File

@ -19,33 +19,38 @@
*/
package org.sonar.plugins.core.timemachine;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Map;
import org.apache.commons.codec.digest.DigestUtils;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang.ObjectUtils;
import org.apache.commons.lang.StringUtils;
import org.codehaus.plexus.util.StringInputStream;
import org.sonar.api.batch.*;
import org.sonar.api.batch.Decorator;
import org.sonar.api.batch.DecoratorBarriers;
import org.sonar.api.batch.DecoratorContext;
import org.sonar.api.batch.DependedUpon;
import org.sonar.api.batch.DependsUpon;
import org.sonar.api.database.model.RuleFailureModel;
import org.sonar.api.database.model.SnapshotSource;
import org.sonar.api.resources.Project;
import org.sonar.api.resources.Resource;
import org.sonar.api.rules.Rule;
import org.sonar.api.rules.RuleFinder;
import org.sonar.api.rules.Violation;
import org.sonar.api.utils.SonarException;
import org.sonar.batch.components.PastViolationsLoader;
import org.sonar.batch.index.ViolationPersister;
import java.io.IOException;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Lists;
import com.google.common.collect.Multimap;
@DependsUpon(DecoratorBarriers.END_OF_VIOLATIONS_GENERATION)
@DependedUpon("ViolationPersisterDecorator") /* temporary workaround - see NewViolationsDecorator */
@DependedUpon("ViolationPersisterDecorator")
/* temporary workaround - see NewViolationsDecorator */
public class ViolationPersisterDecorator implements Decorator {
/**
@ -53,14 +58,12 @@ public class ViolationPersisterDecorator implements Decorator {
*/
private static final String SPACE_CHARS = "\t\n\r ";
private RuleFinder ruleFinder;
private PastViolationsLoader pastViolationsLoader;
private ViolationPersister violationPersister;
List<String> checksums = Lists.newArrayList();
List<String> checksums;
public ViolationPersisterDecorator(RuleFinder ruleFinder, PastViolationsLoader pastViolationsLoader, ViolationPersister violationPersister) {
this.ruleFinder = ruleFinder;
public ViolationPersisterDecorator(PastViolationsLoader pastViolationsLoader, ViolationPersister violationPersister) {
this.pastViolationsLoader = pastViolationsLoader;
this.violationPersister = violationPersister;
}
@ -73,40 +76,125 @@ public class ViolationPersisterDecorator implements Decorator {
if (context.getViolations().isEmpty()) {
return;
}
// Load new violations
List<Violation> newViolations = context.getViolations();
// Load past violations
List<RuleFailureModel> pastViolations = pastViolationsLoader.getPastViolations(resource);
// Load current source and calculate checksums
// Load current source code and calculate checksums for each line
checksums = getChecksums(pastViolationsLoader.getSource(resource));
// Save violations
compareWithPastViolations(context, pastViolations);
// Map new violations with old ones
Map<Violation, RuleFailureModel> violationMap = mapViolations(newViolations, pastViolations);
for (Violation newViolation : newViolations) {
String checksum = getChecksumForLine(checksums, newViolation.getLineId());
violationPersister.saveViolation(context.getProject(), newViolation, violationMap.get(newViolation), checksum);
}
violationPersister.commit();
// Clear cache
checksums.clear();
}
private void compareWithPastViolations(DecoratorContext context, List<RuleFailureModel> pastViolations) {
Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
Map<Violation, RuleFailureModel> mapViolations(List<Violation> newViolations, List<RuleFailureModel> pastViolations) {
Map<Violation, RuleFailureModel> violationMap = new IdentityHashMap<Violation, RuleFailureModel>();
Multimap<Integer, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
for (RuleFailureModel pastViolation : pastViolations) {
Rule rule = ruleFinder.findById(pastViolation.getRuleId());
pastViolationsByRule.put(rule, pastViolation);
pastViolationsByRule.put(pastViolation.getRuleId(), pastViolation);
}
// for each violation, search equivalent past violation
for (Violation violation : context.getViolations()) {
RuleFailureModel pastViolation = selectPastViolation(violation, pastViolationsByRule);
if (pastViolation != null) {
// remove violation, since would be updated and shouldn't affect other violations anymore
pastViolationsByRule.remove(violation.getRule(), pastViolation);
// Try first an exact matching : same rule, same message, same line and same checkum
for (Violation newViolation : newViolations) {
mapViolation(newViolation,
findPastViolationWithSameLineAndChecksumAndMessage(newViolation, pastViolationsByRule.get(newViolation.getRule().getId())),
pastViolationsByRule, violationMap);
}
// If each new violation matches an old one we can stop the matching mechanism
if (violationMap.size() != newViolations.size()) {
// Try then to match violations on same rule with same message and with same checkum
for (Violation newViolation : newViolations) {
if (isNotAlreadyMapped(newViolation, violationMap)) {
mapViolation(newViolation,
findPastViolationWithSameChecksumAndMessage(newViolation, pastViolationsByRule.get(newViolation.getRule().getId())),
pastViolationsByRule, violationMap);
}
}
// Try then to match violations on same rule with same line and with same message
for (Violation newViolation : newViolations) {
if (isNotAlreadyMapped(newViolation, violationMap)) {
mapViolation(newViolation,
findPastViolationWithSameLineAndMessage(newViolation, pastViolationsByRule.get(newViolation.getRule().getId())),
pastViolationsByRule, violationMap);
}
}
String checksum = getChecksumForLine(checksums, violation.getLineId());
violationPersister.saveViolation(context.getProject(), violation, pastViolation, checksum);
}
violationPersister.commit();
return violationMap;
}
private final boolean isNotAlreadyMapped(Violation newViolation, Map<Violation, RuleFailureModel> violationMap) {
return violationMap.get(newViolation) == null;
}
private RuleFailureModel findPastViolationWithSameLineAndMessage(Violation newViolation, Collection<RuleFailureModel> pastViolations) {
for (RuleFailureModel pastViolation : pastViolations) {
if (isSameLine(newViolation, pastViolation) && isSameMessage(newViolation, pastViolation)) {
return pastViolation;
}
}
return null;
}
private RuleFailureModel findPastViolationWithSameChecksumAndMessage(Violation newViolation, Collection<RuleFailureModel> pastViolations) {
for (RuleFailureModel pastViolation : pastViolations) {
if (isSameChecksum(newViolation, pastViolation) && isSameMessage(newViolation, pastViolation)) {
return pastViolation;
}
}
return null;
}
private RuleFailureModel findPastViolationWithSameLineAndChecksumAndMessage(Violation newViolation,
Collection<RuleFailureModel> pastViolations) {
for (RuleFailureModel pastViolation : pastViolations) {
if (isSameLine(newViolation, pastViolation) && isSameChecksum(newViolation, pastViolation)
&& isSameMessage(newViolation, pastViolation)) {
return pastViolation;
}
}
return null;
}
private final boolean isSameChecksum(Violation newViolation, RuleFailureModel pastViolation) {
return pastViolation.getChecksum().equals(getChecksumForLine(checksums, newViolation.getLineId()));
}
private final boolean isSameLine(Violation newViolation, RuleFailureModel pastViolation) {
return pastViolation.getLine() == newViolation.getLineId();
}
private final boolean isSameMessage(Violation newViolation, RuleFailureModel pastViolation) {
return StringUtils.equals(RuleFailureModel.abbreviateMessage(newViolation.getMessage()), pastViolation.getMessage());
}
private void mapViolation(Violation newViolation, RuleFailureModel pastViolation,
Multimap<Integer, RuleFailureModel> pastViolationsByRule, Map<Violation, RuleFailureModel> violationMap) {
if (pastViolation != null) {
pastViolationsByRule.remove(newViolation.getRule().getId(), pastViolation);
violationMap.put(newViolation, pastViolation);
}
}
/**
* @return checksums, never null
*/
private List<String> getChecksums(SnapshotSource source) {
return source == null || source.getData() == null ? Collections.<String>emptyList() : getChecksums(source.getData());
return source == null || source.getData() == null ? Collections.<String> emptyList() : getChecksums(source.getData());
}
static List<String> getChecksums(String data) {
@ -119,7 +207,7 @@ public class ViolationPersisterDecorator implements Decorator {
}
} catch (IOException e) {
throw new SonarException("Unable to calculate checksums", e);
} finally {
IOUtils.closeQuietly(stream);
}
@ -141,53 +229,6 @@ public class ViolationPersisterDecorator implements Decorator {
return checksums.get(line - 1);
}
/**
* Search for past violation.
*/
RuleFailureModel selectPastViolation(Violation violation, Multimap<Rule, RuleFailureModel> pastViolationsByRule) {
Collection<RuleFailureModel> pastViolations = pastViolationsByRule.get(violation.getRule());
if (pastViolations==null || pastViolations.isEmpty()) {
// skip violation, if there is no past violations with same rule
return null;
}
String dbFormattedMessage = RuleFailureModel.abbreviateMessage(violation.getMessage());
RuleFailureModel found = selectPastViolationUsingLine(violation, dbFormattedMessage, pastViolations);
if (found == null) {
found = selectPastViolationUsingChecksum(violation, dbFormattedMessage, pastViolations);
}
return found;
}
/**
* Search for past violation with same message and line.
*/
private RuleFailureModel selectPastViolationUsingLine(Violation violation, String dbFormattedMessage, Collection<RuleFailureModel> pastViolations) {
for (RuleFailureModel pastViolation : pastViolations) {
if (ObjectUtils.equals(violation.getLineId(), pastViolation.getLine()) && StringUtils.equals(dbFormattedMessage, pastViolation.getMessage())) {
return pastViolation;
}
}
return null;
}
/**
* Search for past violation with same message and checksum.
*/
private RuleFailureModel selectPastViolationUsingChecksum(Violation violation, String dbFormattedMessage, Collection<RuleFailureModel> pastViolations) {
String checksum = getChecksumForLine(checksums, violation.getLineId());
// skip violation, which not attached to line
if (checksum == null) {
return null;
}
for (RuleFailureModel pastViolation : pastViolations) {
String pastChecksum = pastViolation.getChecksum();
if (StringUtils.equals(checksum, pastChecksum) && StringUtils.equals(dbFormattedMessage, pastViolation.getMessage())) {
return pastViolation;
}
}
return null;
}
@Override
public String toString() {
return getClass().getSimpleName();

View File

@ -19,21 +19,22 @@
*/
package org.sonar.plugins.core.timemachine;
import com.google.common.collect.LinkedHashMultimap;
import com.google.common.collect.Multimap;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import java.util.List;
import java.util.Map;
import org.junit.Before;
import org.junit.Test;
import org.sonar.api.database.model.RuleFailureModel;
import org.sonar.api.rules.Rule;
import org.sonar.api.rules.Violation;
import java.util.List;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.not;
import static org.hamcrest.Matchers.nullValue;
import static org.junit.Assert.assertThat;
import com.google.common.collect.Lists;
public class ViolationPersisterDecoratorTest {
@ -41,7 +42,7 @@ public class ViolationPersisterDecoratorTest {
@Before
public void setUp() {
decorator = new ViolationPersisterDecorator(null, null, null);
decorator = new ViolationPersisterDecorator(null, null);
}
@Test
@ -59,95 +60,97 @@ public class ViolationPersisterDecoratorTest {
}
@Test
public void sameRuleLineMessage() {
Rule rule = Rule.create().setKey("rule");
rule.setId(50);
Violation violation = Violation.create(rule, null)
.setLineId(1).setMessage("message");
public void checksumShouldHaveGreaterPriorityThanLine() {
RuleFailureModel pastViolation1 = newPastViolation("message", 1, 50, "checksum1");
RuleFailureModel pastViolation2 = newPastViolation("message", 3, 50, "checksum2");
RuleFailureModel pastViolation = newPastViolation(rule, 1, "message");
Violation newViolation1 = newViolation("message", 3, 50, "checksum1");
Violation newViolation2 = newViolation("message", 5, 50, "checksum2");
Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
pastViolationsByRule.put(rule, pastViolation);
RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
assertThat(found, equalTo(pastViolation));
Map<Violation, RuleFailureModel> mapping = decorator.mapViolations(Lists.newArrayList(newViolation1, newViolation2),
Lists.newArrayList(pastViolation1, pastViolation2));
assertThat(mapping.get(newViolation1), equalTo(pastViolation1));
assertThat(mapping.get(newViolation2), equalTo(pastViolation2));
}
@Test
public void sameRuleAndMessageButDifferentLine() {
Rule rule = Rule.create().setKey("rule");
rule.setId(50);
Violation violation = Violation.create(rule, null)
.setLineId(1).setMessage("message");
decorator.checksums = ViolationPersisterDecorator.getChecksums("violation");
public void sameRuleAndLineMessage() {
Violation newViolation = newViolation("message", 1, 50, "checksum1");
RuleFailureModel pastViolation = newPastViolation("message", 1, 50, "checksum2");
RuleFailureModel pastViolation = newPastViolation(rule, 2, "message");
pastViolation.setChecksum(ViolationPersisterDecorator.getChecksum("violation"));
Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
pastViolationsByRule.put(rule, pastViolation);
RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
assertThat(found, equalTo(pastViolation));
Map<Violation, RuleFailureModel> mapping = decorator.mapViolations(Lists.newArrayList(newViolation), Lists.newArrayList(pastViolation));
assertThat(mapping.get(newViolation), equalTo(pastViolation));
}
@Test
public void shouldCreateNewViolation() {
Rule rule = Rule.create().setKey("rule");
Violation violation = Violation.create(rule, null)
.setLineId(1).setMessage("message");
public void sameRuleAndMessageAndChecksumButDifferentLine() {
Violation newViolation = newViolation("message", 1, 50, "checksum1");
RuleFailureModel pastViolation = newPastViolation("message", 2, 50, "checksum1");
RuleFailureModel pastViolation = newPastViolation(rule, 2, "message");
Map<Violation, RuleFailureModel> mapping = decorator.mapViolations(Lists.newArrayList(newViolation), Lists.newArrayList(pastViolation));
assertThat(mapping.get(newViolation), equalTo(pastViolation));
}
Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
pastViolationsByRule.put(rule, pastViolation);
@Test
public void shouldCreateNewViolationWhenSameRuleSameMessageButDifferentLineAndChecksum() {
Violation newViolation = newViolation("message", 1, 50, "checksum1");
RuleFailureModel pastViolation = newPastViolation("message", 2, 50, "checksum2");
RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
assertThat(found, nullValue());
Map<Violation, RuleFailureModel> mapping = decorator.mapViolations(Lists.newArrayList(newViolation), Lists.newArrayList(pastViolation));
assertThat(mapping.get(newViolation), is(nullValue()));
}
@Test
public void shouldNotTrackViolationIfDifferentRule() {
Rule rule = Rule.create().setKey("rule");
rule.setId(50);
Violation violation = Violation.create(rule, null)
.setLineId(1).setMessage("message");
Violation newViolation = newViolation("message", 1, 50, "checksum1");
RuleFailureModel pastViolation = newPastViolation("message", 1, 51, "checksum1");
Rule otherRule = Rule.create().setKey("anotherRule");
otherRule.setId(244);
RuleFailureModel pastViolationOnOtherRule = newPastViolation(otherRule, 1, "message");
Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
pastViolationsByRule.put(otherRule, pastViolationOnOtherRule);
RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
assertThat(found, nullValue());
Map<Violation, RuleFailureModel> mapping = decorator.mapViolations(Lists.newArrayList(newViolation), Lists.newArrayList(pastViolation));
assertThat(mapping.get(newViolation), is(nullValue()));
}
@Test
public void shouldCompareViolationsWithDatabaseFormat() {
// violation messages are trimmed and can be abbreviated when persisted in database.
// Comparing violation messages must use the same format.
Rule rule = Rule.create().setKey("rule");
rule.setId(50);
Violation violation = Violation.create(rule, null)
.setLineId(30).setMessage(" message "); // starts and ends with whitespaces
Violation newViolation = newViolation("message", 1, 50, "checksum1");
RuleFailureModel pastViolation = newPastViolation(" message ", 1, 50, "checksum2");
RuleFailureModel pastViolation = newPastViolation(rule, 30, "message"); // trimmed in database
Multimap<Rule, RuleFailureModel> pastViolationsByRule = LinkedHashMultimap.create();
pastViolationsByRule.put(rule, pastViolation);
RuleFailureModel found = decorator.selectPastViolation(violation, pastViolationsByRule);
assertThat(found, equalTo(pastViolation));
Map<Violation, RuleFailureModel> mapping = decorator.mapViolations(Lists.newArrayList(newViolation), Lists.newArrayList(pastViolation));
assertThat(mapping.get(newViolation), equalTo(pastViolation));
}
private RuleFailureModel newPastViolation(Rule rule, Integer line, String message) {
private Violation newViolation(String message, int lineId, int ruleId) {
Rule rule = Rule.create().setKey("rule");
rule.setId(ruleId);
Violation violation = Violation.create(rule, null).setLineId(lineId).setMessage(message);
return violation;
}
private Violation newViolation(String message, int lineId, int ruleId, String lineChecksum) {
Violation violation = newViolation(message, lineId, ruleId);
if (decorator.checksums == null) {
decorator.checksums = Lists.newArrayListWithExpectedSize(100);
}
for (int i = decorator.checksums.size() - 1; i < lineId; i++) {
decorator.checksums.add("");
}
decorator.checksums.set(lineId - 1, ViolationPersisterDecorator.getChecksum(lineChecksum));
return violation;
}
private RuleFailureModel newPastViolation(String message, int lineId, int ruleId) {
RuleFailureModel pastViolation = new RuleFailureModel();
pastViolation.setLine(line);
pastViolation.setId(lineId + ruleId);
pastViolation.setLine(lineId);
pastViolation.setMessage(message);
pastViolation.setRuleId(rule.getId());
pastViolation.setRuleId(ruleId);
return pastViolation;
}
private RuleFailureModel newPastViolation(String message, int lineId, int ruleId, String lineChecksum) {
RuleFailureModel pastViolation = newPastViolation(message, lineId, ruleId);
pastViolation.setChecksum(ViolationPersisterDecorator.getChecksum(lineChecksum));
return pastViolation;
}