opendev/gerrit
Code Issues Proposed changes

Propagate IOException when reading refs.

Browse Source 
Currently, Repository.getAllRefs() and Repository.getTags() silently
ignores an IOException and instead returns an empty map. Repository
is a public API in JGit and as such cannot be changed until the next
major revision change. Until then, update all call sites to use the
RefDatabase directly, since it propagates the error.

Change-Id: Ia3894a40fbc99482cbb4e1d6b3e4b69e5ddacba2
This commit is contained in:
Colby Ranger 2013-10-07 12:04:23 -07:00
parent 9c7aeb008f
commit 9e85d51493
15 changed files with 107 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java
View File

@ -56,6 +56,7 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.FooterLine;
@ -496,8 +497,9 @@ public class ChangeUtil {
return next;
}
public static PatchSet.Id nextPatchSetId(Repository git, PatchSet.Id id) {
return nextPatchSetId(git.getAllRefs(), id);
public static PatchSet.Id nextPatchSetId(Repository git, PatchSet.Id id)
throws IOException {
return nextPatchSetId(git.getRefDatabase().getRefs(RefDatabase.ALL), id);
}
private static PatchSet.Id nextPatchSetId(PatchSet.Id id) {

3
gerrit-server/src/main/java/com/google/gerrit/server/change/Mergeable.java
View File

@ -40,6 +40,7 @@ import org.eclipse.jgit.lib.AnyObjectId;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevFlag;
@ -98,7 +99,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
Repository git = gitManager.openRepository(change.getProject());
try {
Map<String, Ref> refs = git.getAllRefs();
Map<String, Ref> refs = git.getRefDatabase().getRefs(RefDatabase.ALL);
Ref ref = refs.get(change.getDest().get());
if (isStale(change, ref)) {
result.mergeable =

4
gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
View File

@ -163,7 +163,7 @@ public class PatchSetInserter {
return this;
}
public PatchSet.Id getPatchSetId() {
public PatchSet.Id getPatchSetId() throws IOException {
init();
return patchSet.getId();
}
@ -325,7 +325,7 @@ public class PatchSetInserter {
return updatedChange;
}
private void init() {
private void init() throws IOException {
if (sshInfo == null) {
sshInfo = new NoSshInfo();
}

12
gerrit-server/src/main/java/com/google/gerrit/server/git/MergeOp.java
View File

@ -19,6 +19,7 @@ import static java.util.concurrent.TimeUnit.HOURS;
import static java.util.concurrent.TimeUnit.MILLISECONDS;
import static java.util.concurrent.TimeUnit.MINUTES;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.common.base.Objects;
import com.google.common.collect.ArrayListMultimap;
@ -404,7 +405,7 @@ public class MergeOp {
}
try {
for (final Ref r : repo.getAllRefs().values()) {
for (final Ref r : repo.getRefDatabase().getRefs(ALL).values()) {
if (r.getName().startsWith(Constants.R_HEADS)) {
try {
alreadyAccepted.add(rw.parseCommit(r.getObjectId()));
@ -425,8 +426,15 @@ public class MergeOp {
final ListMultimap<SubmitType, Change> toSubmit =
ArrayListMultimap.create();
final Map<String, Ref> allRefs;
try {
allRefs = repo.getRefDatabase().getRefs(ALL);
} catch (IOException e) {
throw new MergeException(e.getMessage(), e);
}
final Set<ObjectId> tips = new HashSet<ObjectId>();
for (final Ref r : repo.getAllRefs().values()) {
for (final Ref r : allRefs.values()) {
tips.add(r.getObjectId());
}

17
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
View File

@ -18,6 +18,7 @@ import static com.google.gerrit.server.git.MultiProgressMonitor.UNKNOWN;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromApprovals;
import static com.google.gerrit.server.mail.MailUtil.getRecipientsFromFooters;
import static org.eclipse.jgit.lib.Constants.R_HEADS;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import static org.eclipse.jgit.transport.ReceiveCommand.Result.NOT_ATTEMPTED;
import static org.eclipse.jgit.transport.ReceiveCommand.Result.OK;
import static org.eclipse.jgit.transport.ReceiveCommand.Result.REJECTED_MISSING_OBJECT;
@ -110,6 +111,7 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectReader;
import org.eclipse.jgit.lib.PersonIdent;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.notes.NoteMap;
import org.eclipse.jgit.revwalk.FooterKey;
@ -123,6 +125,7 @@ import org.eclipse.jgit.transport.AdvertiseRefsHook;
import org.eclipse.jgit.transport.AdvertiseRefsHookChain;
import org.eclipse.jgit.transport.BaseReceivePack;
import org.eclipse.jgit.transport.ReceiveCommand;
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
import org.eclipse.jgit.transport.ReceiveCommand.Result;
import org.eclipse.jgit.transport.ReceivePack;
import org.eclipse.jgit.transport.UploadPack;
@ -395,10 +398,18 @@ public class ReceiveCommits {
List<AdvertiseRefsHook> advHooks = new ArrayList<AdvertiseRefsHook>(3);
advHooks.add(new AdvertiseRefsHook() {
@Override
public void advertiseRefs(BaseReceivePack rp) {
public void advertiseRefs(BaseReceivePack rp)
throws ServiceMayNotContinueException {
allRefs = rp.getAdvertisedRefs();
if (allRefs == null) {
allRefs = rp.getRepository().getAllRefs();
try {
allRefs = rp.getRepository().getRefDatabase().getRefs(ALL);
} catch (IOException e) {
ServiceMayNotContinueException ex =
new ServiceMayNotContinueException(e.getMessage());
ex.initCause(e);
throw ex;
}
}
rp.setAdvertisedRefs(allRefs, rp.getAdvertisedObjects());
}
@ -1975,7 +1986,7 @@ public class ReceiveCommits {
private Ref findMergedInto(final String first, final RevCommit commit) {
try {
final Map<String, Ref> all = repo.getAllRefs();
final Map<String, Ref> all = repo.getRefDatabase().getRefs(ALL);
Ref firstRef = all.get(first);
if (firstRef != null && isMergedInto(commit, firstRef)) {
return firstRef;

15
gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommitsAdvertiseRefsHook.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server.git;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.Change;
@ -30,6 +32,7 @@ import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.AdvertiseRefsHook;
import org.eclipse.jgit.transport.BaseReceivePack;
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
import org.eclipse.jgit.transport.UploadPack;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@ -59,10 +62,18 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
}
@Override
public void advertiseRefs(BaseReceivePack rp) {
public void advertiseRefs(BaseReceivePack rp)
throws ServiceMayNotContinueException {
Map<String, Ref> oldRefs = rp.getAdvertisedRefs();
if (oldRefs == null) {
oldRefs = rp.getRepository().getAllRefs();
try {
oldRefs = rp.getRepository().getRefDatabase().getRefs(ALL);
} catch (IOException e) {
ServiceMayNotContinueException ex =
new ServiceMayNotContinueException(e.getMessage());
ex.initCause(e);
throw ex;
}
}
Map<String, Ref> r = Maps.newHashMapWithExpectedSize(oldRefs.size());
for (Map.Entry<String, Ref> e : oldRefs.entrySet()) {

5
gerrit-server/src/main/java/com/google/gerrit/server/git/TagSet.java
View File

@ -26,6 +26,7 @@ import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectIdOwnerMap;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevSort;
@ -159,7 +160,7 @@ class TagSet {
TagWalk rw = new TagWalk(git);
rw.setRetainBody(false);
try {
for (Ref ref : git.getAllRefs().values()) {
for (Ref ref : git.getRefDatabase().getRefs(RefDatabase.ALL).values()) {
if (skip(ref)) {
continue;
@ -186,7 +187,7 @@ class TagSet {
}
}
} catch (IOException e) {
log.warn("Repository " + projectName + " has corruption", e);
log.warn("Error building tags for repository " + projectName, e);
} finally {
rw.release();
}

16
gerrit-server/src/main/java/com/google/gerrit/server/git/VisibleRefFilter.java
View File

@ -24,12 +24,15 @@ import com.google.gerrit.server.project.ProjectControl;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevWalk;
import org.eclipse.jgit.transport.AbstractAdvertiseRefsHook;
import org.eclipse.jgit.transport.ServiceMayNotContinueException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
@ -120,9 +123,16 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
}
@Override
protected Map<String, Ref> getAdvertisedRefs(
Repository repository, RevWalk revWalk) {
return filter(repository.getAllRefs());
protected Map<String, Ref> getAdvertisedRefs(Repository repository,
RevWalk revWalk) throws ServiceMayNotContinueException {
try {
return filter(repository.getRefDatabase().getRefs(RefDatabase.ALL));
} catch (IOException e) {
ServiceMayNotContinueException ex =
new ServiceMayNotContinueException(e.getMessage());
ex.initCause(e);
throw ex;
}
}
private Map<String, Ref> filter(Map<String, Ref> refs) {

5
gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeBatchIndexer.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server.index;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.common.base.Stopwatch;
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableList;
@ -44,6 +46,7 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.ObjectInserter;
import org.eclipse.jgit.lib.ProgressMonitor;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import org.eclipse.jgit.revwalk.RevCommit;
import org.eclipse.jgit.revwalk.RevObject;
@ -203,7 +206,7 @@ public class ChangeBatchIndexer {
Multimap<ObjectId, ChangeData> byId = ArrayListMultimap.create();
Repository repo = repoManager.openRepository(project);
try {
Map<String, Ref> refs = repo.getAllRefs();
Map<String, Ref> refs = repo.getRefDatabase().getRefs(ALL);
for (Change c : db.get().changes().byProject(project)) {
Ref r = refs.get(c.currentPatchSetId().toRefName());
if (r != null) {

4
gerrit-server/src/main/java/com/google/gerrit/server/project/CreateBranch.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server.project;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.common.errors.InvalidRevisionException;
import com.google.gerrit.extensions.restapi.AuthException;
@ -214,7 +216,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, Input> {
} catch (IncorrectObjectTypeException err) {
throw new InvalidRevisionException();
}
for (final Ref r : repo.getAllRefs().values()) {
for (final Ref r : repo.getRefDatabase().getRefs(ALL).values()) {
try {
rw.markUninteresting(rw.parseAny(r.getObjectId()));
} catch (MissingObjectException err) {

3
gerrit-server/src/main/java/com/google/gerrit/server/project/ListBranches.java
View File

@ -24,6 +24,7 @@ import com.google.inject.Inject;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.Repository;
import java.io.IOException;
@ -59,7 +60,7 @@ public class ListBranches implements RestReadView<ProjectResource> {
}
try {
final Map<String, Ref> all = db.getAllRefs();
final Map<String, Ref> all = db.getRefDatabase().getRefs(RefDatabase.ALL);
if (!all.containsKey(Constants.HEAD)) {
// The branch pointed to by HEAD doesn't exist yet, so getAllRefs

5
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.server.project;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.PageLinks;
@ -493,7 +495,8 @@ public class ProjectControl {
try {
Repository repo = repoManager.openRepository(projName);
try {
for (Entry<String, Ref> entry : repo.getAllRefs().entrySet()) {
Map<String, Ref> allRefs = repo.getRefDatabase().getRefs(ALL);
for (Entry<String, Ref> entry : allRefs.entrySet()) {
RevCommit tip;
try {
tip = rw.parseCommit(entry.getValue().getObjectId());

11
gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_56.java
View File

@ -24,6 +24,7 @@ import com.google.inject.Provider;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.lib.Constants;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.lib.RefUpdate;
import org.eclipse.jgit.lib.Repository;
@ -61,7 +62,15 @@ public class Schema_56 extends SchemaVersion {
continue;
}
try {
Map<String, Ref> all = git.getAllRefs();
Map<String, Ref> all;
try {
all = git.getRefDatabase().getRefs(RefDatabase.ALL);
} catch (IOException e) {
ui.message("warning: " + name.get() + ": Cannot read refs: "
+ e.getMessage());
e.printStackTrace();
continue;
}
if (all.keySet().equals(keysOne) || all.keySet().equals(keysTwo)) {
try {
RefUpdate update = git.updateRef(Constants.HEAD);

33
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/LsUserRefs.java
View File

@ -14,6 +14,8 @@
package com.google.gerrit.sshd.commands;
import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.reviewdb.client.Account;
@ -88,30 +90,33 @@ public class LsUserRefs extends SshCommand {
IdentifiedUser user = userFactory.create(userAccount.getId());
ProjectControl userProjectControl = projectControl.forUser(user);
Repository repo = null;
Repository repo;
try {
repo = repoManager.openRepository(userProjectControl.getProject()
.getNameKey());
Map<String, Ref> refsMap =
new VisibleRefFilter(tagCache, changeCache, repo, userProjectControl,
db, true).filter(repo.getAllRefs(), false);
for (final String ref : refsMap.keySet()) {
if (!onlyRefsHeads || ref.startsWith(Branch.R_HEADS)) {
stdout.println(ref);
}
}
} catch (RepositoryNotFoundException e) {
throw new UnloggedFailure("fatal: '"
+ projectControl.getProject().getNameKey() + "': not a git archive");
} catch (IOException e) {
throw new UnloggedFailure("fatal: Error opening: '"
+ projectControl.getProject().getNameKey());
} finally {
if (repo != null) {
repo.close();
}
try {
Map<String, Ref> refsMap =
new VisibleRefFilter(tagCache, changeCache, repo, userProjectControl,
db, true).filter(repo.getRefDatabase().getRefs(ALL), false);
for (final String ref : refsMap.keySet()) {
if (!onlyRefsHeads || ref.startsWith(Branch.R_HEADS)) {
stdout.println(ref);
}
}
} catch (IOException e) {
throw new Failure(1, "fatal: Error reading refs: '"
+ projectControl.getProject().getNameKey(), e);
} finally {
repo.close();
}
}
}

5
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java
View File

@ -28,6 +28,7 @@ import com.google.inject.Inject;
import org.eclipse.jgit.errors.TooLargeObjectInPackException;
import org.eclipse.jgit.errors.UnpackException;
import org.eclipse.jgit.lib.Ref;
import org.eclipse.jgit.lib.RefDatabase;
import org.eclipse.jgit.transport.AdvertiseRefsHook;
import org.eclipse.jgit.transport.ReceivePack;
import org.kohsuke.args4j.Option;
@ -141,8 +142,10 @@ final class Receive extends AbstractGitCommand {
+ ref.getName() + "\n");
}
Map<String, Ref> allRefs =
rp.getRepository().getRefDatabase().getRefs(RefDatabase.ALL);
List<Ref> hidden = new ArrayList<Ref>();
for (Ref ref : rp.getRepository().getAllRefs().values()) {
for (Ref ref : allRefs.values()) {
if (!adv.containsKey(ref.getName())) {
hidden.add(ref);
}