From cc95ce6bda3089ed3d89b9cf35a91143ba25f3ef Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Tue, 9 Jul 2013 18:39:47 +0200 Subject: [PATCH] SONAR-3526 Display the author (SCM account) of an issue if exists --- .../plugins/core/issue/IssueTracking.java | 65 +++++++++++---- .../resources/org/sonar/l10n/core.properties | 1 + .../plugins/core/issue/IssueTrackingTest.java | 81 ++++++++++++++++--- .../WEB-INF/app/views/issue/_issue.html.erb | 8 +- 4 files changed, 129 insertions(+), 26 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 b433614c630..57b461727bc 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 @@ -29,24 +29,41 @@ import com.google.common.collect.Multimap; import org.sonar.api.BatchExtension; import org.sonar.api.batch.SonarIndex; import org.sonar.api.issue.internal.DefaultIssue; +import org.sonar.api.measures.CoreMetrics; +import org.sonar.api.measures.FileLinesContext; +import org.sonar.api.measures.FileLinesContextFactory; import org.sonar.api.resources.Resource; +import org.sonar.api.resources.ResourceUtils; import org.sonar.api.rule.RuleKey; import org.sonar.batch.scan.LastSnapshots; import org.sonar.core.issue.db.IssueDto; -import org.sonar.plugins.core.issue.tracking.*; +import org.sonar.plugins.core.issue.tracking.HashedSequence; +import org.sonar.plugins.core.issue.tracking.HashedSequenceComparator; +import org.sonar.plugins.core.issue.tracking.RollingHashSequence; +import org.sonar.plugins.core.issue.tracking.RollingHashSequenceComparator; +import org.sonar.plugins.core.issue.tracking.SourceChecksum; +import org.sonar.plugins.core.issue.tracking.StringText; +import org.sonar.plugins.core.issue.tracking.StringTextComparator; +import org.sonar.plugins.core.issue.tracking.ViolationTrackingBlocksRecognizer; import javax.annotation.Nullable; -import java.util.*; +import java.util.Collection; +import java.util.Collections; +import java.util.Comparator; +import java.util.List; +import java.util.Map; public class IssueTracking implements BatchExtension { private final LastSnapshots lastSnapshots; private final SonarIndex index; + private FileLinesContextFactory fileLineContextFactory; - public IssueTracking(LastSnapshots lastSnapshots, SonarIndex index) { + public IssueTracking(LastSnapshots lastSnapshots, SonarIndex index, FileLinesContextFactory fileLineContextFactory) { this.lastSnapshots = lastSnapshots; this.index = index; + this.fileLineContextFactory = fileLineContextFactory; } public IssueTrackingResult track(Resource resource, Collection dbIssues, Collection newIssues) { @@ -55,11 +72,29 @@ public class IssueTracking implements BatchExtension { String source = index.getSource(resource); setChecksumOnNewIssues(newIssues, source); + setScmAuthorOnNewIssues(resource, newIssues); + // Map new issues with old ones mapIssues(newIssues, dbIssues, source, resource, result); return result; } + @VisibleForTesting + void setScmAuthorOnNewIssues(Resource resource, Collection newIssues) { + if (ResourceUtils.isFile(resource)) { + FileLinesContext fileLineContext = fileLineContextFactory.createFor(resource); + for (DefaultIssue issue : newIssues) { + if (issue.line() != null) { + // TODO When issue is on line 0 then who is the author? + String scmAuthorLogin = fileLineContext.getStringValue(CoreMetrics.SCM_AUTHORS_BY_LINE_KEY, issue.line()); + if (scmAuthorLogin != null) { + issue.setAuthorLogin(scmAuthorLogin); + } + } + } + } + } + private void setChecksumOnNewIssues(Collection issues, String source) { List checksums = SourceChecksum.lineChecksumsOfFile(source); for (DefaultIssue issue : issues) { @@ -102,9 +137,9 @@ public class IssueTracking implements BatchExtension { for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue, result)) { mapIssue( - newIssue, - findLastIssueWithSameLineAndChecksum(newIssue, result.unmatchedForRule(newIssue.ruleKey())), - result); + newIssue, + findLastIssueWithSameLineAndChecksum(newIssue, result.unmatchedForRule(newIssue.ruleKey())), + result); } } } @@ -179,9 +214,9 @@ public class IssueTracking implements BatchExtension { for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue, result)) { mapIssue( - newIssue, - findLastIssueWithSameChecksumAndMessage(newIssue, result.unmatchedForRule(newIssue.ruleKey())), - result); + newIssue, + findLastIssueWithSameChecksumAndMessage(newIssue, result.unmatchedForRule(newIssue.ruleKey())), + result); } } @@ -189,9 +224,9 @@ public class IssueTracking implements BatchExtension { for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue, result)) { mapIssue( - newIssue, - findLastIssueWithSameLineAndMessage(newIssue, result.unmatchedForRule(newIssue.ruleKey())), - result); + newIssue, + findLastIssueWithSameLineAndMessage(newIssue, result.unmatchedForRule(newIssue.ruleKey())), + result); } } @@ -200,9 +235,9 @@ public class IssueTracking implements BatchExtension { for (DefaultIssue newIssue : newIssues) { if (isNotAlreadyMapped(newIssue, result)) { mapIssue( - newIssue, - findLastIssueWithSameChecksum(newIssue, result.unmatchedForRule(newIssue.ruleKey())), - result); + newIssue, + findLastIssueWithSameChecksum(newIssue, result.unmatchedForRule(newIssue.ruleKey())), + result); } } } diff --git a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties index aca30d943ba..c5a4b3ef0ab 100644 --- a/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties +++ b/plugins/sonar-core-plugin/src/main/resources/org/sonar/l10n/core.properties @@ -505,6 +505,7 @@ issue.manual.missing_rule=Missing rule issue.manual.no_rules.admin=Manual rules must be defined before manual issues can be created. issue.manual.no_rules.non_admin=At least one manual rule must exist before manual issues can be created. Please contact your project administrator. issue.reported_by=Reported by +issue.authorLogin=Author: issue.component_deleted=Removed issue.changelog.changed_to={0} changed to {1} issue.changelog.was=was {0} 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 cd2867c0d2f..349229e05b6 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingTest.java @@ -26,8 +26,11 @@ import org.junit.Before; import org.junit.Test; import org.sonar.api.issue.Issue; import org.sonar.api.issue.internal.DefaultIssue; +import org.sonar.api.measures.FileLinesContext; +import org.sonar.api.measures.FileLinesContextFactory; import org.sonar.api.resources.Project; import org.sonar.api.resources.Resource; +import org.sonar.api.resources.Scopes; import org.sonar.api.rule.RuleKey; import org.sonar.batch.scan.LastSnapshots; import org.sonar.core.issue.db.IssueDto; @@ -46,13 +49,15 @@ public class IssueTrackingTest { Resource project; LastSnapshots lastSnapshots; long violationId = 0; + private FileLinesContextFactory fileLineContextFactory; @Before public void before() { lastSnapshots = mock(LastSnapshots.class); project = mock(Project.class); - tracking = new IssueTracking(lastSnapshots, null); + fileLineContextFactory = mock(FileLinesContextFactory.class); + tracking = new IssueTracking(lastSnapshots, null, fileLineContextFactory); } @Test @@ -267,9 +272,9 @@ public class IssueTrackingTest { IssueTrackingResult result = new IssueTrackingResult(); tracking.mapIssues( - Arrays.asList(newIssue1, newIssue2, newIssue3), - Arrays.asList(referenceIssue1), - source, project, result); + Arrays.asList(newIssue1, newIssue2, newIssue3), + Arrays.asList(referenceIssue1), + source, project, result); assertThat(result.matching(newIssue1)).isNull(); assertThat(result.matching(newIssue2)).isSameAs(referenceIssue1); @@ -283,24 +288,27 @@ public class IssueTrackingTest { IssueDto referenceIssue1 = newReferenceIssue("Avoid unused local variables such as 'j'.", 6, "squid", "AvoidCycle", "63c11570fc0a76434156be5f8138fa03"); IssueDto referenceIssue2 = newReferenceIssue("Avoid unused private methods such as 'myMethod()'.", 13, "squid", "NullDeref", "ef23288705d1ef1e512448ace287586e"); - IssueDto referenceIssue3 = newReferenceIssue("Method 'avoidUtilityClass' is not designed for extension - needs to be abstract, final or empty.", 9, "pmd", "UnusedLocalVariable", "ed5cdd046fda82727d6fedd1d8e3a310"); + IssueDto referenceIssue3 = newReferenceIssue("Method 'avoidUtilityClass' is not designed for extension - needs to be abstract, final or empty.", 9, "pmd", + "UnusedLocalVariable", "ed5cdd046fda82727d6fedd1d8e3a310"); // New issue DefaultIssue newIssue1 = newDefaultIssue("Avoid unused local variables such as 'msg'.", 18, RuleKey.of("squid", "AvoidCycle"), "a24254126be2bf1a9b9a8db43f633733"); // 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"); + 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"); + 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"); IssueTrackingResult result = new IssueTrackingResult(); tracking.mapIssues( - Arrays.asList(newIssue1, newIssue2, newIssue3, newIssue4, newIssue5), - Arrays.asList(referenceIssue1, referenceIssue2, referenceIssue3), - source, project, result); + Arrays.asList(newIssue1, newIssue2, newIssue3, newIssue4, newIssue5), + Arrays.asList(referenceIssue1, referenceIssue2, referenceIssue3), + source, project, result); assertThat(result.matching(newIssue1)).isNull(); assertThat(result.matching(newIssue2)).isSameAs(referenceIssue2); @@ -309,6 +317,59 @@ public class IssueTrackingTest { assertThat(result.matching(newIssue5)).isSameAs(referenceIssue1); } + @Test + public void should_update_scm_author_on_new_issues() { + Resource resource = mock(Resource.class); + FileLinesContext context = mock(FileLinesContext.class); + when(context.getStringValue("authors_by_line", 1)).thenReturn("julien"); + when(fileLineContextFactory.createFor(resource)).thenReturn(context); + + DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); + + when(resource.getScope()).thenReturn(Scopes.FILE); + tracking.setScmAuthorOnNewIssues(resource, Arrays.asList(newIssue)); + assertThat(newIssue.authorLogin()).isEqualTo("julien"); + } + + @Test + public void should_not_update_scm_author_when_resource_is_not_a_file() { + Resource resource = mock(Resource.class); + + DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); + + when(resource.getScope()).thenReturn(Scopes.PROJECT); + tracking.setScmAuthorOnNewIssues(resource, Arrays.asList(newIssue)); + assertThat(newIssue.authorLogin()).isNull(); + } + + @Test + public void should_not_update_scm_author_when_issue_is_on_line_0() { + Resource resource = mock(Resource.class); + FileLinesContext context = mock(FileLinesContext.class); + when(context.getStringValue("authors_by_line", 1)).thenReturn("julien"); + when(fileLineContextFactory.createFor(resource)).thenReturn(context); + + DefaultIssue newIssue = newDefaultIssue("message", null, RuleKey.of("squid", "AvoidCycle"), "checksum1"); + + when(resource.getScope()).thenReturn(Scopes.FILE); + tracking.setScmAuthorOnNewIssues(resource, Arrays.asList(newIssue)); + assertThat(newIssue.authorLogin()).isNull(); + } + + @Test + public void should_not_update_scm_author_when_unknow_scm_author() { + Resource resource = mock(Resource.class); + FileLinesContext context = mock(FileLinesContext.class); + when(context.getStringValue("authors_by_line", 1)).thenReturn(null); + when(fileLineContextFactory.createFor(resource)).thenReturn(context); + + DefaultIssue newIssue = newDefaultIssue("message", 1, RuleKey.of("squid", "AvoidCycle"), "checksum1"); + + when(resource.getScope()).thenReturn(Scopes.FILE); + tracking.setScmAuthorOnNewIssues(resource, Arrays.asList(newIssue)); + assertThat(newIssue.authorLogin()).isNull(); + } + private static String load(String name) throws IOException { return Resources.toString(IssueTrackingTest.class.getResource("IssueTrackingTest/" + name + ".txt"), Charsets.UTF_8); } diff --git a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_issue.html.erb b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_issue.html.erb index ffdf06a3693..ff010384541 100644 --- a/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_issue.html.erb +++ b/sonar-server/src/main/webapp/WEB-INF/app/views/issue/_issue.html.erb @@ -29,6 +29,12 @@ <%= message('issue.reported_by') -%> <%= @issue_results.user(issue.reporter).name -%>   <% end %> + <% if issue.authorLogin %> + +   + <%= message('issue.authorLogin') -%> <%= issue.authorLogin -%> +   + <% end %> @@ -147,4 +153,4 @@ <% end %> <% end %> - \ No newline at end of file + -- 2.39.5