SONAR-3526 Some refactoring

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<IssueDto> dbIssues, Collection<DefaultIssue> 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<DefaultIssue> 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<DefaultIssue> issues, String source) {
     List<String> 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<IssueDto> 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<DefaultIssue> 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);
   }