opendev/gerrit
Code Issues Proposed changes

Replace our RepositoryCache with JGit's RepositoryCache

Browse Source 
JGit can now manage a cached set of Repository instances, and its
cache implementation uses better concurrency primitives than what
we had in Gerrit's simple RepositoryCache class.

The JGit contract however requires that we close the Repository
we were given, as the Repository handles are reference counted
to determine "in-use" status.  Failure to close a handle might
permit its resources to leak in memory.

Signed-off-by: Shawn O. Pearce <sop@google.com>
This commit is contained in:
Shawn O. Pearce
2009-07-27 09:09:50 -07:00
parent 0a3ac75205
commit 6a66a63818
16 changed files with 215 additions and 328 deletions
Download Patch File Download Diff File Expand all files Collapse all files

47
src/main/java/com/google/gerrit/git/GitMetaUtil.java
View File

@@ -1,47 +0,0 @@
// Copyright (C) 2008 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.git;
import org.spearce.jgit.lib.Repository;
import java.io.File;
import java.io.IOException;
public class GitMetaUtil {
public static boolean isGitRepository(final File gitdir) {
return new File(gitdir, "config").isFile()
&& new File(gitdir, "HEAD").isFile()
&& new File(gitdir, "objects").isDirectory()
&& new File(gitdir, "refs/heads").isDirectory();
}
public static Repository open(final File gitdir) throws IOException {
if (isGitRepository(gitdir)) {
return new Repository(gitdir);
}
if (isGitRepository(new File(gitdir, ".git"))) {
return new Repository(new File(gitdir, ".git"));
}
final String name = gitdir.getName();
final File parent = gitdir.getParentFile();
if (isGitRepository(new File(parent, name + ".git"))) {
return new Repository(new File(parent, name + ".git"));
}
return null;
}
}

28
src/main/java/com/google/gerrit/git/InvalidRepositoryException.java
View File

@@ -1,28 +0,0 @@
// Copyright (C) 2008 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.git;
/** Indicates a directory does not contain a valid Git repository. */
public class InvalidRepositoryException extends Exception {
private static final long serialVersionUID = 1L;
public InvalidRepositoryException(final String name) {
super(name);
}
public InvalidRepositoryException(final String name, final Throwable err) {
super(name, err);
}
}

8
src/main/java/com/google/gerrit/git/MergeOp.java
View File

@@ -39,6 +39,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spearce.jgit.errors.IncorrectObjectTypeException;
import org.spearce.jgit.errors.MissingObjectException;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.AnyObjectId;
import org.spearce.jgit.lib.Commit;
import org.spearce.jgit.lib.Constants;
@@ -133,6 +134,9 @@ public class MergeOp {
try {
mergeImpl();
} finally {
if (db != null) {
db.close();
}
schema.close();
schema = null;
}
@@ -165,8 +169,8 @@ public class MergeOp {
private void openRepository() throws MergeException {
final String name = destBranch.getParentKey().get();
try {
db = server.getRepositoryCache().get(name);
} catch (InvalidRepositoryException notGit) {
db = server.openRepository(name);
} catch (RepositoryNotFoundException notGit) {
final String m = "Repository \"" + name + "\" unknown.";
throw new MergeException(m, notGit);
}

11
src/main/java/com/google/gerrit/git/PushOp.java
View File

@@ -23,6 +23,7 @@ import com.jcraft.jsch.JSchException;
import org.slf4j.Logger;
import org.spearce.jgit.errors.NoRemoteRepositoryException;
import org.spearce.jgit.errors.NotSupportedException;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.errors.TransportException;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.NullProgressMonitor;
@@ -98,7 +99,7 @@ class PushOp implements Runnable {
} catch (XsrfException e) {
log.error("Cannot open database", e);
} catch (InvalidRepositoryException e) {
} catch (RepositoryNotFoundException e) {
log.error("Cannot replicate " + projectName + "; " + e.getMessage());
} catch (NoRemoteRepositoryException e) {
@@ -125,6 +126,10 @@ class PushOp implements Runnable {
} catch (Error e) {
log.error("Unexpected error during replication to " + uri, e);
} finally {
if (db != null) {
db.close();
}
}
}
@@ -133,9 +138,9 @@ class PushOp implements Runnable {
return (mirror ? "mirror " : "push ") + uri;
}
private void openRepository() throws InvalidRepositoryException,
private void openRepository() throws RepositoryNotFoundException,
OrmException, XsrfException {
db = GerritServer.getInstance().getRepositoryCache().get(projectName);
db = GerritServer.getInstance().openRepository(projectName);
}
private void runImpl() throws IOException {

2
src/main/java/com/google/gerrit/git/PushQueue.java
View File

@@ -118,7 +118,7 @@ public class PushQueue {
try {
final GerritServer gs = GerritServer.getInstance();
path = gs.getSitePath();
if (path == null || gs.getRepositoryCache() == null) {
if (path == null) {
return Collections.emptyList();
}
} catch (OrmException e) {

97
src/main/java/com/google/gerrit/git/RepositoryCache.java
View File

@@ -1,97 +0,0 @@
// Copyright (C) 2008 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.git;
import org.spearce.jgit.lib.Repository;
import java.io.File;
import java.io.IOException;
import java.lang.ref.Reference;
import java.lang.ref.SoftReference;
import java.util.HashMap;
import java.util.Map;
/** Cache of active Git repositories being used by the manager. */
public class RepositoryCache {
private final File base;
private final Map<String, Reference<Repository>> cache;
/**
* Create a new cache to manage a specific base directory (and below).
*
* @param basedir top level directory that contains all repositories.
*/
public RepositoryCache(final File basedir) {
base = basedir;
cache = new HashMap<String, Reference<Repository>>();
}
/**
* @return the base directory which contains all known repositories.
*/
public File getBaseDirectory() {
return base;
}
/**
* Get (or open) a repository by name.
*
* @param name the repository name, relative to the base directory supplied
* when the cache was created.
* @return the cached Repository instance.
* @throws InvalidRepositoryException the name does not denote an existing
* repository, or the name cannot be read as a repository.
*/
public synchronized Repository get(String name)
throws InvalidRepositoryException {
if (name.endsWith(".git")) {
name = name.substring(0, name.length() - 4);
}
final Reference<Repository> ref = cache.get(name);
Repository db = ref != null ? ref.get() : null;
if (db == null) {
if (isUnreasonableName(name)) {
throw new InvalidRepositoryException(name);
}
try {
db = GitMetaUtil.open(new File(base, name));
if (db == null) {
throw new InvalidRepositoryException(name);
}
} catch (IOException err) {
throw new InvalidRepositoryException(name, err);
}
cache.put(name, new SoftReference<Repository>(db));
}
return db;
}
private boolean isUnreasonableName(final String name) {
if (name.length() == 0) return true; // no empty paths
if (name.indexOf('\\') >= 0) return true; // no windows/dos stlye paths
if (name.charAt(0) == '/') return true; // no absolute paths
if (new File(name).isAbsolute()) return true; // no absolute paths
if (name.startsWith("../")) return true; // no "l../etc/passwd"
if (name.contains("/../")) return true; // no "foo/../etc/passwd"
if (name.contains("/./")) return true; // "foo/./foo" is insane to ask
if (name.contains("//")) return true; // windows UNC path can be "//..."
return false; // is a reasonable name
}
}

7
src/main/java/com/google/gerrit/pgm/ReimportPatchSets.java
View File

@@ -19,11 +19,11 @@ import com.google.gerrit.client.reviewdb.PatchSet;
import com.google.gerrit.client.reviewdb.Project;
import com.google.gerrit.client.reviewdb.ReviewDb;
import com.google.gerrit.client.rpc.Common;
import com.google.gerrit.git.InvalidRepositoryException;
import com.google.gerrit.git.PatchSetImporter;
import com.google.gerrit.server.GerritServer;
import com.google.gwtorm.client.OrmException;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.ObjectId;
import org.spearce.jgit.lib.ProgressMonitor;
import org.spearce.jgit.lib.Repository;
@@ -88,8 +88,8 @@ public class ReimportPatchSets extends AbstractProgram {
final String projectName = projectKey.get();
final Repository repo;
try {
repo = gs.getRepositoryCache().get(projectName);
} catch (InvalidRepositoryException ie) {
repo = gs.openRepository(projectName);
} catch (RepositoryNotFoundException ie) {
System.err.println();
System.err.println("NoProject " + psid);
System.err.println("NoProject " + ie.getMessage());
@@ -101,6 +101,7 @@ public class ReimportPatchSets extends AbstractProgram {
rw.parseCommit(ObjectId.fromString(ps.getRevision().get()));
new PatchSetImporter(gs, db, projectKey, repo, src, ps, false).run();
pm.update(1);
repo.close();
}
} catch (OrmException e) {
System.err.println();

10
src/main/java/com/google/gerrit/server/CatServlet.java
View File

@@ -24,12 +24,12 @@ import com.google.gerrit.client.reviewdb.PatchSet;
import com.google.gerrit.client.reviewdb.Project;
import com.google.gerrit.client.reviewdb.ReviewDb;
import com.google.gerrit.client.rpc.Common;
import com.google.gerrit.git.InvalidRepositoryException;
import com.google.gwtjsonrpc.server.XsrfException;
import com.google.gwtorm.client.OrmException;
import eu.medsea.mimeutil.MimeType;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.ObjectId;
import org.spearce.jgit.lib.Repository;
@@ -162,10 +162,8 @@ public class CatServlet extends HttpServlet {
final Repository repo;
try {
repo =
server.getRepositoryCache()
.get(change.getDest().getParentKey().get());
} catch (InvalidRepositoryException e) {
repo = server.openRepository(change.getDest().getParentKey().get());
} catch (RepositoryNotFoundException e) {
getServletContext().log("Cannot open repository", e);
rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
return;
@@ -219,6 +217,8 @@ public class CatServlet extends HttpServlet {
getServletContext().log("Cannot read repository", e);
rsp.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
return;
} finally {
repo.close();
}
final long when = fromCommit.getCommitTime() * 1000L;

57
src/main/java/com/google/gerrit/server/GerritServer.java
View File

@@ -38,7 +38,6 @@ import com.google.gerrit.client.workflow.SubmitFunction;
import com.google.gerrit.git.MergeQueue;
import com.google.gerrit.git.PushAllProjectsOp;
import com.google.gerrit.git.PushQueue;
import com.google.gerrit.git.RepositoryCache;
import com.google.gerrit.git.WorkQueue;
import com.google.gerrit.server.mail.EmailException;
import com.google.gerrit.server.patch.DiffCacheEntryFactory;
@@ -66,10 +65,14 @@ import org.apache.commons.net.smtp.SMTPReply;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spearce.jgit.errors.ConfigInvalidException;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.PersonIdent;
import org.spearce.jgit.lib.Repository;
import org.spearce.jgit.lib.RepositoryCache;
import org.spearce.jgit.lib.RepositoryConfig;
import org.spearce.jgit.lib.WindowCache;
import org.spearce.jgit.lib.WindowCacheConfig;
import org.spearce.jgit.lib.RepositoryCache.FileKey;
import java.io.File;
import java.io.FileInputStream;
@@ -189,7 +192,7 @@ public class GerritServer {
private final SignedToken xsrf;
private final SignedToken account;
private final SignedToken emailReg;
private final RepositoryCache repositories;
private final File basepath;
private final SelfPopulatingCache diffCache;
private final SelfPopulatingCache sshKeysCache;
@@ -236,9 +239,9 @@ public class GerritServer {
if (!root.isAbsolute()) {
root = new File(getSitePath(), basePath);
}
repositories = new RepositoryCache(root);
basepath = root;
} else {
repositories = null;
basepath = null;
}
final ReviewDb c = db.open();
@@ -885,9 +888,49 @@ public class GerritServer {
return gerritConfigFile;
}
/** Get the repositories maintained by this server. */
public RepositoryCache getRepositoryCache() {
return repositories;
/**
* Get (or open) a repository by name.
*
* @param name the repository name, relative to the base directory.
* @return the cached Repository instance. Caller must call {@code close()}
* when done to decrement the resource handle.
* @throws RepositoryNotFoundException the name does not denote an existing
* repository, or the name cannot be read as a repository.
*/
public Repository openRepository(String name)
throws RepositoryNotFoundException {
if (basepath == null) {
throw new RepositoryNotFoundException("No gerrit.basepath configured");
}
if (isUnreasonableName(name)) {
throw new RepositoryNotFoundException("Invalid name: " + name);
}
try {
final FileKey loc = FileKey.lenient(new File(basepath, name));
return RepositoryCache.open(loc);
} catch (IOException e1) {
final RepositoryNotFoundException e2;
e2 = new RepositoryNotFoundException("Cannot open repository " + name);
e2.initCause(e1);
throw e2;
}
}
private boolean isUnreasonableName(final String name) {
if (name.length() == 0) return true; // no empty paths
if (name.indexOf('\\') >= 0) return true; // no windows/dos stlye paths
if (name.charAt(0) == '/') return true; // no absolute paths
if (new File(name).isAbsolute()) return true; // no absolute paths
if (name.startsWith("../")) return true; // no "l../etc/passwd"
if (name.contains("/../")) return true; // no "foo/../etc/passwd"
if (name.contains("/./")) return true; // "foo/./foo" is insane to ask
if (name.contains("//")) return true; // windows UNC path can be "//..."
return false; // is a reasonable name
}
/** Get all registered caches. */

144
src/main/java/com/google/gerrit/server/ProjectAdminServiceImpl.java
View File

@@ -31,7 +31,6 @@ import com.google.gerrit.client.rpc.Common;
import com.google.gerrit.client.rpc.InvalidNameException;
import com.google.gerrit.client.rpc.InvalidRevisionException;
import com.google.gerrit.client.rpc.NoSuchEntityException;
import com.google.gerrit.git.InvalidRepositoryException;
import com.google.gerrit.git.PushQueue;
import com.google.gwt.user.client.rpc.AsyncCallback;
import com.google.gwtjsonrpc.client.VoidResult;
@@ -41,6 +40,7 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spearce.jgit.errors.IncorrectObjectTypeException;
import org.spearce.jgit.errors.MissingObjectException;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.Constants;
import org.spearce.jgit.lib.LockFile;
import org.spearce.jgit.lib.ObjectId;
@@ -123,7 +123,7 @@ public class ProjectAdminServiceImpl extends BaseServiceImplementation
final Repository e;
final LockFile f;
e = server.getRepositoryCache().get(proj.getName());
e = server.openRepository(proj.getName());
f = new LockFile(new File(e.getDirectory(), "description"));
if (f.lock()) {
String d = proj.getDescription();
@@ -135,9 +135,10 @@ public class ProjectAdminServiceImpl extends BaseServiceImplementation
f.write(Constants.encode(d));
f.commit();
}
} catch (IOException e) {
e.close();
} catch (RepositoryNotFoundException e) {
log.error("Cannot update description for " + proj.getName(), e);
} catch (InvalidRepositoryException e) {
} catch (IOException e) {
log.error("Cannot update description for " + proj.getName(), e);
}
}
@@ -285,8 +286,8 @@ public class ProjectAdminServiceImpl extends BaseServiceImplementation
final Repository r;
try {
r = server.getRepositoryCache().get(k.getParentKey().get());
} catch (InvalidRepositoryException e) {
r = server.openRepository(k.getParentKey().get());
} catch (RepositoryNotFoundException e) {
throw new Failure(new NoSuchEntityException());
}
@@ -297,6 +298,7 @@ public class ProjectAdminServiceImpl extends BaseServiceImplementation
result = u.delete();
} catch (IOException e) {
log.error("Cannot delete " + k, e);
r.close();
continue;
}
@@ -319,6 +321,7 @@ public class ProjectAdminServiceImpl extends BaseServiceImplementation
log.error("Cannot delete " + k + ": " + result.name());
break;
}
r.close();
}
return deleted;
}
@@ -348,81 +351,90 @@ public class ProjectAdminServiceImpl extends BaseServiceImplementation
assertAmProjectOwner(db, projectId);
final String repoName = pce.getProject().getName();
final Branch.NameKey name =
new Branch.NameKey(pce.getProject().getNameKey(), refname);
final Repository repo;
try {
repo = server.getRepositoryCache().get(repoName);
} catch (InvalidRepositoryException e1) {
repo = server.openRepository(repoName);
} catch (RepositoryNotFoundException e1) {
throw new Failure(new NoSuchEntityException());
}
// Convert the name given by the user into a valid object.
//
final ObjectId revid;
try {
revid = repo.resolve(startingRevision);
if (revid == null) {
// Convert the name given by the user into a valid object.
//
final ObjectId revid;
try {
revid = repo.resolve(startingRevision);
if (revid == null) {
throw new Failure(new InvalidRevisionException());
}
} catch (IOException err) {
log.error("Cannot resolve \"" + startingRevision + "\" in "
+ repoName, err);
throw new Failure(new InvalidRevisionException());
}
} catch (IOException err) {
log.error("Cannot resolve \"" + startingRevision + "\" in "
+ repoName, err);
throw new Failure(new InvalidRevisionException());
}
// Ensure it is fully connected in this repository. If not,
// we can't safely create a ref to it as objects are missing
//
final RevCommit revcommit;
final ObjectWalk rw = new ObjectWalk(repo);
try {
// Ensure it is fully connected in this repository. If not,
// we can't safely create a ref to it as objects are missing
//
final RevCommit revcommit;
final ObjectWalk rw = new ObjectWalk(repo);
try {
revcommit = rw.parseCommit(revid);
rw.markStart(revcommit);
try {
revcommit = rw.parseCommit(revid);
rw.markStart(revcommit);
} catch (IncorrectObjectTypeException err) {
throw new Failure(new InvalidRevisionException());
}
for (final Ref r : repo.getAllRefs().values()) {
try {
rw.markUninteresting(rw.parseAny(r.getObjectId()));
} catch (MissingObjectException err) {
continue;
}
}
rw.checkConnectivity();
} catch (IncorrectObjectTypeException err) {
throw new Failure(new InvalidRevisionException());
} catch (MissingObjectException err) {
throw new Failure(new InvalidRevisionException());
} catch (IOException err) {
log.error("Repository " + repoName + " possibly corrupt", err);
throw new Failure(new InvalidRevisionException());
}
for (final Ref r : repo.getAllRefs().values()) {
try {
rw.markUninteresting(rw.parseAny(r.getObjectId()));
} catch (MissingObjectException err) {
continue;
}
}
rw.checkConnectivity();
} catch (IncorrectObjectTypeException err) {
throw new Failure(new InvalidRevisionException());
} catch (MissingObjectException err) {
throw new Failure(new InvalidRevisionException());
} catch (IOException err) {
log.error("Repository " + repoName + " possibly corrupt", err);
throw new Failure(new InvalidRevisionException());
}
final HttpServletRequest hreq =
GerritJsonServlet.getCurrentCall().getHttpServletRequest();
final Branch.NameKey name =
new Branch.NameKey(pce.getProject().getNameKey(), refname);
try {
final RefUpdate u = repo.updateRef(refname);
u.setExpectedOldObjectId(ObjectId.zeroId());
u.setNewObjectId(revid);
u.setRefLogIdent(ChangeUtil.toReflogIdent(me, new InetSocketAddress(
hreq.getRemoteHost(), hreq.getRemotePort())));
u.setRefLogMessage("created via web from " + startingRevision, false);
final RefUpdate.Result result = u.update(rw);
switch (result) {
case FAST_FORWARD:
case NEW:
case NO_CHANGE:
PushQueue.scheduleUpdate(name.getParentKey(), refname);
break;
default:
log.error("Cannot create branch " + name + ": " + result.name());
throw new Failure(new IOException(result.name()));
final HttpServletRequest hreq =
GerritJsonServlet.getCurrentCall().getHttpServletRequest();
try {
final RefUpdate u = repo.updateRef(refname);
u.setExpectedOldObjectId(ObjectId.zeroId());
u.setNewObjectId(revid);
u.setRefLogIdent(ChangeUtil.toReflogIdent(me,
new InetSocketAddress(hreq.getRemoteHost(), hreq
.getRemotePort())));
u.setRefLogMessage("created via web from " + startingRevision,
false);
final RefUpdate.Result result = u.update(rw);
switch (result) {
case FAST_FORWARD:
case NEW:
case NO_CHANGE:
PushQueue.scheduleUpdate(name.getParentKey(), refname);
break;
default: {
final String msg =
"Cannot create branch " + name + ": " + result.name();
log.error(msg);
throw new Failure(new IOException(result.name()));
}
}
} catch (IOException err) {
log.error("Cannot create branch " + name, err);
throw new Failure(err);
}
} catch (IOException err) {
log.error("Cannot create branch " + name, err);
throw new Failure(err);
} finally {
repo.close();
}
final Branch.Id id = new Branch.Id(db.nextBranchId());

10
src/main/java/com/google/gerrit/server/mail/CommentSender.java
View File

@@ -18,11 +18,11 @@ import com.google.gerrit.client.reviewdb.Change;
import com.google.gerrit.client.reviewdb.Patch;
import com.google.gerrit.client.reviewdb.PatchLineComment;
import com.google.gerrit.client.reviewdb.PatchSet;
import com.google.gerrit.git.InvalidRepositoryException;
import com.google.gerrit.server.GerritServer;
import com.google.gerrit.server.patch.PatchFile;
import com.google.gwtorm.client.OrmException;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.Repository;
import java.util.Collections;
@@ -109,6 +109,10 @@ public class CommentSender extends ReplyToChangeSender {
appendText(c.getMessage().trim());
appendText("\n\n");
}
if (repo != null) {
repo.close();
}
}
private Map<Patch.Key, Patch> getPatchMap() {
@@ -124,8 +128,8 @@ public class CommentSender extends ReplyToChangeSender {
private Repository getRepository() {
try {
return server.getRepositoryCache().get(projectName);
} catch (InvalidRepositoryException e) {
return server.openRepository(projectName);
} catch (RepositoryNotFoundException e) {
return null;
}
}

18
src/main/java/com/google/gerrit/server/patch/DiffCacheEntryFactory.java
View File

@@ -14,8 +14,6 @@
package com.google.gerrit.server.patch;
import com.google.gerrit.client.reviewdb.Project;
import com.google.gerrit.git.InvalidRepositoryException;
import com.google.gerrit.server.GerritServer;
import net.sf.ehcache.constructs.blocking.CacheEntryFactory;
@@ -37,7 +35,16 @@ public class DiffCacheEntryFactory implements CacheEntryFactory {
public Object createEntry(Object genericKey) throws Exception {
final DiffCacheKey key = (DiffCacheKey) genericKey;
final Repository db = open(key.getProjectKey());
final Repository db = server.openRepository(key.getProjectKey().get());
try {
return createEntry(key, db);
} finally {
db.close();
}
}
private DiffCacheContent createEntry(final DiffCacheKey key,
final Repository db) throws Exception {
final ObjectId newId = key.getNewId();
final List<String> args = new ArrayList<String>();
args.add("git");
@@ -106,9 +113,4 @@ public class DiffCacheEntryFactory implements CacheEntryFactory {
return DiffCacheContent.create(file);
}
private Repository open(final Project.NameKey key)
throws InvalidRepositoryException {
return server.getRepositoryCache().get(key.get());
}
}

8
src/main/java/com/google/gerrit/server/patch/PatchDetailServiceImpl.java
View File

@@ -39,7 +39,6 @@ import com.google.gerrit.client.rpc.BaseServiceImplementation;
import com.google.gerrit.client.rpc.Common;
import com.google.gerrit.client.rpc.NoSuchAccountException;
import com.google.gerrit.client.rpc.NoSuchEntityException;
import com.google.gerrit.git.RepositoryCache;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.GerritJsonServlet;
import com.google.gerrit.server.GerritServer;
@@ -79,12 +78,7 @@ public class PatchDetailServiceImpl extends BaseServiceImplementation implements
callback.onFailure(new NoSuchEntityException());
return;
}
final RepositoryCache rc = server.getRepositoryCache();
if (rc == null) {
callback.onFailure(new Exception("No Repository Cache configured"));
return;
}
run(callback, new PatchScriptAction(server, rc, patchKey, psa, psb, s));
run(callback, new PatchScriptAction(server, patchKey, psa, psb, s));
}
public void patchComments(final Patch.Key patchKey, final PatchSet.Id psa,

67
src/main/java/com/google/gerrit/server/patch/PatchScriptAction.java
View File

@@ -33,8 +33,6 @@ import com.google.gerrit.client.rpc.CorruptEntityException;
import com.google.gerrit.client.rpc.NoSuchEntityException;
import com.google.gerrit.client.rpc.BaseServiceImplementation.Action;
import com.google.gerrit.client.rpc.BaseServiceImplementation.Failure;
import com.google.gerrit.git.InvalidRepositoryException;
import com.google.gerrit.git.RepositoryCache;
import com.google.gerrit.server.GerritServer;
import com.google.gwtorm.client.OrmException;
@@ -43,6 +41,7 @@ import net.sf.ehcache.Element;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.ObjectId;
import org.spearce.jgit.lib.ObjectWriter;
import org.spearce.jgit.lib.Repository;
@@ -57,7 +56,6 @@ class PatchScriptAction implements Action<PatchScript> {
LoggerFactory.getLogger(PatchScriptAction.class);
private final GerritServer server;
private final RepositoryCache rc;
private final Patch.Key patchKey;
private final PatchSet.Id psa;
private final PatchSet.Id psb;
@@ -71,11 +69,10 @@ class PatchScriptAction implements Action<PatchScript> {
private Project.NameKey projectKey;
private Repository git;
PatchScriptAction(final GerritServer gs, final RepositoryCache rc,
final Patch.Key patchKey, final PatchSet.Id psa, final PatchSet.Id psb,
PatchScriptAction(final GerritServer gs, final Patch.Key patchKey,
final PatchSet.Id psa, final PatchSet.Id psb,
final PatchScriptSettings settings) {
this.server = gs;
this.rc = rc;
this.patchKey = patchKey;
this.psa = psa;
this.psb = psb;
@@ -98,38 +95,42 @@ class PatchScriptAction implements Action<PatchScript> {
projectKey = change.getDest().getParentKey();
try {
git = rc.get(projectKey.get());
} catch (InvalidRepositoryException e) {
git = server.openRepository(projectKey.get());
} catch (RepositoryNotFoundException e) {
log.error("Repository " + projectKey + " not found", e);
throw new Failure(new NoSuchEntityException());
}
final PatchScriptBuilder b = newBuilder();
final ObjectId bId = toObjectId(db, psb);
final ObjectId aId = psa == null ? ancestor(bId) : toObjectId(db, psa);
final DiffCacheKey key = keyFor(bId, aId);
final DiffCacheContent contentWS = get(key);
final CommentDetail comments = allComments(db);
final DiffCacheContent contentActual;
if (settings.getWhitespace() != Whitespace.IGNORE_NONE) {
// If we are ignoring whitespace in some form, we still need to know
// where the post-image differs so we can ensure the post-image lines
// are still packed for the client to display.
//
final PatchScriptSettings s = new PatchScriptSettings(settings);
s.setWhitespace(Whitespace.IGNORE_NONE);
contentActual = get(new DiffCacheKey(projectKey, aId, bId, patch, s));
} else {
contentActual = contentWS;
}
try {
return b.toPatchScript(key, contentWS, comments, contentActual);
} catch (CorruptEntityException e) {
log.error("File content for " + key + " unavailable", e);
throw new Failure(new NoSuchEntityException());
final PatchScriptBuilder b = newBuilder();
final ObjectId bId = toObjectId(db, psb);
final ObjectId aId = psa == null ? ancestor(bId) : toObjectId(db, psa);
final DiffCacheKey key = keyFor(bId, aId);
final DiffCacheContent contentWS = get(key);
final CommentDetail comments = allComments(db);
final DiffCacheContent contentActual;
if (settings.getWhitespace() != Whitespace.IGNORE_NONE) {
// If we are ignoring whitespace in some form, we still need to know
// where the post-image differs so we can ensure the post-image lines
// are still packed for the client to display.
//
final PatchScriptSettings s = new PatchScriptSettings(settings);
s.setWhitespace(Whitespace.IGNORE_NONE);
contentActual = get(new DiffCacheKey(projectKey, aId, bId, patch, s));
} else {
contentActual = contentWS;
}
try {
return b.toPatchScript(key, contentWS, comments, contentActual);
} catch (CorruptEntityException e) {
log.error("File content for " + key + " unavailable", e);
throw new Failure(new NoSuchEntityException());
}
} finally {
git.close();
}
}

10
src/main/java/com/google/gerrit/server/ssh/AbstractCommand.java
View File

@@ -21,7 +21,6 @@ import com.google.gerrit.client.reviewdb.ApprovalCategory;
import com.google.gerrit.client.reviewdb.ReviewDb;
import com.google.gerrit.client.rpc.BaseServiceImplementation;
import com.google.gerrit.client.rpc.Common;
import com.google.gerrit.git.RepositoryCache;
import com.google.gerrit.server.GerritServer;
import com.google.gwtjsonrpc.server.XsrfException;
import com.google.gwtorm.client.OrmException;
@@ -105,15 +104,6 @@ abstract class AbstractCommand implements Command, SessionAware {
}
}
protected RepositoryCache getRepositoryCache() throws Failure {
final RepositoryCache rc = getGerritServer().getRepositoryCache();
if (rc == null) {
throw new Failure(128, "fatal: Gerrit repositories are not available",
new IllegalStateException("gerrit.basePath not set"));
}
return rc;
}
protected ReviewDb openReviewDb() throws Failure {
if (db == null) {
try {

19
src/main/java/com/google/gerrit/server/ssh/AbstractGitCommand.java
View File

@@ -20,9 +20,9 @@ import com.google.gerrit.client.reviewdb.ApprovalCategory;
import com.google.gerrit.client.reviewdb.Project;
import com.google.gerrit.client.reviewdb.ProjectRight;
import com.google.gerrit.client.rpc.Common;
import com.google.gerrit.git.InvalidRepositoryException;
import org.kohsuke.args4j.Argument;
import org.spearce.jgit.errors.RepositoryNotFoundException;
import org.spearce.jgit.lib.Repository;
import java.io.IOException;
@@ -75,19 +75,22 @@ abstract class AbstractGitCommand extends AbstractCommand {
new SecurityException("Account lacks Read permission"));
}
try {
repo = getRepositoryCache().get(proj.getName());
} catch (InvalidRepositoryException e) {
throw new Failure(1, "fatal: '" + reqProjName + "': not a git archive", e);
}
userAccount = Common.getAccountCache().get(getAccountId(), db);
if (userAccount == null) {
throw new Failure(1, "fatal: cannot query user database",
new IllegalStateException("Account record no longer in database"));
}
runImpl();
try {
repo = getGerritServer().openRepository(proj.getName());
} catch (RepositoryNotFoundException e) {
throw new Failure(1, "fatal: '" + reqProjName + "': not a git archive", e);
}
try {
runImpl();
} finally {
repo.close();
}
}
protected boolean canPerform(final ApprovalCategory.Id actionId,