SONAR-5644 Apply some feedback on SCM API

3 files changed, 42 insertions, 29 deletions

diff --git a/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnBlameCommand.java b/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnBlameCommand.java
index 48b65ffee6d..27647c8cad2 100644
--- a/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnBlameCommand.java
+++ b/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnBlameCommand.java
@@ -22,8 +22,6 @@ package org.sonar.plugins.scm.svn;
 import com.google.common.annotations.VisibleForTesting;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
-import org.sonar.api.BatchComponent;
-import org.sonar.api.batch.InstantiationStrategy;
 import org.sonar.api.batch.fs.FileSystem;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.scm.BlameCommand;
@@ -44,8 +42,7 @@ import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.Future;
 
-@InstantiationStrategy(InstantiationStrategy.PER_BATCH)
-public class SvnBlameCommand implements BlameCommand, BatchComponent {
+public class SvnBlameCommand extends BlameCommand {
 
   private static final Logger LOG = LoggerFactory.getLogger(SvnBlameCommand.class);
   private final CommandExecutor commandExecutor;
@@ -61,12 +58,13 @@ public class SvnBlameCommand implements BlameCommand, BatchComponent {
   }
 
   @Override
-  public void blame(final FileSystem fs, Iterable<InputFile> files, final BlameResult result) {
+  public void blame(final BlameInput input, final BlameOutput output) {
+    FileSystem fs = input.fileSystem();
     LOG.debug("Working directory: " + fs.baseDir().getAbsolutePath());
     ExecutorService executorService = Executors.newFixedThreadPool(Runtime.getRuntime().availableProcessors() + 1);
     List<Future<Void>> tasks = new ArrayList<Future<Void>>();
-    for (InputFile inputFile : files) {
-      tasks.add(submitTask(fs, result, executorService, inputFile));
+    for (InputFile inputFile : input.filesToBlame()) {
+      tasks.add(submitTask(fs, output, executorService, inputFile));
     }
 
     for (Future<Void> task : tasks) {
@@ -81,7 +79,7 @@ public class SvnBlameCommand implements BlameCommand, BatchComponent {
     }
   }
 
-  private Future<Void> submitTask(final FileSystem fs, final BlameResult result, ExecutorService executorService, final InputFile inputFile) {
+  private Future<Void> submitTask(final FileSystem fs, final BlameOutput result, ExecutorService executorService, final InputFile inputFile) {
     return executorService.submit(new Callable<Void>() {
       @Override
       public Void call() {
@@ -91,7 +89,7 @@ public class SvnBlameCommand implements BlameCommand, BatchComponent {
     });
   }
 
-  private void blame(final FileSystem fs, final InputFile inputFile, final BlameResult result) {
+  private void blame(final FileSystem fs, final InputFile inputFile, final BlameOutput output) {
     String filename = inputFile.relativePath();
     Command cl = createCommandLine(fs.baseDir(), filename);
     SvnBlameConsumer consumer = new SvnBlameConsumer(filename);
@@ -111,7 +109,7 @@ public class SvnBlameCommand implements BlameCommand, BatchComponent {
       // SONARPLUGINS-3097 SVN do not report blame on last empty line
       lines.add(lines.get(lines.size() - 1));
     }
-    result.add(inputFile, lines);
+    output.blameResult(inputFile, lines);
   }
 
   private int execute(Command cl, StreamConsumer consumer, StreamConsumer stderr) {
diff --git a/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnBlameConsumer.java b/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnBlameConsumer.java
index dcf67a50572..8224fab347d 100644
--- a/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnBlameConsumer.java
+++ b/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnBlameConsumer.java
@@ -43,6 +43,8 @@ import org.slf4j.LoggerFactory;
 import org.sonar.api.batch.scm.BlameLine;
 import org.sonar.api.utils.command.StreamConsumer;
 
+import javax.annotation.CheckForNull;
+
 import java.text.ParseException;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
@@ -101,13 +103,14 @@ public class SvnBlameConsumer implements StreamConsumer {
       String date = matcher.group(1);
       String time = matcher.group(2);
       Date dateTime = parseDateTime(date + " " + time);
-      lines.add(new BlameLine(dateTime, revision, author));
+      lines.add(new BlameLine().revision(revision).author(author).date(dateTime));
       lineNumber = 0;
       revision = null;
       author = null;
     }
   }
 
+  @CheckForNull
   protected Date parseDateTime(String dateTimeStr) {
     try {
       return dateFormat.parse(dateTimeStr);
diff --git a/plugins/sonar-svn-plugin/src/test/java/org/sonar/plugins/scm/svn/SvnBlameCommandTest.java b/plugins/sonar-svn-plugin/src/test/java/org/sonar/plugins/scm/svn/SvnBlameCommandTest.java
index f7bb1fc66dc..b85b71d2fdb 100644
--- a/plugins/sonar-svn-plugin/src/test/java/org/sonar/plugins/scm/svn/SvnBlameCommandTest.java
+++ b/plugins/sonar-svn-plugin/src/test/java/org/sonar/plugins/scm/svn/SvnBlameCommandTest.java
@@ -31,7 +31,8 @@ import org.mockito.stubbing.Answer;
 import org.sonar.api.batch.fs.InputFile;
 import org.sonar.api.batch.fs.internal.DefaultFileSystem;
 import org.sonar.api.batch.fs.internal.DefaultInputFile;
-import org.sonar.api.batch.scm.BlameCommand.BlameResult;
+import org.sonar.api.batch.scm.BlameCommand.BlameInput;
+import org.sonar.api.batch.scm.BlameCommand.BlameOutput;
 import org.sonar.api.batch.scm.BlameLine;
 import org.sonar.api.config.PropertyDefinitions;
 import org.sonar.api.config.Settings;
@@ -48,7 +49,9 @@ import java.util.List;
 import static org.fest.assertions.Assertions.assertThat;
 import static org.mockito.Matchers.any;
 import static org.mockito.Matchers.anyLong;
-import static org.mockito.Mockito.*;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
 
 public class SvnBlameCommandTest {
 
@@ -60,12 +63,15 @@ public class SvnBlameCommandTest {
 
   private DefaultFileSystem fs;
   private File baseDir;
+  private BlameInput input;
 
   @Before
   public void prepare() throws IOException {
     baseDir = temp.newFolder();
     fs = new DefaultFileSystem();
     fs.setBaseDir(baseDir);
+    input = mock(BlameInput.class);
+    when(input.fileSystem()).thenReturn(fs);
   }
 
   @Test
@@ -75,7 +81,7 @@ public class SvnBlameCommandTest {
     DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setAbsolutePath(new File(baseDir, "src/foo.xoo").getAbsolutePath());
     fs.add(inputFile);
 
-    BlameResult result = mock(BlameResult.class);
+    BlameOutput result = mock(BlameOutput.class);
     CommandExecutor commandExecutor = mock(CommandExecutor.class);
 
     when(commandExecutor.execute(any(Command.class), any(StreamConsumer.class), any(StreamConsumer.class), anyLong())).thenAnswer(new Answer<Integer>() {
@@ -91,12 +97,14 @@ public class SvnBlameCommandTest {
       }
     });
 
-    new SvnBlameCommand(commandExecutor, mock(SvnConfiguration.class)).blame(fs, Arrays.<InputFile>asList(inputFile), result);
-    verify(result).add(inputFile,
+    when(input.filesToBlame()).thenReturn(Arrays.<InputFile>asList(inputFile));
+
+    new SvnBlameCommand(commandExecutor, mock(SvnConfiguration.class)).blame(input, result);
+    verify(result).blameResult(inputFile,
       Arrays.asList(
-        new BlameLine(DateUtils.parseDateTime("2009-04-18T10:29:59+0000"), "9491", "simon.brandhof"),
-        new BlameLine(DateUtils.parseDateTime("2009-04-18T10:29:59+0000"), "9491", "simon.brandhof"),
-        new BlameLine(DateUtils.parseDateTime("2009-08-31T22:32:17+0000"), "10558", "david")));
+        new BlameLine().date(DateUtils.parseDateTime("2009-04-18T10:29:59+0000")).revision("9491").author("simon.brandhof"),
+        new BlameLine().date(DateUtils.parseDateTime("2009-04-18T10:29:59+0000")).revision("9491").author("simon.brandhof"),
+        new BlameLine().date(DateUtils.parseDateTime("2009-08-31T22:32:17+0000")).revision("10558").author("david")));
   }
 
   @Test
@@ -106,7 +114,7 @@ public class SvnBlameCommandTest {
     DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setAbsolutePath(new File(baseDir, "src/foo.xoo").getAbsolutePath());
     fs.add(inputFile);
 
-    BlameResult result = mock(BlameResult.class);
+    BlameOutput result = mock(BlameOutput.class);
     CommandExecutor commandExecutor = mock(CommandExecutor.class);
 
     when(commandExecutor.execute(any(Command.class), any(StreamConsumer.class), any(StreamConsumer.class), anyLong())).thenAnswer(new Answer<Integer>() {
@@ -122,12 +130,14 @@ public class SvnBlameCommandTest {
       }
     });
 
-    new SvnBlameCommand(commandExecutor, mock(SvnConfiguration.class)).blame(fs, Arrays.<InputFile>asList(inputFile), result);
-    verify(result).add(inputFile,
+    when(input.filesToBlame()).thenReturn(Arrays.<InputFile>asList(inputFile));
+
+    new SvnBlameCommand(commandExecutor, mock(SvnConfiguration.class)).blame(input, result);
+    verify(result).blameResult(inputFile,
       Arrays.asList(
-        new BlameLine(DateUtils.parseDateTime("2009-04-18T10:29:59+0000"), "9491", "simon.brandhof"),
-        new BlameLine(DateUtils.parseDateTime("2009-04-01T10:29:59+0000"), "1", null),
-        new BlameLine(DateUtils.parseDateTime("2009-08-31T22:32:17+0000"), "10558", "david")));
+        new BlameLine().date(DateUtils.parseDateTime("2009-04-18T10:29:59+0000")).revision("9491").author("simon.brandhof"),
+        new BlameLine().date(DateUtils.parseDateTime("2009-04-01T10:29:59+0000")).revision("1"),
+        new BlameLine().date(DateUtils.parseDateTime("2009-08-31T22:32:17+0000")).revision("10558").author("david")));
   }
 
   @Test
@@ -137,7 +147,7 @@ public class SvnBlameCommandTest {
     DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setAbsolutePath(new File(baseDir, "src/foo.xoo").getAbsolutePath());
     fs.add(inputFile);
 
-    BlameResult result = mock(BlameResult.class);
+    BlameOutput result = mock(BlameOutput.class);
     CommandExecutor commandExecutor = mock(CommandExecutor.class);
 
     when(commandExecutor.execute(any(Command.class), any(StreamConsumer.class), any(StreamConsumer.class), anyLong())).thenAnswer(new Answer<Integer>() {
@@ -156,7 +166,8 @@ public class SvnBlameCommandTest {
     thrown.expect(IllegalStateException.class);
     thrown.expectMessage("Unable to blame file src/foo.xoo. No blame info at line 2. Is file commited?");
 
-    new SvnBlameCommand(commandExecutor, mock(SvnConfiguration.class)).blame(fs, Arrays.<InputFile>asList(inputFile), result);
+    when(input.filesToBlame()).thenReturn(Arrays.<InputFile>asList(inputFile));
+    new SvnBlameCommand(commandExecutor, mock(SvnConfiguration.class)).blame(input, result);
   }
 
   @Test
@@ -166,7 +177,7 @@ public class SvnBlameCommandTest {
     DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setAbsolutePath(new File(baseDir, "src/foo.xoo").getAbsolutePath());
     fs.add(inputFile);
 
-    BlameResult result = mock(BlameResult.class);
+    BlameOutput result = mock(BlameOutput.class);
     CommandExecutor commandExecutor = mock(CommandExecutor.class);
 
     when(commandExecutor.execute(any(Command.class), any(StreamConsumer.class), any(StreamConsumer.class), anyLong())).thenAnswer(new Answer<Integer>() {
@@ -182,7 +193,8 @@ public class SvnBlameCommandTest {
     thrown.expect(IllegalStateException.class);
     thrown.expectMessage("The svn blame command [svn blame --xml src/foo.xoo --non-interactive] failed: My error");
 
-    new SvnBlameCommand(commandExecutor, mock(SvnConfiguration.class)).blame(fs, Arrays.<InputFile>asList(inputFile), result);
+    when(input.filesToBlame()).thenReturn(Arrays.<InputFile>asList(inputFile));
+    new SvnBlameCommand(commandExecutor, mock(SvnConfiguration.class)).blame(input, result);
   }
 
   @Test