diff options
9 files changed, 262 insertions, 64 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 4299ce8c699..e51aeefa2fc 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 @@ -283,6 +283,7 @@ public class CorePlugin extends SonarPlugin { extensions.add(CloseReviewsDecorator.class); extensions.add(ReferenceAnalysis.class); extensions.add(ManualMeasureDecorator.class); + extensions.add(ManualViolationInjector.class); extensions.add(UpdateReviewsDecorator.class); // time machine diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java index 86e849d2c0a..c7c12be103e 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/CloseReviewsDecorator.java @@ -19,15 +19,9 @@ */ package org.sonar.plugins.core.sensors; -import java.util.List; - import org.slf4j.Logger; import org.slf4j.LoggerFactory; -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.batch.*; import org.sonar.api.database.DatabaseSession; import org.sonar.api.database.model.Snapshot; import org.sonar.api.database.model.User; @@ -41,6 +35,8 @@ import org.sonar.batch.index.ResourcePersister; import org.sonar.core.NotDryRun; import org.sonar.jpa.entity.Review; +import java.util.List; + /** * Decorator that handles the life cycle of a review (for instance, closes a review when its corresponding violation has been fixed). */ @@ -92,13 +88,17 @@ public class CloseReviewsDecorator implements Decorator { */ protected int closeReviews(Resource resource, int resourceId, int snapshotId) { String conditions = " WHERE status!='CLOSED' AND resource_id=" + resourceId - + " AND rule_failure_permanent_id NOT IN " + "(SELECT permanent_id FROM rule_failures WHERE snapshot_id=" + snapshotId + + " AND manual_violation=:manualViolation AND rule_failure_permanent_id NOT IN " + "(SELECT permanent_id FROM rule_failures WHERE snapshot_id=" + snapshotId + " AND permanent_id IS NOT NULL)"; - List<Review> reviews = databaseSession.getEntityManager().createNativeQuery("SELECT * FROM reviews " + conditions, Review.class).getResultList(); + List<Review> reviews = databaseSession.getEntityManager().createNativeQuery("SELECT * FROM reviews " + conditions, Review.class) + .setParameter("manualViolation", false) + .getResultList(); for (Review review : reviews) { notifyClosed(resource, review); } - int rowUpdated = databaseSession.createNativeQuery("UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP" + conditions).executeUpdate(); + int rowUpdated = databaseSession.createNativeQuery("UPDATE reviews SET status='CLOSED', updated_at=CURRENT_TIMESTAMP" + conditions) + .setParameter("manualViolation", false) + .executeUpdate(); LOG.debug("- {} reviews set to 'closed' on resource #{}", rowUpdated, resourceId); return rowUpdated; } diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ManualViolationInjector.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ManualViolationInjector.java new file mode 100644 index 00000000000..4e9901e3a1d --- /dev/null +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/sensors/ManualViolationInjector.java @@ -0,0 +1,81 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2011 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * Sonar is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.core.sensors; + +import org.slf4j.LoggerFactory; +import org.sonar.api.batch.Decorator; +import org.sonar.api.batch.DecoratorContext; +import org.sonar.api.batch.Phase; +import org.sonar.api.resources.Project; +import org.sonar.api.resources.Resource; +import org.sonar.api.rules.RuleFinder; +import org.sonar.api.rules.RulePriority; +import org.sonar.api.rules.Violation; +import org.sonar.persistence.dao.ReviewDao; +import org.sonar.persistence.model.Review; +import org.sonar.persistence.model.ReviewQuery; + + +import java.util.List; + +@Phase(name = Phase.Name.PRE) +public class ManualViolationInjector implements Decorator { + + private ReviewDao reviewDao; + private RuleFinder ruleFinder; + + public ManualViolationInjector(ReviewDao reviewDao, RuleFinder ruleFinder) { + this.reviewDao = reviewDao; + this.ruleFinder = ruleFinder; + } + + public boolean shouldExecuteOnProject(Project project) { + return true; + } + + public void decorate(Resource resource, DecoratorContext context) { + if (resource.getId() != null) { + ReviewQuery query = ReviewQuery.create().setManualViolation(true).setResourceId(resource.getId()).setStatus(Review.STATUS_OPEN); + List<Review> reviews = reviewDao.selectByQuery(query); + for (Review review : reviews) { + if (review.getRuleId()==null) { + LoggerFactory.getLogger(getClass()).warn("No rule is defined on the review with id: " + review.getId()); + } + if (review.getViolationPermanentId()==null) { + LoggerFactory.getLogger(getClass()).warn("Permanent id of manual violation is missing on the review with id: " + review.getId()); + } + Violation violation = Violation.create(ruleFinder.findById(review.getRuleId()), resource); + violation.setManual(true); + violation.setLineId(review.getLine()); + violation.setPermanentId(review.getViolationPermanentId()); + violation.setSwitchedOff(false); + violation.setCreatedAt(review.getCreatedAt()); + violation.setMessage(review.getTitle()); + violation.setSeverity(RulePriority.valueOf(review.getSeverity())); + context.saveViolation(violation); + } + } + } + + @Override + public String toString() { + return getClass().getSimpleName(); + } +} diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationTrackingDecorator.java index 93347cd2f17..62d259df24c 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationTrackingDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/timemachine/ViolationTrackingDecorator.java @@ -90,13 +90,23 @@ public class ViolationTrackingDecorator implements Decorator { pastViolationsByRule.put(pastViolation.getRuleId(), pastViolation); } - // Try first to match violations on same rule with same line and with same checkum (but not necessarily with same message) + // Match the permanent id of the violation. This id is for example set explicitly when injecting manual violations for (Violation newViolation : newViolations) { mapViolation(newViolation, - findPastViolationWithSameLineAndChecksum(newViolation, pastViolationsByRule.get(newViolation.getRule().getId())), + findPastViolationWithSamePermanentId(newViolation, pastViolationsByRule.get(newViolation.getRule().getId())), pastViolationsByRule, referenceViolationsMap); } + + // Try first to match violations on same rule with same line and with same checkum (but not necessarily with same message) + for (Violation newViolation : newViolations) { + if (isNotAlreadyMapped(newViolation, referenceViolationsMap)) { + mapViolation(newViolation, + findPastViolationWithSameLineAndChecksum(newViolation, pastViolationsByRule.get(newViolation.getRule().getId())), + pastViolationsByRule, referenceViolationsMap); + } + } + // If each new violation matches an old one we can stop the matching mechanism if (referenceViolationsMap.size() != newViolations.size()) { // Try then to match violations on same rule with same message and with same checkum @@ -170,6 +180,15 @@ public class ViolationTrackingDecorator implements Decorator { return null; } + private RuleFailureModel findPastViolationWithSamePermanentId(Violation newViolation, Collection<RuleFailureModel> pastViolations) { + for (RuleFailureModel pastViolation : pastViolations) { + if (isSamePermanentId(newViolation, pastViolation)) { + return pastViolation; + } + } + return null; + } + private boolean isSameChecksum(Violation newViolation, RuleFailureModel pastViolation) { return StringUtils.equals(pastViolation.getChecksum(), newViolation.getChecksum()); } @@ -182,6 +201,10 @@ public class ViolationTrackingDecorator implements Decorator { return StringUtils.equals(RuleFailureModel.abbreviateMessage(newViolation.getMessage()), pastViolation.getMessage()); } + private boolean isSamePermanentId(Violation newViolation, RuleFailureModel pastViolation) { + return newViolation.getPermanentId() != null && newViolation.getPermanentId().equals(pastViolation.getPermanentId()); + } + private void mapViolation(Violation newViolation, RuleFailureModel pastViolation, Multimap<Integer, RuleFailureModel> pastViolationsByRule, Map<Violation, RuleFailureModel> violationMap) { if (pastViolation != null) { @@ -190,7 +213,7 @@ public class ViolationTrackingDecorator implements Decorator { newViolation.setNew(false); pastViolationsByRule.remove(newViolation.getRule().getId(), pastViolation); violationMap.put(newViolation, pastViolation); - + } else { newViolation.setNew(true); newViolation.setCreatedAt(project.getAnalysisDate()); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ManualViolationInjectorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ManualViolationInjectorTest.java new file mode 100644 index 00000000000..bb7c3e8b483 --- /dev/null +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/sensors/ManualViolationInjectorTest.java @@ -0,0 +1,68 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2011 SonarSource + * mailto:contact AT sonarsource DOT com + * + * Sonar is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 3 of the License, or (at your option) any later version. + * + * Sonar is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with Sonar; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02 + */ +package org.sonar.plugins.core.sensors; + +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.junit.Test; +import org.mockito.Matchers; +import org.sonar.api.batch.DecoratorContext; +import org.sonar.api.resources.Project; +import org.sonar.api.rules.Rule; +import org.sonar.api.rules.RuleFinder; +import org.sonar.api.rules.RulePriority; +import org.sonar.api.rules.Violation; +import org.sonar.api.utils.DateUtils; +import org.sonar.persistence.dao.ReviewDao; +import org.sonar.persistence.model.Review; +import org.sonar.persistence.model.ReviewQuery; + +import java.util.Arrays; +import java.util.Date; + +import static org.mockito.Mockito.*; + +public class ManualViolationInjectorTest { + + @Test + public void shouldInjectManualViolationsDefinedByReviews() { + ReviewDao reviewDao = mock(ReviewDao.class); + final Date reviewCreatedAt = DateUtils.parseDate("2011-12-25"); + Review review = new Review().setRuleId(3).setViolationPermanentId(100).setCreatedAt(reviewCreatedAt).setSeverity("BLOCKER"); + when(reviewDao.selectByQuery(Matchers.<ReviewQuery>anyObject())).thenReturn(Arrays.<Review>asList(review)); + RuleFinder ruleFinder = mock(RuleFinder.class); + when(ruleFinder.findById(3)).thenReturn(new Rule()); + DecoratorContext context = mock(DecoratorContext.class); + ManualViolationInjector injector = new ManualViolationInjector(reviewDao, ruleFinder); + + injector.decorate(new Project("key").setId(100), context); + + verify(context, times(1)).saveViolation(argThat(new BaseMatcher<Violation>() { + public boolean matches(Object o) { + Violation v = (Violation) o; + return v.getPermanentId() == 100 && v.getRule() != null && v.isManual() && v.getCreatedAt().equals(reviewCreatedAt) + && v.getSeverity().equals(RulePriority.BLOCKER); + } + + public void describeTo(Description description) { + } + })); + } +} diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationTrackingDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationTrackingDecoratorTest.java index 5aee8dc0586..4f5671d75be 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationTrackingDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/timemachine/ViolationTrackingDecoratorTest.java @@ -49,7 +49,21 @@ public class ViolationTrackingDecoratorTest { when(project.getAnalysisDate()).thenReturn(analysisDate); decorator = new ViolationTrackingDecorator(project, null, null); } - + + @Test + public void permanentIdShouldBeThePrioritaryFieldToCheck() { + RuleFailureModel referenceViolation1 = newReferenceViolation("message", 10, 1, "checksum1").setPermanentId(100); + RuleFailureModel referenceViolation2 = newReferenceViolation("message", 18, 1, "checksum2").setPermanentId(200); + Violation newViolation = newViolation("message", 10, 1, "checksum1"); // exactly the fields of referenceViolation1 + newViolation.setPermanentId(200); + + Map<Violation, RuleFailureModel> mapping = decorator.mapViolations(Lists.newArrayList(newViolation), + Lists.newArrayList(referenceViolation1, referenceViolation2)); + + assertThat(mapping.get(newViolation), equalTo(referenceViolation2));// same permanent id + assertThat(newViolation.isNew(), is(false)); + } + @Test public void checksumShouldHaveGreaterPriorityThanLine() { RuleFailureModel referenceViolation1 = newReferenceViolation("message", 1, 50, "checksum1"); diff --git a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java index 94becc59ec7..3adbd331509 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java +++ b/sonar-batch/src/main/java/org/sonar/batch/index/DefaultIndex.java @@ -19,15 +19,9 @@ */ package org.sonar.batch.index; -import java.util.Collection; -import java.util.Collections; -import java.util.Date; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Set; - +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Sets; import org.apache.commons.lang.ObjectUtils; import org.apache.commons.lang.StringUtils; import org.slf4j.Logger; @@ -36,17 +30,9 @@ import org.sonar.api.batch.Event; import org.sonar.api.batch.SonarIndex; import org.sonar.api.database.model.ResourceModel; import org.sonar.api.design.Dependency; -import org.sonar.api.measures.Measure; -import org.sonar.api.measures.MeasuresFilter; -import org.sonar.api.measures.MeasuresFilters; -import org.sonar.api.measures.Metric; -import org.sonar.api.measures.MetricFinder; +import org.sonar.api.measures.*; import org.sonar.api.profiles.RulesProfile; -import org.sonar.api.resources.Project; -import org.sonar.api.resources.ProjectLink; -import org.sonar.api.resources.Resource; -import org.sonar.api.resources.ResourceUtils; -import org.sonar.api.resources.Scopes; +import org.sonar.api.resources.*; import org.sonar.api.rules.ActiveRule; import org.sonar.api.rules.Violation; import org.sonar.api.utils.SonarException; @@ -56,9 +42,7 @@ import org.sonar.batch.ProjectTree; import org.sonar.batch.ResourceFilters; import org.sonar.batch.ViolationFilters; -import com.google.common.collect.Lists; -import com.google.common.collect.Maps; -import com.google.common.collect.Sets; +import java.util.*; public class DefaultIndex extends SonarIndex { @@ -315,9 +299,9 @@ public class DefaultIndex extends SonarIndex { List<Violation> filteredViolations = Lists.newArrayList(); ViolationQuery.SwitchMode mode = violationQuery.getSwitchMode(); for (Violation violation : bucket.getViolations()) { - if (mode== ViolationQuery.SwitchMode.BOTH || - (mode== ViolationQuery.SwitchMode.OFF && violation.isSwitchedOff()) || - (mode== ViolationQuery.SwitchMode.ON && !violation.isSwitchedOff())) { + if (mode == ViolationQuery.SwitchMode.BOTH || + (mode == ViolationQuery.SwitchMode.OFF && violation.isSwitchedOff()) || + (mode == ViolationQuery.SwitchMode.ON && !violation.isSwitchedOff())) { filteredViolations.add(violation); } } @@ -341,19 +325,26 @@ public class DefaultIndex extends SonarIndex { if (bucket != null && !bucket.isExcluded()) { boolean isIgnored = !force && violationFilters != null && violationFilters.isIgnored(violation); if (!isIgnored) { - ActiveRule activeRule = profile.getActiveRule(violation.getRule()); - if (activeRule == null) { - if (currentProject.getReuseExistingRulesConfig()) { - violation.setSeverity(violation.getRule().getSeverity()); - doAddViolation(violation, bucket); - } else { - LoggerFactory.getLogger(getClass()).debug("Rule is not activated, ignoring violation {}", violation); - } + // TODO this code is not the responsibility of this index. It should be moved somewhere else. - } else { - violation.setSeverity(activeRule.getSeverity()); + if (violation.isManual()) { doAddViolation(violation, bucket); + } else { + ActiveRule activeRule = profile.getActiveRule(violation.getRule()); + if (activeRule == null) { + if (currentProject.getReuseExistingRulesConfig()) { + violation.setSeverity(violation.getRule().getSeverity()); + doAddViolation(violation, bucket); + + } else { + LoggerFactory.getLogger(getClass()).debug("Rule is not activated, ignoring violation {}", violation); + } + + } else { + violation.setSeverity(activeRule.getSeverity()); + doAddViolation(violation, bucket); + } } } } @@ -400,7 +391,7 @@ public class DefaultIndex extends SonarIndex { Event event = new Event(name, description, category); event.setDate(date); event.setCreatedAt(new Date()); - + persistence.saveEvent(resource, event); return null; } diff --git a/sonar-core/src/main/java/org/sonar/persistence/model/Review.java b/sonar-core/src/main/java/org/sonar/persistence/model/Review.java index 24877895b66..54fb8ce2d96 100644 --- a/sonar-core/src/main/java/org/sonar/persistence/model/Review.java +++ b/sonar-core/src/main/java/org/sonar/persistence/model/Review.java @@ -25,6 +25,10 @@ import java.util.Date; * @since 2.13 */ public class Review { + + public static final String STATUS_OPEN = "OPEN"; + public static final String STATUS_CLOSED = "CLOSED"; + private Long id; private Integer userId; private Integer assigneeId; @@ -63,103 +67,116 @@ public class Review { return assigneeId; } - public void setAssigneeId(Integer assigneeId) { + public Review setAssigneeId(Integer assigneeId) { this.assigneeId = assigneeId; + return this; } public String getTitle() { return title; } - public void setTitle(String title) { + public Review setTitle(String title) { this.title = title; + return this; } public String getStatus() { return status; } - public void setStatus(String status) { + public Review setStatus(String status) { this.status = status; + return this; } public String getResolution() { return resolution; } - public void setResolution(String resolution) { + public Review setResolution(String resolution) { this.resolution = resolution; + return this; } public Integer getViolationPermanentId() { return violationPermanentId; } - public void setViolationPermanentId(Integer violationPermanentId) { + public Review setViolationPermanentId(Integer violationPermanentId) { this.violationPermanentId = violationPermanentId; + return this; } public Integer getProjectId() { return projectId; } - public void setProjectId(Integer projectId) { + public Review setProjectId(Integer projectId) { this.projectId = projectId; + return this; } public Integer getResourceId() { return resourceId; } - public void setResourceId(Integer resourceId) { + public Review setResourceId(Integer resourceId) { this.resourceId = resourceId; + return this; } public Integer getLine() { return line; } - public void setLine(Integer line) { + public Review setLine(Integer line) { this.line = line; + return this; } public Date getCreatedAt() { return createdAt; } - public void setCreatedAt(Date createdAt) { + public Review setCreatedAt(Date createdAt) { this.createdAt = createdAt; + return this; } public Date getUpdatedAt() { return updatedAt; } - public void setUpdatedAt(Date updatedAt) { + public Review setUpdatedAt(Date updatedAt) { this.updatedAt = updatedAt; + return this; } public String getSeverity() { return severity; } - public void setSeverity(String severity) { + public Review setSeverity(String severity) { this.severity = severity; + return this; } public Integer getRuleId() { return ruleId; } - public void setRuleId(Integer ruleId) { + public Review setRuleId(Integer ruleId) { this.ruleId = ruleId; + return this; } public Boolean getManualViolation() { return manualViolation; } - public void setManualViolation(Boolean manualViolation) { - this.manualViolation = manualViolation; + public Review setManualViolation(Boolean b) { + this.manualViolation = b; + return this; } } diff --git a/sonar-core/src/main/java/org/sonar/persistence/model/ReviewQuery.java b/sonar-core/src/main/java/org/sonar/persistence/model/ReviewQuery.java index 69ee246e600..ccb7cf7c5ee 100644 --- a/sonar-core/src/main/java/org/sonar/persistence/model/ReviewQuery.java +++ b/sonar-core/src/main/java/org/sonar/persistence/model/ReviewQuery.java @@ -19,6 +19,9 @@ */ package org.sonar.persistence.model; +/** + * @since 2.13 + */ public final class ReviewQuery { private Boolean manualViolation; private Integer resourceId; |