Fix peer review issues

This commit is contained in:
René Pfeuffer
2020-06-02 21:31:33 +02:00
parent 273bcb6f1a
commit a32bd01c45
6 changed files with 12 additions and 101 deletions

View File

@@ -40,7 +40,7 @@ import java.util.concurrent.ConcurrentHashMap;
* requested for a repository with {@link #getWorkingCopy(SimpleWorkingCopyFactory.WorkingCopyContext)},
* this implementation fetches a new directory from the {@link WorkdirProvider}.
* On {@link #contextClosed(SimpleWorkingCopyFactory.WorkingCopyContext, File)},
* the directory is not deleted, buy put into a map with the repository id as key.
* the directory is not deleted, but put into a map with the repository id as key.
* When a working copy is requested with {@link #getWorkingCopy(SimpleWorkingCopyFactory.WorkingCopyContext)}
* for a repository with such an existing directory, it is taken from the map, reclaimed and
* returned as {@link WorkingCopy}.

View File

@@ -55,97 +55,10 @@ import java.io.File;
* <dt>{@link #closeRepository(R)}</dt>
* <dd>Closes resources allocated for the central repository.</dd>
* </dl>
* <pre>
* ┌─────────────┐ ┌───────────────────────────┐ ┌───────────────┐ ┌───────────────┐
* │ModifyCommand│ │SimpleGitWorkingCopyFactory│ │WorkingCopyPool│ │WorkdirProvider│
* └──────┬──────┘ └─────────────┬─────────────┘ └───────┬───────┘ └───────┬───────┘
* │ createWorkingCopy │ │ │
* │──────────────────────────────>│ │ │
* │ │ │ │
* │ │ create ┌──────────────────┐ │ │
* │ │──────────────────────> │WorkingCopyContext│ │ │
* │ │ └────────┬─────────┘ │ │
* │ │ getWorkingCopy │ │
* │ │─────────────────────────────────────────────────────────────>│ │
* │ │ │ │ │
* │ │ │ │ │
* │ │ │ │ │
* │ │ │ │ │
* │ │ │ reclaim │ │
* │ │ │ <──────────────────────────│ │
* │ │ │ │ │
* │ │ │ │ │
* │ │ ╔══════╤═══════════╪════════════════════════════╪═════════════════════════════════════════════════╪═════════════════╗
* │ │ ║ ALT │ reclaim successful │ │ ║
* │ │ ╟──────┘ │ │ │ ║
* │ │ ║ │ │ ┌───────────┐ │ ║
* │ │ ║ │ ─────────────────────────────────────────────>│WorkingCopy│ │ ║
* │ │ ║ │ │ └─────┬─────┘ │ ║
* │ │ ║ │ WorkingCopy │ │ ║
* │ │ ║ │ ──────────────────────────>│ │ ║
* │ │ ╠══════════════════╪════════════════════════════╪═════════════════════════════════════════════════╪═════════════════╣
* │ │ ║ [reclaim fails; create new] │ │ ║
* │ │ ║ │ ReclaimFailedException │ │ ║
* │ │ ║ │ ──────────────────────────>│ │ ║
* │ │ ║ │ │ │ ║
* │ │ ║ │ │ createNewWorkdir │ ║
* │ │ ║ │ │────────────────────────────────────────────────>│ ║
* │ │ ║ │ │ │ ║
* │ │ ║ │ │ │ ║
* │ │ ║ │ │<────────────────────────────────────────────────│ ║
* │ │ ║ │ │ │ ║
* │ │ ║ │ initialize │ │ ║
* │ │ ║ │ <──────────────────────────│ │ ║
* │ │ ║ │ │ │ ║
* │ │ ║ │ │ ┌───────────┐ │ ║
* │ │ ║ │ ─────────────────────────────────────────────>│WorkingCopy│ │ ║
* │ │ ║ │ │ └─────┬─────┘ │ ║
* │ │ ║ │ WorkingCopy │ │ │ ║
* │ │ ║ │ ──────────────────────────>│ │ │ ║
* │ │ ╚══════════════════╪════════════════════════════╪════════════════════════╪════════════════════════╪═════════════════╝
* │ │ │ │ │ │
* │ │ WorkingCopy │ │ │
* │ │<─────────────────────────────────────────────────────────────│ │ │
* │ │ │ │ │ │
* │ WorkingCopy │ │ │ │ │
* │<──────────────────────────────│ │ │ │ │
* │ │ │ │ │ │
* . . . . . .
* . . . . . .
* . . . . . .
* . . . . . .
* │ │ doWork │ │ │ │
* │──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────>│ │
* │ │ │ │ │ │
* . . . . . .
* . . . . . .
* . . . . . .
* . . . . . .
* │ │ close │ │ │ │
* │──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────>│ │
* │ │ │ │ │ │
* │ │ │ close │ │ │
* │ │<──────────────────────────────────────────────────────────────────────────────────────│ │
* │ │ │ │ │ │
* │ ────┐ │ │ │ │
* │ │ closeWorkingCopy │ │ │ │
* │ <───┘ │ │ │ │
* │ │ │ │ │ │
* │ ────┐ │ │ │ │
* │ │ closeRepository │ │ │ │
* │ <───┘ │ │ │ │
* │ │ │ │ │ │
* │ │ contextClosed │ │ │
* │ │─────────────────────────────────────────────────────────────>│ │ │
* │ │ │ │ │ │
* │ │ │ ────┐ │ │
* │ │ │ │ cacheDirectory │ │
* │ │ │ <───┘ │ │
* ┌──────┴──────┐ ┌─────────────┴─────────────┐ ┌────────┴─────────┐ ┌───────┴───────┐ ┌─────┴─────┐ ┌───────┴───────┐
* │ModifyCommand│ │SimpleGitWorkingCopyFactory│ │WorkingCopyContext│ │WorkingCopyPool│ │WorkingCopy│ │WorkdirProvider│
* └─────────────┘ └───────────────────────────┘ └──────────────────┘ └───────────────┘ └───────────┘ └───────────────┘
* </pre>
* <img src="http://www.plantuml.com/plantuml/png/jLF1JiCm3BtdAtAkr7r0aQf9XN42JGE9SvHumyA9goHbAr-FnZgKDbCPGXnZl_ViFDlB49MFdINnm0QtVSFMAcVA-WbjIt2FyONz6xfTmss_KZgoxsKbjGSL8Kc96NnPooJOi8jmY4LHdKJccKbipKpL3bAOs7dkMldiUnbPUj2aq8e9fwppwjKPgoYUUJ9qMaC8suv4pXYf5CL5H2sdxQQz0WNuhhLLIE5cy54ws5yKF6I2cnD_fP30t1qqj17PNVwoGR_s_8u6_E3r8-o7X9W0odfgzLKseiE8Yl03_iSoP_8svbQpabVlP3rQ-35niLXCxo59LuQFhvzGcZYCR9azgW4-WxY2diJ_gBI1bWCUtx-xJtqQR7FKo6UNmvL-XLlqy2Kdbk1CP-aJ"/>
* <br>
* The general process looks like this:
* <br>
* <img src="doc-files/SimpleWorkingCopyFactory_Sequence.png"/>
* @param <R> Type of central repository location
* @param <W> Type of working copy for repository
* @param <C> Type of repository context

Binary file not shown.

After

Width:  |  Height:  |  Size: 58 KiB

View File

@@ -24,6 +24,7 @@
package sonia.scm.repository.spi;
import com.google.common.base.Stopwatch;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.errors.GitAPIException;
import org.eclipse.jgit.lib.Constants;
@@ -54,7 +55,7 @@ class GitWorkingCopyInitializer {
public ParentAndClone<Repository, Repository> initialize(File target, String initialBranch) {
LOG.trace("clone repository {}", context.getRepository().getId());
long start = System.nanoTime();
Stopwatch stopwatch = Stopwatch.createStarted();
try {
Repository clone = Git.cloneRepository()
.setURI(simpleGitWorkingCopyFactory.createScmTransportProtocolUri(context.getDirectory()))
@@ -73,9 +74,7 @@ class GitWorkingCopyInitializer {
} catch (GitAPIException | IOException e) {
throw new InternalRepositoryException(context.getRepository(), "could not clone working copy of repository", e);
} finally {
long end = System.nanoTime();
long duration = end - start;
LOG.trace("took {} ns to clone repository {}", duration, context.getRepository().getId());
LOG.trace("took {} to clone repository {}", stopwatch.stop(), context.getRepository().getId());
}
}
}

View File

@@ -24,6 +24,7 @@
package sonia.scm.repository.spi;
import com.google.common.base.Stopwatch;
import org.eclipse.jgit.api.Git;
import org.eclipse.jgit.api.ResetCommand;
import org.eclipse.jgit.api.errors.GitAPIException;
@@ -49,7 +50,7 @@ class GitWorkingCopyReclaimer {
public ParentAndClone<Repository, Repository> reclaim(File target, String initialBranch) throws SimpleWorkingCopyFactory.ReclaimFailedException {
LOG.trace("reclaim repository {}", context.getRepository().getId());
long start = System.nanoTime();
Stopwatch stopwatch = Stopwatch.createStarted();
Repository repo = openTarget(target);
try (Git git = Git.open(target)) {
git.reset().setMode(ResetCommand.ResetType.HARD).call();
@@ -62,9 +63,7 @@ class GitWorkingCopyReclaimer {
} catch (GitAPIException | IOException e) {
throw new SimpleWorkingCopyFactory.ReclaimFailedException(e);
} finally {
long end = System.nanoTime();
long duration = end - start;
LOG.trace("took {} ns to reclaim repository {}\n", duration, context.getRepository().getId());
LOG.trace("took {} to reclaim repository {}", stopwatch.stop(), context.getRepository().getId());
}
}

View File

@@ -45,7 +45,7 @@ public class WorkingCopyPoolModule extends AbstractModule {
Class<? extends WorkingCopyPool> strategyClass = (Class<? extends WorkingCopyPool>) classLoader.loadClass(workingCopyPoolStrategy);
bind(WorkingCopyPool.class).to(strategyClass);
} catch (Exception e) {
throw new RuntimeException("could not instantiate class for working copy pool: " + workingCopyPoolStrategy, e);
throw new IllegalStateException("could not instantiate class for working copy pool: " + workingCopyPoolStrategy, e);
}
}
}