From 9219e33eabb8d01c9c06beee690ac0904ae23145 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 23 Feb 2016 11:44:30 +0100 Subject: [PATCH] Migrate more callers to ChangeNotes.Factory#createChecked Change-Id: I9e570251ca845d0c4a3edbc9dfb7e17b743824b5 Signed-off-by: Edwin Kempin --- .../google/gerrit/server/change/Rebase.java | 8 ++-- .../google/gerrit/server/change/Submit.java | 12 +++--- .../gerrit/server/git/GroupCollector.java | 17 ++++++--- .../gerrit/server/git/ReceiveCommits.java | 37 +++++++++++-------- .../server/index/ReindexAfterUpdate.java | 6 ++- .../gerrit/server/schema/Schema_108.java | 12 +++--- 6 files changed, 54 insertions(+), 38 deletions(-) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java index 940d9285b2..05311a598f 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Rebase.java @@ -130,7 +130,7 @@ public class Rebase implements RestModifyView, private String findBaseRev(RevWalk rw, RevisionResource rsrc, RebaseInput input) throws AuthException, ResourceConflictException, - OrmException, IOException { + OrmException, IOException, NoSuchChangeException { if (input == null || input.base == null) { return null; } @@ -194,7 +194,7 @@ public class Rebase implements RestModifyView, } private Base parseBase(RevisionResource rsrc, String base) - throws OrmException { + throws OrmException, NoSuchChangeException { ReviewDb db = dbProvider.get(); // Try parsing the base as a ref string. @@ -237,12 +237,12 @@ public class Rebase implements RestModifyView, } private ChangeControl controlFor(RevisionResource rsrc, Change.Id id) - throws OrmException { + throws OrmException, NoSuchChangeException { if (rsrc.getChange().getId().equals(id)) { return rsrc.getControl(); } ChangeNotes notes = - notesFactory.create(dbProvider.get(), rsrc.getProject(), id); + notesFactory.createChecked(dbProvider.get(), rsrc.getProject(), id); return rsrc.getControl().getProjectControl().controlFor(notes); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java index c0c570bc52..154f8d963d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java @@ -46,6 +46,7 @@ import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.git.MergeSuperSet; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gwtorm.server.OrmException; @@ -203,13 +204,14 @@ public class Submit implements RestModifyView, try (MergeOp op = mergeOpProvider.get()) { ReviewDb db = dbProvider.get(); op.merge(db, change, caller, true, input); - change = changeNotesFactory - .create(db, change.getProject(), change.getId()).getChange(); + try { + change = changeNotesFactory + .createChecked(db, change.getProject(), change.getId()).getChange(); + } catch (NoSuchChangeException e) { + throw new ResourceConflictException("change is deleted"); + } } - if (change == null) { - throw new ResourceConflictException("change is deleted"); - } switch (change.getStatus()) { case MERGED: return new Output(change); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java index 8d13f7e8ca..8f9add5708 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java @@ -37,6 +37,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.server.OrmException; import org.eclipse.jgit.lib.ObjectId; @@ -103,7 +104,8 @@ public class GroupCollector { } private static interface Lookup { - List lookup(PatchSet.Id psId) throws OrmException; + List lookup(PatchSet.Id psId) + throws OrmException, NoSuchChangeException; } private final Multimap patchSetsBySha; @@ -120,10 +122,11 @@ public class GroupCollector { transformRefs(changeRefsById), new Lookup() { @Override - public List lookup(PatchSet.Id psId) throws OrmException { + public List lookup(PatchSet.Id psId) + throws OrmException, NoSuchChangeException { // TODO(dborowitz): Reuse open repository from caller. ChangeNotes notes = - notesFactory.create(db, project, psId.getParentKey()); + notesFactory.createChecked(db, project, psId.getParentKey()); PatchSet ps = psUtil.get(db, notes, psId); return ps != null ? ps.getGroups() : null; } @@ -239,7 +242,8 @@ public class GroupCollector { } } - public SortedSetMultimap getGroups() throws OrmException { + public SortedSetMultimap getGroups() + throws OrmException, NoSuchChangeException { done = true; SortedSetMultimap result = MultimapBuilder .hashKeys(groups.keySet().size()) @@ -270,7 +274,8 @@ public class GroupCollector { } private Set resolveGroups(ObjectId forCommit, - Collection candidates) throws OrmException { + Collection candidates) + throws OrmException, NoSuchChangeException { Set actual = Sets.newTreeSet(); Set done = Sets.newHashSetWithExpectedSize(candidates.size()); Set seen = Sets.newHashSetWithExpectedSize(candidates.size()); @@ -307,7 +312,7 @@ public class GroupCollector { } private Iterable resolveGroup(ObjectId forCommit, String group) - throws OrmException { + throws OrmException, NoSuchChangeException { ObjectId id = parseGroup(forCommit, group); if (id != null) { PatchSet.Id psId = Iterables.getFirst(patchSetsBySha.get(id), null); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index bb1940e6bd..6b58f5d7d7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -113,6 +113,7 @@ import com.google.gerrit.server.notedb.ChangeUpdate; import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.project.ChangeControl; +import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectState; @@ -1460,14 +1461,14 @@ public class ReceiveCommits { final Change changeEnt; try { - changeEnt = - notesFactory.create(db, project.getNameKey(), changeId).getChange(); + changeEnt = notesFactory.createChecked(db, project.getNameKey(), changeId) + .getChange(); } catch (OrmException e) { log.error("Cannot lookup existing change " + changeId, e); reject(cmd, "database error"); return; - } - if (changeEnt == null) { + } catch (NoSuchChangeException e) { + log.error("Change not found " + changeId, e); reject(cmd, "change " + changeId + " not found"); return; } @@ -1671,7 +1672,7 @@ public class ReceiveCommits { for (UpdateGroupsRequest update : updateGroups) { update.groups = ImmutableList.copyOf((groups.get(update.commit))); } - } catch (OrmException e) { + } catch (OrmException | NoSuchChangeException e) { log.error("Error collecting groups for changes", e); reject(magicBranch.cmd, "internal server error"); return; @@ -1736,7 +1737,8 @@ public class ReceiveCommits { requestScopePropagator.wrap(new Callable() { @Override public Void call() throws OrmException, RestApiException, - UpdateException, RepositoryNotFoundException, IOException { + UpdateException, RepositoryNotFoundException, IOException, + NoSuchChangeException { try (RequestState state = requestState(caller)) { insertChange(state); } @@ -1747,8 +1749,8 @@ public class ReceiveCommits { return Futures.makeChecked(future, INSERT_EXCEPTION); } - private void insertChange(RequestState state) - throws OrmException, IOException, RestApiException, UpdateException { + private void insertChange(RequestState state) throws OrmException, + IOException, RestApiException, UpdateException, NoSuchChangeException { RevCommit commit = state.rw.parseCommit(commitId); state.rw.parseBody(commit); final PatchSet.Id psId = ins.setGroups(groups).getPatchSetId(); @@ -1801,7 +1803,7 @@ public class ReceiveCommits { } private void submit(ChangeControl changeCtl, PatchSet ps) - throws OrmException, RestApiException { + throws OrmException, RestApiException, NoSuchChangeException { Submit submit = submitProvider.get(); RevisionResource rsrc = new RevisionResource(changes.parse(changeCtl), ps); try (MergeOp op = mergeOpProvider.get()) { @@ -1810,7 +1812,8 @@ public class ReceiveCommits { } addMessage(""); Change c = notesFactory - .create(db, project.getNameKey(), rsrc.getChange().getId()).getChange(); + .createChecked(db, project.getNameKey(), rsrc.getChange().getId()) + .getChange(); switch (c.getStatus()) { case MERGED: addMessage("Change " + c.getChangeId() + " merged."); @@ -2128,7 +2131,7 @@ public class ReceiveCommits { requestScopePropagator.wrap(new Callable() { @Override public PatchSet.Id call() throws OrmException, IOException, - RestApiException, UpdateException { + RestApiException, UpdateException, NoSuchChangeException { try { if (magicBranch != null && magicBranch.edit) { return upsertEdit(); @@ -2154,8 +2157,8 @@ public class ReceiveCommits { return psId; } - PatchSet.Id insertPatchSet(RequestState state) - throws OrmException, IOException, RestApiException, UpdateException { + PatchSet.Id insertPatchSet(RequestState state) throws OrmException, + IOException, RestApiException, UpdateException, NoSuchChangeException { RevCommit newCommit = state.rw.parseCommit(newCommitId); state.rw.parseBody(newCommit); @@ -2475,9 +2478,11 @@ public class ReceiveCommits { String refName = cmd.getRefName(); Change.Id cid = psi.getParentKey(); - Change change = - notesFactory.create(db, project.getNameKey(), cid).getChange(); - if (change == null) { + Change change; + try { + change = + notesFactory.createChecked(db, project.getNameKey(), cid).getChange(); + } catch (NoSuchChangeException e) { log.warn(project.getName() + " change " + cid + " is missing"); return null; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java index 2897572ab5..c0f4cfc7a7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/index/ReindexAfterUpdate.java @@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.git.QueueProvider.QueueType; import com.google.gerrit.server.notedb.ChangeNotes; +import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.OneOffRequestContext; @@ -146,11 +147,12 @@ public class ReindexAfterUpdate implements GitReferenceUpdatedListener { } @Override - protected Void impl(RequestContext ctx) throws OrmException, IOException { + protected Void impl(RequestContext ctx) + throws OrmException, IOException, NoSuchChangeException { // Reload change, as some time may have passed since GetChanges. ReviewDb db = ctx.getReviewDbProvider().get(); Change c = notesFactory - .create(db, new Project.NameKey(event.getProjectName()), id) + .createChecked(db, new Project.NameKey(event.getProjectName()), id) .getChange(); indexerFactory.create(executor, indexes).index(db, c); return null; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_108.java b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_108.java index 5c45930479..946ddcfbce 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_108.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/schema/Schema_108.java @@ -30,6 +30,7 @@ import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GroupCollector; +import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; import com.google.inject.Provider; @@ -75,7 +76,7 @@ public class Schema_108 extends SchemaVersion { try (Repository repo = repoManager.openRepository(e.getKey()); RevWalk rw = new RevWalk(repo)) { updateProjectGroups(db, repo, rw, (Set) e.getValue(), ui); - } catch (IOException err) { + } catch (IOException | NoSuchChangeException err) { throw new OrmException(err); } if (++i % 100 == 0) { @@ -85,9 +86,9 @@ public class Schema_108 extends SchemaVersion { ui.message("done"); } - private void updateProjectGroups(ReviewDb db, Repository repo, - RevWalk rw, Set changes, UpdateUI ui) - throws OrmException, IOException { + private void updateProjectGroups(ReviewDb db, Repository repo, RevWalk rw, + Set changes, UpdateUI ui) + throws OrmException, IOException, NoSuchChangeException { // Match sorting in ReceiveCommits. rw.reset(); rw.sort(RevSort.TOPO); @@ -131,7 +132,8 @@ public class Schema_108 extends SchemaVersion { } private static void updateGroups(ReviewDb db, GroupCollector collector, - Multimap patchSetsBySha) throws OrmException { + Multimap patchSetsBySha) + throws OrmException, NoSuchChangeException { Map patchSets = db.patchSets().toMap(db.patchSets().get(patchSetsBySha.values())); for (Map.Entry> e