Simplify workdir factory

scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java

@@ -9,7 +9,7 @@ import java.io.File;
 import java.io.IOException;
 import java.nio.file.Files;
 
-public abstract class SimpleWorkdirFactory<R extends AutoCloseable, C> implements WorkdirFactory<R, C> {
+public abstract class SimpleWorkdirFactory<R, C> implements WorkdirFactory<R, C> {
 
   private static final Logger logger = LoggerFactory.getLogger(SimpleWorkdirFactory.class);
 
@@ -33,8 +33,8 @@ public abstract class SimpleWorkdirFactory<R extends AutoCloseable, C> implement
   public WorkingCopy<R> createWorkingCopy(C context) {
     try {
       File directory = createNewWorkdir();
-      R clone = cloneProvider.cloneRepository(context, directory);
-      return new WorkingCopy<>(clone, this::close, directory);
+      ParentAndClone<R> parentAndClone = cloneProvider.cloneRepository(context, directory);
+      return new WorkingCopy<>(parentAndClone.getClone(), parentAndClone.getParent(), this::close, directory);
     } catch (IOException e) {
       throw new InternalRepositoryException(getRepository(context), "could not create temporary directory for clone of repository", e);
     }
@@ -42,19 +42,40 @@ public abstract class SimpleWorkdirFactory<R extends AutoCloseable, C> implement
 
   protected abstract Repository getRepository(C context);
 
+  protected abstract void closeRepository(R repository);
+
   private File createNewWorkdir() throws IOException {
     return Files.createTempDirectory(poolDirectory.toPath(),"workdir").toFile();
   }
 
   private void close(R repository) {
     try {
-      repository.close();
+      closeRepository(repository);
     } catch (Exception e) {
       logger.warn("could not close temporary repository clone", e);
     }
   }
 
-  public interface CloneProvider<R, C> {
-    R cloneRepository(C context, File target) throws IOException;
+  @FunctionalInterface
+  protected interface CloneProvider<R, C> {
+    ParentAndClone<R> cloneRepository(C context, File target) throws IOException;
+  }
+
+  protected static class ParentAndClone<R> {
+    private final R parent;
+    private final R clone;
+
+    public ParentAndClone(R parent, R clone) {
+      this.parent = parent;
+      this.clone = clone;
+    }
+
+    public R getParent() {
+      return parent;
+    }
+
+    public R getClone() {
+      return clone;
+    }
   }
 }

scm-core/src/main/java/sonia/scm/repository/util/WorkdirFactory.java

@@ -1,5 +1,5 @@
 package sonia.scm.repository.util;
 
-public interface WorkdirFactory<R extends AutoCloseable, C> {
+public interface WorkdirFactory<R, C> {
   WorkingCopy<R> createWorkingCopy(C context);
 }

scm-core/src/main/java/sonia/scm/repository/util/WorkingCopy.java

@@ -8,20 +8,34 @@ import java.io.File;
 import java.io.IOException;
 import java.util.function.Consumer;
 
-public class WorkingCopy<R extends AutoCloseable> extends CloseableWrapper<R> {
+public class WorkingCopy<R> implements AutoCloseable {
 
   private static final Logger LOG = LoggerFactory.getLogger(WorkingCopy.class);
 
   private final File directory;
+  private final R workingRepository;
+  private final R centralRepository;
 
-  public WorkingCopy(R wrappedRepository, Consumer<R> cleanup, File directory) {
-    super(wrappedRepository, cleanup);
+  public WorkingCopy(R workingRepository, R centralRepository, Consumer<R> cleanup, File directory) {
     this.directory = directory;
+    this.workingRepository = workingRepository;
+    this.centralRepository = centralRepository;
+  }
+
+  public R getWorkingRepository() {
+    return workingRepository;
+  }
+
+  public R getCentralRepository() {
+    return centralRepository;
+  }
+
+  public File getDirectory() {
+    return directory;
   }
 
   @Override
   public void close() {
-    super.close();
     try {
       IOUtil.delete(directory);
     } catch (IOException e) {

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

@@ -59,7 +59,7 @@ public class GitBranchCommand extends AbstractGitCommand implements BranchComman
   @Override
   public Branch branch(BranchRequest request) {
     try (WorkingCopy<org.eclipse.jgit.lib.Repository> workingCopy = workdirFactory.createWorkingCopy(context)) {
-      Git clone = new Git(workingCopy.get());
+      Git clone = new Git(workingCopy.getWorkingRepository());
       if (request.getParentBranch() != null) {
         clone.checkout().setName(request.getParentBranch());
       }

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

@@ -50,7 +50,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
     RepositoryPermissions.push(context.getRepository().getId()).check();
 
     try (WorkingCopy<Repository> workingCopy = workdirFactory.createWorkingCopy(context)) {
-      Repository repository = workingCopy.get();
+      Repository repository = workingCopy.getWorkingRepository();
       logger.debug("cloned repository to folder {}", repository.getWorkTree());
       return new MergeWorker(repository, request).merge();
     } catch (IOException e) {

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

@@ -23,13 +23,13 @@ public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory<Repository, Gi
   private static class GitCloneProvider implements CloneProvider<Repository, GitContext> {
 
     @Override
-    public Repository cloneRepository(GitContext context, File target) {
+    public ParentAndClone<Repository> cloneRepository(GitContext context, File target) {
       try {
-        return Git.cloneRepository()
+        return new ParentAndClone<>(null, Git.cloneRepository()
           .setURI(createScmTransportProtocolUri(context.getDirectory()))
           .setDirectory(target)
           .call()
-          .getRepository();
+          .getRepository());
       } catch (GitAPIException e) {
         throw new InternalRepositoryException(context.getRepository(), "could not clone working copy of repository", e);
       }
@@ -40,6 +40,11 @@ public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory<Repository, Gi
     }
   }
 
+  @Override
+  protected void closeRepository(Repository repository) {
+    repository.close();
+  }
+
   @Override
   protected sonia.scm.repository.Repository getRepository(GitContext context) {
     return context.getRepository();

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

@@ -1,6 +1,5 @@
 package sonia.scm.repository.spi;
 
-import org.eclipse.jgit.api.errors.GitAPIException;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.transport.ScmTransportProtocol;
 import org.eclipse.jgit.transport.Transport;
@@ -47,11 +46,11 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase {
 
     try (WorkingCopy<Repository> workingCopy = factory.createWorkingCopy(createContext())) {
 
-      assertThat(workingCopy.get().getDirectory())
+      assertThat(workingCopy.getDirectory())
         .exists()
         .isNotEqualTo(masterRepo)
         .isDirectory();
-      assertThat(new File(workingCopy.get().getWorkTree(), "a.txt"))
+      assertThat(new File(workingCopy.getWorkingRepository().getWorkTree(), "a.txt"))
         .exists()
         .isFile()
         .hasContent("a\nline for blame");
@@ -64,10 +63,10 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase {
 
     File firstDirectory;
     try (WorkingCopy<Repository> workingCopy = factory.createWorkingCopy(createContext())) {
-      firstDirectory = workingCopy.get().getDirectory();
+      firstDirectory = workingCopy.getDirectory();
     }
     try (WorkingCopy<Repository> workingCopy = factory.createWorkingCopy(createContext())) {
-      File secondDirectory = workingCopy.get().getDirectory();
+      File secondDirectory = workingCopy.getDirectory();
       assertThat(secondDirectory).isNotEqualTo(firstDirectory);
     }
   }
@@ -78,7 +77,7 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase {
 
     File directory;
     try (WorkingCopy<Repository> workingCopy = factory.createWorkingCopy(createContext())) {
-      directory = workingCopy.get().getWorkTree();
+      directory = workingCopy.getWorkingRepository().getWorkTree();
     }
     assertThat(directory).doesNotExist();
   }

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBranchCommand.java

@@ -32,6 +32,7 @@ package sonia.scm.repository.spi;
 
 import com.aragost.javahg.Changeset;
 import com.aragost.javahg.commands.CommitCommand;
+import com.aragost.javahg.commands.PullCommand;
 import com.aragost.javahg.commands.UpdateCommand;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -59,8 +60,8 @@ public class HgBranchCommand extends AbstractCommand implements BranchCommand {
 
   @Override
   public Branch branch(BranchRequest request) throws IOException {
-    try (WorkingCopy<RepositoryCloseableWrapper> workingCopy = workdirFactory.createWorkingCopy(getContext())) {
-      com.aragost.javahg.Repository repository = workingCopy.get().get();
+    try (WorkingCopy<com.aragost.javahg.Repository> workingCopy = workdirFactory.createWorkingCopy(getContext())) {
+      com.aragost.javahg.Repository repository = workingCopy.getWorkingRepository();
       if (request.getParentBranch() != null) {
         UpdateCommand.on(repository).rev(request.getParentBranch()).execute();
       }
@@ -75,6 +76,14 @@ public class HgBranchCommand extends AbstractCommand implements BranchCommand {
       LOG.debug("Created new branch '{}' in repository {} with changeset {}",
         request.getNewBranch(), getRepository().getNamespaceAndName(), emptyChangeset.getNode());
 
+      try {
+        com.aragost.javahg.commands.PullCommand pullCommand = PullCommand.on(workingCopy.getCentralRepository());
+        workdirFactory.configure(pullCommand);
+        pullCommand.execute(workingCopy.getDirectory().getAbsolutePath());
+      } catch (Exception e) {
+        throw new RuntimeException(e);
+      }
+
       return Branch.normalBranch(request.getNewBranch(), emptyChangeset.getNode());
     }
   }

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgWorkdirFactory.java

@@ -1,6 +1,9 @@
 package sonia.scm.repository.spi;
 
+import com.aragost.javahg.Repository;
+import com.aragost.javahg.commands.PullCommand;
 import sonia.scm.repository.util.WorkdirFactory;
 
-public interface HgWorkdirFactory extends WorkdirFactory<RepositoryCloseableWrapper, HgCommandContext> {
+public interface HgWorkdirFactory extends WorkdirFactory<Repository, HgCommandContext> {
+  void configure(PullCommand pullCommand);
 }

scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/SimpleHgWorkdirFactory.java

@@ -12,77 +12,44 @@ import java.io.File;
 import java.io.IOException;
 import java.util.Map;
 import java.util.function.BiConsumer;
-import java.util.function.Consumer;
 
-public class SimpleHgWorkdirFactory extends SimpleWorkdirFactory<RepositoryCloseableWrapper, HgCommandContext> implements HgWorkdirFactory {
+public class SimpleHgWorkdirFactory extends SimpleWorkdirFactory<Repository, HgCommandContext> implements HgWorkdirFactory {
 
   @Inject
   public SimpleHgWorkdirFactory(Provider<HgRepositoryEnvironmentBuilder> hgRepositoryEnvironmentBuilder) {
-    this(hgRepositoryEnvironmentBuilder, new HookConfigurer());
+    super(new HgCloneProvider(hgRepositoryEnvironmentBuilder));
   }
 
-  SimpleHgWorkdirFactory(Provider<HgRepositoryEnvironmentBuilder> hgRepositoryEnvironmentBuilder, Consumer<PullCommand> hookConfigurer) {
-    super(new HgCloneProvider(hgRepositoryEnvironmentBuilder, hookConfigurer));
-  }
-
-  private static class HgCloneProvider implements CloneProvider<RepositoryCloseableWrapper, HgCommandContext> {
+  private static class HgCloneProvider implements CloneProvider<Repository, HgCommandContext> {
 
     private final Provider<HgRepositoryEnvironmentBuilder> hgRepositoryEnvironmentBuilder;
-    private final Consumer<PullCommand> hookConfigurer;
 
-    private HgCloneProvider(Provider<HgRepositoryEnvironmentBuilder> hgRepositoryEnvironmentBuilder, Consumer<PullCommand> hookConfigurer) {
+    private HgCloneProvider(Provider<HgRepositoryEnvironmentBuilder> hgRepositoryEnvironmentBuilder) {
       this.hgRepositoryEnvironmentBuilder = hgRepositoryEnvironmentBuilder;
-      this.hookConfigurer = hookConfigurer;
     }
 
     @Override
-    public RepositoryCloseableWrapper cloneRepository(HgCommandContext context, File target) throws IOException {
-      BiConsumer<sonia.scm.repository.Repository, Map<String, String>> repositoryMapBiConsumer = (repository, environment) -> {
-        hgRepositoryEnvironmentBuilder.get().buildFor(repository, null, environment);
-      };
+    public ParentAndClone<Repository> cloneRepository(HgCommandContext context, File target) throws IOException {
+      BiConsumer<sonia.scm.repository.Repository, Map<String, String>> repositoryMapBiConsumer =
+        (repository, environment) -> hgRepositoryEnvironmentBuilder.get().buildFor(repository, null, environment);
       Repository centralRepository = context.openWithSpecialEnvironment(repositoryMapBiConsumer);
       CloneCommand.on(centralRepository).execute(target.getAbsolutePath());
-      return new RepositoryCloseableWrapper(Repository.open(target), centralRepository, hookConfigurer);
+      return new ParentAndClone<>(centralRepository, Repository.open(target));
     }
   }
 
+  @Override
+  protected void closeRepository(Repository repository) {
+    repository.close();
+  }
+
   @Override
   protected sonia.scm.repository.Repository getRepository(HgCommandContext context) {
     return null;
   }
-}
-
-class RepositoryCloseableWrapper implements AutoCloseable {
-  private final Repository delegate;
-  private final Repository centralRepository;
-  private final Consumer<PullCommand> hookConfigurer;
-
-  RepositoryCloseableWrapper(Repository delegate, Repository centralRepository, Consumer<PullCommand> hookConfigurer) {
-    this.delegate = delegate;
-    this.centralRepository = centralRepository;
-    this.hookConfigurer = hookConfigurer;
-  }
-
-  Repository get() {
-    return delegate;
-  }
 
   @Override
-  public void close() {
-    try {
-      PullCommand pullCommand = PullCommand.on(centralRepository);
-      hookConfigurer.accept(pullCommand);
-      pullCommand.execute(delegate.getDirectory().getAbsolutePath());
-    } catch (Exception e) {
-      throw new RuntimeException(e);
-    }
-    centralRepository.close();
-  }
-}
-
-class HookConfigurer implements Consumer<PullCommand> {
-  @Override
-  public void accept(PullCommand pullCommand) {
+  public void configure(PullCommand pullCommand) {
     pullCommand.cmdAppend("--config", "hooks.changegroup.scm=python:scmhooks.postHook");
     pullCommand.cmdAppend("--config", "hooks.pretxnchangegroup.scm=python:scmhooks.preHook");
   }

scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBranchCommandTest.java

@@ -19,7 +19,7 @@ public class HgBranchCommandTest extends AbstractHgCommandTestBase {
     HgRepositoryEnvironmentBuilder hgRepositoryEnvironmentBuilder =
       new HgRepositoryEnvironmentBuilder(handler, HgTestUtil.createHookManager());
 
-    SimpleHgWorkdirFactory workdirFactory = new SimpleHgWorkdirFactory(Providers.of(hgRepositoryEnvironmentBuilder), pc -> {});
+    SimpleHgWorkdirFactory workdirFactory = new SimpleHgWorkdirFactory(Providers.of(hgRepositoryEnvironmentBuilder));
 
     BranchRequest branchRequest = new BranchRequest();
     branchRequest.setNewBranch("new_branch");