From: Simon Brandhof Date: Mon, 20 May 2013 18:52:57 +0000 (+0200) Subject: SONAR-3755 close issues when rule is disabled or deleted X-Git-Tag: 3.6~332 X-Git-Url: https://source.dussan.org/?a=commitdiff_plain;h=97e7d751080fdc0b45845a066efc03bc7a14ac6c;p=sonarqube.git SONAR-3755 close issues when rule is disabled or deleted --- diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java index c5a6b2cb3a1..d4fb3288a69 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensor.java @@ -49,7 +49,7 @@ public class InitialOpenIssuesSensor implements Sensor { @Override public void analyse(Project project, SensorContext context) { Date loadingDate = new Date(); - List dtos = issueDao.selectOpenIssues(project.getId()); + List dtos = issueDao.selectNonClosedIssuesByRootComponent(project.getId()); initialOpenIssuesStack.setIssues(dtos, loadingDate); } diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java index c4ab555d068..0738bd5ca1f 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTracking.java @@ -22,13 +22,14 @@ package org.sonar.plugins.core.issue; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Objects; -import com.google.common.collect.*; +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Lists; +import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; import org.sonar.api.BatchExtension; import org.sonar.api.batch.SonarIndex; -import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; -import org.sonar.api.rules.RuleFinder; -import org.sonar.api.utils.KeyValueFormat; +import org.sonar.api.rule.RuleKey; import org.sonar.batch.scan.LastSnapshots; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.db.IssueDto; @@ -37,7 +38,6 @@ import org.sonar.plugins.core.timemachine.ViolationTrackingBlocksRecognizer; import org.sonar.plugins.core.timemachine.tracking.*; import javax.annotation.Nullable; - import java.util.*; public class IssueTracking implements BatchExtension { @@ -52,22 +52,10 @@ public class IssueTracking implements BatchExtension { } } }; - private final Project project; - private final RuleFinder ruleFinder; private final LastSnapshots lastSnapshots; private final SonarIndex index; - /** - * Live collection of unmapped past issues. - */ - private Set unmappedLastIssues = Sets.newHashSet(); - /** - * Map of old issues by new issues - */ - private Map referenceIssuesMap = Maps.newIdentityHashMap(); - public IssueTracking(Project project, RuleFinder ruleFinder, LastSnapshots lastSnapshots, SonarIndex index) { - this.project = project; - this.ruleFinder = ruleFinder; + public IssueTracking(LastSnapshots lastSnapshots, SonarIndex index) { this.lastSnapshots = lastSnapshots; this.index = index; } @@ -75,16 +63,15 @@ public class IssueTracking implements BatchExtension { /** * @return untracked issues */ - public Set track(Resource resource, Collection dbIssues, Collection newIssues) { - referenceIssuesMap.clear(); - unmappedLastIssues.clear(); + public IssueTrackingResult track(Resource resource, Collection dbIssues, Collection newIssues) { + IssueTrackingResult result = new IssueTrackingResult(); String source = index.getSource(resource); setChecksumOnNewIssues(newIssues, source); // Map new issues with old ones - mapIssues(newIssues, dbIssues, source, resource); - return unmappedLastIssues; + mapIssues(newIssues, dbIssues, source, resource, result); + return result; } private void setChecksumOnNewIssues(Collection issues, String source) { @@ -95,67 +82,56 @@ public class IssueTracking implements BatchExtension { } @VisibleForTesting - Map mapIssues(Collection newIssues, @Nullable List lastIssues) { - return mapIssues(newIssues, lastIssues, null, null); - } - - @VisibleForTesting - Map mapIssues(Collection newIssues, @Nullable Collection lastIssues, @Nullable String source, @Nullable Resource resource) { + void mapIssues(Collection newIssues, @Nullable Collection lastIssues, @Nullable String source, @Nullable Resource resource, IssueTrackingResult result) { boolean hasLastScan = false; - Multimap lastIssuesByRule = LinkedHashMultimap.create(); if (lastIssues != null) { hasLastScan = true; - mapLastIssues(newIssues, lastIssues, lastIssuesByRule); + mapLastIssues(newIssues, lastIssues, result); } // If each new issue matches an old one we can stop the matching mechanism - if (referenceIssuesMap.size() != newIssues.size()) { + if (result.matched().size() != newIssues.size()) { if (source != null && resource != null && hasLastScan) { String referenceSource = lastSnapshots.getSource(resource); if (referenceSource != null) { - mapNewissues(referenceSource, newIssues, lastIssuesByRule, source); + mapNewissues(referenceSource, newIssues, source, result); } } - mapIssuesOnSameRule(newIssues, lastIssuesByRule); + mapIssuesOnSameRule(newIssues, result); } - - return referenceIssuesMap; } - private void mapLastIssues(Collection newIssues, Collection lastIssues, Multimap lastIssuesByRule) { - unmappedLastIssues.addAll(lastIssues); - + private void mapLastIssues(Collection newIssues, Collection lastIssues, IssueTrackingResult result) { for (IssueDto lastIssue : lastIssues) { - lastIssuesByRule.put(getRuleId(lastIssue), lastIssue); + result.addUnmatched(lastIssue); } // Match the key of the issue. (For manual issues) for (DefaultIssue newIssue : newIssues) { - mapIssue(newIssue, - findLastIssueWithSameKey(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), - lastIssuesByRule, referenceIssuesMap); + mapIssue(newIssue, findLastIssueWithSameKey(newIssue, result.unmatchedForRule(newIssue.ruleKey())), result); } // Try first to match issues on same rule with same line and with same checksum (but not necessarily with same message) for (DefaultIssue newIssue : newIssues) { - if (isNotAlreadyMapped(newIssue)) { - mapIssue(newIssue, - findLastIssueWithSameLineAndChecksum(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), - lastIssuesByRule, referenceIssuesMap); + if (isNotAlreadyMapped(newIssue, result)) { + mapIssue( + newIssue, + findLastIssueWithSameLineAndChecksum(newIssue, result.unmatchedForRule(newIssue.ruleKey())), + result); } } } - private void mapNewissues(String referenceSource, Collection newIssues, Multimap lastIssuesByRule, String source) { + private void mapNewissues(String referenceSource, Collection newIssues, String source, IssueTrackingResult result) { HashedSequence hashedReference = HashedSequence.wrap(new StringText(referenceSource), StringTextComparator.IGNORE_WHITESPACE); HashedSequence hashedSource = HashedSequence.wrap(new StringText(source), StringTextComparator.IGNORE_WHITESPACE); HashedSequenceComparator hashedComparator = new HashedSequenceComparator(StringTextComparator.IGNORE_WHITESPACE); ViolationTrackingBlocksRecognizer rec = new ViolationTrackingBlocksRecognizer(hashedReference, hashedSource, hashedComparator); - Multimap newIssuesByLines = newIssuesByLines(newIssues, rec); - Multimap lastIssuesByLines = lastIssuesByLines(unmappedLastIssues, rec); + Multimap newIssuesByLines = newIssuesByLines(newIssues, rec, result); + Multimap lastIssuesByLines = lastIssuesByLines(result.unmatched(), rec); RollingHashSequence> a = RollingHashSequence.wrap(hashedReference, hashedComparator, 5); RollingHashSequence> b = RollingHashSequence.wrap(hashedSource, hashedComparator, 5); @@ -189,7 +165,7 @@ public class IssueTracking implements BatchExtension { for (HashOccurrence hashOccurrence : map.values()) { if (hashOccurrence.countA == 1 && hashOccurrence.countB == 1) { // Guaranteed that lineA has been moved to lineB, so we can map all issues on lineA to all issues on lineB - map(newIssuesByLines.get(hashOccurrence.lineB), lastIssuesByLines.get(hashOccurrence.lineA), lastIssuesByRule); + map(newIssuesByLines.get(hashOccurrence.lineB), lastIssuesByLines.get(hashOccurrence.lineA), result); lastIssuesByLines.removeAll(hashOccurrence.lineA); newIssuesByLines.removeAll(hashOccurrence.lineB); } @@ -207,47 +183,50 @@ public class IssueTracking implements BatchExtension { Collections.sort(possibleLinePairs, LINE_PAIR_COMPARATOR); for (LinePair linePair : possibleLinePairs) { // High probability that lineA has been moved to lineB, so we can map all Issues on lineA to all Issues on lineB - map(newIssuesByLines.get(linePair.lineB), lastIssuesByLines.get(linePair.lineA), lastIssuesByRule); + map(newIssuesByLines.get(linePair.lineB), lastIssuesByLines.get(linePair.lineA), result); } } } - private void mapIssuesOnSameRule(Collection newIssues, Multimap lastIssuesByRule) { + private void mapIssuesOnSameRule(Collection newIssues, IssueTrackingResult result) { // Try then to match issues on same rule with same message and with same checksum for (DefaultIssue newIssue : newIssues) { - if (isNotAlreadyMapped(newIssue)) { - mapIssue(newIssue, - findLastIssueWithSameChecksumAndMessage(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), - lastIssuesByRule, referenceIssuesMap); + if (isNotAlreadyMapped(newIssue, result)) { + mapIssue( + newIssue, + findLastIssueWithSameChecksumAndMessage(newIssue, result.unmatchedForRule(newIssue.ruleKey())), + result); } } // Try then to match issues on same rule with same line and with same message for (DefaultIssue newIssue : newIssues) { - if (isNotAlreadyMapped(newIssue)) { - mapIssue(newIssue, - findLastIssueWithSameLineAndMessage(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), - lastIssuesByRule, referenceIssuesMap); + if (isNotAlreadyMapped(newIssue, result)) { + mapIssue( + newIssue, + findLastIssueWithSameLineAndMessage(newIssue, result.unmatchedForRule(newIssue.ruleKey())), + result); } } // Last check: match issue if same rule and same checksum but different line and different message // See SONAR-2812 for (DefaultIssue newIssue : newIssues) { - if (isNotAlreadyMapped(newIssue)) { - mapIssue(newIssue, - findLastIssueWithSameChecksum(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), - lastIssuesByRule, referenceIssuesMap); + if (isNotAlreadyMapped(newIssue, result)) { + mapIssue( + newIssue, + findLastIssueWithSameChecksum(newIssue, result.unmatchedForRule(newIssue.ruleKey())), + result); } } } - private void map(Collection newIssues, Collection lastIssues, Multimap lastIssuesByRule) { + private void map(Collection newIssues, Collection lastIssues, IssueTrackingResult result) { for (DefaultIssue newIssue : newIssues) { - if (isNotAlreadyMapped(newIssue)) { + if (isNotAlreadyMapped(newIssue, result)) { for (IssueDto pastIssue : lastIssues) { - if (isNotAlreadyMapped(pastIssue) && Objects.equal(getRuleId(newIssue), getRuleId(pastIssue))) { - mapIssue(newIssue, pastIssue, lastIssuesByRule, referenceIssuesMap); + if (isNotAlreadyMapped(pastIssue, result) && Objects.equal(newIssue.ruleKey(), RuleKey.of(pastIssue.getRuleRepo(), pastIssue.getRule()))) { + mapIssue(newIssue, pastIssue, result); break; } } @@ -255,10 +234,10 @@ public class IssueTracking implements BatchExtension { } } - private Multimap newIssuesByLines(Collection newIssues, ViolationTrackingBlocksRecognizer rec) { + private Multimap newIssuesByLines(Collection newIssues, ViolationTrackingBlocksRecognizer rec, IssueTrackingResult result) { Multimap newIssuesByLines = LinkedHashMultimap.create(); for (DefaultIssue newIssue : newIssues) { - if (isNotAlreadyMapped(newIssue) && rec.isValidLineInSource(newIssue.line())) { + if (isNotAlreadyMapped(newIssue, result) && rec.isValidLineInSource(newIssue.line())) { newIssuesByLines.put(newIssue.line(), newIssue); } } @@ -320,12 +299,12 @@ public class IssueTracking implements BatchExtension { return null; } - private boolean isNotAlreadyMapped(IssueDto pastIssue) { - return unmappedLastIssues.contains(pastIssue); + private boolean isNotAlreadyMapped(IssueDto pastIssue, IssueTrackingResult result) { + return result.unmatched().contains(pastIssue); } - private boolean isNotAlreadyMapped(DefaultIssue newIssue) { - return !referenceIssuesMap.containsKey(newIssue); + private boolean isNotAlreadyMapped(DefaultIssue newIssue, IssueTrackingResult result) { + return !result.isMatched(newIssue); } private boolean isSameChecksum(DefaultIssue newIssue, IssueDto pastIssue) { @@ -344,53 +323,12 @@ public class IssueTracking implements BatchExtension { return Objects.equal(newIssue.key(), pastIssue.getKee()); } - private void mapIssue(DefaultIssue newIssue, IssueDto pastIssue, Multimap lastIssuesByRule, Map issueMap) { - if (pastIssue != null) { - newIssue.setKey(pastIssue.getKee()); - if (pastIssue.isManualSeverity()) { - newIssue.setManualSeverity(true); - newIssue.setSeverity(pastIssue.getSeverity()); - } else if (!Objects.equal(pastIssue.getSeverity(), newIssue.severity())) { - // TODO register diff - } - newIssue.setResolution(pastIssue.getResolution()); - newIssue.setStatus(pastIssue.getStatus()); - newIssue.setNew(false); - newIssue.setAlive(true); - newIssue.setAssignee(pastIssue.getAssignee()); - newIssue.setAuthorLogin(pastIssue.getAuthorLogin()); - newIssue.setAssignee(pastIssue.getAssignee()); - if (pastIssue.getAttributes() != null) { - //FIXME do not override new attributes - newIssue.setAttributes(KeyValueFormat.parse(pastIssue.getAttributes())); - } - newIssue.setTechnicalCreationDate(pastIssue.getCreatedAt()); - newIssue.setTechnicalUpdateDate(pastIssue.getUpdatedAt()); - newIssue.setCreationDate(pastIssue.getIssueCreationDate()); - newIssue.setUpdateDate(project.getAnalysisDate()); - - // should be null - newIssue.setCloseDate(pastIssue.getIssueCloseDate()); - - lastIssuesByRule.remove(getRuleId(newIssue), pastIssue); - issueMap.put(newIssue, pastIssue); - unmappedLastIssues.remove(pastIssue); + private void mapIssue(DefaultIssue issue, @Nullable IssueDto ref, IssueTrackingResult result) { + if (ref != null) { + result.setMatch(issue, ref); } } - @VisibleForTesting - IssueDto getReferenceIssue(DefaultIssue issue) { - return referenceIssuesMap.get(issue); - } - - private Integer getRuleId(DefaultIssue issue) { - return ruleFinder.findByKey(issue.ruleKey()).getId(); - } - - private Integer getRuleId(IssueDto issue) { - return issue.getRuleId(); - } - @Override public String toString() { return getClass().getSimpleName(); diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java index 9ded6ce1565..0bc6875731f 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java @@ -19,46 +19,57 @@ */ package org.sonar.plugins.core.issue; +import com.google.common.annotations.VisibleForTesting; +import com.google.common.base.Objects; +import com.google.common.base.Strings; import com.google.common.collect.Lists; 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.component.ResourcePerspectives; +import org.sonar.api.issue.Issuable; import org.sonar.api.issue.Issue; import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; import org.sonar.api.resources.ResourceUtils; -import org.sonar.api.resources.Scopes; -import org.sonar.batch.issue.ScanIssues; +import org.sonar.api.rules.Rule; +import org.sonar.api.rules.RuleFinder; +import org.sonar.api.utils.KeyValueFormat; +import org.sonar.batch.issue.IssueCache; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.issue.db.IssueDto; import org.sonar.core.issue.workflow.IssueWorkflow; import java.util.Collection; -import java.util.Set; @DependedUpon(DecoratorBarriers.END_OF_ISSUES_UPDATES) public class IssueTrackingDecorator implements Decorator { - private final ScanIssues scanIssues; + private final IssueCache issueCache; private final InitialOpenIssuesStack initialOpenIssues; private final IssueTracking tracking; private final IssueFilters filters; private final IssueHandlers handlers; private final IssueWorkflow workflow; private final IssueChangeContext changeContext; + private final ResourcePerspectives perspectives; + private final RuleFinder ruleFinder; - public IssueTrackingDecorator(ScanIssues scanIssues, InitialOpenIssuesStack initialOpenIssues, IssueTracking tracking, + public IssueTrackingDecorator(IssueCache issueCache, InitialOpenIssuesStack initialOpenIssues, IssueTracking tracking, IssueFilters filters, IssueHandlers handlers, IssueWorkflow workflow, - Project project) { - this.scanIssues = scanIssues; + Project project, ResourcePerspectives perspectives, + RuleFinder ruleFinder) { + this.issueCache = issueCache; this.initialOpenIssues = initialOpenIssues; this.tracking = tracking; this.filters = filters; this.handlers = handlers; this.workflow = workflow; this.changeContext = IssueChangeContext.createScan(project.getAnalysisDate()); + this.perspectives = perspectives; + this.ruleFinder = ruleFinder; } public boolean shouldExecuteOnProject(Project project) { @@ -66,40 +77,88 @@ public class IssueTrackingDecorator implements Decorator { } public void decorate(Resource resource, DecoratorContext context) { - if (canHaveIssues(resource)) { - // all the issues created by rule engines during this module scan - Collection issues = Lists.newArrayList(); - for (Issue issue : scanIssues.issues(resource.getEffectiveKey())) { - scanIssues.remove(issue); - if (filters.accept(issue)) { - issues.add((DefaultIssue) issue); - } - } + Issuable issuable = perspectives.as(Issuable.class, resource); + if (issuable != null) { + doDecorate(resource); - // all the issues that are open in db before starting this module scan - Collection dbOpenIssues = initialOpenIssues.selectAndRemove(resource.getId()); - Set unmatchedDbIssues = tracking.track(resource, dbOpenIssues, issues); - // TODO register manual issues (isAlive=true, isNew=false) ? Or are they included in unmatchedDbIssues ? - addUnmatched(unmatchedDbIssues, issues); + } + } - if (ResourceUtils.isProject(resource)) { - // issues that relate to deleted components - addDead(issues); + @VisibleForTesting + void doDecorate(Resource resource) { + Collection issues = Lists.newArrayList(); + for (Issue issue : issueCache.byComponent(resource.getEffectiveKey())) { + issueCache.remove(issue); + if (filters.accept(issue)) { + issues.add((DefaultIssue) issue); } + } + // issues = all the issues created by rule engines during this module scan and not excluded by filters + + // all the issues that are not closed in db before starting this module scan, including manual issues + Collection dbOpenIssues = initialOpenIssues.selectAndRemove(resource.getId()); + + IssueTrackingResult trackingResult = tracking.track(resource, dbOpenIssues, issues); + + // unmatched = issues that have been resolved + issues on disabled/removed rules + manual issues + addUnmatched(trackingResult.unmatched(), issues); + + mergeMatched(trackingResult); + + if (ResourceUtils.isProject(resource)) { + // issues that relate to deleted components + addDead(issues); + } - for (DefaultIssue issue : issues) { - workflow.doAutomaticTransition(issue, changeContext); - handlers.execute(issue, changeContext); - scanIssues.addOrUpdate(issue); + for (DefaultIssue issue : issues) { + workflow.doAutomaticTransition(issue, changeContext); + handlers.execute(issue, changeContext); + issueCache.put(issue); + } + } + + private void mergeMatched(IssueTrackingResult result) { + for (DefaultIssue issue : result.matched()) { + IssueDto ref = result.matching(issue); + + issue.setKey(ref.getKee()); + if (ref.isManualSeverity()) { + issue.setManualSeverity(true); + issue.setSeverity(ref.getSeverity()); + } else if (!Objects.equal(ref.getSeverity(), issue.severity())) { + // TODO register diff + } + issue.setResolution(ref.getResolution()); + issue.setStatus(ref.getStatus()); + issue.setNew(false); + issue.setAlive(true); + issue.setAssignee(ref.getAssignee()); + issue.setAuthorLogin(ref.getAuthorLogin()); + issue.setAssignee(ref.getAssignee()); + if (ref.getAttributes() != null) { + //FIXME do not override new attributes + issue.setAttributes(KeyValueFormat.parse(ref.getAttributes())); } + issue.setTechnicalCreationDate(ref.getCreatedAt()); + issue.setTechnicalUpdateDate(ref.getUpdatedAt()); + issue.setCreationDate(ref.getIssueCreationDate()); + // FIXME issue.setUpdateDate(project.getAnalysisDate()); + + // should be null + issue.setCloseDate(ref.getIssueCloseDate()); } } - private void addUnmatched(Set unmatchedDbIssues, Collection issues) { - for (IssueDto unmatchedDto : unmatchedDbIssues) { + private void addUnmatched(Collection unmatchedIssues, Collection issues) { + for (IssueDto unmatchedDto : unmatchedIssues) { DefaultIssue unmatched = unmatchedDto.toDefaultIssue(); - unmatched.setAlive(false); unmatched.setNew(false); + + Rule rule = ruleFinder.findByKey(unmatched.ruleKey()); + boolean manualIssue = !Strings.isNullOrEmpty(unmatched.reporter()); + boolean onExistingRule = rule != null && !Rule.STATUS_REMOVED.equals(rule.getStatus()); + unmatched.setAlive(manualIssue && onExistingRule); + issues.add(unmatched); } } @@ -112,9 +171,4 @@ public class IssueTrackingDecorator implements Decorator { issues.add(dead); } } - - private boolean canHaveIssues(Resource resource) { - // TODO check existence of perspective Issuable ? - return Scopes.isHigherThanOrEquals(resource.getScope(), Scopes.FILE); - } } diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingResult.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingResult.java new file mode 100644 index 00000000000..08e8c39b51c --- /dev/null +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingResult.java @@ -0,0 +1,69 @@ +/* + * SonarQube, open source software quality management tool. + * Copyright (C) 2008-2013 SonarSource + * mailto:contact AT sonarsource DOT com + * + * SonarQube 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. + * + * SonarQube 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 this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA. + */ +package org.sonar.plugins.core.issue; + +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Maps; +import com.google.common.collect.Multimap; +import com.google.common.collect.Sets; +import org.sonar.api.rule.RuleKey; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.db.IssueDto; + +import java.util.Collection; +import java.util.HashSet; +import java.util.IdentityHashMap; + +class IssueTrackingResult { + private final HashSet unmatched = Sets.newHashSet(); + private final Multimap unmatchedByRule = LinkedHashMultimap.create(); + private final IdentityHashMap matched = Maps.newIdentityHashMap(); + + Collection unmatched() { + return unmatched; + } + + Collection unmatchedForRule(RuleKey ruleKey) { + return unmatchedByRule.get(ruleKey); + } + + Collection matched() { + return matched.keySet(); + } + + boolean isMatched(DefaultIssue issue) { + return matched.containsKey(issue); + } + + IssueDto matching(DefaultIssue issue) { + return matched.get(issue); + } + + void addUnmatched(IssueDto i) { + unmatched.add(i); + unmatchedByRule.put(RuleKey.of(i.getRuleRepo(), i.getRule()), i); + } + + void setMatch(DefaultIssue issue, IssueDto matching) { + matched.put(issue, matching); + unmatchedByRule.remove(RuleKey.of(matching.getRuleRepo(), matching.getRule()), matching); + unmatched.remove(matching); + } +} diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java index 8aa8d5f31fa..dc8053e8712 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/InitialOpenIssuesSensorTest.java @@ -46,7 +46,7 @@ public class InitialOpenIssuesSensorTest { project.setId(1); sensor.analyse(project, null); - verify(issueDao).selectOpenIssues(1); + verify(issueDao).selectNonClosedIssuesByRootComponent(1); verify(stack).setIssues(anyListOf(IssueDto.class), any(Date.class)); } } diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java index 0b74ff29986..dba4a9c6133 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java @@ -19,16 +19,16 @@ */ package org.sonar.plugins.core.issue; -import com.google.common.collect.Sets; import org.junit.Before; import org.junit.Test; import org.mockito.ArgumentMatcher; -import org.mockito.Mockito; import org.sonar.api.batch.DecoratorContext; +import org.sonar.api.component.ResourcePerspectives; import org.sonar.api.resources.File; import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; -import org.sonar.batch.issue.ScanIssues; +import org.sonar.api.rules.RuleFinder; +import org.sonar.batch.issue.IssueCache; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueChangeContext; import org.sonar.core.issue.db.IssueDto; @@ -44,19 +44,27 @@ import static org.mockito.Mockito.*; public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { IssueTrackingDecorator decorator; - ScanIssues scanIssues = mock(ScanIssues.class); + IssueCache issueCache = mock(IssueCache.class); InitialOpenIssuesStack initialOpenIssues = mock(InitialOpenIssuesStack.class); - IssueTracking tracking = mock(IssueTracking.class); + IssueTracking tracking = mock(IssueTracking.class, RETURNS_MOCKS); IssueFilters filters = mock(IssueFilters.class); IssueHandlers handlers = mock(IssueHandlers.class); IssueWorkflow workflow = mock(IssueWorkflow.class); + ResourcePerspectives perspectives = mock(ResourcePerspectives.class); Date loadedDate = new Date(); + RuleFinder ruleFinder = mock(RuleFinder.class); @Before public void init() { when(initialOpenIssues.getLoadedDate()).thenReturn(loadedDate); - decorator = new IssueTrackingDecorator(scanIssues, initialOpenIssues, tracking, - filters, handlers, workflow, mock(Project.class)); + decorator = new IssueTrackingDecorator( + issueCache, + initialOpenIssues, + tracking, + filters, handlers, workflow, + mock(Project.class), + perspectives, + ruleFinder); } @Test @@ -77,7 +85,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { public void should_not_be_executed_on_classes_not_methods() throws Exception { DecoratorContext context = mock(DecoratorContext.class); decorator.decorate(JavaClass.create("org.foo.Bar"), context); - verifyZeroInteractions(context, scanIssues, tracking, filters, handlers, workflow); + verifyZeroInteractions(context, issueCache, tracking, filters, handlers, workflow); } @Test @@ -86,12 +94,12 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { final DefaultIssue issue = new DefaultIssue(); // INPUT : one issue, no open issues during previous scan, no filtering - when(scanIssues.issues("struts:Action.java")).thenReturn(Arrays.asList(issue)); + when(issueCache.byComponent("struts:Action.java")).thenReturn(Arrays.asList(issue)); when(filters.accept(issue)).thenReturn(true); List dbIssues = Collections.emptyList(); when(initialOpenIssues.selectAndRemove(123)).thenReturn(dbIssues); - decorator.decorate(file, mock(DecoratorContext.class, Mockito.RETURNS_MOCKS)); + decorator.doDecorate(file); // Apply filters, track, apply transitions, notify extensions then update cache verify(filters).accept(issue); @@ -104,7 +112,7 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { })); verify(workflow).doAutomaticTransition(eq(issue), any(IssueChangeContext.class)); verify(handlers).execute(eq(issue), any(IssueChangeContext.class)); - verify(scanIssues).addOrUpdate(issue); + verify(issueCache).put(issue); } @Test @@ -114,19 +122,22 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { DefaultIssue openIssue = new DefaultIssue(); // INPUT : one issue, one open issue during previous scan, no filtering - when(scanIssues.issues("struts:Action.java")).thenReturn(Arrays.asList(openIssue)); + when(issueCache.byComponent("struts:Action.java")).thenReturn(Arrays.asList(openIssue)); when(filters.accept(openIssue)).thenReturn(true); IssueDto unmatchedIssue = new IssueDto().setKee("ABCDE").setResolution("OPEN").setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle"); - List unmatchedIssues = Arrays.asList(unmatchedIssue); - when(tracking.track(eq(file), anyCollection(), anyCollection())).thenReturn(Sets.newHashSet(unmatchedIssues)); - decorator.decorate(file, mock(DecoratorContext.class, Mockito.RETURNS_MOCKS)); + IssueTrackingResult trackingResult = new IssueTrackingResult(); + trackingResult.addUnmatched(unmatchedIssue); + + when(tracking.track(eq(file), anyCollection(), anyCollection())).thenReturn(trackingResult); + + decorator.doDecorate(file); verify(workflow, times(2)).doAutomaticTransition(any(DefaultIssue.class), any(IssueChangeContext.class)); verify(handlers, times(2)).execute(any(DefaultIssue.class), any(IssueChangeContext.class)); - verify(scanIssues, times(2)).addOrUpdate(any(DefaultIssue.class)); + verify(issueCache, times(2)).put(any(DefaultIssue.class)); - verify(scanIssues).addOrUpdate(argThat(new ArgumentMatcher() { + verify(issueCache).put(argThat(new ArgumentMatcher() { @Override public boolean matches(Object o) { DefaultIssue issue = (DefaultIssue) o; @@ -139,19 +150,19 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { public void should_register_issues_on_deleted_components() throws Exception { Project project = new Project("struts"); DefaultIssue openIssue = new DefaultIssue(); - when(scanIssues.issues("struts")).thenReturn(Arrays.asList(openIssue)); + when(issueCache.byComponent("struts")).thenReturn(Arrays.asList(openIssue)); when(filters.accept(openIssue)).thenReturn(true); IssueDto deadIssue = new IssueDto().setKee("ABCDE").setResolution("OPEN").setStatus("OPEN").setRuleKey_unit_test_only("squid", "AvoidCycle"); when(initialOpenIssues.getAllIssues()).thenReturn(Arrays.asList(deadIssue)); - decorator.decorate(project, mock(DecoratorContext.class, Mockito.RETURNS_MOCKS)); + decorator.doDecorate(project); // the dead issue must be closed -> apply automatic transition, notify handlers and add to cache verify(workflow, times(2)).doAutomaticTransition(any(DefaultIssue.class), any(IssueChangeContext.class)); verify(handlers, times(2)).execute(any(DefaultIssue.class), any(IssueChangeContext.class)); - verify(scanIssues, times(2)).addOrUpdate(any(DefaultIssue.class)); + verify(issueCache, times(2)).put(any(DefaultIssue.class)); - verify(scanIssues).addOrUpdate(argThat(new ArgumentMatcher() { + verify(issueCache).put(argThat(new ArgumentMatcher() { @Override public boolean matches(Object o) { DefaultIssue dead = (DefaultIssue) o; diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java index 9a81055c54a..ea72dc39bbe 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java @@ -26,9 +26,8 @@ import org.junit.Before; import org.junit.Test; import org.sonar.api.issue.Issue; import org.sonar.api.resources.Project; +import org.sonar.api.resources.Resource; import org.sonar.api.rule.RuleKey; -import org.sonar.api.rules.Rule; -import org.sonar.api.rules.RuleFinder; import org.sonar.api.utils.DateUtils; import org.sonar.batch.scan.LastSnapshots; import org.sonar.core.issue.DefaultIssue; @@ -37,7 +36,6 @@ import org.sonar.core.issue.db.IssueDto; import java.io.IOException; import java.util.Arrays; import java.util.Date; -import java.util.Map; import static com.google.common.collect.Lists.newArrayList; import static org.fest.assertions.Assertions.assertThat; @@ -49,62 +47,44 @@ public class IssueTrackingTest { private final Date analysisDate = DateUtils.parseDate("2013-04-11"); IssueTracking tracking; - Project project; - RuleFinder ruleFinder; + Resource project; LastSnapshots lastSnapshots; long violationId = 0; @Before public void before() { - Rule rule1 = Rule.create("squid", "AvoidCycle"); - rule1.setId(1); - Rule rule2 = Rule.create("squid", "NullDeref"); - rule2.setId(2); - Rule rule3 = Rule.create("pmd", "UnusedLocalVariable"); - rule3.setId(3); - - ruleFinder = mock(RuleFinder.class); - when(ruleFinder.findById(1)).thenReturn(rule1); - when(ruleFinder.findById(2)).thenReturn(rule2); - when(ruleFinder.findById(3)).thenReturn(rule3); - when(ruleFinder.findByKey(RuleKey.of("squid", "AvoidCycle"))).thenReturn(rule1); - when(ruleFinder.findByKey(RuleKey.of("squid", "NullDeref"))).thenReturn(rule2); - when(ruleFinder.findByKey(RuleKey.of("pmd", "UnusedLocalVariable"))).thenReturn(rule3); - lastSnapshots = mock(LastSnapshots.class); project = mock(Project.class); - when(project.getAnalysisDate()).thenReturn(analysisDate); - tracking = new IssueTracking(project, ruleFinder, lastSnapshots, null); + tracking = new IssueTracking(lastSnapshots, null); } @Test public void key_should_be_the_prioritary_field_to_check() { - IssueDto referenceIssue1 = newReferenceIssue("message", 10, 1, "checksum1").setKee("100"); - IssueDto referenceIssue2 = newReferenceIssue("message", 10, 1, "checksum2").setKee("200"); + IssueDto referenceIssue1 = newReferenceIssue("message", 10, "squid", "AvoidCycle", "checksum1").setKee("100"); + IssueDto referenceIssue2 = newReferenceIssue("message", 10, "squid", "AvoidCycle", "checksum2").setKee("200"); // exactly the fields of referenceIssue1 but not the same key DefaultIssue newIssue = newDefaultIssue("message", 10, RuleKey.of("squid", "AvoidCycle"), "checksum1").setKey("200"); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue1, referenceIssue2)); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue1, referenceIssue2), null, null, result); // same key - assertThat(tracking.getReferenceIssue(newIssue)).isSameAs(referenceIssue2); - assertThat(newIssue.isNew()).isFalse(); + assertThat(result.matching(newIssue)).isSameAs(referenceIssue2); } @Test public void checksum_should_have_greater_priority_than_line() { - IssueDto referenceIssue1 = newReferenceIssue("message", 1, 1, "checksum1"); - IssueDto referenceIssue2 = newReferenceIssue("message", 3, 1, "checksum2"); + IssueDto referenceIssue1 = newReferenceIssue("message", 1, "squid", "AvoidCycle", "checksum1"); + IssueDto referenceIssue2 = newReferenceIssue("message", 3, "squid", "AvoidCycle", "checksum2"); DefaultIssue newIssue1 = newDefaultIssue("message", 3, RuleKey.of("squid", "AvoidCycle"), "checksum1"); DefaultIssue newIssue2 = newDefaultIssue("message", 5, RuleKey.of("squid", "AvoidCycle"), "checksum2"); - tracking.mapIssues(newArrayList(newIssue1, newIssue2), newArrayList(referenceIssue1, referenceIssue2)); - assertThat(tracking.getReferenceIssue(newIssue1)).isSameAs(referenceIssue1); - assertThat(newIssue1.isNew()).isFalse(); - assertThat(tracking.getReferenceIssue(newIssue2)).isSameAs(referenceIssue2); - assertThat(newIssue2.isNew()).isFalse(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue1, newIssue2), newArrayList(referenceIssue1, referenceIssue2), null, null, result); + assertThat(result.matching(newIssue1)).isSameAs(referenceIssue1); + assertThat(result.matching(newIssue2)).isSameAs(referenceIssue2); } /** @@ -113,51 +93,51 @@ public class IssueTrackingTest { @Test public void same_rule_and_null_line_and_checksum_but_different_messages() { DefaultIssue newIssue = newDefaultIssue("new message", null, RuleKey.of("squid", "AvoidCycle"), "checksum1"); - IssueDto referenceIssue = newReferenceIssue("old message", null, 1, "checksum1"); + IssueDto referenceIssue = newReferenceIssue("old message", null, "squid", "AvoidCycle", "checksum1"); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isSameAs(referenceIssue); - assertThat(newIssue.isNew()).isFalse(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void same_rule_and_line_and_checksum_but_different_messages() { DefaultIssue newIssue = newDefaultIssue("new message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); - IssueDto referenceIssue = newReferenceIssue("old message", 1, 1, "checksum1"); + IssueDto referenceIssue = newReferenceIssue("old message", 1, "squid", "AvoidCycle", "checksum1"); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isSameAs(referenceIssue); - assertThat(newIssue.isNew()).isFalse(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void same_rule_and_line_message() { DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); - IssueDto referenceIssue = newReferenceIssue("message", 1, 1, "checksum2"); + IssueDto referenceIssue = newReferenceIssue("message", 1, "squid", "AvoidCycle", "checksum2"); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isSameAs(referenceIssue); - assertThat(newIssue.isNew()).isFalse(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void should_ignore_reference_measure_without_checksum() { DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), null); - IssueDto referenceIssue = newReferenceIssue("message", 1, 2, null); + IssueDto referenceIssue = newReferenceIssue("message", 1, "squid", "NullDeref", null); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isNull(); - assertThat(newIssue.isNew()).isTrue(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isNull(); } @Test public void same_rule_and_message_and_checksum_but_different_line() { DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); - IssueDto referenceIssue = newReferenceIssue("message", 2, 1, "checksum1"); + IssueDto referenceIssue = newReferenceIssue("message", 2, "squid", "AvoidCycle", "checksum1"); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isSameAs(referenceIssue); - assertThat(newIssue.isNew()).isFalse(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } /** @@ -166,31 +146,31 @@ public class IssueTrackingTest { @Test public void same_checksum_and_rule_but_different_line_and_different_message() { DefaultIssue newIssue = newDefaultIssue("new message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); - IssueDto referenceIssue = newReferenceIssue("old message", 2, 1, "checksum1"); + IssueDto referenceIssue = newReferenceIssue("old message", 2, "squid", "AvoidCycle", "checksum1"); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isSameAs(referenceIssue); - assertThat(newIssue.isNew()).isFalse(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test public void should_create_new_issue_when_same_rule_same_message_but_different_line_and_checksum() { DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); - IssueDto referenceIssue = newReferenceIssue("message", 2, 1, "checksum2"); + IssueDto referenceIssue = newReferenceIssue("message", 2, "squid", "AvoidCycle", "checksum2"); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isNull(); - assertThat(newIssue.isNew()).isTrue(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isNull(); } @Test public void should_not_track_issue_if_different_rule() { DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); - IssueDto referenceIssue = newReferenceIssue("message", 1, 2, "checksum1"); + IssueDto referenceIssue = newReferenceIssue("message", 1, "squid", "NullDeref", "checksum1"); - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isNull(); - assertThat(newIssue.isNew()).isTrue(); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isNull(); } @Test @@ -198,37 +178,11 @@ public class IssueTrackingTest { // issue messages are trimmed and can be abbreviated when persisted in database. // Comparing issue messages must use the same format. DefaultIssue newIssue = newDefaultIssue(" message ", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); - IssueDto referenceIssue = newReferenceIssue("message", 1, 1, "checksum2"); - - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(tracking.getReferenceIssue(newIssue)).isSameAs(referenceIssue); - assertThat(newIssue.isNew()).isFalse(); - } - - @Test - public void should_set_severity_if_severity_has_been_changed_by_user() { - DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum").setSeverity("MAJOR"); - IssueDto referenceIssue = newReferenceIssue("message", 1, 1, "checksum").setSeverity("MINOR").setManualSeverity(true); - - tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(newIssue.severity()).isEqualTo("MINOR"); - } + IssueDto referenceIssue = newReferenceIssue("message", 1, "squid", "AvoidCycle", "checksum2"); - @Test - public void should_copy_some_fields_when_not_new() { - DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum"); - IssueDto referenceIssue = newReferenceIssue("", 1, 1, "checksum").setAuthorLogin("arthur").setAssignee("perceval"); - Date referenceDate = DateUtils.parseDate("2009-05-18"); - referenceIssue.setIssueCreationDate(referenceDate); - assertThat(newIssue.creationDate()).isNull(); - - Map mapping = tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue)); - assertThat(mapping.size()).isEqualTo(1); - assertThat(newIssue.isNew()).isFalse(); - - assertThat(newIssue.creationDate()).isEqualTo(referenceDate); - assertThat(newIssue.assignee()).isEqualTo("perceval"); - assertThat(newIssue.authorLogin()).isEqualTo("arthur"); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), null, null, result); + assertThat(result.matching(newIssue)).isSameAs(referenceIssue); } @Test @@ -237,16 +191,12 @@ public class IssueTrackingTest { String source = load("example2-v2"); DefaultIssue newIssue = newDefaultIssue("Indentation", 9, RuleKey.of("squid", "AvoidCycle"), "foo"); - IssueDto referenceIssue = newReferenceIssue("2 branches need to be covered", null, 1, null); - + IssueDto referenceIssue = newReferenceIssue("2 branches need to be covered", null, "squid", "AvoidCycle", null); - Map mapping = tracking.mapIssues( - newArrayList(newIssue), - newArrayList(referenceIssue), - source, project); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), source, project, result); - assertThat(mapping.isEmpty()).isTrue(); - assertThat(newIssue.isNew()).isTrue(); + assertThat(result.matched()).isEmpty(); } @Test @@ -255,15 +205,12 @@ public class IssueTrackingTest { String source = load("example2-v2"); DefaultIssue newIssue = newDefaultIssue("1 branch need to be covered", null, RuleKey.of("squid", "AvoidCycle"), "foo"); - IssueDto referenceIssue = newReferenceIssue("Indentationd", 7, 1, null); + IssueDto referenceIssue = newReferenceIssue("Indentationd", 7, "squid", "AvoidCycle", null); - Map mapping = tracking.mapIssues( - newArrayList(newIssue), - newArrayList(referenceIssue), - source, project); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), source, project, result); - assertThat(mapping.isEmpty()).isTrue(); - assertThat(newIssue.isNew()).isTrue(); + assertThat(result.matched()).isEmpty(); } /** @@ -275,15 +222,12 @@ public class IssueTrackingTest { String source = load("example2-v2"); DefaultIssue newIssue = newDefaultIssue("1 branch need to be covered", null, RuleKey.of("squid", "AvoidCycle"), null); - IssueDto referenceIssue = newReferenceIssue("2 branches need to be covered", null, 1, null); + IssueDto referenceIssue = newReferenceIssue("2 branches need to be covered", null, "squid", "AvoidCycle", null); - Map mapping = tracking.mapIssues( - newArrayList(newIssue), - newArrayList(referenceIssue), - source, project); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(newArrayList(newIssue), newArrayList(referenceIssue), source, project, result); - assertThat(newIssue.isNew()).isFalse(); - assertThat(mapping.get(newIssue)).isEqualTo(referenceIssue); + assertThat(result.matching(newIssue)).isEqualTo(referenceIssue); } /** @@ -294,25 +238,21 @@ public class IssueTrackingTest { when(lastSnapshots.getSource(project)).thenReturn(load("example1-v1")); String source = load("example1-v2"); - IssueDto referenceIssue1 = newReferenceIssue("Indentation", 7, 1, null); - IssueDto referenceIssue2 = newReferenceIssue("Indentation", 11, 1, null); + IssueDto referenceIssue1 = newReferenceIssue("Indentation", 7, "squid", "AvoidCycle", null); + IssueDto referenceIssue2 = newReferenceIssue("Indentation", 11, "squid", "AvoidCycle", null); DefaultIssue newIssue1 = newDefaultIssue("Indentation", 9, RuleKey.of("squid", "AvoidCycle"), null); DefaultIssue newIssue2 = newDefaultIssue("Indentation", 13, RuleKey.of("squid", "AvoidCycle"), null); DefaultIssue newIssue3 = newDefaultIssue("Indentation", 17, RuleKey.of("squid", "AvoidCycle"), null); DefaultIssue newIssue4 = newDefaultIssue("Indentation", 21, RuleKey.of("squid", "AvoidCycle"), null); - Map mapping = tracking.mapIssues( - Arrays.asList(newIssue1, newIssue2, newIssue3, newIssue4), - Arrays.asList(referenceIssue1, referenceIssue2), - source, project); - - assertThat(newIssue1.isNew()).isTrue(); - assertThat(newIssue2.isNew()).isTrue(); - assertThat(newIssue3.isNew()).isFalse(); - assertThat(mapping.get(newIssue3)).isEqualTo(referenceIssue1); - assertThat(newIssue4.isNew()).isFalse(); - assertThat(mapping.get(newIssue4)).isEqualTo(referenceIssue2); + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues(Arrays.asList(newIssue1, newIssue2, newIssue3, newIssue4), Arrays.asList(referenceIssue1, referenceIssue2), source, project, result); + + assertThat(result.matching(newIssue1)).isNull(); + assertThat(result.matching(newIssue2)).isNull(); + assertThat(result.matching(newIssue3)).isSameAs(referenceIssue1); + assertThat(result.matching(newIssue4)).isSameAs(referenceIssue2); } /** @@ -323,21 +263,21 @@ public class IssueTrackingTest { when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); String source = load("example2-v2"); - IssueDto referenceIssue1 = newReferenceIssue("SystemPrintln", 5, 1, null); + IssueDto referenceIssue1 = newReferenceIssue("SystemPrintln", 5, "squid", "AvoidCycle", null); DefaultIssue newIssue1 = newDefaultIssue("SystemPrintln", 6, RuleKey.of("squid", "AvoidCycle"), null); DefaultIssue newIssue2 = newDefaultIssue("SystemPrintln", 10, RuleKey.of("squid", "AvoidCycle"), null); DefaultIssue newIssue3 = newDefaultIssue("SystemPrintln", 14, RuleKey.of("squid", "AvoidCycle"), null); - Map mapping = tracking.mapIssues( + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues( Arrays.asList(newIssue1, newIssue2, newIssue3), Arrays.asList(referenceIssue1), - source, project); + source, project, result); - assertThat(newIssue1.isNew()).isTrue(); - assertThat(newIssue2.isNew()).isFalse(); - assertThat(mapping.get(newIssue2)).isEqualTo(referenceIssue1); - assertThat(newIssue3.isNew()).isTrue(); + assertThat(result.matching(newIssue1)).isNull(); + assertThat(result.matching(newIssue2)).isSameAs(referenceIssue1); + assertThat(result.matching(newIssue3)).isNull(); } @Test @@ -345,9 +285,9 @@ public class IssueTrackingTest { when(lastSnapshots.getSource(project)).thenReturn(load("example3-v1")); String source = load("example3-v2"); - IssueDto referenceIssue1 = newReferenceIssue("Avoid unused local variables such as 'j'.", 6, 1, "63c11570fc0a76434156be5f8138fa03"); - IssueDto referenceIssue2 = newReferenceIssue("Avoid unused private methods such as 'myMethod()'.", 13, 2, "ef23288705d1ef1e512448ace287586e"); - IssueDto referenceIssue3 = newReferenceIssue("Method 'avoidUtilityClass' is not designed for extension - needs to be abstract, final or empty.", 9, 3, "ed5cdd046fda82727d6fedd1d8e3a310"); + IssueDto referenceIssue1 = newReferenceIssue("Avoid unused local variables such as 'j'.", 6, "squid", "AvoidCycle", "63c11570fc0a76434156be5f8138fa03"); + IssueDto referenceIssue2 = newReferenceIssue("Avoid unused private methods such as 'myMethod()'.", 13, "squid", "NullDeref", "ef23288705d1ef1e512448ace287586e"); + IssueDto referenceIssue3 = newReferenceIssue("Method 'avoidUtilityClass' is not designed for extension - needs to be abstract, final or empty.", 9, "pmd", "UnusedLocalVariable", "ed5cdd046fda82727d6fedd1d8e3a310"); // New issue DefaultIssue newIssue1 = newDefaultIssue("Avoid unused local variables such as 'msg'.", 18, RuleKey.of("squid", "AvoidCycle"), "a24254126be2bf1a9b9a8db43f633733"); @@ -360,19 +300,17 @@ public class IssueTrackingTest { // Same as referenceIssue1 DefaultIssue newIssue5 = newDefaultIssue("Avoid unused local variables such as 'j'.", 6, RuleKey.of("squid", "AvoidCycle"), "4432a2675ec3e1620daefe38386b51ef"); - Map mapping = tracking.mapIssues( + IssueTrackingResult result = new IssueTrackingResult(); + tracking.mapIssues( Arrays.asList(newIssue1, newIssue2, newIssue3, newIssue4, newIssue5), Arrays.asList(referenceIssue1, referenceIssue2, referenceIssue3), - source, project); - - assertThat(newIssue1.isNew()).isTrue(); - assertThat(newIssue2.isNew()).isFalse(); - assertThat(newIssue3.isNew()).isFalse(); - assertThat(newIssue4.isNew()).isTrue(); - assertThat(newIssue5.isNew()).isFalse(); - assertThat(mapping.get(newIssue2)).isEqualTo(referenceIssue2); - assertThat(mapping.get(newIssue3)).isEqualTo(referenceIssue3); - assertThat(mapping.get(newIssue5)).isEqualTo(referenceIssue1); + source, project, result); + + assertThat(result.matching(newIssue1)).isNull(); + assertThat(result.matching(newIssue2)).isSameAs(referenceIssue2); + assertThat(result.matching(newIssue3)).isSameAs(referenceIssue3); + assertThat(result.matching(newIssue4)).isNull(); + assertThat(result.matching(newIssue5)).isSameAs(referenceIssue1); } private static String load(String name) throws IOException { @@ -383,18 +321,17 @@ public class IssueTrackingTest { return new DefaultIssue().setMessage(message).setLine(line).setRuleKey(ruleKey).setChecksum(checksum).setStatus(Issue.STATUS_OPEN); } - private IssueDto newReferenceIssue(String message, Integer lineId, int ruleId, String lineChecksum) { + private IssueDto newReferenceIssue(String message, Integer lineId, String ruleRepo, String ruleKey, String lineChecksum) { IssueDto referenceIssue = new IssueDto(); Long id = violationId++; referenceIssue.setId(id); referenceIssue.setKee(Long.toString(id)); referenceIssue.setLine(lineId); referenceIssue.setMessage(message); - referenceIssue.setRuleId(ruleId); + referenceIssue.setRuleKey_unit_test_only(ruleRepo, ruleKey); referenceIssue.setChecksum(lineChecksum); referenceIssue.setResolution(null); referenceIssue.setStatus(Issue.STATUS_OPEN); return referenceIssue; } - } diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java b/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java index 6c0b44c03d9..151e1783b3e 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/DefaultIssuable.java @@ -33,11 +33,13 @@ import java.util.Collection; public class DefaultIssuable implements Issuable { private final ScanIssues scanIssues; + private final IssueCache cache; private final Component component; - DefaultIssuable(Component component, ScanIssues scanIssues) { + DefaultIssuable(Component component, ScanIssues scanIssues, IssueCache cache) { this.component = component; this.scanIssues = scanIssues; + this.cache = cache; } @Override @@ -53,7 +55,7 @@ public class DefaultIssuable implements Issuable { @SuppressWarnings("unchecked") @Override public Collection issues() { - return (Collection)scanIssues.issues(component.key()); + return (Collection)cache.byComponent(component.key()); } @Override diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java index 8ad43865f77..3e13158a761 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuableFactory.java @@ -34,10 +34,12 @@ import javax.annotation.CheckForNull; public class IssuableFactory extends PerspectiveBuilder { private final ScanIssues scanIssues; + private final IssueCache cache; - public IssuableFactory(ScanIssues scanIssues) { + public IssuableFactory(ScanIssues scanIssues, IssueCache cache) { super(Issuable.class); this.scanIssues = scanIssues; + this.cache = cache; } @CheckForNull @@ -47,6 +49,6 @@ public class IssuableFactory extends PerspectiveBuilder { if (component instanceof ResourceComponent) { supported = Scopes.isHigherThanOrEquals(((ResourceComponent) component).scope(), Scopes.FILE); } - return supported ? new DefaultIssuable(component, scanIssues) : null; + return supported ? new DefaultIssuable(component, scanIssues, cache) : null; } } diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssues.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssues.java index bb9b10e62a9..ab06a7f9cca 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssues.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssues.java @@ -19,17 +19,12 @@ */ package org.sonar.batch.issue; -import com.google.common.base.Preconditions; -import com.google.common.base.Strings; import org.sonar.api.BatchComponent; -import org.sonar.api.issue.Issue; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.resources.Project; import org.sonar.api.rules.ActiveRule; import org.sonar.core.issue.DefaultIssue; -import java.util.Collection; - /** * Central component to manage issues */ @@ -45,20 +40,6 @@ public class ScanIssues implements BatchComponent { this.project = project; } - public Collection issues(String componentKey) { - return cache.byComponent(componentKey); - } - - public Collection issues() { - return cache.all(); - } - - public ScanIssues addOrUpdate(DefaultIssue issue) { - Preconditions.checkState(!Strings.isNullOrEmpty(issue.key()), "Missing issue key"); - cache.put(issue); - return this; - } - public boolean initAndAddIssue(DefaultIssue issue) { ActiveRule activeRule = qProfile.getActiveRule(issue.ruleKey().repository(), issue.ruleKey().rule()); if (activeRule == null || activeRule.getRule() == null) { @@ -75,8 +56,4 @@ public class ScanIssues implements BatchComponent { return true; } - public boolean remove(Issue issue) { - return cache.remove(issue); - } - } diff --git a/sonar-batch/src/main/java/org/sonar/batch/phases/PhaseExecutor.java b/sonar-batch/src/main/java/org/sonar/batch/phases/PhaseExecutor.java index f12c4dfd6a5..3a85c4c42bb 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/phases/PhaseExecutor.java +++ b/sonar-batch/src/main/java/org/sonar/batch/phases/PhaseExecutor.java @@ -121,7 +121,7 @@ public final class PhaseExecutor { persistenceManager.setDelayedMode(false); if (module.isRoot()) { - sonarReport.execute(sensorContext); + sonarReport.execute(); LOGGER.info("Store results in database"); for (ScanPersister persister : persisters) { diff --git a/sonar-batch/src/main/java/org/sonar/batch/report/SonarReport.java b/sonar-batch/src/main/java/org/sonar/batch/report/SonarReport.java index f02a8c466be..d6e24c7aa63 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/report/SonarReport.java +++ b/sonar-batch/src/main/java/org/sonar/batch/report/SonarReport.java @@ -35,6 +35,7 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.scan.filesystem.ModuleFileSystem; import org.sonar.api.utils.DateUtils; import org.sonar.api.utils.SonarException; +import org.sonar.batch.issue.IssueCache; import org.sonar.batch.issue.ScanIssues; import org.sonar.core.i18n.RuleI18nManager; import org.sonar.core.issue.DefaultIssue; @@ -58,17 +59,17 @@ public class SonarReport implements BatchComponent { private final ModuleFileSystem fileSystem; private final Server server; private final RuleI18nManager ruleI18nManager; - private final ScanIssues scanIssues; + private final IssueCache issueCache; - public SonarReport(Settings settings, ModuleFileSystem fileSystem, Server server, RuleI18nManager ruleI18nManager, ScanIssues scanIssues) { + public SonarReport(Settings settings, ModuleFileSystem fileSystem, Server server, RuleI18nManager ruleI18nManager, IssueCache issueCache) { this.settings = settings; this.fileSystem = fileSystem; this.server = server; this.ruleI18nManager = ruleI18nManager; - this.scanIssues = scanIssues; + this.issueCache = issueCache; } - public void execute(SensorContext context) { + public void execute() { if (settings.getBoolean(CoreProperties.DRY_RUN)) { exportResults(); } @@ -178,6 +179,6 @@ public class SonarReport implements BatchComponent { @VisibleForTesting Collection getIssues() { - return scanIssues.issues(); + return issueCache.all(); } } diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java index 16358a6546f..f661546cb32 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/IssuableFactoryTest.java @@ -34,10 +34,11 @@ import static org.mockito.Mockito.mock; public class IssuableFactoryTest { ScanIssues issueFactory = mock(ScanIssues.class); + IssueCache cache = mock(IssueCache.class); @Test public void file_should_be_issuable() throws Exception { - IssuableFactory factory = new IssuableFactory(issueFactory); + IssuableFactory factory = new IssuableFactory(issueFactory, cache); Component component = new ResourceComponent(new File("foo/bar.c")); Issuable issuable = factory.loadPerspective(Issuable.class, component); @@ -48,7 +49,7 @@ public class IssuableFactoryTest { @Test public void project_should_be_issuable() throws Exception { - IssuableFactory factory = new IssuableFactory(issueFactory); + IssuableFactory factory = new IssuableFactory(issueFactory, cache); Component component = new ResourceComponent(new Project("Foo")); Issuable issuable = factory.loadPerspective(Issuable.class, component); @@ -59,7 +60,7 @@ public class IssuableFactoryTest { @Test public void java_file_should_be_issuable() throws Exception { - IssuableFactory factory = new IssuableFactory(issueFactory); + IssuableFactory factory = new IssuableFactory(issueFactory, cache); Component component = new ResourceComponent(new JavaFile("bar.Foo")); Issuable issuable = factory.loadPerspective(Issuable.class, component); @@ -70,7 +71,7 @@ public class IssuableFactoryTest { @Test public void java_class_should_not_be_issuable() throws Exception { - IssuableFactory factory = new IssuableFactory(issueFactory); + IssuableFactory factory = new IssuableFactory(issueFactory, cache); Component component = new ResourceComponent(JavaClass.create("bar", "Foo")); Issuable issuable = factory.loadPerspective(Issuable.class, component); diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssuesTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssuesTest.java index c19ae73fc02..730a0508faf 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssuesTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssuesTest.java @@ -42,12 +42,6 @@ public class ScanIssuesTest { Project project = mock(Project.class); ScanIssues scanIssues = new ScanIssues(qProfile, cache, project); - @Test - public void should_get_issues() throws Exception { - scanIssues.issues("key"); - verify(cache).byComponent("key"); - } - @Test public void should_ignore_null_active_rule() throws Exception { when(qProfile.getActiveRule(anyString(), anyString())).thenReturn(null); diff --git a/sonar-batch/src/test/java/org/sonar/batch/report/SonarReportTest.java b/sonar-batch/src/test/java/org/sonar/batch/report/SonarReportTest.java index 55253b35fb6..b0f3aed03ae 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/report/SonarReportTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/report/SonarReportTest.java @@ -27,7 +27,6 @@ import org.junit.Before; import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.sonar.api.CoreProperties; -import org.sonar.api.batch.SensorContext; import org.sonar.api.config.Settings; import org.sonar.api.issue.Issue; import org.sonar.api.platform.Server; @@ -36,7 +35,7 @@ import org.sonar.api.rule.RuleKey; import org.sonar.api.rules.Rule; import org.sonar.api.scan.filesystem.ModuleFileSystem; import org.sonar.api.utils.DateUtils; -import org.sonar.batch.issue.ScanIssues; +import org.sonar.batch.issue.IssueCache; import org.sonar.core.i18n.RuleI18nManager; import org.sonar.core.issue.DefaultIssue; @@ -54,13 +53,12 @@ public class SonarReportTest { @org.junit.Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); SonarReport sonarReport; - SensorContext sensorContext = mock(SensorContext.class); Resource resource = mock(Resource.class); ModuleFileSystem fileSystem = mock(ModuleFileSystem.class); Server server = mock(Server.class); RuleI18nManager ruleI18nManager = mock(RuleI18nManager.class); Settings settings; - ScanIssues scanIssues = mock(ScanIssues.class); + IssueCache issueCache = mock(IssueCache.class); @Before public void before() { @@ -69,7 +67,7 @@ public class SonarReportTest { settings = new Settings(); settings.setProperty(CoreProperties.DRY_RUN, true); - sonarReport = new SonarReport(settings, fileSystem, server, ruleI18nManager, scanIssues); + sonarReport = new SonarReport(settings, fileSystem, server, ruleI18nManager, issueCache); } @Test @@ -236,7 +234,7 @@ public class SonarReportTest { settings.setProperty("sonar.report.export.path", "output.json"); when(fileSystem.workingDir()).thenReturn(sonarDirectory); - sonarReport.execute(sensorContext); + sonarReport.execute(); assertThat(new File(sonarDirectory, "output.json")).exists(); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java index 7ec803e8cdb..6a67dcf6774 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDao.java @@ -59,11 +59,11 @@ public class IssueDao implements BatchComponent, ServerComponent { } } - // TODO rename selectOpenIssuesByProject. Is it by module or project ?? - public List selectOpenIssues(Integer componentId) { + public List selectNonClosedIssuesByRootComponent(int componentId) { SqlSession session = mybatis.openSession(); try { - return session.selectList("org.sonar.core.issue.db.IssueMapper.selectOpenIssues", componentId); + IssueMapper mapper = session.getMapper(IssueMapper.class); + return mapper.selectNonClosedIssues(componentId); } finally { MyBatis.closeQuietly(session); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java index f31e977f6aa..e7bf13f7e5a 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueDto.java @@ -310,7 +310,7 @@ public final class IssueDto { return ToStringBuilder.reflectionToString(this, ToStringStyle.SHORT_PREFIX_STYLE); } - public static IssueDto toDto(DefaultIssue issue, Integer componentId, Integer ruleId) { + public static IssueDto toDtoForInsert(DefaultIssue issue, Integer componentId, Integer ruleId) { return new IssueDto() .setKee(issue.key()) .setLine(issue.line()) @@ -335,6 +335,29 @@ public final class IssueDto { .setIssueUpdateDate(issue.updateDate()); } + public static IssueDto toDtoForUpdate(DefaultIssue issue) { + // Invariant fields, like key and rule, can't be updated + return new IssueDto() + .setKee(issue.key()) + .setLine(issue.line()) + .setMessage(issue.message()) + .setEffortToFix(issue.effortToFix()) + .setResolution(issue.resolution()) + .setStatus(issue.status()) + .setSeverity(issue.severity()) + .setChecksum(issue.getChecksum()) + .setManualSeverity(issue.manualSeverity()) + .setReporter(issue.reporter()) + .setAssignee(issue.assignee()) + .setActionPlanKey(issue.actionPlanKey()) + .setAttributes(issue.attributes() != null ? KeyValueFormat.format(issue.attributes()) : "") + .setAuthorLogin(issue.authorLogin()) + .setUpdatedAt(issue.technicalUpdateDate()) + .setIssueCreationDate(issue.creationDate()) + .setIssueCloseDate(issue.closeDate()) + .setIssueUpdateDate(issue.updateDate()); + } + public DefaultIssue toDefaultIssue() { DefaultIssue issue = new DefaultIssue(); issue.setKey(kee); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java index cc32f7272fa..478a66da894 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueMapper.java @@ -31,6 +31,8 @@ public interface IssueMapper { List select(IssueQuery query); + List selectNonClosedIssues(int rootComponentId); + void insert(IssueDto issue); int update(IssueDto issue); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java index af308fb0029..8064e7502c3 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/db/IssueStorage.java @@ -54,14 +54,15 @@ public abstract class IssueStorage { List conflicts = Lists.newArrayList(); for (DefaultIssue issue : issues) { - int ruleId = ruleId(issue); - int componentId = componentId(issue); - - IssueDto dto = IssueDto.toDto(issue, componentId, ruleId); if (issue.isNew()) { + int componentId = componentId(issue); + int ruleId = ruleId(issue); + IssueDto dto = IssueDto.toDtoForInsert(issue, componentId, ruleId); issueMapper.insert(dto); + } else /* TODO if hasChanges */ { // TODO manage condition on update date + IssueDto dto = IssueDto.toDtoForUpdate(issue); int count = issueMapper.update(dto); if (count < 1) { conflicts.add(issue); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java b/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java index 3b2f2943ad6..926b769bcb9 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/workflow/IssueWorkflow.java @@ -80,7 +80,7 @@ public class IssueWorkflow implements BatchComponent, ServerComponent, Startable // Close the issues that do not exist anymore. Note that isAlive() is true on manual issues .transition(Transition.builder("automaticclose") .from(Issue.STATUS_OPEN).to(Issue.STATUS_CLOSED) - .conditions(new IsAlive(false), new IsManual(false)) + .conditions(new IsAlive(false)) .functions(new SetResolution(Issue.RESOLUTION_FIXED), new SetCloseDate(true)) .automatic() .build()) diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml index 36ce2d7dde2..40fc177e27d 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/db/IssueMapper.xml @@ -59,10 +59,11 @@ #{issueUpdateDate}, #{issueCloseDate}, #{createdAt}, #{updatedAt}) + update issues set - resource_id=#{resourceId}, - rule_id=#{ruleId}, action_plan_key=#{actionPlanKey}, severity=#{severity}, manual_severity=#{manualSeverity}, @@ -79,7 +80,6 @@ issue_creation_date=#{issueCreationDate}, issue_update_date=#{issueUpdateDate}, issue_close_date=#{issueCloseDate}, - created_at=#{createdAt}, updated_at=#{updatedAt} where kee = #{kee} @@ -91,12 +91,12 @@ where i.kee=#{kee} and i.rule_id=r.id and p.id=i.resource_id - select distinct from issues i, rules r, projects p where i.status <> 'CLOSED' - and (p.root_id=#{componentId} or (p.root_id is null and p.id=#{componentId})) + and (p.root_id=#{id} or (p.root_id is null and p.id=#{id})) and i.resource_id=p.id and r.id=i.rule_id diff --git a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java index 1ac718d52cb..2bc8932b731 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/db/IssueDaoTest.java @@ -246,7 +246,7 @@ public class IssueDaoTest extends AbstractDaoTestCase { public void should_select_open_issues() { setupData("shared", "should_select_open_issues"); - List dtos = dao.selectOpenIssues(400); + List dtos = dao.selectNonClosedIssuesByRootComponent(400); assertThat(dtos).hasSize(2); IssueDto issue = dtos.get(0); diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/testUpdate-result.xml b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/testUpdate-result.xml index 67d9024f5c7..57bc88451ea 100644 --- a/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/testUpdate-result.xml +++ b/sonar-core/src/test/resources/org/sonar/core/issue/db/IssueMapperTest/testUpdate-result.xml @@ -38,7 +38,7 @@ issue_creation_date="2013-05-18" issue_update_date="2013-05-19" issue_close_date="2013-05-20" - created_at="2013-05-21" + created_at="[null]" updated_at="2013-05-22" action_plan_key="current_sprint" />