From 7e2f9917c56be4d7a4aa76586a27e316518b75de Mon Sep 17 00:00:00 2001 From: Julien HENRY Date: Wed, 10 Jul 2013 09:35:36 +0200 Subject: [PATCH] SONAR-3526 Some refactoring --- .../plugins/core/issue/IssueTracking.java | 26 +---- .../core/issue/IssueTrackingDecorator.java | 27 ++++- .../issue/IssueTrackingDecoratorTest.java | 100 ++++++++++++++++-- .../plugins/core/issue/IssueTrackingTest.java | 61 +---------- 4 files changed, 118 insertions(+), 96 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 57b461727bc..b10d53a6750 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,11 +29,7 @@ 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; @@ -58,12 +54,10 @@ public class IssueTracking implements BatchExtension { private final LastSnapshots lastSnapshots; private final SonarIndex index; - private FileLinesContextFactory fileLineContextFactory; - public IssueTracking(LastSnapshots lastSnapshots, SonarIndex index, FileLinesContextFactory fileLineContextFactory) { + public IssueTracking(LastSnapshots lastSnapshots, SonarIndex index) { this.lastSnapshots = lastSnapshots; this.index = index; - this.fileLineContextFactory = fileLineContextFactory; } public IssueTrackingResult track(Resource resource, Collection dbIssues, Collection newIssues) { @@ -72,29 +66,11 @@ 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) { diff --git a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java index 4943d1ff52a..c49d279ea56 100644 --- a/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java +++ b/plugins/sonar-core-plugin/src/main/java/org/sonar/plugins/core/issue/IssueTrackingDecorator.java @@ -19,6 +19,10 @@ */ package org.sonar.plugins.core.issue; +import org.sonar.api.measures.FileLinesContextFactory; + +import org.sonar.api.measures.CoreMetrics; +import org.sonar.api.measures.FileLinesContext; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Strings; import com.google.common.collect.Lists; @@ -57,6 +61,7 @@ public class IssueTrackingDecorator implements Decorator { private final ResourcePerspectives perspectives; private final RulesProfile rulesProfile; private final RuleFinder ruleFinder; + private FileLinesContextFactory fileLineContextFactory; public IssueTrackingDecorator(IssueCache issueCache, InitialOpenIssuesStack initialOpenIssues, IssueTracking tracking, IssueHandlers handlers, IssueWorkflow workflow, @@ -64,13 +69,15 @@ public class IssueTrackingDecorator implements Decorator { Project project, ResourcePerspectives perspectives, RulesProfile rulesProfile, - RuleFinder ruleFinder) { + RuleFinder ruleFinder, + FileLinesContextFactory fileLineContextFactory) { this.issueCache = issueCache; this.initialOpenIssues = initialOpenIssues; this.tracking = tracking; this.handlers = handlers; this.workflow = workflow; this.updater = updater; + this.fileLineContextFactory = fileLineContextFactory; this.changeContext = IssueChangeContext.createScan(project.getAnalysisDate()); this.perspectives = perspectives; this.rulesProfile = rulesProfile; @@ -100,6 +107,8 @@ public class IssueTrackingDecorator implements Decorator { // all the issues that are not closed in db before starting this module scan, including manual issues Collection dbOpenIssues = initialOpenIssues.selectAndRemove(resource.getEffectiveKey()); + setScmAuthorOnNewIssues(resource, issues); + IssueTrackingResult trackingResult = tracking.track(resource, dbOpenIssues, issues); // unmatched = issues that have been resolved + issues on disabled/removed rules + manual issues @@ -119,6 +128,22 @@ public class IssueTrackingDecorator implements Decorator { } } + @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 mergeMatched(IssueTrackingResult result) { for (DefaultIssue issue : result.matched()) { IssueDto ref = result.matching(issue); diff --git a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java index 54d54bc6b65..9e8d0e9dbd4 100644 --- a/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java +++ b/plugins/sonar-core-plugin/src/test/java/org/sonar/plugins/core/issue/IssueTrackingDecoratorTest.java @@ -27,10 +27,13 @@ import org.sonar.api.batch.DecoratorContext; import org.sonar.api.component.ResourcePerspectives; import org.sonar.api.issue.internal.DefaultIssue; import org.sonar.api.issue.internal.IssueChangeContext; +import org.sonar.api.measures.FileLinesContext; +import org.sonar.api.measures.FileLinesContextFactory; import org.sonar.api.profiles.RulesProfile; import org.sonar.api.resources.File; 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.api.rules.Rule; import org.sonar.api.rules.RuleFinder; @@ -47,7 +50,18 @@ import java.util.Collections; import java.util.List; import static org.fest.assertions.Assertions.assertThat; -import static org.mockito.Mockito.*; +import static org.mockito.Matchers.any; +import static org.mockito.Matchers.anyCollection; +import static org.mockito.Matchers.anyString; +import static org.mockito.Matchers.argThat; +import static org.mockito.Matchers.eq; +import static org.mockito.Mockito.RETURNS_MOCKS; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyZeroInteractions; +import static org.mockito.Mockito.when; public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { @@ -61,20 +75,23 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { ResourcePerspectives perspectives = mock(ResourcePerspectives.class); RulesProfile profile = mock(RulesProfile.class); RuleFinder ruleFinder = mock(RuleFinder.class); + private FileLinesContextFactory fileLineContextFactory; @Before public void init() { + fileLineContextFactory = mock(FileLinesContextFactory.class); decorator = new IssueTrackingDecorator( - issueCache, - initialOpenIssues, - tracking, - handlers, - workflow, - updater, - mock(Project.class), - perspectives, - profile, - ruleFinder); + issueCache, + initialOpenIssues, + tracking, + handlers, + workflow, + updater, + mock(Project.class), + perspectives, + profile, + ruleFinder, + fileLineContextFactory); } @Test @@ -223,4 +240,65 @@ public class IssueTrackingDecoratorTest extends AbstractDaoTestCase { } })); } + + @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 = mock(DefaultIssue.class); + when(newIssue.line()).thenReturn(1); + + when(resource.getScope()).thenReturn(Scopes.FILE); + decorator.setScmAuthorOnNewIssues(resource, Arrays.asList(newIssue)); + + verify(newIssue).setAuthorLogin("julien"); + } + + @Test + public void should_not_update_scm_author_when_resource_is_not_a_file() { + Resource resource = mock(Resource.class); + + DefaultIssue newIssue = mock(DefaultIssue.class); + when(newIssue.line()).thenReturn(1); + + when(resource.getScope()).thenReturn(Scopes.PROJECT); + decorator.setScmAuthorOnNewIssues(resource, Arrays.asList(newIssue)); + + verify(newIssue, never()).setAuthorLogin(anyString()); + } + + @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 = mock(DefaultIssue.class); + when(newIssue.line()).thenReturn(null); + + when(resource.getScope()).thenReturn(Scopes.FILE); + decorator.setScmAuthorOnNewIssues(resource, Arrays.asList(newIssue)); + + verify(newIssue, never()).setAuthorLogin(anyString()); + } + + @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 = mock(DefaultIssue.class); + when(newIssue.line()).thenReturn(1); + + when(resource.getScope()).thenReturn(Scopes.FILE); + decorator.setScmAuthorOnNewIssues(resource, Arrays.asList(newIssue)); + + verify(newIssue, never()).setAuthorLogin(anyString()); + } } 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 349229e05b6..72d70638ab5 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,11 +26,8 @@ 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; @@ -49,15 +46,14 @@ 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); - fileLineContextFactory = mock(FileLinesContextFactory.class); - tracking = new IssueTracking(lastSnapshots, null, fileLineContextFactory); + + tracking = new IssueTracking(lastSnapshots, null); } @Test @@ -317,59 +313,6 @@ 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); } -- 2.39.5