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 a1073e24f2..f301022d30 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 @@ -8,7 +8,7 @@ import sonia.scm.repository.Repository; import java.io.File; import java.io.IOException; -public abstract class SimpleWorkdirFactory implements WorkdirFactory { +public abstract class SimpleWorkdirFactory implements WorkdirFactory { private static final Logger logger = LoggerFactory.getLogger(SimpleWorkdirFactory.class); @@ -19,11 +19,11 @@ public abstract class SimpleWorkdirFactory implements WorkdirFactory } @Override - public WorkingCopy createWorkingCopy(C context, String initialBranch) { + public WorkingCopy createWorkingCopy(C context, String initialBranch) { try { File directory = workdirProvider.createNewWorkdir(); - ParentAndClone parentAndClone = cloneRepository(context, directory, initialBranch); - return new WorkingCopy<>(parentAndClone.getClone(), parentAndClone.getParent(), this::close, directory); + ParentAndClone parentAndClone = cloneRepository(context, directory, initialBranch); + return new WorkingCopy<>(parentAndClone.getClone(), parentAndClone.getParent(), this::closeWorkdir, this::closeCentral, directory); } catch (IOException e) { throw new InternalRepositoryException(getScmRepository(context), "could not clone repository in temporary directory", e); } @@ -34,10 +34,11 @@ public abstract class SimpleWorkdirFactory implements WorkdirFactory @SuppressWarnings("squid:S00112") // We do allow implementations to throw arbitrary exceptions here, so that we can handle them in close protected abstract void closeRepository(R repository) throws Exception; + protected abstract void closeWorkdirInternal(W workdir) throws Exception; - protected abstract ParentAndClone cloneRepository(C context, File target, String initialBranch) throws IOException; + protected abstract ParentAndClone cloneRepository(C context, File target, String initialBranch) throws IOException; - private void close(R repository) { + private void closeCentral(R repository) { try { closeRepository(repository); } catch (Exception e) { @@ -45,11 +46,19 @@ public abstract class SimpleWorkdirFactory implements WorkdirFactory } } - protected static class ParentAndClone { - private final R parent; - private final R clone; + private void closeWorkdir(W repository) { + try { + closeWorkdirInternal(repository); + } catch (Exception e) { + logger.warn("could not close temporary repository clone", e); + } + } - public ParentAndClone(R parent, R clone) { + protected static class ParentAndClone { + private final R parent; + private final W clone; + + public ParentAndClone(R parent, W clone) { this.parent = parent; this.clone = clone; } @@ -58,7 +67,7 @@ public abstract class SimpleWorkdirFactory implements WorkdirFactory return parent; } - public R getClone() { + public W 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 bddf03adaa..e1df5e99f3 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 { - WorkingCopy createWorkingCopy(C context, String initialBranch); +public interface WorkdirFactory { + WorkingCopy createWorkingCopy(C context, String initialBranch); } 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 3c96184142..12c859fb8c 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,23 +8,25 @@ import java.io.File; import java.io.IOException; import java.util.function.Consumer; -public class WorkingCopy implements AutoCloseable { +public class WorkingCopy implements AutoCloseable { private static final Logger LOG = LoggerFactory.getLogger(WorkingCopy.class); private final File directory; - private final R workingRepository; + private final W workingRepository; private final R centralRepository; - private final Consumer cleanup; + private final Consumer cleanupWorkdir; + private final Consumer cleanupCentral; - public WorkingCopy(R workingRepository, R centralRepository, Consumer cleanup, File directory) { + public WorkingCopy(W workingRepository, R centralRepository, Consumer cleanupWorkdir, Consumer cleanupCentral, File directory) { this.directory = directory; this.workingRepository = workingRepository; this.centralRepository = centralRepository; - this.cleanup = cleanup; + this.cleanupCentral = cleanupCentral; + this.cleanupWorkdir = cleanupWorkdir; } - public R getWorkingRepository() { + public W getWorkingRepository() { return workingRepository; } @@ -39,8 +41,8 @@ public class WorkingCopy implements AutoCloseable { @Override public void close() { try { - cleanup.accept(workingRepository); - cleanup.accept(centralRepository); + cleanupWorkdir.accept(workingRepository); + cleanupCentral.accept(centralRepository); IOUtil.delete(directory); } catch (IOException e) { LOG.warn("could not delete temporary workdir '{}'", directory, e); diff --git a/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java b/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java index 4a1a1a4179..04e7b72202 100644 --- a/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java +++ b/scm-core/src/test/java/sonia/scm/repository/util/SimpleWorkdirFactoryTest.java @@ -24,14 +24,14 @@ public class SimpleWorkdirFactoryTest { @Rule public TemporaryFolder temporaryFolder = new TemporaryFolder(); - private SimpleWorkdirFactory simpleWorkdirFactory; + private SimpleWorkdirFactory simpleWorkdirFactory; private String initialBranchForLastCloneCall; @Before public void initFactory() throws IOException { WorkdirProvider workdirProvider = new WorkdirProvider(temporaryFolder.newFolder()); - simpleWorkdirFactory = new SimpleWorkdirFactory(workdirProvider) { + simpleWorkdirFactory = new SimpleWorkdirFactory(workdirProvider) { @Override protected Repository getScmRepository(Context context) { return REPOSITORY; @@ -43,7 +43,12 @@ public class SimpleWorkdirFactoryTest { } @Override - protected ParentAndClone cloneRepository(Context context, File target, String initialBranch) { + protected void closeWorkdirInternal(Closeable workdir) throws Exception { + workdir.close(); + } + + @Override + protected ParentAndClone cloneRepository(Context context, File target, String initialBranch) { initialBranchForLastCloneCall = initialBranch; return new ParentAndClone<>(parent, clone); } @@ -53,7 +58,7 @@ public class SimpleWorkdirFactoryTest { @Test public void shouldCreateParentAndClone() { Context context = new Context(); - try (WorkingCopy workingCopy = simpleWorkdirFactory.createWorkingCopy(context, null)) { + try (WorkingCopy workingCopy = simpleWorkdirFactory.createWorkingCopy(context, null)) { assertThat(workingCopy.getCentralRepository()).isSameAs(parent); assertThat(workingCopy.getWorkingRepository()).isSameAs(clone); } @@ -62,7 +67,7 @@ public class SimpleWorkdirFactoryTest { @Test public void shouldCloseParent() throws IOException { Context context = new Context(); - try (WorkingCopy workingCopy = simpleWorkdirFactory.createWorkingCopy(context, null)) {} + try (WorkingCopy workingCopy = simpleWorkdirFactory.createWorkingCopy(context, null)) {} verify(parent).close(); } @@ -70,7 +75,7 @@ public class SimpleWorkdirFactoryTest { @Test public void shouldCloseClone() throws IOException { Context context = new Context(); - try (WorkingCopy workingCopy = simpleWorkdirFactory.createWorkingCopy(context, null)) {} + try (WorkingCopy workingCopy = simpleWorkdirFactory.createWorkingCopy(context, null)) {} verify(clone).close(); } @@ -78,7 +83,7 @@ public class SimpleWorkdirFactoryTest { @Test public void shouldPropagateInitialBranch() { Context context = new Context(); - try (WorkingCopy workingCopy = simpleWorkdirFactory.createWorkingCopy(context, "some")) { + try (WorkingCopy workingCopy = simpleWorkdirFactory.createWorkingCopy(context, "some")) { assertThat(initialBranchForLastCloneCall).isEqualTo("some"); } } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirFactory.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirFactory.java index d3ed353677..9b2be467e8 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirFactory.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/GitWorkdirFactory.java @@ -4,5 +4,5 @@ import org.eclipse.jgit.lib.Repository; import sonia.scm.repository.spi.GitContext; import sonia.scm.repository.util.WorkdirFactory; -public interface GitWorkdirFactory extends WorkdirFactory { +public interface GitWorkdirFactory extends WorkdirFactory { } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java index 2531888a8b..73159da5c1 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/repository/spi/AbstractGitCommand.java @@ -142,7 +142,7 @@ class AbstractGitCommand } > R inClone(Function workerSupplier, GitWorkdirFactory workdirFactory, String initialBranch) { - try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context, initialBranch)) { + try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context, initialBranch)) { Repository repository = workingCopy.getWorkingRepository(); logger.debug("cloned repository to folder {}", repository.getWorkTree()); return workerSupplier.apply(new Git(repository)).run(); 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 e8675068b9..fe39006a66 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 @@ -58,7 +58,7 @@ public class GitBranchCommand extends AbstractGitCommand implements BranchComman @Override public Branch branch(BranchRequest request) { - try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context, request.getParentBranch())) { + try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context, request.getParentBranch())) { Git clone = new Git(workingCopy.getWorkingRepository()); Ref ref = clone.branchCreate().setName(request.getNewBranch()).call(); Iterable call = clone.push().add(request.getNewBranch()).call(); 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 a0fda7cd3b..f7b0c5567c 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 @@ -18,7 +18,7 @@ import java.io.IOException; import static sonia.scm.ContextEntry.ContextBuilder.entity; import static sonia.scm.NotFoundException.notFound; -public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory implements GitWorkdirFactory { +public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory implements GitWorkdirFactory { @Inject public SimpleGitWorkdirFactory(WorkdirProvider workdirProvider) { @@ -26,7 +26,7 @@ public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory cloneRepository(GitContext context, File target, String initialBranch) { + public ParentAndClone cloneRepository(GitContext context, File target, String initialBranch) { try { Repository clone = Git.cloneRepository() .setURI(createScmTransportProtocolUri(context.getDirectory())) @@ -60,6 +60,13 @@ public class SimpleGitWorkdirFactory extends SimpleWorkdirFactory workingCopy = factory.createWorkingCopy(createContext(), null)) { + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), null)) { assertThat(workingCopy.getDirectory()) .exists() @@ -62,7 +62,7 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { public void shouldCheckoutInitialBranch() { SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(workdirProvider); - try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), "test-branch")) { + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), "test-branch")) { assertThat(new File(workingCopy.getWorkingRepository().getWorkTree(), "a.txt")) .exists() .isFile() @@ -75,10 +75,10 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(workdirProvider); File firstDirectory; - try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), null)) { + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), null)) { firstDirectory = workingCopy.getDirectory(); } - try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), null)) { + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), null)) { File secondDirectory = workingCopy.getDirectory(); assertThat(secondDirectory).isNotEqualTo(firstDirectory); } @@ -89,7 +89,7 @@ public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase { SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(workdirProvider); File directory; - try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), null)) { + try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext(), null)) { 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 5e9e01c9d8..e41dbf96da 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 @@ -59,7 +59,7 @@ public class HgBranchCommand extends AbstractCommand implements BranchCommand { @Override public Branch branch(BranchRequest request) { - try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(getContext(), request.getParentBranch())) { + try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(getContext(), request.getParentBranch())) { com.aragost.javahg.Repository repository = workingCopy.getWorkingRepository(); Changeset emptyChangeset = createNewBranchWithEmptyCommit(request, repository); @@ -83,7 +83,7 @@ public class HgBranchCommand extends AbstractCommand implements BranchCommand { .execute(); } - private void pullNewBranchIntoCentralRepository(BranchRequest request, WorkingCopy workingCopy) { + private void pullNewBranchIntoCentralRepository(BranchRequest request, WorkingCopy workingCopy) { try { PullCommand pullCommand = PullCommand.on(workingCopy.getCentralRepository()); workdirFactory.configure(pullCommand); diff --git a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java index 0294b6902b..419b7de161 100644 --- a/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java +++ b/scm-plugins/scm-hg-plugin/src/main/java/sonia/scm/repository/spi/HgModifyCommand.java @@ -29,7 +29,7 @@ public class HgModifyCommand implements ModifyCommand { @Override public String execute(ModifyCommandRequest request) { - try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context, request.getBranch())) { + try (WorkingCopy workingCopy = workdirFactory.createWorkingCopy(context, request.getBranch())) { Repository workingRepository = workingCopy.getWorkingRepository(); request.getRequests().forEach( partialRequest -> { @@ -85,7 +85,7 @@ public class HgModifyCommand implements ModifyCommand { } } - private List pullModifyChangesToCentralRepository(ModifyCommandRequest request, WorkingCopy workingCopy) { + private List pullModifyChangesToCentralRepository(ModifyCommandRequest request, WorkingCopy workingCopy) { try { com.aragost.javahg.commands.PullCommand pullCommand = PullCommand.on(workingCopy.getCentralRepository()); workdirFactory.configure(pullCommand); 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 515ee62c98..2920abc422 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 @@ -4,6 +4,6 @@ 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 619f8b0892..412e9b7bf6 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 @@ -16,7 +16,7 @@ import java.io.IOException; import java.util.Map; import java.util.function.BiConsumer; -public class SimpleHgWorkdirFactory extends SimpleWorkdirFactory implements HgWorkdirFactory { +public class SimpleHgWorkdirFactory extends SimpleWorkdirFactory implements HgWorkdirFactory { private final Provider hgRepositoryEnvironmentBuilder; @@ -26,7 +26,7 @@ public class SimpleHgWorkdirFactory extends SimpleWorkdirFactory cloneRepository(HgCommandContext context, File target, String initialBranch) throws IOException { + public ParentAndClone cloneRepository(HgCommandContext context, File target, String initialBranch) throws IOException { BiConsumer> repositoryMapBiConsumer = (repository, environment) -> hgRepositoryEnvironmentBuilder.get().buildFor(repository, null, environment); Repository centralRepository = context.openWithSpecialEnvironment(repositoryMapBiConsumer); @@ -46,6 +46,11 @@ public class SimpleHgWorkdirFactory extends SimpleWorkdirFactory