SONAR-5843 Use the original SVN author on merges

6 files changed, 157 insertions, 14 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 2e1038f162b..f088967589a 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
@@ -128,6 +128,9 @@ public class SvnBlameCommand extends BlameCommand {
     cl.setDirectory(baseDir);
     cl.addArgument("blame");
     cl.addArgument("--xml");
+    if (configuration.useMergeHistory()) {
+      cl.addArgument("--use-merge-history");
+    }
     cl.addArgument("--non-interactive");
     cl.addArgument("-x");
     cl.addArgument("-w");
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 8224fab347d..3bf42c756b3 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
@@ -68,6 +68,9 @@ public class SvnBlameConsumer implements StreamConsumer {
 
   private static final Pattern DATE_PATTERN = Pattern.compile("<date>(.*)T(.*)\\.(.*)Z</date>");
 
+  private boolean insideCommitSection = false;
+  private boolean insideMergedSection = false;
+
   private SimpleDateFormat dateFormat;
 
   private List<BlameLine> lines = new ArrayList<BlameLine>();
@@ -82,31 +85,61 @@ public class SvnBlameConsumer implements StreamConsumer {
 
   private int lineNumber = 0;
 
-  private String revision;
-
+  private String committerRevision;
+  private String committer;
+  private Date committerDate;
+  private String authorRevision;
   private String author;
+  private Date authorDate;
 
   @Override
   public void consumeLine(String line) {
     Matcher matcher;
     if ((matcher = LINE_PATTERN.matcher(line)).find()) {
-      if (lineNumber != 0) {
-        throw new IllegalStateException("Unable to blame file " + filename + ". No blame info at line " + lineNumber + ". Is file commited?");
-      }
       String lineNumberStr = matcher.group(1);
       lineNumber = Integer.parseInt(lineNumberStr);
+      insideCommitSection = false;
+      insideMergedSection = false;
+    } else if (line.contains("<commit") && !insideMergedSection) {
+      insideCommitSection = true;
+    } else if (line.contains("<merged")) {
+      insideMergedSection = true;
+      insideCommitSection = false;
     } else if ((matcher = REVISION_PATTERN.matcher(line)).find()) {
-      revision = matcher.group(1);
+      if (insideCommitSection) {
+        committerRevision = matcher.group(1);
+      } else if (insideMergedSection) {
+        authorRevision = matcher.group(1);
+      }
     } else if ((matcher = AUTHOR_PATTERN.matcher(line)).find()) {
-      author = matcher.group(1);
+      if (insideCommitSection) {
+        committer = matcher.group(1);
+      } else if (insideMergedSection) {
+        author = matcher.group(1);
+      }
     } else if ((matcher = DATE_PATTERN.matcher(line)).find()) {
       String date = matcher.group(1);
       String time = matcher.group(2);
       Date dateTime = parseDateTime(date + " " + time);
-      lines.add(new BlameLine().revision(revision).author(author).date(dateTime));
-      lineNumber = 0;
-      revision = null;
+      if (insideCommitSection) {
+        committerDate = dateTime;
+      } else if (insideMergedSection) {
+        authorDate = dateTime;
+      }
+    } else if (line.contains("</entry>")) {
+      if (authorRevision != null) {
+        lines.add(new BlameLine().revision(authorRevision).author(author).date(authorDate));
+      } else if (committerRevision != null) {
+        lines.add(new BlameLine().revision(committerRevision).author(committer).date(committerDate));
+      } else {
+        throw new IllegalStateException("Unable to blame file " + filename + ". No blame info at line " + lineNumber + ". Is file commited?");
+      }
+      insideCommitSection = false;
+      insideMergedSection = false;
       author = null;
+      committer = null;
+      committerRevision = null;
+      authorRevision = null;
     }
   }
 
diff --git a/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnConfiguration.java b/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnConfiguration.java
index 16942c8a0f8..8c7db3efb6d 100644
--- a/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnConfiguration.java
+++ b/plugins/sonar-svn-plugin/src/main/java/org/sonar/plugins/scm/svn/SvnConfiguration.java
@@ -40,6 +40,7 @@ public class SvnConfiguration implements BatchComponent {
   public static final String PASSWORD_PROP_KEY = "sonar.svn.password.secured";
   public static final String CONFIG_DIR_PROP_KEY = "sonar.svn.config_dir";
   public static final String TRUST_SERVER_PROP_KEY = "sonar.svn.trust_server_cert";
+  public static final String USE_MERGE_HISTORY_KEY = "sonar.svn.use_merge_history";
   private final Settings settings;
 
   public SvnConfiguration(Settings settings) {
@@ -84,6 +85,18 @@ public class SvnConfiguration implements BatchComponent {
         .category(CoreProperties.CATEGORY_SCM)
         .subCategory(CATEGORY_SVN)
         .index(3)
+        .build(),
+      PropertyDefinition
+        .builder(USE_MERGE_HISTORY_KEY)
+        .name("Use merge history for blame")
+        .description(
+          "Use merge history (--use-merge-history) to get real author of a modification instead of commiter of the merge. May not be supported by your SVN server/client.")
+        .type(PropertyType.BOOLEAN)
+        .defaultValue("false")
+        .onQualifiers(Qualifiers.PROJECT)
+        .category(CoreProperties.CATEGORY_SCM)
+        .subCategory(CATEGORY_SVN)
+        .index(4)
         .build());
   }
 
@@ -106,4 +119,8 @@ public class SvnConfiguration implements BatchComponent {
     return settings.getBoolean(TRUST_SERVER_PROP_KEY);
   }
 
+  public boolean useMergeHistory() {
+    return settings.getBoolean(USE_MERGE_HISTORY_KEY);
+  }
+
 }
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 6bc94b51ddd..f306fe3fc60 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
@@ -141,6 +141,39 @@ public class SvnBlameCommandTest {
   }
 
   @Test
+  public void testParsingOfOutputWithMergeHistory() throws IOException {
+    File source = new File(baseDir, "src/foo.xoo");
+    FileUtils.write(source, "sample content");
+    DefaultInputFile inputFile = new DefaultInputFile("foo", "src/foo.xoo").setAbsolutePath(new File(baseDir, "src/foo.xoo").getAbsolutePath());
+    fs.add(inputFile);
+
+    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>() {
+
+      @Override
+      public Integer answer(InvocationOnMock invocation) throws Throwable {
+        StreamConsumer outConsumer = (StreamConsumer) invocation.getArguments()[1];
+        List<String> lines = IOUtils.readLines(getClass().getResourceAsStream("/blame-with-merge-history.xml"), "UTF-8");
+        for (String line : lines) {
+          outConsumer.consumeLine(line);
+        }
+        return 0;
+      }
+    });
+
+    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().date(DateUtils.parseDateTime("2012-07-19T11:44:57+0200")).revision("9490").author("dgageot"),
+        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
   public void shouldFailIfFileContainsLocalModification() throws IOException {
     File source = new File(baseDir, "src/foo.xoo");
     FileUtils.write(source, "sample content");
@@ -215,10 +248,13 @@ public class SvnBlameCommandTest {
 
     settings.setProperty(SvnConfiguration.CONFIG_DIR_PROP_KEY, "/home/julien/.svn");
     settings.setProperty(SvnConfiguration.TRUST_SERVER_PROP_KEY, "true");
+    settings.setProperty(SvnConfiguration.USE_MERGE_HISTORY_KEY, "true");
     commandLine = svnBlameCommand.createCommandLine(baseDir, "src/main/java/Foo.java");
     assertThat(commandLine.toCommandLine())
-      .isEqualTo("svn blame --xml --non-interactive -x -w --config-dir /home/julien/.svn --username myUser --password myPass --trust-server-cert src/main/java/Foo.java");
-    assertThat(commandLine.toString()).isEqualTo(
-      "svn blame --xml --non-interactive -x -w --config-dir /home/julien/.svn --username ******** --password ******** --trust-server-cert src/main/java/Foo.java");
+      .isEqualTo(
+        "svn blame --xml --use-merge-history --non-interactive -x -w --config-dir /home/julien/.svn --username myUser --password myPass --trust-server-cert src/main/java/Foo.java");
+    assertThat(commandLine.toString())
+      .isEqualTo(
+        "svn blame --xml --use-merge-history --non-interactive -x -w --config-dir /home/julien/.svn --username ******** --password ******** --trust-server-cert src/main/java/Foo.java");
   }
 }
diff --git a/plugins/sonar-svn-plugin/src/test/java/org/sonar/plugins/scm/svn/SvnPluginTest.java b/plugins/sonar-svn-plugin/src/test/java/org/sonar/plugins/scm/svn/SvnPluginTest.java
index 7c8ea23a91a..745067f838e 100644
--- a/plugins/sonar-svn-plugin/src/test/java/org/sonar/plugins/scm/svn/SvnPluginTest.java
+++ b/plugins/sonar-svn-plugin/src/test/java/org/sonar/plugins/scm/svn/SvnPluginTest.java
@@ -27,6 +27,6 @@ public class SvnPluginTest {
 
   @Test
   public void getExtensions() {
-    assertThat(new SvnPlugin().getExtensions()).hasSize(7);
+    assertThat(new SvnPlugin().getExtensions()).hasSize(8);
   }
 }
diff --git a/plugins/sonar-svn-plugin/src/test/resources/blame-with-merge-history.xml b/plugins/sonar-svn-plugin/src/test/resources/blame-with-merge-history.xml
new file mode 100644
index 00000000000..efe87073a7a
--- /dev/null
+++ b/plugins/sonar-svn-plugin/src/test/resources/blame-with-merge-history.xml
@@ -0,0 +1,54 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<blame>
+<target
+   path="pom.xml">
+<entry
+   line-number="1">
+<commit
+   revision="9491">
+<author>automatic-merge</author>
+<date>2009-04-18T10:29:59.077093Z</date>
+</commit>
+<merged
+   path="/dummy-svn/pom.xml">
+<commit
+   revision="9490">
+<author>dgageot</author>
+<date>2012-07-19T09:44:57.393222Z</date>
+</commit>
+</merged>
+</entry>
+<entry
+   line-number="2">
+<commit
+   revision="9491">
+<author>simon.brandhof</author>
+<date>2009-04-18T10:29:59.077093Z</date>
+</commit>
+<merged
+   path="/dummy-svn/pom.xml">
+<commit
+   revision="9491">
+<author>simon.brandhof</author>
+<date>2009-04-18T10:29:59.077093Z</date>
+</commit>
+</merged>
+</entry>
+<entry
+   line-number="3">
+<commit
+   revision="10558">
+<author>david</author>
+<date>2009-08-31T22:32:17.361675Z</date>
+</commit>
+<merged
+   path="/dummy-svn/pom.xml">
+<commit
+   revision="10558">
+<author>david</author>
+<date>2009-08-31T22:32:17.361675Z</date>
+</commit>
+</merged>
+</entry>
+</target>
+</blame>