From 1162536e212c5600215f36690b8a2872c8102f72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Fri, 29 Mar 2019 10:02:38 +0100 Subject: [PATCH] Simplify workdir factory --- .../repository/util/SimpleWorkdirFactory.java | 33 ++++++++-- .../scm/repository/util/WorkdirFactory.java | 2 +- .../scm/repository/util/WorkingCopy.java | 22 +++++-- .../scm/repository/spi/GitBranchCommand.java | 2 +- .../scm/repository/spi/GitMergeCommand.java | 2 +- .../spi/SimpleGitWorkdirFactory.java | 11 +++- .../spi/SimpleGitWorkdirFactoryTest.java | 11 ++-- .../scm/repository/spi/HgBranchCommand.java | 13 +++- .../scm/repository/spi/HgWorkdirFactory.java | 5 +- .../spi/SimpleHgWorkdirFactory.java | 61 +++++-------------- .../repository/spi/HgBranchCommandTest.java | 2 +- 11 files changed, 91 insertions(+), 73 deletions(-) diff --git a/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java b/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java index 5f7cbd111c..37ba1874db 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/SimpleWorkdirFactory.java +++ b/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 implements WorkdirFactory { +public abstract class SimpleWorkdirFactory implements WorkdirFactory { private static final Logger logger = LoggerFactory.getLogger(SimpleWorkdirFactory.class); @@ -33,8 +33,8 @@ public abstract class SimpleWorkdirFactory implement public WorkingCopy createWorkingCopy(C context) { try { File directory = createNewWorkdir(); - R clone = cloneProvider.cloneRepository(context, directory); - return new WorkingCopy<>(clone, this::close, directory); + ParentAndClone 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 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 cloneRepository(C context, File target) throws IOException; + @FunctionalInterface + protected interface CloneProvider { + ParentAndClone cloneRepository(C context, File target) throws IOException; + } + + protected static class ParentAndClone { + 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; + } } } diff --git a/scm-core/src/main/java/sonia/scm/repository/util/WorkdirFactory.java b/scm-core/src/main/java/sonia/scm/repository/util/WorkdirFactory.java index ff01ae3da6..1b67c7f1eb 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/WorkdirFactory.java +++ b/scm-core/src/main/java/sonia/scm/repository/util/WorkdirFactory.java @@ -1,5 +1,5 @@ package sonia.scm.repository.util; -public interface WorkdirFactory { +public interface WorkdirFactory { WorkingCopy createWorkingCopy(C context); } diff --git a/scm-core/src/main/java/sonia/scm/repository/util/WorkingCopy.java b/scm-core/src/main/java/sonia/scm/repository/util/WorkingCopy.java index ac949876fb..353fca30d2 100644 --- a/scm-core/src/main/java/sonia/scm/repository/util/WorkingCopy.java +++ b/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 extends CloseableWrapper { +public class WorkingCopy 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 cleanup, File directory) { - super(wrappedRepository, cleanup); + public WorkingCopy(R workingRepository, R centralRepository, Consumer 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) { diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBranchCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBranchCommand.java index 2040cd24be..787cfb78bf 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitBranchCommand.java +++ b/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 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()); } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java index f402d63c34..6f4d09e4f6 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/GitMergeCommand.java +++ b/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 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) { diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java index d9fed92968..1346504b12 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/SimpleGitWorkdirFactory.java @@ -23,13 +23,13 @@ public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory { @Override - public Repository cloneRepository(GitContext context, File target) { + public ParentAndClone 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 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 workingCopy = factory.createWorkingCopy(createContext())) { - firstDirectory = workingCopy.get().getDirectory(); + firstDirectory = workingCopy.getDirectory(); } try (WorkingCopy 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 workingCopy = factory.createWorkingCopy(createContext())) { - directory = workingCopy.get().getWorkTree(); + directory = workingCopy.getWorkingRepository().getWorkTree(); } assertThat(directory).doesNotExist(); } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBranchCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBranchCommand.java index c4a10ce19f..24fb88ce50 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgBranchCommand.java +++ b/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 workingCopy = workdirFactory.createWorkingCopy(getContext())) { - com.aragost.javahg.Repository repository = workingCopy.get().get(); + try (WorkingCopy 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()); } } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgWorkdirFactory.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgWorkdirFactory.java index b32b485cca..515ee62c98 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgWorkdirFactory.java +++ b/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 { +public interface HgWorkdirFactory extends WorkdirFactory { + void configure(PullCommand pullCommand); } diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/SimpleHgWorkdirFactory.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/SimpleHgWorkdirFactory.java index 2abf4253ff..8bf4bd2a3c 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/SimpleHgWorkdirFactory.java +++ b/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 implements HgWorkdirFactory { +public class SimpleHgWorkdirFactory extends SimpleWorkdirFactory implements HgWorkdirFactory { @Inject public SimpleHgWorkdirFactory(Provider hgRepositoryEnvironmentBuilder) { - this(hgRepositoryEnvironmentBuilder, new HookConfigurer()); + super(new HgCloneProvider(hgRepositoryEnvironmentBuilder)); } - SimpleHgWorkdirFactory(Provider hgRepositoryEnvironmentBuilder, Consumer hookConfigurer) { - super(new HgCloneProvider(hgRepositoryEnvironmentBuilder, hookConfigurer)); - } - - private static class HgCloneProvider implements CloneProvider { + private static class HgCloneProvider implements CloneProvider { private final Provider hgRepositoryEnvironmentBuilder; - private final Consumer hookConfigurer; - private HgCloneProvider(Provider hgRepositoryEnvironmentBuilder, Consumer hookConfigurer) { + private HgCloneProvider(Provider hgRepositoryEnvironmentBuilder) { this.hgRepositoryEnvironmentBuilder = hgRepositoryEnvironmentBuilder; - this.hookConfigurer = hookConfigurer; } @Override - public RepositoryCloseableWrapper cloneRepository(HgCommandContext context, File target) throws IOException { - BiConsumer> repositoryMapBiConsumer = (repository, environment) -> { - hgRepositoryEnvironmentBuilder.get().buildFor(repository, null, environment); - }; + public ParentAndClone cloneRepository(HgCommandContext context, File target) throws IOException { + BiConsumer> 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 hookConfigurer; - - RepositoryCloseableWrapper(Repository delegate, Repository centralRepository, Consumer 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 { - @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"); } diff --git a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBranchCommandTest.java b/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBranchCommandTest.java index 34f1f5994d..9f8fb1c792 100644 --- a/scm-plugins/scm-hg-plugin/src/test/java/sonia/scm/repository/spi/HgBranchCommandTest.java +++ b/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");