Move conflict computation to merge command

Therefore revert changes to diff command and introduce new
MergeConflictResult instead of streaming result.

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/Differ.java

@@ -8,7 +8,6 @@ import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevTree;
 import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.treewalk.EmptyTreeIterator;
-import org.eclipse.jgit.treewalk.FileTreeIterator;
 import org.eclipse.jgit.treewalk.TreeWalk;
 import org.eclipse.jgit.treewalk.filter.PathFilter;
 import sonia.scm.repository.GitUtil;
@@ -38,17 +37,6 @@ final class Differ implements AutoCloseable {
   private static Differ create(Repository repository, DiffCommandRequest request) throws IOException {
       RevWalk walk = new RevWalk(repository);
 
-    if (!Strings.isNullOrEmpty(request.getMergeChangeset()))
-    {
-      ObjectId otherRevision = repository.resolve(request.getMergeChangeset());
-      RevTree tree = walk.parseCommit(otherRevision).getTree();
-      TreeWalk treeWalk = new TreeWalk(repository);
-      treeWalk.addTree(tree);
-      treeWalk.addTree(new FileTreeIterator( repository ));
-      return new Differ(null, walk, treeWalk);
-    } else {
-
-
       ObjectId revision = repository.resolve(request.getRevision());
       RevCommit commit = walk.parseCommit(revision);
 
@@ -58,32 +46,40 @@ final class Differ implements AutoCloseable {
       treeWalk.reset();
       treeWalk.setRecursive(true);
 
-      if (Util.isNotEmpty(request.getPath())) {
+      if (Util.isNotEmpty(request.getPath()))
+      {
         treeWalk.setFilter(PathFilter.create(request.getPath()));
       }
 
 
-      if (!Strings.isNullOrEmpty(request.getAncestorChangeset())) {
+      if (!Strings.isNullOrEmpty(request.getAncestorChangeset()))
+      {
         ObjectId otherRevision = repository.resolve(request.getAncestorChangeset());
         ObjectId ancestorId = GitUtil.computeCommonAncestor(repository, revision, otherRevision);
         RevTree tree = walk.parseCommit(ancestorId).getTree();
         treeWalk.addTree(tree);
-      } else if (commit.getParentCount() > 0) {
+      }
+      else if (commit.getParentCount() > 0)
+      {
         RevTree tree = commit.getParent(0).getTree();
 
-        if (tree != null) {
+        if (tree != null)
+        {
           treeWalk.addTree(tree);
-        } else {
+        }
+        else
+        {
           treeWalk.addTree(new EmptyTreeIterator());
         }
-      } else {
+      }
+      else
+      {
         treeWalk.addTree(new EmptyTreeIterator());
       }
 
       treeWalk.addTree(commit.getTree());
 
-      return new Differ(commit, walk, treeWalk);
-    }
+    return new Differ(commit, walk, treeWalk);
   }
 
   private Diff diff() throws IOException {

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitDiffCommand.java

@@ -31,21 +31,12 @@
 
 package sonia.scm.repository.spi;
 
-import com.google.common.base.Strings;
-import org.eclipse.jgit.api.MergeCommand;
-import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.diff.DiffEntry;
 import org.eclipse.jgit.diff.DiffFormatter;
-import org.eclipse.jgit.lib.ObjectId;
-import org.eclipse.jgit.lib.ObjectReader;
-import org.eclipse.jgit.treewalk.CanonicalTreeParser;
-import sonia.scm.repository.GitWorkdirFactory;
-import sonia.scm.repository.InternalRepositoryException;
 import sonia.scm.repository.Repository;
 import sonia.scm.repository.api.DiffCommandBuilder;
 
 import java.io.IOException;
-import java.util.List;
 
 /**
  *
@@ -53,64 +44,22 @@ import java.util.List;
  */
 public class GitDiffCommand extends AbstractGitCommand implements DiffCommand {
 
-  private final GitWorkdirFactory workdirFactory;
-
-  GitDiffCommand(GitContext context, Repository repository, GitWorkdirFactory workdirFactory) {
+  GitDiffCommand(GitContext context, Repository repository) {
     super(context, repository);
-    this.workdirFactory = workdirFactory;
   }
 
   @Override
   public DiffCommandBuilder.OutputStreamConsumer getDiffResult(DiffCommandRequest request) throws IOException {
-    if (Strings.isNullOrEmpty(request.getMergeChangeset())) {
-      Differ.Diff diff = Differ.diff(open(), request);
-      return computeDiff(diff.getEntries(), open());
-    } else {
-      WorkingCopyCloser closer = new WorkingCopyCloser();
-      return inCloneWithPostponedClose(git -> new GitCloneWorker<DiffCommandBuilder.OutputStreamConsumer>(git) {
-        @Override
-        DiffCommandBuilder.OutputStreamConsumer run() throws IOException {
-          ObjectId sourceRevision = resolveRevision(request.getRevision());
-          try {
-            getClone().merge()
-              .setFastForward(MergeCommand.FastForwardMode.NO_FF)
-              .setCommit(false) // we want to set the author manually
-              .include(request.getRevision(), sourceRevision)
-              .call();
-          } catch (GitAPIException e) {
-            throw new InternalRepositoryException(context.getRepository(), "could not merge branch " + request.getRevision() + " into " + request.getMergeChangeset(), e);
-          }
+    @SuppressWarnings("squid:S2095") // repository will be closed with the RepositoryService
+    org.eclipse.jgit.lib.Repository repository = open();
 
-          return outputStream -> {
-            CanonicalTreeParser treeParser = new CanonicalTreeParser();
-            ObjectId treeId = git.getRepository().resolve(request.getMergeChangeset() + "^{tree}");
-            try (ObjectReader reader = git.getRepository().newObjectReader()) {
-              treeParser.reset(reader, treeId);
-              git
-                .diff()
-                .setOldTree(treeParser)
-                .setOutputStream(outputStream)
-                .call();
-              DiffCommandRequest clone = request.clone();
-              clone.setRevision(sourceRevision.name());
-            } catch (GitAPIException e) {
-              throw new InternalRepositoryException(repository, "could not calculate diff", e);
-            } finally {
-              closer.close();
-            }
-          };
-        }
-      }, workdirFactory, request.getMergeChangeset(), closer);
-    }
-  }
-
-  private DiffCommandBuilder.OutputStreamConsumer computeDiff(List<DiffEntry> entries, org.eclipse.jgit.lib.Repository repository) throws IOException {
+    Differ.Diff diff = Differ.diff(repository, request);
 
     return output -> {
       try (DiffFormatter formatter = new DiffFormatter(output)) {
         formatter.setRepository(repository);
 
-        for (DiffEntry e : entries) {
+        for (DiffEntry e : diff.getEntries()) {
           if (!e.getOldId().equals(e.getNewId())) {
             formatter.format(e);
           }

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java

@@ -4,11 +4,15 @@ import com.google.common.base.Strings;
 import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.MergeCommand.FastForwardMode;
 import org.eclipse.jgit.api.MergeResult;
+import org.eclipse.jgit.api.Status;
 import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.merge.MergeStrategy;
 import org.eclipse.jgit.merge.ResolveMerger;
+import org.eclipse.jgit.treewalk.CanonicalTreeParser;
+import org.eclipse.jgit.treewalk.filter.PathFilter;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.repository.GitWorkdirFactory;
@@ -17,6 +21,7 @@ import sonia.scm.repository.Person;
 import sonia.scm.repository.api.MergeCommandResult;
 import sonia.scm.repository.api.MergeDryRunCommandResult;
 
+import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.text.MessageFormat;
 
@@ -55,6 +60,12 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
     }
   }
 
+  @Override
+  public MergeConflictResult computeConflicts(MergeCommandRequest request) {
+    WorkingCopyCloser closer = new WorkingCopyCloser();
+    return inClone(git -> new ConflictWorker(git, request, closer), workdirFactory, request.getTargetBranch());
+  }
+
   private class MergeWorker extends GitCloneWorker<MergeCommandResult> {
 
     private final String target;
@@ -115,4 +126,82 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
       return MergeCommandResult.failure(result.getConflicts().keySet());
     }
   }
+
+  private class ConflictWorker extends GitCloneWorker<MergeConflictResult> {
+    private final Git git;
+    private final MergeCommandRequest request;
+    private final WorkingCopyCloser closer;
+
+    private ConflictWorker(Git git, MergeCommandRequest request, WorkingCopyCloser closer) {
+      super(git);
+      this.git = git;
+      this.request = request;
+      this.closer = closer;
+    }
+
+    @Override
+    MergeConflictResult run() throws IOException {
+      ObjectId sourceRevision = resolveRevision(request.getBranchToMerge());
+      MergeResult mergeResult;
+      try {
+        mergeResult = getClone().merge()
+          .setFastForward(FastForwardMode.NO_FF)
+          .setCommit(false)
+          .include(request.getBranchToMerge(), sourceRevision)
+          .call();
+      } catch (GitAPIException e) {
+        throw new InternalRepositoryException(context.getRepository(), "could not merge branch " + request.getBranchToMerge() + " into " + request.getTargetBranch(), e);
+      }
+
+      if (mergeResult.getConflicts() == null) {
+        return new MergeConflictResult();
+      }
+      Status status;
+      try {
+        status = getClone().status().call();
+      } catch (GitAPIException e) {
+        throw new InternalRepositoryException(context.getRepository(), "could not get status", e);
+      }
+
+      MergeConflictResult result = new MergeConflictResult();
+
+      CanonicalTreeParser treeParser = new CanonicalTreeParser();
+      ObjectId treeId = git.getRepository().resolve(request.getTargetBranch() + "^{tree}");
+
+      ByteArrayOutputStream diffBuffer = new ByteArrayOutputStream();
+
+      status.getConflictingStageState().entrySet().forEach(conflictEntry -> {
+
+        String path = conflictEntry.getKey();
+        switch (conflictEntry.getValue()) {
+          case BOTH_MODIFIED:
+            diffBuffer.reset();
+            try (ObjectReader reader = git.getRepository().newObjectReader()) {
+              treeParser.reset(reader, treeId);
+              git
+                .diff()
+                .setOldTree(treeParser)
+                .setPathFilter(PathFilter.create(path))
+                .setOutputStream(diffBuffer)
+                .call();
+              result.addBothModified(path, diffBuffer.toString());
+            } catch (GitAPIException | IOException e) {
+              throw new InternalRepositoryException(repository, "could not calculate diff for path " + path, e);
+            } finally {
+              closer.close();
+            }
+            break;
+          case DELETED_BY_THEM:
+            result.addDeletedByThem(path);
+            break;
+          case DELETED_BY_US:
+            result.addDeletedByUs(path);
+            break;
+          default:
+            throw new InternalRepositoryException(context.getRepository(), "unexpected conflict type: " + conflictEntry.getValue());
+        }
+      });
+      return result;
+    }
+  }
 }

scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitRepositoryServiceProvider.java

@@ -169,7 +169,7 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider
   @Override
   public DiffCommand getDiffCommand()
   {
-    return new GitDiffCommand(context, repository, handler.getWorkdirFactory());
+    return new GitDiffCommand(context, repository);
   }
 
   @Override

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffCommandTest.java

@@ -40,7 +40,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase {
 
   @Test
   public void diffForOneRevisionShouldCreateDiff() throws IOException {
-    GitDiffCommand gitDiffCommand = createDiffCommand();
+    GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository);
     DiffCommandRequest diffCommandRequest = new DiffCommandRequest();
     diffCommandRequest.setRevision("3f76a12f08a6ba0dc988c68b7f0b2cd190efc3c4");
     ByteArrayOutputStream output = new ByteArrayOutputStream();
@@ -50,7 +50,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase {
 
   @Test
   public void diffForOneBranchShouldCreateDiff() throws IOException {
-    GitDiffCommand gitDiffCommand = createDiffCommand();
+    GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository);
     DiffCommandRequest diffCommandRequest = new DiffCommandRequest();
     diffCommandRequest.setRevision("test-branch");
     ByteArrayOutputStream output = new ByteArrayOutputStream();
@@ -60,7 +60,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase {
 
   @Test
   public void diffForPathShouldCreateLimitedDiff() throws IOException {
-    GitDiffCommand gitDiffCommand = createDiffCommand();
+    GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository);
     DiffCommandRequest diffCommandRequest = new DiffCommandRequest();
     diffCommandRequest.setRevision("test-branch");
     diffCommandRequest.setPath("a.txt");
@@ -71,7 +71,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase {
 
   @Test
   public void diffBetweenTwoBranchesShouldCreateDiff() throws IOException {
-    GitDiffCommand gitDiffCommand = createDiffCommand();
+    GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository);
     DiffCommandRequest diffCommandRequest = new DiffCommandRequest();
     diffCommandRequest.setRevision("master");
     diffCommandRequest.setAncestorChangeset("test-branch");
@@ -82,7 +82,7 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase {
 
   @Test
   public void diffBetweenTwoBranchesForPathShouldCreateLimitedDiff() throws IOException {
-    GitDiffCommand gitDiffCommand = createDiffCommand();
+    GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository);
     DiffCommandRequest diffCommandRequest = new DiffCommandRequest();
     diffCommandRequest.setRevision("master");
     diffCommandRequest.setAncestorChangeset("test-branch");
@@ -91,8 +91,4 @@ public class GitDiffCommandTest extends AbstractGitCommandTestBase {
     gitDiffCommand.getDiffResult(diffCommandRequest).accept(output);
     assertEquals(DIFF_FILE_A_MULTIPLE_REVISIONS, output.toString());
   }
-
-  private GitDiffCommand createDiffCommand() {
-    return new GitDiffCommand(createContext(), repository, null);
-  }
 }

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitDiffCommand_Merge_Test.java

@@ -1,74 +0,0 @@
-package sonia.scm.repository.spi;
-
-import org.junit.Rule;
-import org.junit.Test;
-import sonia.scm.repository.util.WorkdirProvider;
-
-import java.io.ByteArrayOutputStream;
-import java.io.IOException;
-
-import static org.assertj.core.api.Assertions.assertThat;
-
-public class GitDiffCommand_Merge_Test extends AbstractGitCommandTestBase {
-
-  static final String DIFF_HEADER = "diff --git a/Main.java b/Main.java";
-  static final String DIFF_FILE_NO_CONFLICT = "--- a/Main.java\n" +
-    "+++ b/Main.java\n" +
-    "@@ -1,5 +1,5 @@\n" +
-    " class Main {\n" +
-    "-    public static void main(String[] args) {\n" +
-    "+    public static void main(String[] arguments) {\n" +
-    "         System.out.println(\"Expect nothing more to happen.\");\n" +
-    "         System.out.println(\"This is for demonstration, only.\");\n" +
-    "     }\n";
-  static final String DIFF_FILE_CONFLICT = "--- a/Main.java\n" +
-    "+++ b/Main.java\n" +
-    "@@ -1,6 +1,13 @@\n" +
-    "+import java.util.Arrays;\n" +
-    "+\n" +
-    " class Main {\n" +
-    "     public static void main(String[] args) {\n" +
-    "         System.out.println(\"Expect nothing more to happen.\");\n" +
-    "+<<<<<<< HEAD\n" +
-    "         System.out.println(\"This is for demonstration, only.\");\n" +
-    "+=======\n" +
-    "+        System.out.println(\"Parameters:\");\n" +
-    "+        Arrays.stream(args).map(arg -> \"- \" + arg).forEach(System.out::println);\n" +
-    "+>>>>>>> feature/print_args\n" +
-    "     }\n" +
-    " }\n";
-
-  @Rule
-  public BindTransportProtocolRule transportProtocolRule = new BindTransportProtocolRule();
-
-  @Test
-  public void diffBetweenTwoBranchesWithoutConflict() throws IOException {
-    GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository, new SimpleGitWorkdirFactory(new WorkdirProvider()));
-    DiffCommandRequest diffCommandRequest = new DiffCommandRequest();
-    diffCommandRequest.setRevision("feature/rename_variable");
-    diffCommandRequest.setConflictBranch("integration");
-    ByteArrayOutputStream output = new ByteArrayOutputStream();
-    gitDiffCommand.getDiffResult(diffCommandRequest).accept(output);
-    assertThat(output.toString())
-      .contains(DIFF_HEADER)
-      .contains(DIFF_FILE_NO_CONFLICT);
-  }
-
-  @Test
-  public void diffBetweenTwoBranchesWithSimpleConflict() throws IOException {
-    GitDiffCommand gitDiffCommand = new GitDiffCommand(createContext(), repository, new SimpleGitWorkdirFactory(new WorkdirProvider()));
-    DiffCommandRequest diffCommandRequest = new DiffCommandRequest();
-    diffCommandRequest.setRevision("feature/print_args");
-    diffCommandRequest.setConflictBranch("integration");
-    ByteArrayOutputStream output = new ByteArrayOutputStream();
-    gitDiffCommand.getDiffResult(diffCommandRequest).accept(output);
-    assertThat(output.toString())
-      .contains(DIFF_HEADER)
-      .contains(DIFF_FILE_CONFLICT);
-  }
-
-  @Override
-  protected String getZippedRepositoryResource() {
-    return "sonia/scm/repository/spi/scm-git-spi-merge-diff-test.zip";
-  }
-}

scm-plugins/scm-git-plugin/src/test/java/sonia/scm/repository/spi/GitMergeCommand_Conflict_Test.java

@@ -0,0 +1,81 @@
+package sonia.scm.repository.spi;
+
+import org.junit.Rule;
+import org.junit.Test;
+import sonia.scm.repository.spi.MergeConflictResult.SingleMergeConflict;
+import sonia.scm.repository.util.WorkdirProvider;
+
+import java.io.IOException;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.BOTH_MODIFIED;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.DELETED_BY_THEM;
+import static sonia.scm.repository.spi.MergeConflictResult.ConflictTypes.DELETED_BY_US;
+
+public class GitMergeCommand_Conflict_Test extends AbstractGitCommandTestBase {
+
+  static final String DIFF_HEADER = "diff --git a/Main.java b/Main.java";
+  static final String DIFF_FILE_CONFLICT = "--- a/Main.java\n" +
+    "+++ b/Main.java\n" +
+    "@@ -1,6 +1,13 @@\n" +
+    "+import java.util.Arrays;\n" +
+    "+\n" +
+    " class Main {\n" +
+    "     public static void main(String[] args) {\n" +
+    "         System.out.println(\"Expect nothing more to happen.\");\n" +
+    "+<<<<<<< HEAD\n" +
+    "         System.out.println(\"This is for demonstration, only.\");\n" +
+    "+=======\n" +
+    "+        System.out.println(\"Parameters:\");\n" +
+    "+        Arrays.stream(args).map(arg -> \"- \" + arg).forEach(System.out::println);\n" +
+    "+>>>>>>> feature/print_args\n" +
+    "     }\n" +
+    " }\n";
+
+  @Rule
+  public BindTransportProtocolRule transportProtocolRule = new BindTransportProtocolRule();
+
+  @Test
+  public void diffBetweenTwoBranchesWithoutConflict() throws IOException {
+    MergeConflictResult result = computeMergeConflictResult("feature/rename_variable", "integration");
+    assertThat(result.getConflicts()).isEmpty();
+  }
+
+  @Test
+  public void diffBetweenTwoBranchesWithSimpleConflict() throws IOException {
+    MergeConflictResult result = computeMergeConflictResult("feature/print_args", "integration");
+    SingleMergeConflict conflict = result.getConflicts().get(0);
+    assertThat(conflict.getType()).isEqualTo(BOTH_MODIFIED);
+    assertThat(conflict.getPath()).isEqualTo("Main.java");
+    assertThat(conflict.getDiff()).contains(DIFF_HEADER, DIFF_FILE_CONFLICT);
+  }
+
+  @Test
+  public void diffBetweenTwoBranchesWithDeletedByThem() throws IOException {
+    MergeConflictResult result = computeMergeConflictResult("feature/remove_class", "integration");
+    SingleMergeConflict conflict = result.getConflicts().get(0);
+    assertThat(conflict.getType()).isEqualTo(DELETED_BY_THEM);
+    assertThat(conflict.getPath()).isEqualTo("Main.java");
+  }
+
+  @Test
+  public void diffBetweenTwoBranchesWithDeletedByUs() throws IOException {
+    MergeConflictResult result = computeMergeConflictResult("integration", "feature/remove_class");
+    SingleMergeConflict conflict = result.getConflicts().get(0);
+    assertThat(conflict.getType()).isEqualTo(DELETED_BY_US);
+    assertThat(conflict.getPath()).isEqualTo("Main.java");
+  }
+
+  private MergeConflictResult computeMergeConflictResult(String branchToMerge, String targetBranch) {
+    GitMergeCommand gitMergeCommand = new GitMergeCommand(createContext(), repository, new SimpleGitWorkdirFactory(new WorkdirProvider()));
+    MergeCommandRequest mergeCommandRequest = new MergeCommandRequest();
+    mergeCommandRequest.setBranchToMerge(branchToMerge);
+    mergeCommandRequest.setTargetBranch(targetBranch);
+    return gitMergeCommand.computeConflicts(mergeCommandRequest);
+  }
+
+  @Override
+  protected String getZippedRepositoryResource() {
+    return "sonia/scm/repository/spi/scm-git-spi-merge-diff-test.zip";
+  }
+}