Allow GitRepositoryManager methods to throw IOExceptions

RepositoryNotFoundException should really mean "this repository
definitely doesn't exist", not "something went wrong when we tried to
read this repo (maybe it doesn't exist?)". Since GitRepositoryManager
could only throw RepositoryNotFoundExceptions, the easiest
implementation was to wrap *all* exceptions in that class, i.e. to take
the latter interpretation. We have some downstream caching code that
treated RepositoryNotFoundExceptions as definite negatives, i.e. the
former interpretation. This was causing poisoned caches on otherwise
mostly harmless transient errors.

Change GitRepositoryManager to distinguish between these two cases by
using IOException for "something went wrong". This complicates the
signature, but since RepositoryNotFound derives from IOException, many
callers that just want to wrap-and-throw can still do so with minimal
changes.

Change-Id: Ib2aa2e2443cc3b530ae7fc50fe4eb20059bb16de
This commit is contained in:
Dave Borowitz
2012-05-14 11:09:09 -07:00
committed by gerrit code review
parent e0d188ffd5
commit 7f4fbc8756
21 changed files with 53 additions and 27 deletions

View File

@@ -167,7 +167,12 @@ public class GitOverHttpServlet extends GitServlet {
} }
req.setAttribute(ATT_CONTROL, pc); req.setAttribute(ATT_CONTROL, pc);
return manager.openRepository(pc.getProject().getNameKey()); try {
return manager.openRepository(pc.getProject().getNameKey());
} catch (IOException e) {
throw new RepositoryNotFoundException(
pc.getProject().getNameKey().get(), e);
}
} }
} }

View File

@@ -141,6 +141,9 @@ class PatchScriptFactory extends Handler<PatchScript> {
} catch (RepositoryNotFoundException e) { } catch (RepositoryNotFoundException e) {
log.error("Repository " + projectKey + " not found", e); log.error("Repository " + projectKey + " not found", e);
throw new NoSuchChangeException(changeId, e); throw new NoSuchChangeException(changeId, e);
} catch (IOException e) {
log.error("Cannot open repository " + projectKey, e);
throw new NoSuchChangeException(changeId, e);
} }
try { try {
final PatchList list = listFor(keyFor(diffPrefs.getIgnoreWhitespace())); final PatchList list = listFor(keyFor(diffPrefs.getIgnoreWhitespace()));

View File

@@ -65,7 +65,8 @@ class ChangeProjectSettings extends Handler<ProjectDetail> {
} }
@Override @Override
public ProjectDetail call() throws NoSuchProjectException, OrmException { public ProjectDetail call() throws NoSuchProjectException, OrmException,
IOException {
final Project.NameKey projectName = update.getNameKey(); final Project.NameKey projectName = update.getNameKey();
projectControlFactory.ownerFor(projectName); projectControlFactory.ownerFor(projectName);
@@ -74,6 +75,8 @@ class ChangeProjectSettings extends Handler<ProjectDetail> {
md = metaDataUpdateFactory.create(projectName); md = metaDataUpdateFactory.create(projectName);
} catch (RepositoryNotFoundException notFound) { } catch (RepositoryNotFoundException notFound) {
throw new NoSuchProjectException(projectName); throw new NoSuchProjectException(projectName);
} catch (IOException e) {
throw new OrmException(e);
} }
try { try {
// TODO We really should take advantage of the Git commit DAG and // TODO We really should take advantage of the Git commit DAG and

View File

@@ -80,7 +80,7 @@ class DeleteBranches extends Handler<Set<Branch.NameKey>> {
@Override @Override
public Set<Branch.NameKey> call() throws NoSuchProjectException, public Set<Branch.NameKey> call() throws NoSuchProjectException,
RepositoryNotFoundException, OrmException { RepositoryNotFoundException, OrmException, IOException {
final ProjectControl projectControl = final ProjectControl projectControl =
projectControlFactory.controlFor(projectName); projectControlFactory.controlFor(projectName);

View File

@@ -62,7 +62,7 @@ class ListBranches extends Handler<ListBranchesResult> {
} }
@Override @Override
public ListBranchesResult call() throws NoSuchProjectException { public ListBranchesResult call() throws NoSuchProjectException, IOException {
final ProjectControl pctl = projectControlFactory.validateFor( // final ProjectControl pctl = projectControlFactory.validateFor( //
projectName, // projectName, //
ProjectControl.OWNER | ProjectControl.VISIBLE); ProjectControl.OWNER | ProjectControl.VISIBLE);

View File

@@ -51,7 +51,7 @@ class ProjectDetailFactory extends Handler<ProjectDetail> {
} }
@Override @Override
public ProjectDetail call() throws NoSuchProjectException { public ProjectDetail call() throws NoSuchProjectException, IOException {
final ProjectControl pc = final ProjectControl pc =
projectControlFactory.validateFor(projectName, ProjectControl.OWNER projectControlFactory.validateFor(projectName, ProjectControl.OWNER
| ProjectControl.VISIBLE); | ProjectControl.VISIBLE);

View File

@@ -14,7 +14,6 @@
package com.google.gerrit.httpd.rpc.project; package com.google.gerrit.httpd.rpc.project;
import com.google.gerrit.common.data.ProjectDetail; import com.google.gerrit.common.data.ProjectDetail;
import com.google.gerrit.httpd.rpc.Handler; import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
@@ -22,6 +21,7 @@ import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.inject.Inject; import com.google.inject.Inject;
import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.Comparator; import java.util.Comparator;
@@ -50,6 +50,7 @@ class VisibleProjectDetails extends Handler<List<ProjectDetail>> {
try { try {
result.add(projectDetailFactory.create(projectName).call()); result.add(projectDetailFactory.create(projectName).call());
} catch (NoSuchProjectException e) { } catch (NoSuchProjectException e) {
} catch (IOException e) {
} }
} }
Collections.sort(result, new Comparator<ProjectDetail>() { Collections.sort(result, new Comparator<ProjectDetail>() {

View File

@@ -33,7 +33,6 @@ import com.google.inject.Injector;
import org.eclipse.jgit.errors.IncorrectObjectTypeException; import org.eclipse.jgit.errors.IncorrectObjectTypeException;
import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.lib.TextProgressMonitor; import org.eclipse.jgit.lib.TextProgressMonitor;
@@ -109,7 +108,7 @@ public class ScanTrackingIds extends SiteProgram {
final Repository git; final Repository git;
try { try {
git = gitManager.openRepository(project); git = gitManager.openRepository(project);
} catch (RepositoryNotFoundException e) { } catch (IOException e) {
return; return;
} }
try { try {

View File

@@ -50,7 +50,6 @@ import com.google.inject.AbstractModule;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.Config;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.RefUpdate;
@@ -203,7 +202,7 @@ public class ChangeHookRunner implements ChangeHooks {
private Repository openRepository(final Project.NameKey name) { private Repository openRepository(final Project.NameKey name) {
try { try {
return repoManager.openRepository(name); return repoManager.openRepository(name);
} catch (RepositoryNotFoundException err) { } catch (IOException err) {
log.warn("Cannot open repository " + name.get(), err); log.warn("Cannot open repository " + name.get(), err);
return null; return null;
} }

View File

@@ -189,6 +189,8 @@ public class RulesCache {
git = gitMgr.openRepository(project); git = gitMgr.openRepository(project);
} catch (RepositoryNotFoundException e) { } catch (RepositoryNotFoundException e) {
throw new CompileException("Cannot open repository " + project, e); throw new CompileException("Cannot open repository " + project, e);
} catch (IOException e) {
throw new CompileException("Cannot open repository " + project, e);
} }
try { try {
ObjectLoader ldr = git.open(rulesId, Constants.OBJ_BLOB); ObjectLoader ldr = git.open(rulesId, Constants.OBJ_BLOB);

View File

@@ -42,6 +42,7 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
import java.util.Map; import java.util.Map;
public final class StoredValues { public final class StoredValues {
@@ -100,6 +101,8 @@ public final class StoredValues {
repo = gitMgr.openRepository(projectKey); repo = gitMgr.openRepository(projectKey);
} catch (RepositoryNotFoundException e) { } catch (RepositoryNotFoundException e) {
throw new SystemException(e.getMessage()); throw new SystemException(e.getMessage());
} catch (IOException e) {
throw new SystemException(e.getMessage());
} }
env.addToCleanup(new Runnable() { env.addToCleanup(new Runnable() {
@Override @Override
@@ -130,4 +133,4 @@ public final class StoredValues {
private StoredValues() { private StoredValues() {
} }
} }

View File

@@ -59,10 +59,11 @@ public interface GitRepositoryManager {
* @return the cached Repository instance. Caller must call {@code close()} * @return the cached Repository instance. Caller must call {@code close()}
* when done to decrement the resource handle. * when done to decrement the resource handle.
* @throws RepositoryNotFoundException the name does not denote an existing * @throws RepositoryNotFoundException the name does not denote an existing
* repository, or the name cannot be read as a repository. * repository.
* @throws IOException the name cannot be read as a repository.
*/ */
public abstract Repository openRepository(Project.NameKey name) public abstract Repository openRepository(Project.NameKey name)
throws RepositoryNotFoundException; throws RepositoryNotFoundException, IOException;
/** /**
* Create (and open) a repository by name. * Create (and open) a repository by name.
@@ -73,9 +74,11 @@ public interface GitRepositoryManager {
* @throws RepositoryCaseMismatchException the name collides with an existing * @throws RepositoryCaseMismatchException the name collides with an existing
* repository name, but only in case of a character within the name. * repository name, but only in case of a character within the name.
* @throws RepositoryNotFoundException the name is invalid. * @throws RepositoryNotFoundException the name is invalid.
* @throws IOException the repository cannot be created.
*/ */
public abstract Repository createRepository(Project.NameKey name) public abstract Repository createRepository(Project.NameKey name)
throws RepositoryCaseMismatchException, RepositoryNotFoundException; throws RepositoryCaseMismatchException, RepositoryNotFoundException,
IOException;
/** @return set of all known projects, sorted by natural NameKey order. */ /** @return set of all known projects, sorted by natural NameKey order. */
public abstract SortedSet<Project.NameKey> list(); public abstract SortedSet<Project.NameKey> list();

View File

@@ -319,6 +319,9 @@ public class MergeOp {
} catch (RepositoryNotFoundException notGit) { } catch (RepositoryNotFoundException notGit) {
final String m = "Repository \"" + name.get() + "\" unknown."; final String m = "Repository \"" + name.get() + "\" unknown.";
throw new MergeException(m, notGit); throw new MergeException(m, notGit);
} catch (IOException err) {
final String m = "Error opening repository \"" + name.get() + '"';
throw new MergeException(m, err);
} }
rw = new RevWalk(repo) { rw = new RevWalk(repo) {

View File

@@ -25,6 +25,8 @@ import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
/** Helps with the updating of a {@link VersionedMetaData}. */ /** Helps with the updating of a {@link VersionedMetaData}. */
public class MetaDataUpdate { public class MetaDataUpdate {
public static class User { public static class User {
@@ -49,7 +51,7 @@ public class MetaDataUpdate {
} }
public MetaDataUpdate create(Project.NameKey name) public MetaDataUpdate create(Project.NameKey name)
throws RepositoryNotFoundException { throws RepositoryNotFoundException, IOException {
MetaDataUpdate md = factory.create(name, mgr.openRepository(name)); MetaDataUpdate md = factory.create(name, mgr.openRepository(name));
md.getCommitBuilder().setAuthor(userIdent); md.getCommitBuilder().setAuthor(userIdent);
md.getCommitBuilder().setCommitter(serverIdent); md.getCommitBuilder().setCommitter(serverIdent);
@@ -71,7 +73,7 @@ public class MetaDataUpdate {
} }
public MetaDataUpdate create(Project.NameKey name) public MetaDataUpdate create(Project.NameKey name)
throws RepositoryNotFoundException { throws RepositoryNotFoundException, IOException {
MetaDataUpdate md = factory.create(name, mgr.openRepository(name)); MetaDataUpdate md = factory.create(name, mgr.openRepository(name));
md.getCommitBuilder().setAuthor(serverIdent); md.getCommitBuilder().setAuthor(serverIdent);
md.getCommitBuilder().setCommitter(serverIdent); md.getCommitBuilder().setCommitter(serverIdent);

View File

@@ -488,6 +488,10 @@ public class PushReplication implements ReplicationQueue {
log.error("Internal error: project " + project log.error("Internal error: project " + project
+ " not found during replication", err); + " not found during replication", err);
return; return;
} catch (IOException err) {
log.error("Internal error: unable to open project " + project
+ " during replication", err);
return;
} }
try { try {
Ref head = git.getRef(Constants.HEAD); Ref head = git.getRef(Constants.HEAD);

View File

@@ -24,7 +24,6 @@ import com.google.gerrit.server.patch.PatchList;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
import java.io.IOException; import java.io.IOException;
@@ -140,7 +139,7 @@ public class CommentSender extends ReplyToChangeSender {
private Repository getRepository() { private Repository getRepository() {
try { try {
return args.server.openRepository(projectState.getProject().getNameKey()); return args.server.openRepository(projectState.getProject().getNameKey());
} catch (RepositoryNotFoundException e) { } catch (IOException e) {
return null; return null;
} }
} }

View File

@@ -28,7 +28,6 @@ import com.google.inject.Inject;
import com.google.inject.Singleton; import com.google.inject.Singleton;
import org.eclipse.jgit.errors.MissingObjectException; import org.eclipse.jgit.errors.MissingObjectException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
@@ -83,7 +82,7 @@ public class PatchSetInfoFactory {
Repository repo; Repository repo;
try { try {
repo = repoManager.openRepository(change.getProject()); repo = repoManager.openRepository(change.getProject());
} catch (RepositoryNotFoundException e) { } catch (IOException e) {
throw new PatchSetInfoNotAvailableException(e); throw new PatchSetInfoNotAvailableException(e);
} }
try { try {

View File

@@ -130,10 +130,10 @@ public class CreateProject {
} finally { } finally {
repo.close(); repo.close();
} }
} catch (RepositoryNotFoundException doesNotExist) { } catch (IOException ioErr) {
final String msg = "Cannot create " + nameKey; final String msg = "Cannot create " + nameKey;
log.error(msg, err); log.error(msg, err);
throw new ProjectCreationFailedException(msg, err); throw new ProjectCreationFailedException(msg, ioErr);
} }
} catch (Exception e) { } catch (Exception e) {
final String msg = "Cannot create " + nameKey; final String msg = "Cannot create " + nameKey;

View File

@@ -158,9 +158,11 @@ class Schema_53 extends SchemaVersion {
// inheritable permissions. For example 'All-Projects'. // inheritable permissions. For example 'All-Projects'.
try { try {
git = mgr.createRepository(nameKey); git = mgr.createRepository(nameKey);
} catch (RepositoryNotFoundException err) { } catch (IOException err) {
throw new OrmException("Cannot create repository " + name, err); throw new OrmException("Cannot create repository " + name, err);
} }
} catch (IOException e) {
throw new OrmException(e);
} }
try { try {
MetaDataUpdate md = MetaDataUpdate md =

View File

@@ -33,7 +33,6 @@ import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
@@ -88,7 +87,7 @@ public class Schema_64 extends SchemaVersion {
Repository git; Repository git;
try { try {
git = mgr.openRepository(allProjects); git = mgr.openRepository(allProjects);
} catch (RepositoryNotFoundException e) { } catch (IOException e) {
throw new OrmException(e); throw new OrmException(e);
} }
try { try {

View File

@@ -43,8 +43,8 @@ import com.google.gwtorm.jdbc.JdbcSchema;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider; import com.google.inject.Provider;
import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.errors.ConfigInvalidException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.CommitBuilder; import org.eclipse.jgit.lib.CommitBuilder;
import org.eclipse.jgit.lib.PersonIdent; import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.lib.Repository;
@@ -87,7 +87,7 @@ public class Schema_65 extends SchemaVersion {
Repository git; Repository git;
try { try {
git = mgr.openRepository(allProjects); git = mgr.openRepository(allProjects);
} catch (RepositoryNotFoundException e) { } catch (IOException e) {
throw new OrmException(e); throw new OrmException(e);
} }
try { try {