From 7f8c6bc89dbc2f5a1792b17228b1a17b2c8483d6 Mon Sep 17 00:00:00 2001 From: Simon Brandhof Date: Fri, 19 Apr 2013 13:39:23 +0200 Subject: [PATCH] SONAR-3755 rename Issue#message() to description() --- .../plugins/core/issue/IssueTracking.java | 73 +++---- .../core/issue/IssuesWorkflowDecorator.java | 69 ++----- .../plugins/core/issue/IssueTrackingTest.java | 192 +----------------- .../issue/IssuesWorkflowDecoratorTest.java | 12 +- .../batch/issue/DeprecatedViolations.java | 2 +- .../org/sonar/batch/issue/IssuePersister.java | 29 +-- .../sonar/batch/issue/IssuesDecorator.java | 2 +- .../sonar/batch/issue/ScanIssueChanges.java | 4 +- .../batch/issue/IssuesDecoratorTest.java | 2 +- .../batch/issue/ScanIssueChangesTest.java | 7 +- .../org/sonar/core/issue/DefaultIssue.java | 26 ++- .../sonar/core/issue/DefaultIssueBuilder.java | 8 +- .../sonar/core/issue/DefaultIssueFinder.java | 38 +--- .../java/org/sonar/core/issue/IssueDto.java | 92 ++++++--- .../org/sonar/core/user/AuthorizationDao.java | 5 +- .../org/sonar/core/issue/IssueMapper.xml | 59 +++--- .../core/issue/DefaultIssueBuilderTest.java | 6 +- .../core/issue/DefaultIssueFinderTest.java | 12 +- .../sonar/core/issue/DefaultIssueTest.java | 11 + .../org/sonar/core/issue/IssueDaoTest.java | 12 +- .../core/issue/IssueDaoTest/insert-result.xml | 2 +- .../java/org/sonar/api/issue/Issuable.java | 2 +- .../main/java/org/sonar/api/issue/Issue.java | 5 +- .../java/org/sonar/api/issue/IssueChange.java | 20 +- .../org/sonar/api/issue/IssueChanges.java | 2 +- .../java/org/sonar/api/issue/IssueFinder.java | 2 +- .../java/org/sonar/api/issue/IssueQuery.java | 20 +- .../org/sonar/api/issue/IssueChangeTest.java | 6 +- .../org/sonar/api/issue/IssueQueryTest.java | 4 +- .../server/issue/DefaultJRubyIssues.java | 8 +- .../server/issue/ServerIssueChanges.java | 68 +++++++ .../org/sonar/server/platform/Platform.java | 2 + .../app/controllers/api/issues_controller.rb | 8 +- .../app/controllers/issues_controller.rb | 4 +- .../WEB-INF/app/views/issues/_list.html.erb | 13 +- .../server/issue/DefaultJRubyIssuesTest.java | 7 +- .../org/sonar/wsclient/services/Issue.java | 2 +- .../sonar/wsclient/services/IssueQuery.java | 2 +- .../unmarshallers/IssueUnmarshaller.java | 2 +- .../wsclient/services/IssueQueryTest.java | 2 +- .../unmarshallers/IssueUnmarshallerTest.java | 2 +- .../src/test/resources/issues/all_issues.json | 4 +- .../test/resources/issues/single_issue.json | 2 +- 43 files changed, 344 insertions(+), 506 deletions(-) create mode 100644 sonar-server/src/main/java/org/sonar/server/issue/ServerIssueChanges.java 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 LINE_PAIR_COMPARATOR = new Comparator() { 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 referenceIssues, Collection 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 newIssues, Collection lastIssues, Multimap 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 lastIssuesByRule, Map 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 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 issueKeys, Resource resource) { + private void closeResolvedStandardIssues(IssueDto openIssue, Set 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 openIssues, Resource resource) { + private void closeIssuesOnDeletedResources(Collection 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 toDefaultIssues(Collection issues) { return newArrayList(Iterables.transform(issues, new Function() { 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 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 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 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 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 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 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.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.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.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.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.emptyList()); when(initialOpenIssuesStack.selectAndRemove(anyInt())).thenReturn(Collections.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 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 attributes() { - return attributes == null ? Collections.emptyMap() : ImmutableMap.copyOf(attributes); + return attributes == null ? Collections.emptyMap() : ImmutableMap.copyOf(attributes); } public DefaultIssue setAttributes(Map 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 authorizedComponentIds = authorizationDao.keepAuthorizedComponentIds(componentIds, currentUserId, ROLE, sqlSession); + Set authorizedComponentIds = authorizationDao.keepAuthorizedComponentIds(componentIds, currentUserId, role, sqlSession); List 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,19 +267,12 @@ 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; } - /** - * Only for unit tests - */ - public IssueDto setRuleRepo_unit_test_only(String ruleRepo) { - this.ruleRepo = ruleRepo; - return this; - } - /** * Only for unit tests */ @@ -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.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 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}) @@ -45,31 +45,31 @@ 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}) 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} @@ -77,7 +77,10 @@ select 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