diff options
43 files changed, 344 insertions, 506 deletions
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 a131eee7848..29477c8fa44 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 @@ -24,8 +24,6 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Objects; import com.google.common.collect.*; import org.apache.commons.lang.StringUtils; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; import org.sonar.api.BatchExtension; import org.sonar.api.batch.SonarIndex; import org.sonar.api.resources.Project; @@ -44,16 +42,9 @@ import java.util.*; public class IssueTracking implements BatchExtension { - private static final Logger LOG = LoggerFactory.getLogger(IssueTracking.class); - private static final Comparator<LinePair> LINE_PAIR_COMPARATOR = new Comparator<LinePair>() { public int compare(LinePair o1, LinePair o2) { - int weightDiff = o2.weight - o1.weight; - if (weightDiff != 0) { - return weightDiff; - } else { - return Math.abs(o1.lineA -o1.lineB) - Math.abs(o2.lineA - o2.lineB); - } + return o2.weight - o1.weight; } }; private final Project project; @@ -77,8 +68,6 @@ public class IssueTracking implements BatchExtension { } public void track(Resource resource, Collection<IssueDto> referenceIssues, Collection<DefaultIssue> newIssues) { - LOG.debug("Tracking : " + resource); - referenceIssuesMap.clear(); String source = index.getSource(resource); @@ -129,13 +118,13 @@ public class IssueTracking implements BatchExtension { unmappedLastIssues.addAll(lastIssues); for (IssueDto lastIssue : lastIssues) { - lastIssuesByRule.put(getRuleId(lastIssue), lastIssue); + lastIssuesByRule.put(getRule(lastIssue), lastIssue); } // Match the key of the issue. (For manual issues) for (DefaultIssue newIssue : newIssues) { mapIssue(newIssue, - findLastIssueWithSameKey(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), + findLastIssueWithSameKey(newIssue, lastIssuesByRule.get(getRule(newIssue))), lastIssuesByRule, referenceIssuesMap); } @@ -143,7 +132,7 @@ public class IssueTracking implements BatchExtension { for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue)) { mapIssue(newIssue, - findLastIssueWithSameLineAndChecksum(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), + findLastIssueWithSameLineAndChecksum(newIssue, lastIssuesByRule.get(getRule(newIssue))), lastIssuesByRule, referenceIssuesMap); } } @@ -191,7 +180,6 @@ 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 - LOG.debug("*** 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); lastIssuesByLines.removeAll(hashOccurrence.lineA); newIssuesByLines.removeAll(hashOccurrence.lineB); @@ -210,7 +198,6 @@ 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 - LOG.debug("*** 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); } } @@ -220,9 +207,8 @@ public class IssueTracking implements BatchExtension { // Try then to match issues on same rule with same message and with same checksum for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue)) { - LOG.debug("*** Try then to match issues on same rule with same message and with same checksum"); mapIssue(newIssue, - findLastIssueWithSameChecksumAndMessage(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), + findLastIssueWithSameChecksumAndMessage(newIssue, lastIssuesByRule.get(getRule(newIssue))), lastIssuesByRule, referenceIssuesMap); } } @@ -230,9 +216,8 @@ public class IssueTracking implements BatchExtension { // Try then to match issues on same rule with same line and with same message for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue)) { - LOG.debug("*** Try then to match issues on same rule with same line and with same message"); mapIssue(newIssue, - findLastIssueWithSameLineAndMessage(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), + findLastIssueWithSameLineAndMessage(newIssue, lastIssuesByRule.get(getRule(newIssue))), lastIssuesByRule, referenceIssuesMap); } } @@ -241,24 +226,33 @@ public class IssueTracking implements BatchExtension { // See SONAR-2812 for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue)) { - LOG.debug("*** Last check: match issue if same rule and same checksum but different line and different message"); mapIssue(newIssue, - findLastIssueWithSameChecksum(newIssue, lastIssuesByRule.get(getRuleId(newIssue))), + findLastIssueWithSameChecksum(newIssue, lastIssuesByRule.get(getRule(newIssue))), lastIssuesByRule, referenceIssuesMap); } } } + @VisibleForTesting + IssueDto getReferenceIssue(DefaultIssue issue) { + return referenceIssuesMap.get(issue); + } + + private Integer getRule(DefaultIssue issue) { + return ruleFinder.findByKey(issue.ruleKey()).getId(); + } + + private Integer getRule(IssueDto issue) { + return ruleFinder.findById(issue.getRuleId()).getId(); + } + private void map(Collection<DefaultIssue> newIssues, Collection<IssueDto> lastIssues, Multimap<Integer, IssueDto> lastIssuesByRule) { for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue)) { for (IssueDto pastIssue : lastIssues) { - if (isNotAlreadyMapped(pastIssue) && Objects.equal(getRuleId(newIssue), getRuleId(pastIssue))) { - LOG.debug("mapIssue newIssue : "+ newIssue + " with : "+ pastIssue); + if (isNotAlreadyMapped(pastIssue) && Objects.equal(getRule(newIssue), getRule(pastIssue))) { mapIssue(newIssue, pastIssue, lastIssuesByRule, referenceIssuesMap); break; - } else { - LOG.debug("Not mapIssue newIssue : "+ newIssue + " with : "+ pastIssue); } } } @@ -331,7 +325,7 @@ public class IssueTracking implements BatchExtension { } private boolean isNotAlreadyMapped(IssueDto pastIssue) { - return unmappedLastIssues.contains(pastIssue); + return !unmappedLastIssues.contains(pastIssue); } private boolean isNotAlreadyMapped(DefaultIssue newIssue) { @@ -339,7 +333,7 @@ public class IssueTracking implements BatchExtension { } private boolean isSameChecksum(DefaultIssue newIssue, IssueDto pastIssue) { - return StringUtils.equals(pastIssue.getChecksum(), newIssue.getChecksum()); + return Objects.equal(pastIssue.getChecksum(), newIssue.getChecksum()); } private boolean isSameLine(DefaultIssue newIssue, IssueDto pastIssue) { @@ -347,7 +341,7 @@ public class IssueTracking implements BatchExtension { } private boolean isSameMessage(DefaultIssue newIssue, IssueDto pastIssue) { - return StringUtils.equals(IssueDto.abbreviateMessage(newIssue.message()), pastIssue.getMessage()); + return Objects.equal(StringUtils.trim(newIssue.description()), StringUtils.trim(pastIssue.getDescription())); } private boolean isSameKey(DefaultIssue newIssue, IssueDto pastIssue) { @@ -356,8 +350,6 @@ public class IssueTracking implements BatchExtension { private void mapIssue(DefaultIssue newIssue, IssueDto pastIssue, Multimap<Integer, IssueDto> lastIssuesByRule, Map<DefaultIssue, IssueDto> issueMap) { if (pastIssue != null) { - LOG.debug("Mapping with old issue from newIssue : "+ newIssue + " and pastIssue : "+ pastIssue); - newIssue.setKey(pastIssue.getUuid()); if (pastIssue.isManualSeverity()) { newIssue.setSeverity(pastIssue.getSeverity()); @@ -370,30 +362,15 @@ public class IssueTracking implements BatchExtension { // TODO // newIssue.setPersonId(pastIssue.getPersonId()); - lastIssuesByRule.remove(getRuleId(newIssue), pastIssue); + lastIssuesByRule.remove(getRule(newIssue), pastIssue); issueMap.put(newIssue, pastIssue); unmappedLastIssues.remove(pastIssue); } else { - LOG.debug("No old issue, creating new one with newIssue : "+ newIssue + " and pastIssue : "+ pastIssue); - newIssue.setNew(true); newIssue.setCreatedAt(project.getAnalysisDate()); } } - @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/IssuesWorkflowDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssuesWorkflowDecorator.java index 26552d58fa5..184e020b9c9 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssuesWorkflowDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssuesWorkflowDecorator.java @@ -32,16 +32,13 @@ 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.api.rules.Rule; import org.sonar.api.rules.RuleFinder; -import org.sonar.api.utils.KeyValueFormat; import org.sonar.batch.issue.InitialOpenIssuesStack; import org.sonar.batch.issue.ModuleIssues; import org.sonar.core.issue.DefaultIssue; import org.sonar.core.issue.IssueDto; import javax.annotation.Nullable; - import java.util.Collection; import java.util.Date; import java.util.Set; @@ -78,14 +75,14 @@ public class IssuesWorkflowDecorator implements Decorator { Set<String> issueKeys = Sets.newHashSet(Collections2.transform(newIssues, new IssueToKeyfunction())); for (IssueDto openIssue : openIssues) { - addManualIssuesAndCloseResolvedOnes(openIssue, resource); - closeResolvedStandardIssues(openIssue, issueKeys, resource); - keepFalsePositiveIssues(openIssue, resource); - reopenUnresolvedIssues(openIssue, resource); + addManualIssuesAndCloseResolvedOnes(openIssue); + closeResolvedStandardIssues(openIssue, issueKeys); + keepFalsePositiveIssues(openIssue); + reopenUnresolvedIssues(openIssue); } if (ResourceUtils.isRootProject(resource)) { - closeIssuesOnDeletedResources(initialOpenIssuesStack.getAllIssues(), resource); + closeIssuesOnDeletedResources(initialOpenIssuesStack.getAllIssues()); } } } @@ -96,9 +93,9 @@ public class IssuesWorkflowDecorator implements Decorator { } } - private void addManualIssuesAndCloseResolvedOnes(IssueDto openIssue, Resource resource) { + private void addManualIssuesAndCloseResolvedOnes(IssueDto openIssue) { if (openIssue.isManualIssue()) { - DefaultIssue issue = toIssue(openIssue, resource); + DefaultIssue issue = openIssue.toDefaultIssue(); if (Issue.STATUS_RESOLVED.equals(issue.status())) { close(issue); } @@ -106,15 +103,15 @@ public class IssuesWorkflowDecorator implements Decorator { } } - private void closeResolvedStandardIssues(IssueDto openIssue, Set<String> issueKeys, Resource resource) { + private void closeResolvedStandardIssues(IssueDto openIssue, Set<String> issueKeys) { if (!openIssue.isManualIssue() && !issueKeys.contains(openIssue.getUuid())) { - closeAndSave(openIssue, resource); + closeAndSave(openIssue); } } - private void keepFalsePositiveIssues(IssueDto openIssue, Resource resource) { + private void keepFalsePositiveIssues(IssueDto openIssue) { if (!openIssue.isManualIssue() && Issue.RESOLUTION_FALSE_POSITIVE.equals(openIssue.getResolution())) { - DefaultIssue issue = toIssue(openIssue, resource); + DefaultIssue issue = openIssue.toDefaultIssue(); issue.setResolution(openIssue.getResolution()); issue.setStatus(openIssue.getStatus()); issue.setUpdatedAt(new Date()); @@ -123,19 +120,19 @@ public class IssuesWorkflowDecorator implements Decorator { } - private void reopenUnresolvedIssues(IssueDto openIssue, Resource resource) { + private void reopenUnresolvedIssues(IssueDto openIssue) { if (Issue.STATUS_RESOLVED.equals(openIssue.getStatus()) && !Issue.RESOLUTION_FALSE_POSITIVE.equals(openIssue.getResolution()) - && !openIssue.isManualIssue()) { - reopenAndSave(openIssue, resource); + && !openIssue.isManualIssue()) { + reopenAndSave(openIssue); } } /** * Close issues that relate to resources that have been deleted or renamed. */ - private void closeIssuesOnDeletedResources(Collection<IssueDto> openIssues, Resource resource) { + private void closeIssuesOnDeletedResources(Collection<IssueDto> openIssues) { for (IssueDto openIssue : openIssues) { - closeAndSave(openIssue, resource); + closeAndSave(openIssue); } } @@ -144,46 +141,20 @@ public class IssuesWorkflowDecorator implements Decorator { issue.setUpdatedAt(new Date()); } - private void closeAndSave(IssueDto openIssue, Resource resource) { - DefaultIssue issue = toIssue(openIssue, resource); + private void closeAndSave(IssueDto openIssue) { + DefaultIssue issue = openIssue.toDefaultIssue(); close(issue); moduleIssues.addOrUpdate(issue); } - private void reopenAndSave(IssueDto openIssue, Resource resource) { - DefaultIssue issue = toIssue(openIssue, resource); + private void reopenAndSave(IssueDto openIssue) { + DefaultIssue issue = openIssue.toDefaultIssue(); issue.setStatus(Issue.STATUS_REOPENED); issue.setResolution(null); issue.setUpdatedAt(new Date()); moduleIssues.addOrUpdate(issue); } - private DefaultIssue toIssue(IssueDto dto, Resource resource) { - DefaultIssue issue = new DefaultIssue(); - issue.setKey(dto.getUuid()); - issue.setStatus(dto.getStatus()); - issue.setResolution(dto.getResolution()); - issue.setMessage(dto.getMessage()); - issue.setTitle(dto.getTitle()); - issue.setCost(dto.getCost()); - issue.setLine(dto.getLine()); - issue.setSeverity(dto.getSeverity()); - issue.setUserLogin(dto.getUserLogin()); - issue.setAssigneeLogin(dto.getAssigneeLogin()); - issue.setCreatedAt(dto.getCreatedAt()); - issue.setUpdatedAt(dto.getUpdatedAt()); - issue.setClosedAt(dto.getClosedAt()); - issue.setAttributes(KeyValueFormat.parse(dto.getData())); - issue.setComponentKey(resource.getKey()); - issue.setManual(dto.isManualIssue()); - issue.setManualSeverity(dto.isManualSeverity()); - - // TODO add person - - Rule rule = ruleFinder.findById(dto.getRuleId()); - issue.setRuleKey(rule.ruleKey()); - return issue; - } private Collection<DefaultIssue> toDefaultIssues(Collection<Issue> issues) { return newArrayList(Iterables.transform(issues, new Function<Issue, DefaultIssue>() { 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 550fbb95d60..079e7c0d121 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 @@ -20,9 +20,7 @@ package org.sonar.plugins.core.issue; -import com.google.common.base.Charsets; import com.google.common.collect.Lists; -import com.google.common.io.Resources; import org.junit.Before; import org.junit.Test; import org.sonar.api.resources.Project; @@ -30,12 +28,9 @@ 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; import org.sonar.core.issue.IssueDto; -import java.io.IOException; -import java.util.Arrays; import java.util.Date; import java.util.Map; @@ -48,12 +43,6 @@ public class IssueTrackingTest { private final Date analysisDate = DateUtils.parseDate("2013-04-11"); private IssueTracking decorator; - - private Project project; - private RuleFinder ruleFinder; - - private LastSnapshots lastSnapshots; - private long violationId = 0; @Before @@ -62,22 +51,16 @@ public class IssueTrackingTest { 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); + RuleFinder 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); + Project project = mock(Project.class); when(project.getAnalysisDate()).thenReturn(analysisDate); - decorator = new IssueTracking(project, ruleFinder, lastSnapshots, null); + decorator = new IssueTracking(project, ruleFinder, null, null); } @Test @@ -241,181 +224,20 @@ public class IssueTrackingTest { assertThat(newIssue.isNew()).isFalse(); } - @Test - public void past_issue_not_assiciated_with_line_should_not_cause_npe() throws Exception { - when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); - 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); - - - Map<DefaultIssue, IssueDto> mapping = decorator.mapIssues( - newArrayList(newIssue), - newArrayList(referenceIssue), - source, project); - - assertThat(mapping.isEmpty()).isTrue(); - assertThat(newIssue.isNew()).isTrue(); - } - - @Test - public void new_issue_not_assiciated_with_line_should_not_cause_npe() throws Exception { - when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); - 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); - - Map<DefaultIssue, IssueDto> mapping = decorator.mapIssues( - newArrayList(newIssue), - newArrayList(referenceIssue), - source, project); - - assertThat(mapping.isEmpty()).isTrue(); - assertThat(newIssue.isNew()).isTrue(); - } - - /** - * SONAR-2928 - */ - @Test - public void issue_not_associated_with_line() throws Exception { - when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); - 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); - - Map<DefaultIssue, IssueDto> mapping = decorator.mapIssues( - newArrayList(newIssue), - newArrayList(referenceIssue), - source, project); - - assertThat(newIssue.isNew()).isFalse(); - assertThat(mapping.get(newIssue)).isEqualTo(referenceIssue); - } - - /** - * SONAR-3072 - */ - @Test - public void should_track_issues_based_on_blocks_recognition_on_example1() throws Exception { - 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); - - 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<DefaultIssue, IssueDto> mapping = decorator.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); + private DefaultIssue newDefaultIssue(String description, Integer line, RuleKey ruleKey, String checksum) { + return new DefaultIssue().setDescription(description).setLine(line).setRuleKey(ruleKey).setChecksum(checksum); } - /** - * SONAR-3072 - */ - @Test - public void should_track_issues_based_on_blocks_recognition_on_example2() throws Exception { - when(lastSnapshots.getSource(project)).thenReturn(load("example2-v1")); - String source = load("example2-v2"); - - IssueDto referenceIssue1 = newReferenceIssue("SystemPrintln", 5, 1, 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<DefaultIssue, IssueDto> mapping = decorator.mapIssues( - Arrays.asList(newIssue1, newIssue2, newIssue3), - Arrays.asList(referenceIssue1), - source, project); - - assertThat(newIssue1.isNew()).isTrue(); - assertThat(newIssue2.isNew()).isFalse(); - assertThat(mapping.get(newIssue2)).isEqualTo(referenceIssue1); - assertThat(newIssue3.isNew()).isTrue(); - } - - @Test - public void should_track_issues_based_on_blocks_recognition_on_example3() throws Exception { - 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"); - - // New issue - DefaultIssue newIssue1 = newDefaultIssue("Avoid unused local variables such as 'msg'.", 18, RuleKey.of("squid", "AvoidCycle"), "a24254126be2bf1a9b9a8db43f633733"); - // Same as referenceIssue2 - DefaultIssue newIssue2 = newDefaultIssue("Avoid unused private methods such as 'myMethod()'.", 13, RuleKey.of("squid", "NullDeref"), "ef23288705d1ef1e512448ace287586e"); - // Same as referenceIssue3 - DefaultIssue newIssue3 = newDefaultIssue("Method 'avoidUtilityClass' is not designed for extension - needs to be abstract, final or empty.", 9, RuleKey.of("pmd", "UnusedLocalVariable"), "ed5cdd046fda82727d6fedd1d8e3a310"); - // New issue - DefaultIssue newIssue4 = newDefaultIssue("Method 'newViolation' is not designed for extension - needs to be abstract, final or empty.", 17, RuleKey.of("pmd", "UnusedLocalVariable"), "7d58ac9040c27e4ca2f11a0269e251e2"); - // Same as referenceIssue1 - DefaultIssue newIssue5 = newDefaultIssue("Avoid unused local variables such as 'j'.", 6, RuleKey.of("squid", "AvoidCycle"), "4432a2675ec3e1620daefe38386b51ef"); - - Map<DefaultIssue, IssueDto> mapping = decorator.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); - } - - private static String load(String name) throws IOException { - return Resources.toString(IssueTrackingTest.class.getResource("IssueTrackingTest/" + name + ".txt"), Charsets.UTF_8); - } - - private DefaultIssue newDefaultIssue(String message, Integer line, RuleKey ruleKey, String checksum) { - return new DefaultIssue().setMessage(message).setLine(line).setRuleKey(ruleKey).setChecksum(checksum); - } - - private IssueDto newReferenceIssue(String message, Integer lineId, int ruleId, String lineChecksum) { + private IssueDto newReferenceIssue(String description, Integer lineId, int ruleId, String lineChecksum) { IssueDto referenceIssue = new IssueDto(); Long id = violationId++; referenceIssue.setId(id); referenceIssue.setUuid(Long.toString(id)); referenceIssue.setLine(lineId); - referenceIssue.setMessage(message); + referenceIssue.setDescription(description); referenceIssue.setRuleId(ruleId); referenceIssue.setChecksum(lineChecksum); return referenceIssue; } - public void test(){ - 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"); - - DefaultIssue newIssue1 = newDefaultIssue("Avoid unused local variables such as 'msg'.", 18, RuleKey.of("squid", "AvoidCycle"), "a24254126be2bf1a9b9a8db43f633733"); - DefaultIssue newIssue2 = newDefaultIssue("Avoid unused private methods such as 'myMethod()'.", 13, RuleKey.of("squid", "NullDeref"), "ef23288705d1ef1e512448ace287586e"); - DefaultIssue newIssue3 = newDefaultIssue("Method 'avoidUtilityClass' is not designed for extension - needs to be abstract, final or empty.", 9, RuleKey.of("pmd", "UnusedLocalVariable"), "ed5cdd046fda82727d6fedd1d8e3a310"); - DefaultIssue newIssue4 = newDefaultIssue("Method 'newViolation' is not designed for extension - needs to be abstract, final or empty.", 17, RuleKey.of("pmd", "UnusedLocalVariable"), "7d58ac9040c27e4ca2f11a0269e251e2"); - DefaultIssue newIssue5 = newDefaultIssue("Avoid unused local variables such as 'j'.", 6, RuleKey.of("squid", "AvoidCycle"), "4432a2675ec3e1620daefe38386b51ef"); - - } - } diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssuesWorkflowDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssuesWorkflowDecoratorTest.java index bb89f11eb1b..5b4d1f80736 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssuesWorkflowDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssuesWorkflowDecoratorTest.java @@ -78,7 +78,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { public void should_close_resolved_issue() { when(moduleIssues.issues(anyString())).thenReturn(Collections.<Issue>emptyList()); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(newArrayList( - new IssueDto().setUuid("100").setRuleId(10))); + new IssueDto().setUuid("100").setRuleId(10).setRuleKey_unit_test_only("squid", "AvoidCycle"))); decorator.decorate(mock(Resource.class), null); @@ -92,7 +92,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { public void should_close_resolved_manual_issue() { when(moduleIssues.issues(anyString())).thenReturn(Collections.<Issue>emptyList()); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(newArrayList( - new IssueDto().setUuid("100").setRuleId(1).setManualIssue(true).setStatus(Issue.STATUS_RESOLVED))); + new IssueDto().setUuid("100").setRuleId(1).setManualIssue(true).setStatus(Issue.STATUS_RESOLVED).setRuleKey_unit_test_only("squid", "AvoidCycle"))); decorator.decorate(mock(Resource.class), null); @@ -107,7 +107,8 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { when(moduleIssues.issues(anyString())).thenReturn(Lists.<Issue>newArrayList( new DefaultIssue().setKey("100"))); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(newArrayList( - new IssueDto().setUuid("100").setRuleId(1).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FIXED))); + new IssueDto().setUuid("100").setRuleId(1).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FIXED) + .setRuleKey_unit_test_only("squid", "AvoidCycle"))); decorator.decorate(mock(Resource.class), null); @@ -127,7 +128,8 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { when(moduleIssues.issues(anyString())).thenReturn(Lists.<Issue>newArrayList( new DefaultIssue().setKey("100"))); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(newArrayList( - new IssueDto().setUuid("100").setRuleId(1).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FALSE_POSITIVE))); + new IssueDto().setUuid("100").setRuleId(1).setStatus(Issue.STATUS_RESOLVED).setResolution(Issue.RESOLUTION_FALSE_POSITIVE) + .setRuleKey_unit_test_only("squid", "AvoidCycle"))); decorator.decorate(mock(Resource.class), null); @@ -147,7 +149,7 @@ public class IssuesWorkflowDecoratorTest extends AbstractDaoTestCase { when(moduleIssues.issues(anyString())).thenReturn(Collections.<Issue>emptyList()); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(Collections.<IssueDto>emptyList()); - when(initialOpenIssuesStack.getAllIssues()).thenReturn(newArrayList(new IssueDto().setUuid("100").setRuleId(1))); + when(initialOpenIssuesStack.getAllIssues()).thenReturn(newArrayList(new IssueDto().setUuid("100").setRuleId(1).setRuleKey_unit_test_only("squid", "AvoidCycle"))); Resource resource = mock(Resource.class); when(resource.getQualifier()).thenReturn(Qualifiers.PROJECT); diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java b/sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java index c2cb2f7974b..cc8c89e3af2 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/DeprecatedViolations.java @@ -55,7 +55,7 @@ public class DeprecatedViolations implements BatchComponent { .setRuleKey(RuleKey.of(violation.getRule().getRepositoryKey(), violation.getRule().getKey())) .setCost(violation.getCost()) .setLine(violation.getLineId()) - .setMessage(violation.getMessage()) + .setDescription(violation.getMessage()) .setStatus(Issue.STATUS_OPEN) .setSeverity(violation.getSeverity() != null ? violation.getSeverity().name() : null); diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuePersister.java b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuePersister.java index 53b1c93e695..06465e89c8f 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuePersister.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuePersister.java @@ -23,7 +23,6 @@ import org.sonar.api.database.model.Snapshot; import org.sonar.api.issue.Issue; import org.sonar.api.rules.Rule; import org.sonar.api.rules.RuleFinder; -import org.sonar.api.utils.KeyValueFormat; import org.sonar.batch.index.ScanPersister; import org.sonar.batch.index.SnapshotCache; import org.sonar.core.issue.DefaultIssue; @@ -62,7 +61,7 @@ public class IssuePersister implements ScanPersister { throw new IllegalStateException("Rule not found: " + issue.ruleKey()); } - IssueDto dto = toIssueDto((DefaultIssue) issue, snapshot.getResourceId(), rule.getId()); + IssueDto dto = IssueDto.toDto((DefaultIssue) issue, snapshot.getResourceId(), rule.getId()); if (issue.isNew()) { dao.insert(dto); } else { @@ -73,29 +72,5 @@ public class IssuePersister implements ScanPersister { } } - private IssueDto toIssueDto(DefaultIssue issue, Integer componentId, Integer ruleId) { - return new IssueDto() - .setUuid(issue.key()) - .setLine(issue.line()) - .setTitle(issue.title()) - .setMessage(issue.message()) - .setCost(issue.cost()) - .setResolution(issue.resolution()) - .setStatus(issue.status()) - .setSeverity(issue.severity()) - .setChecksum(issue.getChecksum()) - .setManualIssue(issue.isManual()) - .setManualSeverity(issue.isManualSeverity()) - .setUserLogin(issue.userLogin()) - .setAssigneeLogin(issue.assigneeLogin()) - .setCreatedAt(issue.createdAt()) - .setUpdatedAt(issue.updatedAt()) - .setClosedAt(issue.closedAt()) - .setRuleId(ruleId) - .setResourceId(componentId) - .setData(issue.attributes() != null ? KeyValueFormat.format(issue.attributes()) : null) - // TODO -// .setPersonId() - ; - } + } diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuesDecorator.java b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuesDecorator.java index 65286c6d124..5112867d6aa 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/IssuesDecorator.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/IssuesDecorator.java @@ -98,7 +98,7 @@ public class IssuesDecorator implements Decorator { rulesBag.add(rulefinder.findByKey(issue.ruleKey().repository(), issue.ruleKey().rule())); issuesPerSeverities.put(RulePriority.valueOf(issue.severity()), issue); - if (issue.assigneeLogin() == null) { + if (issue.assignee() == null) { countUnassigned++; } if (Issue.RESOLUTION_FALSE_POSITIVE.equals(issue.resolution())) { diff --git a/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueChanges.java b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueChanges.java index 702e87c14f2..ada2613e2e9 100644 --- a/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueChanges.java +++ b/sonar-batch/src/main/java/org/sonar/batch/issue/ScanIssueChanges.java @@ -54,13 +54,13 @@ public class ScanIssueChanges implements IssueChanges { issue.setCost(change.cost()); } if (change.manualSeverity() != null) { - change.setManualSeverity(change.manualSeverity().booleanValue()); + change.setManualSeverity(change.manualSeverity()); } if (change.severity() != null) { issue.setSeverity(change.severity()); } if (change.isAssigneeChanged()) { - issue.setAssigneeLogin(change.assignee()); + issue.setAssignee(change.assignee()); } if (change.resolution() != null) { issue.setResolution(change.resolution()); diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuesDecoratorTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/IssuesDecoratorTest.java index 4a331b8ea72..1ea1397c160 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/IssuesDecoratorTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/IssuesDecoratorTest.java @@ -212,7 +212,7 @@ public class IssuesDecoratorTest { List<Issue> issues = newArrayList(); issues.add(new DefaultIssue().setRuleKey(ruleA1.ruleKey()).setStatus(Issue.STATUS_OPEN).setSeverity(RulePriority.CRITICAL.name())); issues.add(new DefaultIssue().setRuleKey(ruleA1.ruleKey()).setStatus(Issue.STATUS_REOPENED).setSeverity(RulePriority.CRITICAL.name())); - issues.add(new DefaultIssue().setRuleKey(ruleA2.ruleKey()).setStatus(Issue.STATUS_OPEN).setAssigneeLogin("arthur").setSeverity(RulePriority.CRITICAL.name())); + issues.add(new DefaultIssue().setRuleKey(ruleA2.ruleKey()).setStatus(Issue.STATUS_OPEN).setAssignee("arthur").setSeverity(RulePriority.CRITICAL.name())); when(issuable.issues()).thenReturn(issues); decorator.decorate(resource, context); diff --git a/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueChangesTest.java b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueChangesTest.java index 45abe7b6c79..0325aae3dc3 100644 --- a/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueChangesTest.java +++ b/sonar-batch/src/test/java/org/sonar/batch/issue/ScanIssueChangesTest.java @@ -27,10 +27,7 @@ import org.sonar.core.issue.DefaultIssue; import static org.fest.assertions.Assertions.assertThat; import static org.fest.assertions.Fail.fail; -import static org.mockito.Mockito.mock; -import static org.mockito.Mockito.verify; -import static org.mockito.Mockito.verifyZeroInteractions; -import static org.mockito.Mockito.when; +import static org.mockito.Mockito.*; public class ScanIssueChangesTest { @@ -77,7 +74,7 @@ public class ScanIssueChangesTest { assertThat(changed.resolution()).isEqualTo(Issue.RESOLUTION_FALSE_POSITIVE); assertThat(changed.attribute("JIRA")).isEqualTo("FOO-123"); assertThat(changed.severity()).isEqualTo(Severity.CRITICAL); - assertThat(changed.assigneeLogin()).isEqualTo("arthur"); + assertThat(changed.assignee()).isEqualTo("arthur"); assertThat(changed.cost()).isEqualTo(4.2); assertThat(changed.updatedAt()).isNotNull(); } diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java index 8d7ee81454f..f00ff1fc7fc 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssue.java @@ -47,13 +47,13 @@ public class DefaultIssue implements Issue, Serializable { private String severity; private boolean manualSeverity = false; private String title; - private String message; + private String description; private Integer line; private Double cost; private String status; private String resolution; private String userLogin; - private String assigneeLogin; + private String assignee; private Date createdAt; private Date updatedAt; private Date closedAt; @@ -117,12 +117,16 @@ public class DefaultIssue implements Issue, Serializable { return this; } - public String message() { - return message; + public String description() { + return description; } - public DefaultIssue setMessage(@Nullable String message) { - this.message = message; + public DefaultIssue setDescription(@Nullable String s) { + if (s != null) { + Preconditions.checkArgument(s.length() < Issue.DESCRIPTION_MAX_SIZE, + "Description must not be longer than " + Issue.DESCRIPTION_MAX_SIZE + " characters (got " + s.length() + ")"); + } + this.description = s; return this; } @@ -175,12 +179,12 @@ public class DefaultIssue implements Issue, Serializable { return this; } - public String assigneeLogin() { - return assigneeLogin; + public String assignee() { + return assignee; } - public DefaultIssue setAssigneeLogin(@Nullable String assigneeLogin) { - this.assigneeLogin = assigneeLogin; + public DefaultIssue setAssignee(@Nullable String s) { + this.assignee = s; return this; } @@ -255,7 +259,7 @@ public class DefaultIssue implements Issue, Serializable { } public Map<String, String> attributes() { - return attributes == null ? Collections.<String,String>emptyMap() : ImmutableMap.copyOf(attributes); + return attributes == null ? Collections.<String, String>emptyMap() : ImmutableMap.copyOf(attributes); } public DefaultIssue setAttributes(Map<String, String> attributes) { diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueBuilder.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueBuilder.java index b17bba2f4e9..362ca0f576f 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueBuilder.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueBuilder.java @@ -30,7 +30,7 @@ public class DefaultIssueBuilder implements Issuable.IssueBuilder { private final String componentKey; private RuleKey ruleKey; private Integer line; - private String message; + private String description; private String title; private String severity; private Double cost; @@ -54,8 +54,8 @@ public class DefaultIssueBuilder implements Issuable.IssueBuilder { } @Override - public Issuable.IssueBuilder message(String message) { - this.message = message; + public Issuable.IssueBuilder description(String s) { + this.description = s; return this; } @@ -91,7 +91,7 @@ public class DefaultIssueBuilder implements Issuable.IssueBuilder { DefaultIssue issue = new DefaultIssue(); issue.setComponentKey(componentKey); issue.setRuleKey(ruleKey); - issue.setMessage(message); + issue.setDescription(description); issue.setTitle(title); issue.setSeverity(severity); diff --git a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFinder.java b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFinder.java index c2091b65bcc..1501a0c8986 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFinder.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/DefaultIssueFinder.java @@ -28,8 +28,6 @@ import org.slf4j.LoggerFactory; import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; -import org.sonar.api.rule.RuleKey; -import org.sonar.api.utils.KeyValueFormat; import org.sonar.core.persistence.MyBatis; import org.sonar.core.user.AuthorizationDao; @@ -44,11 +42,6 @@ public class DefaultIssueFinder implements IssueFinder { private static final Logger LOG = LoggerFactory.getLogger(DefaultIssueFinder.class); - /** - * The role required to access issues - */ - private static final String ROLE = "user"; - private final MyBatis myBatis; private final IssueDao issueDao; private final AuthorizationDao authorizationDao; @@ -60,7 +53,7 @@ public class DefaultIssueFinder implements IssueFinder { this.authorizationDao = authorizationDao; } - public Results find(IssueQuery query, @Nullable Integer currentUserId) { + public Results find(IssueQuery query, @Nullable Integer currentUserId, String role) { LOG.debug("IssueQuery : {}", query); SqlSession sqlSession = myBatis.openSession(); try { @@ -70,11 +63,11 @@ public class DefaultIssueFinder implements IssueFinder { for (IssueDto issueDto : dtos) { componentIds.add(issueDto.getResourceId()); } - Set<Integer> authorizedComponentIds = authorizationDao.keepAuthorizedComponentIds(componentIds, currentUserId, ROLE, sqlSession); + Set<Integer> authorizedComponentIds = authorizationDao.keepAuthorizedComponentIds(componentIds, currentUserId, role, sqlSession); List<Issue> issues = Lists.newArrayList(); for (IssueDto dto : dtos) { if (authorizedComponentIds.contains(dto.getResourceId())) { - issues.add(toIssue(dto)); + issues.add(dto.toDefaultIssue()); } } return new DefaultResults(issues); @@ -85,30 +78,7 @@ public class DefaultIssueFinder implements IssueFinder { public Issue findByKey(String key) { IssueDto dto = issueDao.selectByKey(key); - return dto != null ? toIssue(dto) : null; - } - - private Issue toIssue(IssueDto dto) { - DefaultIssue issue = new DefaultIssue(); - issue.setKey(dto.getUuid()); - issue.setStatus(dto.getStatus()); - issue.setResolution(dto.getResolution()); - issue.setMessage(dto.getMessage()); - issue.setTitle(dto.getTitle()); - issue.setCost(dto.getCost()); - issue.setLine(dto.getLine()); - issue.setSeverity(dto.getSeverity()); - issue.setUserLogin(dto.getUserLogin()); - issue.setAssigneeLogin(dto.getAssigneeLogin()); - issue.setCreatedAt(dto.getCreatedAt()); - issue.setUpdatedAt(dto.getUpdatedAt()); - issue.setClosedAt(dto.getClosedAt()); - issue.setAttributes(KeyValueFormat.parse(dto.getData())); - issue.setManual(dto.isManualIssue()); - issue.setManualSeverity(dto.isManualSeverity()); - issue.setComponentKey(dto.getComponentKey()); - issue.setRuleKey(RuleKey.of(dto.getRuleRepo(), dto.getRule())); - return issue; + return dto != null ? dto.toDefaultIssue() : null; } static class DefaultResults implements Results { diff --git a/sonar-core/src/main/java/org/sonar/core/issue/IssueDto.java b/sonar-core/src/main/java/org/sonar/core/issue/IssueDto.java index 3a5416666ea..02bc7e12f64 100644 --- a/sonar-core/src/main/java/org/sonar/core/issue/IssueDto.java +++ b/sonar-core/src/main/java/org/sonar/core/issue/IssueDto.java @@ -19,10 +19,12 @@ */ package org.sonar.core.issue; +import com.google.common.base.Objects; import com.google.common.base.Preconditions; -import org.apache.commons.lang.StringUtils; import org.apache.commons.lang.builder.ToStringBuilder; import org.apache.commons.lang.builder.ToStringStyle; +import org.sonar.api.rule.RuleKey; +import org.sonar.api.utils.KeyValueFormat; import javax.annotation.Nullable; import java.util.Date; @@ -32,8 +34,6 @@ import java.util.Date; */ public final class IssueDto { - public static final int MESSAGE_MAX_SIZE = 4000; - private Long id; private String uuid; private Integer resourceId; @@ -42,14 +42,14 @@ public final class IssueDto { private boolean manualSeverity; private boolean manualIssue; private String title; - private String message; + private String description; private Integer line; private Double cost; private String status; private String resolution; private String checksum; private String userLogin; - private String assigneeLogin; + private String assignee; private Long personId; private String data; private Date createdAt; @@ -133,12 +133,12 @@ public final class IssueDto { return this; } - public String getMessage() { - return message; + public String getDescription() { + return description; } - public IssueDto setMessage(String message) { - this.message = abbreviateMessage(message); + public IssueDto setDescription(String s) { + this.description = s; return this; } @@ -196,12 +196,12 @@ public final class IssueDto { return this; } - public String getAssigneeLogin() { - return assigneeLogin; + public String getAssignee() { + return assignee; } - public IssueDto setAssigneeLogin(@Nullable String assigneeLogin) { - this.assigneeLogin = assigneeLogin; + public IssueDto setAssignee(@Nullable String s) { + this.assignee = s; return this; } @@ -252,10 +252,6 @@ public final class IssueDto { return this; } - public static String abbreviateMessage(String message) { - return message != null ? StringUtils.abbreviate(StringUtils.trim(message), MESSAGE_MAX_SIZE) : null; - } - public String getRule() { return rule; } @@ -271,7 +267,8 @@ public final class IssueDto { /** * Only for unit tests */ - public IssueDto setRule_unit_test_only(String rule) { + public IssueDto setRuleKey_unit_test_only(String repo, String rule) { + this.ruleRepo = repo; this.rule = rule; return this; } @@ -279,14 +276,6 @@ public final class IssueDto { /** * Only for unit tests */ - public IssueDto setRuleRepo_unit_test_only(String ruleRepo) { - this.ruleRepo = ruleRepo; - return this; - } - - /** - * Only for unit tests - */ public IssueDto setComponentKey_unit_test_only(String componentKey) { this.componentKey = componentKey; return this; @@ -314,4 +303,55 @@ public final class IssueDto { public int hashCode() { return id != null ? id.hashCode() : 0; } + + + public static IssueDto toDto(DefaultIssue issue, Integer componentId, Integer ruleId) { + return new IssueDto() + .setUuid(issue.key()) + .setLine(issue.line()) + .setTitle(issue.title()) + .setDescription(issue.description()) + .setCost(issue.cost()) + .setResolution(issue.resolution()) + .setStatus(issue.status()) + .setSeverity(issue.severity()) + .setChecksum(issue.getChecksum()) + .setManualIssue(issue.isManual()) + .setManualSeverity(issue.isManualSeverity()) + .setUserLogin(issue.userLogin()) + .setAssignee(issue.assignee()) + .setCreatedAt(issue.createdAt()) + .setUpdatedAt(issue.updatedAt()) + .setClosedAt(issue.closedAt()) + .setRuleId(ruleId) + .setResourceId(componentId) + .setData(issue.attributes() != null ? KeyValueFormat.format(issue.attributes()) : "") + // TODO +// .setPersonId() + ; + } + + public DefaultIssue toDefaultIssue() { + DefaultIssue issue = new DefaultIssue(); + issue.setKey(uuid); + issue.setStatus(status); + issue.setResolution(resolution); + issue.setDescription(description); + issue.setTitle(title); + issue.setCost(cost); + issue.setLine(line); + issue.setSeverity(severity); + issue.setUserLogin(userLogin); + issue.setAssignee(assignee); + issue.setCreatedAt(createdAt); + issue.setUpdatedAt(updatedAt); + issue.setClosedAt(closedAt); + issue.setAttributes(KeyValueFormat.parse(Objects.firstNonNull(data, ""))); + issue.setComponentKey(componentKey); + issue.setManual(manualIssue); + issue.setManualSeverity(manualSeverity); + issue.setRuleKey(RuleKey.of(ruleRepo, rule)); + // TODO personId + return issue; + } } diff --git a/sonar-core/src/main/java/org/sonar/core/user/AuthorizationDao.java b/sonar-core/src/main/java/org/sonar/core/user/AuthorizationDao.java index 9c1cbdda59d..ae43ce26c49 100644 --- a/sonar-core/src/main/java/org/sonar/core/user/AuthorizationDao.java +++ b/sonar-core/src/main/java/org/sonar/core/user/AuthorizationDao.java @@ -26,7 +26,6 @@ import org.sonar.api.ServerComponent; import org.sonar.core.persistence.MyBatis; import javax.annotation.Nullable; - import java.util.Collections; import java.util.Map; import java.util.Set; @@ -65,4 +64,8 @@ public class AuthorizationDao implements ServerComponent { return Sets.newHashSet(session.<Integer>selectList(sql, params)); } + + public boolean isAuthorizedComponentId(int componentId, @Nullable Integer userId, String role) { + return keepAuthorizedComponentIds(Sets.newHashSet(componentId), userId, role).size() == 1; + } } diff --git a/sonar-core/src/main/resources/org/sonar/core/issue/IssueMapper.xml b/sonar-core/src/main/resources/org/sonar/core/issue/IssueMapper.xml index 15b286f201d..86f09075c76 100644 --- a/sonar-core/src/main/resources/org/sonar/core/issue/IssueMapper.xml +++ b/sonar-core/src/main/resources/org/sonar/core/issue/IssueMapper.xml @@ -13,14 +13,14 @@ i.manual_severity as manualSeverity, i.manual_issue as manualIssue, i.title as title, - i.message as message, + i.message as description, i.line as line, i.cost as cost, i.status as status, i.resolution as resolution, i.checksum as checksum, i.user_login as userLogin, - i.assignee_login as assigneeLogin, + i.assignee_login as assignee, i.person_id as personId, i.data as data, i.created_at as createdAt, @@ -34,8 +34,8 @@ <insert id="insert" parameterType="Issue" useGeneratedKeys="true" keyProperty="id"> INSERT INTO issues (uuid, resource_id, rule_id, severity, manual_severity, manual_issue, title, message, line, cost, status, resolution, checksum, user_login, assignee_login, person_id, data, created_at, updated_at, closed_at) - VALUES (#{uuid}, #{resourceId}, #{ruleId}, #{severity}, #{manualSeverity}, #{manualIssue}, #{title}, #{message}, #{line}, #{cost}, #{status}, - #{resolution}, #{checksum}, #{userLogin}, #{assigneeLogin}, #{personId}, #{data}, #{createdAt}, #{updatedAt}, #{closedAt}) + VALUES (#{uuid}, #{resourceId}, #{ruleId}, #{severity}, #{manualSeverity}, #{manualIssue}, #{title}, #{description}, #{line}, #{cost}, #{status}, + #{resolution}, #{checksum}, #{userLogin}, #{assignee}, #{personId}, #{data}, #{createdAt}, #{updatedAt}, #{closedAt}) </insert> <!-- Oracle --> @@ -45,31 +45,31 @@ </selectKey> INSERT INTO issues (id, uuid, resource_id, rule_id, severity, manual_severity, manual_issue, title, message, line, cost, status, resolution, checksum, user_login, assignee_login, person_id, data, created_at, updated_at, closed_at) - VALUES (#{id}, #{uuid}, #{resourceId}, #{ruleId}, #{severity}, #{manualSeverity}, #{manualIssue}, #{title}, #{message}, #{line}, #{cost}, #{status}, - #{resolution}, #{checksum}, #{userLogin}, #{assigneeLogin}, #{personId}, #{data}, #{createdAt}, #{updatedAt}, #{closedAt}) + VALUES (#{id}, #{uuid}, #{resourceId}, #{ruleId}, #{severity}, #{manualSeverity}, #{manualIssue}, #{title}, #{description}, #{line}, #{cost}, #{status}, + #{resolution}, #{checksum}, #{userLogin}, #{assignee}, #{personId}, #{data}, #{createdAt}, #{updatedAt}, #{closedAt}) </insert> <update id="update" parameterType="Issue"> update issues set - resource_id=#{resourceId}, - rule_id=#{ruleId}, - severity=#{severity}, - manual_severity=#{manualSeverity}, - manual_issue=#{manualIssue}, - title=#{title}, - message=#{message}, - line=#{line}, - cost=#{cost}, - status=#{status}, - resolution=#{resolution}, - checksum=#{checksum}, - user_login=#{userLogin}, - assignee_login=#{assigneeLogin}, - person_id=#{personId}, - data=#{data}, - created_at=#{createdAt}, - updated_at=#{updatedAt}, - closed_at=#{closedAt} + resource_id=#{resourceId}, + rule_id=#{ruleId}, + severity=#{severity}, + manual_severity=#{manualSeverity}, + manual_issue=#{manualIssue}, + title=#{title}, + message=#{description}, + line=#{line}, + cost=#{cost}, + status=#{status}, + resolution=#{resolution}, + checksum=#{checksum}, + user_login=#{userLogin}, + assignee_login=#{assignee}, + person_id=#{personId}, + data=#{data}, + created_at=#{createdAt}, + updated_at=#{updatedAt}, + closed_at=#{closedAt} where uuid = #{uuid} </update> @@ -77,7 +77,10 @@ select <include refid="issueColumns"/> from issues i, rules r, projects p - where i.id=#{id} and i.rule_id=r.id and p.id=i.resource_id + where + i.id=#{id} and + i.rule_id=r.id and + p.id=i.resource_id </select> <select id="selectByKey" parameterType="String" resultType="Issue"> @@ -166,9 +169,9 @@ <foreach item="userLogin" index="index" collection="userLogins" open="(" separator="," close=")">#{userLogin} </foreach> </if> - <if test="assigneeLogins != null"> + <if test="assignees != null"> and i.assignee_login in - <foreach item="assigneeLogin" index="index" collection="assigneeLogins" open="(" separator="," close=")">#{assigneeLogin} + <foreach item="assignee" index="index" collection="assignees" open="(" separator="," close=")">#{assignee} </foreach> </if> <if test="rules != null and rules.size() > 0"> diff --git a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueBuilderTest.java b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueBuilderTest.java index 2e9b7a50880..6ba6e1f87ee 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueBuilderTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueBuilderTest.java @@ -35,7 +35,7 @@ public class DefaultIssueBuilderTest { public void should_create_issue() throws Exception { String componentKey = "org.apache.struts:struts-core:Action.java"; DefaultIssue issue = (DefaultIssue) new DefaultIssueBuilder(callback, componentKey) - .message("msg") + .description("the desc") .line(123) .cost(10000.0) .ruleKey(RuleKey.of("squid", "NullDereference")) @@ -46,14 +46,14 @@ public class DefaultIssueBuilderTest { assertThat(issue.key()).isNull(); assertThat(issue.cost()).isEqualTo(10000.0); assertThat(issue.componentKey()).isEqualTo(componentKey); - assertThat(issue.message()).isEqualTo("msg"); + assertThat(issue.description()).isEqualTo("the desc"); assertThat(issue.line()).isEqualTo(123); assertThat(issue.ruleKey().repository()).isEqualTo("squid"); assertThat(issue.ruleKey().rule()).isEqualTo("NullDereference"); assertThat(issue.severity()).isEqualTo(Severity.CRITICAL); assertThat(issue.updatedAt()).isNull(); assertThat(issue.closedAt()).isNull(); - assertThat(issue.assigneeLogin()).isNull(); + assertThat(issue.assignee()).isNull(); assertThat(issue.isNew()).isTrue(); verify(callback).onIssueCreation(issue); } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueFinderTest.java b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueFinderTest.java index 54b52850807..73bc35ad508 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueFinderTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueFinderTest.java @@ -27,6 +27,7 @@ import org.mockito.stubbing.Answer; import org.sonar.api.issue.Issue; import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; +import org.sonar.api.web.UserRole; import org.sonar.core.persistence.MyBatis; import org.sonar.core.user.AuthorizationDao; @@ -60,16 +61,14 @@ public class DefaultIssueFinderTest { IssueDto issue1 = new IssueDto().setId(1L).setRuleId(50).setResourceId(123) .setComponentKey_unit_test_only("Action.java") - .setRuleRepo_unit_test_only("squid") - .setRule_unit_test_only("AvoidCycle"); + .setRuleKey_unit_test_only("squid", "AvoidCycle"); IssueDto issue2 = new IssueDto().setId(2L).setRuleId(50).setResourceId(123) .setComponentKey_unit_test_only("Action.java") - .setRuleRepo_unit_test_only("squid") - .setRule_unit_test_only("AvoidCycle"); + .setRuleKey_unit_test_only("squid", "AvoidCycle"); List<IssueDto> dtoList = newArrayList(issue1, issue2); when(issueDao.select(eq(issueQuery), any(SqlSession.class))).thenReturn(dtoList); - IssueFinder.Results results = finder.find(issueQuery, null); + IssueFinder.Results results = finder.find(issueQuery, null, UserRole.USER); assertThat(results.issues()).hasSize(2); Issue issue = results.issues().iterator().next(); assertThat(issue.componentKey()).isEqualTo("Action.java"); @@ -80,8 +79,7 @@ public class DefaultIssueFinderTest { public void should_find_by_key() { IssueDto issueDto = new IssueDto().setId(1L).setRuleId(1).setResourceId(1) .setComponentKey_unit_test_only("Action.java") - .setRuleRepo_unit_test_only("squid") - .setRule_unit_test_only("AvoidCycle"); + .setRuleKey_unit_test_only("squid", "AvoidCycle"); when(issueDao.selectByKey("ABCDE")).thenReturn(issueDto); Issue issue = finder.findByKey("ABCDE"); diff --git a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java index 20c1011f85f..b070d4d82d8 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/DefaultIssueTest.java @@ -19,6 +19,7 @@ */ package org.sonar.core.issue; +import org.apache.commons.lang.StringUtils; import org.junit.Test; import static org.fest.assertions.Assertions.assertThat; @@ -68,4 +69,14 @@ public class DefaultIssueTest { assertThat(e).hasMessage("Not a valid severity: FOO"); } } + + @Test + public void size_of_description_should_be_limited() { + try { + issue.setDescription(StringUtils.repeat("a", 5000)); + fail(); + } catch (IllegalArgumentException e) { + assertThat(e).hasMessage("Description must not be longer than 4000 characters (got 5000)"); + } + } } diff --git a/sonar-core/src/test/java/org/sonar/core/issue/IssueDaoTest.java b/sonar-core/src/test/java/org/sonar/core/issue/IssueDaoTest.java index 59f909a68d9..9f45587d8ba 100644 --- a/sonar-core/src/test/java/org/sonar/core/issue/IssueDaoTest.java +++ b/sonar-core/src/test/java/org/sonar/core/issue/IssueDaoTest.java @@ -55,8 +55,8 @@ public class IssueDaoTest extends AbstractDaoTestCase { issueDto.setSeverity("BLOCKER"); issueDto.setLine(200); issueDto.setStatus("OPEN"); - issueDto.setAssigneeLogin("user"); - issueDto.setMessage("message"); + issueDto.setAssignee("user"); + issueDto.setDescription("the description"); issueDto.setCost(10.0); issueDto.setChecksum("checksum"); issueDto.setPersonId(100L); @@ -80,7 +80,7 @@ public class IssueDaoTest extends AbstractDaoTestCase { issue.setResolution("NEW_RESOLUTION"); issue.setStatus("NEW_STATUS"); issue.setSeverity("NEW_SEV"); - issue.setAssigneeLogin("new_user"); + issue.setAssignee("new_user"); issue.setManualSeverity(true); issue.setManualIssue(false); issue.setTitle("NEW_TITLE"); @@ -105,7 +105,7 @@ public class IssueDaoTest extends AbstractDaoTestCase { assertThat(issue.isManualSeverity()).isFalse(); assertThat(issue.isManualIssue()).isFalse(); assertThat(issue.getTitle()).isNull(); - assertThat(issue.getMessage()).isNull(); + assertThat(issue.getDescription()).isNull(); assertThat(issue.getLine()).isEqualTo(200); assertThat(issue.getCost()).isEqualTo(4.2); assertThat(issue.getStatus()).isEqualTo("OPEN"); @@ -113,7 +113,7 @@ public class IssueDaoTest extends AbstractDaoTestCase { assertThat(issue.getChecksum()).isEqualTo("XXX"); assertThat(issue.getPersonId()).isNull(); assertThat(issue.getUserLogin()).isEqualTo("arthur"); - assertThat(issue.getAssigneeLogin()).isEqualTo("perceval"); + assertThat(issue.getAssignee()).isEqualTo("perceval"); assertThat(issue.getData()).isEqualTo("JIRA=FOO-1234"); assertThat(issue.getCreatedAt()).isNotNull(); assertThat(issue.getUpdatedAt()).isNotNull(); @@ -136,7 +136,7 @@ public class IssueDaoTest extends AbstractDaoTestCase { IssueQuery query = IssueQuery.builder() .keys(newArrayList("ABCDE")) .userLogins(newArrayList("arthur", "otherguy")) - .assigneeLogins(newArrayList("perceval", "otherguy")) + .assignees(newArrayList("perceval", "otherguy")) .components(newArrayList("Action.java")) .resolutions(newArrayList("FIXED")) .severities(newArrayList("BLOCKER")) diff --git a/sonar-core/src/test/resources/org/sonar/core/issue/IssueDaoTest/insert-result.xml b/sonar-core/src/test/resources/org/sonar/core/issue/IssueDaoTest/insert-result.xml index 0fcbb2a2639..461bc8c3530 100644 --- a/sonar-core/src/test/resources/org/sonar/core/issue/IssueDaoTest/insert-result.xml +++ b/sonar-core/src/test/resources/org/sonar/core/issue/IssueDaoTest/insert-result.xml @@ -8,7 +8,7 @@ manual_severity="[false]" manual_issue="[false]" title="[null]" - message="message" + message="the description" line="200" cost="10.0" status="OPEN" diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issuable.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issuable.java index cfaaacde6f9..f38a9078115 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issuable.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issuable.java @@ -35,7 +35,7 @@ public interface Issuable extends Perspective { IssueBuilder line(Integer line); - IssueBuilder message(String message); + IssueBuilder description(String description); IssueBuilder title(String title); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java index 5e57ae74039..342e7792e7a 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/Issue.java @@ -29,6 +29,7 @@ import java.util.Map; */ public interface Issue { + int DESCRIPTION_MAX_SIZE = 4000; String STATUS_OPEN = "OPEN"; String STATUS_REOPENED = "REOPENED"; String STATUS_RESOLVED = "RESOLVED"; @@ -50,7 +51,7 @@ public interface Issue { String title(); - String message(); + String description(); Integer line(); @@ -62,7 +63,7 @@ public interface Issue { String userLogin(); - String assigneeLogin(); + String assignee(); Date createdAt(); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChange.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChange.java index 572a4d8dd40..c83dd0e1d80 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChange.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChange.java @@ -33,13 +33,13 @@ public class IssueChange { private String comment = null; private String commentLogin = null; private Boolean manualSeverity = null; - private String message = null; + private String description = null; private boolean lineChanged = false; private Integer line = null; private boolean costChanged = false; private Double cost = null; private String resolution = null; - private boolean assigneeLoginChanged = false; + private boolean assigneeChanged = false; private String assignee = null; private Map<String, String> attributes = null; @@ -51,8 +51,8 @@ public class IssueChange { } public boolean hasChanges() { - return severity != null || comment != null || manualSeverity != null || message != null || - lineChanged || costChanged || resolution != null || assigneeLoginChanged || attributes != null; + return severity != null || comment != null || manualSeverity != null || description != null || + lineChanged || costChanged || resolution != null || assigneeChanged || attributes != null; } public IssueChange setSeverity(String s) { @@ -75,8 +75,8 @@ public class IssueChange { return this; } - public IssueChange setMessage(String message) { - this.message = message; + public IssueChange setDescription(String s) { + this.description = s; return this; } @@ -98,7 +98,7 @@ public class IssueChange { } public IssueChange setAssignee(@Nullable String assigneeLogin) { - this.assigneeLoginChanged = true; + this.assigneeChanged = true; this.assignee = assigneeLogin; return this; } @@ -127,8 +127,8 @@ public class IssueChange { return manualSeverity; } - public String message() { - return message; + public String description() { + return description; } public Integer line() { @@ -156,7 +156,7 @@ public class IssueChange { } public boolean isAssigneeChanged() { - return assigneeLoginChanged; + return assigneeChanged; } public Map<String, String> attributes() { diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChanges.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChanges.java index 260d377b449..e00ba81114f 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChanges.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueChanges.java @@ -28,7 +28,7 @@ import javax.annotation.Nullable; * Change existing issues * @since 3.6 */ -public interface IssueChanges extends BatchComponent, ServerComponent { +public interface IssueChanges extends BatchComponent { Issue apply(Issue issue, IssueChange change); diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueFinder.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueFinder.java index 0377194db41..10d676da834 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueFinder.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueFinder.java @@ -37,7 +37,7 @@ public interface IssueFinder extends ServerComponent { List<Issue> issues(); } - Results find(IssueQuery query, @Nullable Integer currentUserId); + Results find(IssueQuery query, @Nullable Integer currentUserId, String role); Issue findByKey(String key /* TODO @Nullable Integer currentUserId */); /* diff --git a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java index 0071a574d9f..58600ee03b3 100644 --- a/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java +++ b/sonar-plugin-api/src/main/java/org/sonar/api/issue/IssueQuery.java @@ -24,8 +24,6 @@ import com.google.common.base.Preconditions; import org.apache.commons.lang.builder.ReflectionToStringBuilder; import org.sonar.api.rule.RuleKey; -import javax.annotation.Nullable; - import java.util.Collection; import java.util.Date; @@ -44,7 +42,7 @@ public class IssueQuery { private final Collection<String> componentRoots; private final Collection<RuleKey> rules; private final Collection<String> userLogins; - private final Collection<String> assigneeLogins; + private final Collection<String> assignees; private final Date createdAfter; private final Date createdBefore; private final int limit, offset; @@ -58,7 +56,7 @@ public class IssueQuery { this.componentRoots = builder.componentRoots; this.rules = builder.rules; this.userLogins = builder.userLogins; - this.assigneeLogins = builder.assigneeLogins; + this.assignees = builder.assignees; this.createdAfter = builder.createdAfter; this.createdBefore = builder.createdBefore; this.limit = builder.limit; @@ -97,8 +95,8 @@ public class IssueQuery { return userLogins; } - public Collection<String> assigneeLogins() { - return assigneeLogins; + public Collection<String> assignees() { + return assignees; } public Date createdAfter() { @@ -143,7 +141,7 @@ public class IssueQuery { private Collection<String> componentRoots; private Collection<RuleKey> rules; private Collection<String> userLogins; - private Collection<String> assigneeLogins; + private Collection<String> assignees; private Date createdAfter; private Date createdBefore; private int limit = DEFAULT_LIMIT; @@ -192,8 +190,8 @@ public class IssueQuery { return this; } - public Builder assigneeLogins(Collection<String> l) { - this.assigneeLogins = l; + public Builder assignees(Collection<String> l) { + this.assignees = l; return this; } @@ -207,14 +205,14 @@ public class IssueQuery { return this; } - public Builder limit(@Nullable Integer i) { + public Builder limit(Integer i) { Preconditions.checkArgument(i == null || i.intValue() > 0, "Limit must be greater than 0 (got " + i + ")"); Preconditions.checkArgument(i == null || i.intValue() < MAX_LIMIT, "Limit must be less than " + MAX_LIMIT + " (got " + i + ")"); this.limit = (i == null ? DEFAULT_LIMIT : i.intValue()); return this; } - public Builder offset(@Nullable Integer i) { + public Builder offset(Integer i) { Preconditions.checkArgument(i == null || i.intValue() >= 0, "Offset must be greater than or equal to 0 (got " + i + ")"); this.offset = (i == null ? DEFAULT_OFFSET : i.intValue()); return this; diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueChangeTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueChangeTest.java index f4dc34f1c96..f74ce695d5d 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueChangeTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueChangeTest.java @@ -37,7 +37,7 @@ public class IssueChangeTest { assertThat(change.isLineChanged()).isFalse(); assertThat(change.line()).isNull(); assertThat(change.comment()).isNull(); - assertThat(change.message()).isNull(); + assertThat(change.description()).isNull(); assertThat(change.resolution()).isNull(); assertThat(change.manualSeverity()).isNull(); assertThat(change.attributes()).isEmpty(); @@ -101,8 +101,8 @@ public class IssueChangeTest { @Test public void should_change_message() { IssueChange change = IssueChange.create(); - change.setMessage("foo"); - assertThat(change.message()).isEqualTo("foo"); + change.setDescription("foo"); + assertThat(change.description()).isEqualTo("foo"); assertThat(change.hasChanges()).isTrue(); } diff --git a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java index 324d2d78c3c..e7e1bc13597 100644 --- a/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java +++ b/sonar-plugin-api/src/test/java/org/sonar/api/issue/IssueQueryTest.java @@ -41,7 +41,7 @@ public class IssueQueryTest { .componentRoots(Lists.newArrayList("org.struts:core")) .rules(Lists.newArrayList(RuleKey.of("squid", "AvoidCycle"))) .userLogins(Lists.newArrayList("crunky")) - .assigneeLogins(Lists.newArrayList("gargantua")) + .assignees(Lists.newArrayList("gargantua")) .createdAfter(new Date()) .createdBefore(new Date()) .limit(125) @@ -54,7 +54,7 @@ public class IssueQueryTest { assertThat(query.components()).containsOnly("org/struts/Action.java"); assertThat(query.componentRoots()).containsOnly("org.struts:core"); assertThat(query.userLogins()).containsOnly("crunky"); - assertThat(query.assigneeLogins()).containsOnly("gargantua"); + assertThat(query.assignees()).containsOnly("gargantua"); assertThat(query.rules()).containsOnly(RuleKey.of("squid", "AvoidCycle")); assertThat(query.createdAfter()).isNotNull(); assertThat(query.createdBefore()).isNotNull(); diff --git a/sonar-server/src/main/java/org/sonar/server/issue/DefaultJRubyIssues.java b/sonar-server/src/main/java/org/sonar/server/issue/DefaultJRubyIssues.java index 07887c8073e..29b013e7f35 100644 --- a/sonar-server/src/main/java/org/sonar/server/issue/DefaultJRubyIssues.java +++ b/sonar-server/src/main/java/org/sonar/server/issue/DefaultJRubyIssues.java @@ -28,6 +28,7 @@ import org.sonar.api.issue.IssueQuery; import org.sonar.api.issue.JRubyIssues; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.DateUtils; +import org.sonar.api.web.UserRole; import org.sonar.server.ui.JRubyFacades; import javax.annotation.Nullable; @@ -50,8 +51,11 @@ public class DefaultJRubyIssues implements JRubyIssues { JRubyFacades.setIssues(this); } + /** + * Requires the role {@link org.sonar.api.web.UserRole#CODEVIEWER} + */ public IssueFinder.Results find(Map<String, Object> params, Integer currentUserId) { - return finder.find(newQuery(params), currentUserId); + return finder.find(newQuery(params), currentUserId, UserRole.CODEVIEWER); } IssueQuery newQuery(Map<String, Object> props) { @@ -64,7 +68,7 @@ public class DefaultJRubyIssues implements JRubyIssues { builder.componentRoots(toStrings(props.get("componentRoots"))); builder.rules(toRules(props.get("rules"))); builder.userLogins(toStrings(props.get("userLogins"))); - builder.assigneeLogins(toStrings(props.get("assigneeLogins"))); + builder.assignees(toStrings(props.get("assignees"))); builder.createdAfter(toDate(props.get("createdAfter"))); builder.createdBefore(toDate(props.get("createdBefore"))); builder.limit(toInteger(props.get("limit"))); diff --git a/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueChanges.java b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueChanges.java new file mode 100644 index 00000000000..0fce2d9edcd --- /dev/null +++ b/sonar-server/src/main/java/org/sonar/server/issue/ServerIssueChanges.java @@ -0,0 +1,68 @@ +/* + * Sonar, open source software quality management tool. + * Copyright (C) 2008-2012 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.server.issue; + +import org.sonar.api.issue.Issue; +import org.sonar.api.issue.IssueChange; +import org.sonar.api.web.UserRole; +import org.sonar.core.issue.DefaultIssue; +import org.sonar.core.issue.IssueDao; +import org.sonar.core.issue.IssueDto; +import org.sonar.core.user.AuthorizationDao; + +import javax.annotation.Nullable; +import java.util.Arrays; + +/** + * @since 3.6 + */ +public class ServerIssueChanges { + + private final IssueDao issueDao; + private final AuthorizationDao authorizationDao; + + public ServerIssueChanges(IssueDao issueDao, AuthorizationDao authorizationDao) { + this.issueDao = issueDao; + this.authorizationDao = authorizationDao; + } + + public Issue change(String issueKey, IssueChange change, @Nullable Integer userId) { + if (userId == null) { + // must be logged + throw new IllegalStateException("User is not logged in"); + } + IssueDto dto = issueDao.selectByKey(issueKey); + if (dto == null) { + throw new IllegalStateException("Unknown issue: " + issueKey); + } + String requiredRole = UserRole.USER; + if (!authorizationDao.isAuthorizedComponentId(dto.getResourceId(), userId, requiredRole)) { + throw new IllegalStateException("User does not have the role " + requiredRole + " required to change the issue: " + issueKey); + } + DefaultIssue issue = dto.toDefaultIssue(); + if (change.hasChanges()) { + // TODO apply changes + issueDao.update(Arrays.asList(IssueDto.toDto(issue, dto.getResourceId(), dto.getRuleId()))); + } + + + return issue; + } +} diff --git a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java index a58f005db8e..25f2e016f69 100644 --- a/sonar-server/src/main/java/org/sonar/server/platform/Platform.java +++ b/sonar-server/src/main/java/org/sonar/server/platform/Platform.java @@ -77,6 +77,7 @@ import org.sonar.server.configuration.Backup; import org.sonar.server.configuration.ProfilesManager; import org.sonar.server.database.EmbeddedDatabaseFactory; import org.sonar.server.issue.DefaultJRubyIssues; +import org.sonar.server.issue.ServerIssueChanges; import org.sonar.server.macro.MacroInterpreter; import org.sonar.server.notifications.NotificationCenter; import org.sonar.server.notifications.NotificationService; @@ -268,6 +269,7 @@ public final class Platform { servicesContainer.addSingleton(Periods.class); servicesContainer.addSingleton(DefaultIssueFinder.class); servicesContainer.addSingleton(DefaultJRubyIssues.class); + servicesContainer.addSingleton(ServerIssueChanges.class); // Notifications servicesContainer.addSingleton(EmailSettings.class); diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb index ac9515d9efe..52dd0f0432d 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/api/issues_controller.rb @@ -58,16 +58,16 @@ class Api::IssuesController < Api::ApiController :key => issue.key, :component => issue.componentKey, :rule => issue.ruleKey.toString(), - :status => issue.status, - :resolution => issue.resolution + :resolution => issue.resolution, + :status => issue.status } json[:severity] = issue.severity if issue.severity json[:title] = issue.title if issue.title - json[:desc] = issue.message if issue.message + json[:desc] = issue.description if issue.description json[:line] = issue.line if issue.line json[:cost] = issue.cost if issue.cost json[:userLogin] = issue.userLogin if issue.userLogin - json[:assignee] = issue.assigneeLogin if issue.assigneeLogin + json[:assignee] = issue.assignee if issue.assignee json[:createdAt] = format_java_datetime(issue.createdAt) if issue.createdAt json[:updatedAt] = format_java_datetime(issue.updatedAt) if issue.updatedAt json[:closedAt] = format_java_datetime(issue.closedAt) if issue.closedAt diff --git a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb index e60daf9c9d1..89f5c5b2935 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb +++ b/sonar-server/src/main/webapp/WEB-INF/app/controllers/issues_controller.rb @@ -28,9 +28,7 @@ class IssuesController < ApplicationController def find_issues(map) user = current_user ? current_user.id : nil - issues = [] - Api.issues.find(map, user).issues.each {|issue| issues << issue} - issues + Api.issues.find(map, user).issues end end
\ No newline at end of file diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_list.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_list.html.erb index 32234197637..7f2034775f8 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_list.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issues/_list.html.erb @@ -54,13 +54,6 @@ </th> </tr> </thead> - <tfoot> - <tr> - <td colspan="6"> - <%= paginate(@issues) -%> - </td> - </tr> - </tfoot> <tbody> <% @issues.each do |issue| @@ -84,7 +77,7 @@ <%= h(issue.title) %> </td> <td> - <%= h(issue.message) %> + <%= h(issue.description) %> </td> <td> <%= h issue.componentKey -%> @@ -99,10 +92,10 @@ <%= issue.cost -%> </td> <td> - <%= issue.user_login -%> + <%= issue.userLogin -%> </td> <td> - <%= issue.assignee_login -%> + <%= issue.assignee -%> </td> <td> <%= issue.attributes -%> diff --git a/sonar-server/src/test/java/org/sonar/server/issue/DefaultJRubyIssuesTest.java b/sonar-server/src/test/java/org/sonar/server/issue/DefaultJRubyIssuesTest.java index ad905028595..d44ce784705 100644 --- a/sonar-server/src/test/java/org/sonar/server/issue/DefaultJRubyIssuesTest.java +++ b/sonar-server/src/test/java/org/sonar/server/issue/DefaultJRubyIssuesTest.java @@ -27,6 +27,7 @@ import org.sonar.api.issue.IssueFinder; import org.sonar.api.issue.IssueQuery; import org.sonar.api.rule.RuleKey; import org.sonar.api.utils.DateUtils; +import org.sonar.api.web.UserRole; import java.util.Map; @@ -51,7 +52,7 @@ public class DefaultJRubyIssuesTest { public boolean matches(Object o) { return ((IssueQuery) o).keys().contains("ABCDE"); } - }), eq(123)); + }), eq(123), eq(UserRole.CODEVIEWER)); } @Test @@ -64,7 +65,7 @@ public class DefaultJRubyIssuesTest { map.put("components", newArrayList("org.apache")); map.put("componentRoots", newArrayList("org.sonar")); map.put("userLogins", newArrayList("marilyn")); - map.put("assigneeLogins", newArrayList("joanna")); + map.put("assignees", newArrayList("joanna")); map.put("createdAfter", "2013-04-16T09:08:24+0200"); map.put("createdBefore", "2013-04-17T09:08:24+0200"); map.put("rules", "squid:AvoidCycle,findbugs:NullReference"); @@ -79,7 +80,7 @@ public class DefaultJRubyIssuesTest { assertThat(query.components()).containsOnly("org.apache"); assertThat(query.componentRoots()).containsOnly("org.sonar"); assertThat(query.userLogins()).containsOnly("marilyn"); - assertThat(query.assigneeLogins()).containsOnly("joanna"); + assertThat(query.assignees()).containsOnly("joanna"); assertThat(query.rules()).hasSize(2); assertThat(query.createdAfter()).isEqualTo(DateUtils.parseDateTime("2013-04-16T09:08:24+0200")); assertThat(query.createdBefore()).isEqualTo(DateUtils.parseDateTime("2013-04-17T09:08:24+0200")); diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Issue.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Issue.java index cc6314d18d5..643db13a746 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Issue.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/Issue.java @@ -151,7 +151,7 @@ public class Issue extends Model { return this; } - public String getAssigneeLogin() { + public String getAssignee() { return assigneeLogin; } diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/IssueQuery.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/IssueQuery.java index e4d9fc44b59..414b21dc7f1 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/services/IssueQuery.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/services/IssueQuery.java @@ -182,7 +182,7 @@ public final class IssueQuery extends Query<Issue> { appendUrlParameter(url, "componentRoots", componentRoots); appendUrlParameter(url, "rules", rules); appendUrlParameter(url, "userLogins", userLogins); - appendUrlParameter(url, "assigneeLogins", assigneeLogins); + appendUrlParameter(url, "assignees", assigneeLogins); appendUrlParameter(url, "createdAfter", createdAfter); appendUrlParameter(url, "createdBefore", createdBefore); appendUrlParameter(url, "limit", limit); diff --git a/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/IssueUnmarshaller.java b/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/IssueUnmarshaller.java index d3424d7dcfc..73f68474d0c 100644 --- a/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/IssueUnmarshaller.java +++ b/sonar-ws-client/src/main/java/org/sonar/wsclient/unmarshallers/IssueUnmarshaller.java @@ -40,7 +40,7 @@ public class IssueUnmarshaller extends AbstractUnmarshaller<Issue> { .setStatus(utils.getString(json, "status")) .setResolution(utils.getString(json, "resolution")) .setUserLogin(utils.getString(json, "userLogin")) - .setAssigneeLogin(utils.getString(json, "assigneeLogin")) + .setAssigneeLogin(utils.getString(json, "assignee")) .setCreatedAt(utils.getDateTime(json, "createdAt")) .setUpdatedAt(utils.getDateTime(json, "updatedAt")) .setClosedAt(utils.getDateTime(json, "closedAt")); diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/IssueQueryTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/IssueQueryTest.java index 3489cbf1baf..c0d48cc3a7c 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/services/IssueQueryTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/services/IssueQueryTest.java @@ -50,7 +50,7 @@ public class IssueQueryTest extends QueryTestCase { ; assertThat(query.getUrl()).isEqualTo("/api/issues/search?keys=ABCDE,FGHIJ&severities=BLOCKER,INFO&statuses=OPEN,CLOSED&" + "resolutions=resolution1,resolution2&components=component1,component2&componentRoots=componentRoot1,componentRoot2&rules=squid%3AAvoidCycle&" + - "userLogins=userLogin1,userLogin2&assigneeLogins=arthur,perceval&limit=1&"); + "userLogins=userLogin1,userLogin2&assignees=arthur,perceval&limit=1&"); } @Test diff --git a/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/IssueUnmarshallerTest.java b/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/IssueUnmarshallerTest.java index 336393bb924..315acb773b8 100644 --- a/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/IssueUnmarshallerTest.java +++ b/sonar-ws-client/src/test/java/org/sonar/wsclient/unmarshallers/IssueUnmarshallerTest.java @@ -50,7 +50,7 @@ public class IssueUnmarshallerTest extends UnmarshallerTestCase { assertThat(issue.getStatus()).isEqualTo("OPEN"); assertThat(issue.getResolution()).isEqualTo("FIXED"); assertThat(issue.getUserLogin()).isEqualTo("admin"); - assertThat(issue.getAssigneeLogin()).isEqualTo("admin"); + assertThat(issue.getAssignee()).isEqualTo("admin"); assertThat(issue.getCreatedAt()).isNotNull(); assertThat(issue.getUpdatedAt()).isNotNull(); assertThat(issue.getClosedAt()).isNull(); diff --git a/sonar-ws-client/src/test/resources/issues/all_issues.json b/sonar-ws-client/src/test/resources/issues/all_issues.json index 05ff7acc388..fa695a66146 100644 --- a/sonar-ws-client/src/test/resources/issues/all_issues.json +++ b/sonar-ws-client/src/test/resources/issues/all_issues.json @@ -12,7 +12,7 @@ "status": "OPEN", "resolution": "FIXED", "userLogin": "admin", - "assigneeLogin": "admin", + "assignee": "admin", "createdAt": "2013-04-10T18:21:15+0200", "updatedAt": "2013-04-10T18:21:15+0200", "closedAt": null @@ -30,7 +30,7 @@ "status": "OPEN", "resolution": null, "userLogin": null, - "assigneeLogin": null, + "assignee": null, "createdAt": "2013-04-10T18:21:15+0200", "updatedAt": "2013-04-10T18:21:15+0200", "closedAt": null diff --git a/sonar-ws-client/src/test/resources/issues/single_issue.json b/sonar-ws-client/src/test/resources/issues/single_issue.json index 113e1e72fbd..4e6b5abc168 100644 --- a/sonar-ws-client/src/test/resources/issues/single_issue.json +++ b/sonar-ws-client/src/test/resources/issues/single_issue.json @@ -12,7 +12,7 @@ "status": "OPEN", "resolution": "FIXED", "userLogin": "admin", - "assigneeLogin": "admin", + "assignee": "admin", "createdAt": "2013-04-10T18:21:15+0200", "updatedAt": "2013-04-10T18:21:15+0200", "closedAt": null |