Start with a simple factory instead of a complex pool

This commit is contained in:
René Pfeuffer
2018-11-07 10:28:27 +01:00
parent 4cdf3e468c
commit 33642352ea
11 changed files with 183 additions and 122 deletions

View File

@@ -91,7 +91,7 @@ public class GitRepositoryHandler
private final Scheduler scheduler;
private final GitWorkdirPool workdirPool;
private final GitWorkdirFactory workdirFactory;
private Task task;
@@ -106,11 +106,11 @@ public class GitRepositoryHandler
* @param scheduler
*/
@Inject
public GitRepositoryHandler(ConfigurationStoreFactory storeFactory, FileSystem fileSystem, Scheduler scheduler, GitWorkdirPool workdirPool)
public GitRepositoryHandler(ConfigurationStoreFactory storeFactory, FileSystem fileSystem, Scheduler scheduler, GitWorkdirFactory workdirFactory)
{
super(storeFactory, fileSystem);
this.scheduler = scheduler;
this.workdirPool = workdirPool;
this.workdirFactory = workdirFactory;
}
//~--- get methods ----------------------------------------------------------
@@ -239,7 +239,7 @@ public class GitRepositoryHandler
return new File(directory, DIRECTORY_REFS).exists();
}
public GitWorkdirPool getWorkdirPool() {
return workdirPool;
public GitWorkdirFactory getWorkdirFactory() {
return workdirFactory;
}
}

View File

@@ -0,0 +1,8 @@
package sonia.scm.repository;
import sonia.scm.repository.spi.GitContext;
import sonia.scm.repository.spi.WorkingCopy;
public interface GitWorkdirFactory {
WorkingCopy createWorkingCopy(GitContext gitContext);
}

View File

@@ -1,46 +0,0 @@
package sonia.scm.repository;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Repository;
import java.io.File;
import java.util.Random;
/**
* Config:
*
* 1. Overall and absolute maximum of temp work directories
* 2. Maximum number of temp work directories pooled overall
* 3. Maximum number of temp work directories pooled for one master repository
*/
public class GitWorkdirPool {
private final Random random = new Random();
private final File poolDirectory;
public GitWorkdirPool() {
this(new File(System.getProperty("java.io.tmpdir")));
}
public GitWorkdirPool(File poolDirectory) {
this.poolDirectory = poolDirectory;
}
public CloseableWrapper<Repository> getWorkingCopy(File bareRepository) {
try {
Git clone = cloneRepository(bareRepository, new File(poolDirectory, Long.toString(random.nextLong())));
return new CloseableWrapper<>(clone.getRepository(), r -> clone.close());
} catch (GitAPIException e) {
throw new InternalRepositoryException("could not clone working copy of repository", e);
}
}
protected Git cloneRepository(File bareRepository, File target) throws GitAPIException {
return Git.cloneRepository()
.setURI(bareRepository.getAbsolutePath())
.setDirectory(target)
.call();
}
}

View File

@@ -106,6 +106,10 @@ public class GitContext implements Closeable
return repository;
}
File getDirectory() {
return directory;
}
//~--- fields ---------------------------------------------------------------
/** Field description */

View File

@@ -2,10 +2,9 @@ package sonia.scm.repository.spi;
import org.eclipse.jgit.merge.MergeStrategy;
import org.eclipse.jgit.merge.ResolveMerger;
import sonia.scm.repository.CloseableWrapper;
import sonia.scm.repository.GitWorkdirPool;
import org.eclipse.jgit.lib.Repository;
import sonia.scm.repository.GitWorkdirFactory;
import sonia.scm.repository.InternalRepositoryException;
import sonia.scm.repository.Repository;
import sonia.scm.repository.api.MergeCommandResult;
import sonia.scm.repository.api.MergeDryRunCommandResult;
@@ -13,17 +12,17 @@ import java.io.IOException;
public class GitMergeCommand extends AbstractGitCommand implements MergeCommand {
private final GitWorkdirPool workdirPool;
private final GitWorkdirFactory workdirPool;
GitMergeCommand(GitContext context, Repository repository, GitWorkdirPool workdirPool) {
GitMergeCommand(GitContext context, sonia.scm.repository.Repository repository, GitWorkdirFactory workdirPool) {
super(context, repository);
this.workdirPool = workdirPool;
}
@Override
public MergeCommandResult merge(MergeCommandRequest request) {
try (CloseableWrapper<org.eclipse.jgit.lib.Repository> workingCopy = workdirPool.getWorkingCopy(context.open().getDirectory())) {
org.eclipse.jgit.lib.Repository repository = workingCopy.get();
try (WorkingCopy workingCopy = workdirPool.createWorkingCopy(context)) {
Repository repository = workingCopy.get();
ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository);
boolean mergeResult = merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch()));
if (mergeResult) {
@@ -38,7 +37,7 @@ public class GitMergeCommand extends AbstractGitCommand implements MergeCommand
@Override
public MergeDryRunCommandResult dryRun(MergeCommandRequest request) {
try {
org.eclipse.jgit.lib.Repository repository = context.open();
Repository repository = context.open();
ResolveMerger merger = (ResolveMerger) MergeStrategy.RECURSIVE.newMerger(repository, true);
return new MergeDryRunCommandResult(merger.merge(repository.resolve(request.getBranchToMerge()), repository.resolve(request.getTargetBranch())));
} catch (IOException e) {

View File

@@ -243,7 +243,7 @@ public class GitRepositoryServiceProvider extends RepositoryServiceProvider
@Override
public MergeCommand getMergeCommand() {
return new GitMergeCommand(context, repository, handler.getWorkdirPool());
return new GitMergeCommand(context, repository, handler.getWorkdirFactory());
}
//~--- fields ---------------------------------------------------------------

View File

@@ -0,0 +1,56 @@
package sonia.scm.repository.spi;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.util.FileUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import sonia.scm.repository.GitWorkdirFactory;
import sonia.scm.repository.InternalRepositoryException;
import java.io.File;
import java.io.IOException;
import java.util.Random;
public class SimpleGitWorkdirFactory implements GitWorkdirFactory {
private static final Logger logger = LoggerFactory.getLogger(SimpleGitWorkdirFactory.class);
private final Random random = new Random();
private final File poolDirectory;
public SimpleGitWorkdirFactory() {
this(new File(System.getProperty("java.io.tmpdir"), "scmm-git-pool"));
}
public SimpleGitWorkdirFactory(File poolDirectory) {
this.poolDirectory = poolDirectory;
}
public WorkingCopy createWorkingCopy(GitContext gitContext) {
try {
Repository clone = cloneRepository(gitContext.getDirectory(), new File(poolDirectory, Long.toString(random.nextLong())));
return new WorkingCopy(clone, this::close);
} catch (GitAPIException e) {
throw new InternalRepositoryException("could not clone working copy of repository", e);
}
}
protected Repository cloneRepository(File bareRepository, File target) throws GitAPIException {
return Git.cloneRepository()
.setURI(bareRepository.getAbsolutePath())
.setDirectory(target)
.call()
.getRepository();
}
private void close(Repository repository) {
repository.close();
try {
FileUtils.delete(repository.getDirectory(), FileUtils.RECURSIVE);
} catch (IOException e) {
logger.warn("could not delete temporary git workdir '{}'", repository.getDirectory(), e);
}
}
}

View File

@@ -0,0 +1,12 @@
package sonia.scm.repository.spi;
import org.eclipse.jgit.lib.Repository;
import sonia.scm.repository.CloseableWrapper;
import java.util.function.Consumer;
public class WorkingCopy extends CloseableWrapper<Repository> {
WorkingCopy(Repository wrapped, Consumer<Repository> cleanup) {
super(wrapped, cleanup);
}
}

View File

@@ -62,7 +62,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase {
private ConfigurationStoreFactory factory;
@Mock
private GitWorkdirPool gitWorkdirPool;
private GitWorkdirFactory gitWorkdirFactory;
@Override
protected void checkDirectory(File directory) {
@@ -87,7 +87,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase {
protected RepositoryHandler createRepositoryHandler(ConfigurationStoreFactory factory,
File directory) {
GitRepositoryHandler repositoryHandler = new GitRepositoryHandler(factory,
new DefaultFileSystem(), scheduler, gitWorkdirPool);
new DefaultFileSystem(), scheduler, gitWorkdirFactory);
repositoryHandler.init(contextProvider);
@@ -103,7 +103,7 @@ public class GitRepositoryHandlerTest extends SimpleRepositoryHandlerTestBase {
@Test
public void getDirectory() {
GitRepositoryHandler repositoryHandler = new GitRepositoryHandler(factory,
new DefaultFileSystem(), scheduler, gitWorkdirPool);
new DefaultFileSystem(), scheduler, gitWorkdirFactory);
GitConfig gitConfig = new GitConfig();
gitConfig.setRepositoryDirectory(new File("/path"));

View File

@@ -1,59 +0,0 @@
package sonia.scm.repository;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Repository;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import sonia.scm.repository.spi.AbstractGitCommandTestBase;
import java.io.File;
import java.io.IOException;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
public class GitWorkdirPoolTest extends AbstractGitCommandTestBase {
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
@Test
public void emptyPoolShouldCreateNewWorkdir() throws IOException {
GitWorkdirPool pool = new GitWorkdirPool(temporaryFolder.newFolder());
File masterRepo = createRepositoryDirectory();
CloseableWrapper<Repository> workingCopy = pool.getWorkingCopy(masterRepo);
assertThat(workingCopy)
.isNotNull()
.extracting(w -> w.get().getDirectory())
.isNotEqualTo(masterRepo);
}
@Test
public void cloneFromPoolShouldBeClosed() throws IOException {
PoolWithSpy pool = new PoolWithSpy(temporaryFolder.newFolder());
File masterRepo = createRepositoryDirectory();
try (CloseableWrapper<Repository> workingCopy = pool.getWorkingCopy(masterRepo)) {
assertThat(workingCopy).isNotNull();
}
verify(pool.createdClone).close();
}
private static class PoolWithSpy extends GitWorkdirPool {
PoolWithSpy(File poolDirectory) {
super(poolDirectory);
}
Git createdClone;
@Override
protected Git cloneRepository(File bareRepository, File destination) throws GitAPIException {
createdClone = spy(super.cloneRepository(bareRepository, destination));
return createdClone;
}
}
}

View File

@@ -0,0 +1,87 @@
package sonia.scm.repository.spi;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Repository;
import org.junit.Rule;
import org.junit.Test;
import org.junit.rules.TemporaryFolder;
import java.io.File;
import java.io.IOException;
import static org.assertj.core.api.Assertions.assertThat;
import static org.mockito.Mockito.spy;
import static org.mockito.Mockito.verify;
public class SimpleGitWorkdirFactoryTest extends AbstractGitCommandTestBase {
@Rule
public TemporaryFolder temporaryFolder = new TemporaryFolder();
@Test
public void emptyPoolShouldCreateNewWorkdir() throws IOException {
SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder());
File masterRepo = createRepositoryDirectory();
try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) {
assertThat(workingCopy.get().getDirectory())
.exists()
.isNotEqualTo(masterRepo)
.isDirectory();
assertThat(new File(workingCopy.get().getWorkTree(), "a.txt"))
.exists()
.isFile()
.hasContent("a\nline for blame");
}
}
@Test
public void cloneFromPoolShouldBeClosed() throws IOException {
PoolWithSpy factory = new PoolWithSpy(temporaryFolder.newFolder());
try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) {
assertThat(workingCopy).isNotNull();
}
verify(factory.createdClone).close();
}
@Test
public void cloneFromPoolShouldNotBeReused() throws IOException {
SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder());
File firstDirectory;
try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) {
firstDirectory = workingCopy.get().getDirectory();
}
try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) {
File secondDirectory = workingCopy.get().getDirectory();
assertThat(secondDirectory).isNotEqualTo(firstDirectory);
}
}
@Test
public void cloneFromPoolShouldBeDeletedOnClose() throws IOException {
SimpleGitWorkdirFactory factory = new SimpleGitWorkdirFactory(temporaryFolder.newFolder());
File directory;
try (WorkingCopy workingCopy = factory.createWorkingCopy(createContext())) {
directory = workingCopy.get().getDirectory();
}
assertThat(directory).doesNotExist();
}
private static class PoolWithSpy extends SimpleGitWorkdirFactory {
PoolWithSpy(File poolDirectory) {
super(poolDirectory);
}
Repository createdClone;
@Override
protected Repository cloneRepository(File bareRepository, File destination) throws GitAPIException {
createdClone = spy(super.cloneRepository(bareRepository, destination));
return createdClone;
}
}
}