Cleanup worker

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

@@ -5,6 +5,7 @@ import org.eclipse.jgit.api.Git;
 import org.eclipse.jgit.api.MergeResult;
 import org.eclipse.jgit.api.Status;
 import org.eclipse.jgit.api.errors.GitAPIException;
+import org.eclipse.jgit.lib.IndexDiff;
 import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.ObjectReader;
 import org.eclipse.jgit.lib.Repository;
@@ -23,6 +24,8 @@ import java.io.IOException;
 import java.util.Set;
 
 import static org.eclipse.jgit.merge.MergeStrategy.RECURSIVE;
+import static sonia.scm.ContextEntry.ContextBuilder.entity;
+import static sonia.scm.NotFoundException.notFound;
 
 public class GitMergeCommand extends AbstractGitCommand implements MergeCommand {
 
@@ -46,8 +49,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
 
   @Override
   public MergeConflictResult computeConflicts(MergeCommandRequest request) {
-    WorkingCopyCloser closer = new WorkingCopyCloser();
+    return inClone(git -> new ConflictWorker(git, request), workdirFactory, request.getTargetBranch());
-    return inClone(git -> new ConflictWorker(git, request, closer), workdirFactory, request.getTargetBranch());
   }
 
   private MergeCommandResult mergeWithStrategy(MergeCommandRequest request) {
@@ -91,43 +93,45 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
   }
 
   private class ConflictWorker extends GitCloneWorker<MergeConflictResult> {
-    private final Git git;
+    private final String branchToMerge;
-    private final MergeCommandRequest request;
+    private final String targetBranch;
-    private final WorkingCopyCloser closer;
     private final CanonicalTreeParser treeParser;
     private final ObjectId treeId;
     private final ByteArrayOutputStream diffBuffer;
 
-    private ConflictWorker(Git git, MergeCommandRequest request, WorkingCopyCloser closer) {
+    private final MergeConflictResult result = new MergeConflictResult();
+
+
+    private ConflictWorker(Git git, MergeCommandRequest request) {
       super(git, context, repository);
-      this.git = git;
+      branchToMerge = request.getBranchToMerge();
-      this.request = request;
+      targetBranch = request.getTargetBranch();
-      this.closer = closer;
 
       treeParser = new CanonicalTreeParser();
-      treeId = git.getRepository().resolve(request.getTargetBranch() + "^{tree}");
       diffBuffer = new ByteArrayOutputStream();
+      try {
+        treeId = git.getRepository().resolve(request.getTargetBranch() + "^{tree}");
+      } catch (IOException e) {
+        throw notFound(entity("branch", request.getTargetBranch()).in(repository));
+      }
     }
 
     @Override
     MergeConflictResult run() throws IOException {
       MergeResult mergeResult = doTemporaryMerge();
-      if (mergeResult.getConflicts() == null) {
+      if (mergeResult.getConflicts() != null) {
-        return new MergeConflictResult();
+        getStatus().getConflictingStageState().forEach(this::computeConflict);
+      }
+      return result;
     }
 
-      Status status = getStatus();
+    private void computeConflict(String path, IndexDiff.StageState stageState) {
-
+      switch (stageState) {
-      MergeConflictResult result = new MergeConflictResult();
-
-
-      status.getConflictingStageState().forEach((path, value) -> {
-        switch (value) {
         case BOTH_MODIFIED:
           diffBuffer.reset();
-            try (ObjectReader reader = git.getRepository().newObjectReader()) {
+          try (ObjectReader reader = getClone().getRepository().newObjectReader()) {
             treeParser.reset(reader, treeId);
-              git
+            getClone()
               .diff()
               .setOldTree(treeParser)
               .setPathFilter(PathFilter.create(path))
@@ -136,8 +140,6 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
             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:
@@ -147,10 +149,21 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
           result.addDeletedByUs(path);
           break;
         default:
-            throw new InternalRepositoryException(context.getRepository(), "unexpected conflict type: " + value);
+          throw new InternalRepositoryException(context.getRepository(), "unexpected conflict type: " + stageState);
+      }
+    }
+
+    private MergeResult doTemporaryMerge() throws IOException {
+      ObjectId sourceRevision = resolveRevision(branchToMerge);
+      try {
+        return getClone().merge()
+          .setFastForward(org.eclipse.jgit.api.MergeCommand.FastForwardMode.NO_FF)
+          .setCommit(false)
+          .include(branchToMerge, sourceRevision)
+          .call();
+      } catch (GitAPIException e) {
+        throw new InternalRepositoryException(context.getRepository(), "could not merge branch " + branchToMerge + " into " + targetBranch, e);
       }
-      });
-      return result;
     }
 
     private Status getStatus() {
@@ -162,20 +175,5 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
       }
       return status;
     }
-
-    private MergeResult doTemporaryMerge() throws IOException {
-      ObjectId sourceRevision = resolveRevision(request.getBranchToMerge());
-      MergeResult mergeResult;
-      try {
-        mergeResult = getClone().merge()
-          .setFastForward(org.eclipse.jgit.api.MergeCommand.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);
-      }
-      return mergeResult;
-    }
   }
 }