Migrate more callers to ChangeNotes.Factory#createChecked

Change-Id: I9e570251ca845d0c4a3edbc9dfb7e17b743824b5
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2016-02-23 11:44:30 +01:00
parent 7b0cd9d7d0
commit 9219e33eab
6 changed files with 54 additions and 38 deletions

View File

@@ -130,7 +130,7 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
private String findBaseRev(RevWalk rw, RevisionResource rsrc, private String findBaseRev(RevWalk rw, RevisionResource rsrc,
RebaseInput input) throws AuthException, ResourceConflictException, RebaseInput input) throws AuthException, ResourceConflictException,
OrmException, IOException { OrmException, IOException, NoSuchChangeException {
if (input == null || input.base == null) { if (input == null || input.base == null) {
return null; return null;
} }
@@ -194,7 +194,7 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
} }
private Base parseBase(RevisionResource rsrc, String base) private Base parseBase(RevisionResource rsrc, String base)
throws OrmException { throws OrmException, NoSuchChangeException {
ReviewDb db = dbProvider.get(); ReviewDb db = dbProvider.get();
// Try parsing the base as a ref string. // Try parsing the base as a ref string.
@@ -237,12 +237,12 @@ public class Rebase implements RestModifyView<RevisionResource, RebaseInput>,
} }
private ChangeControl controlFor(RevisionResource rsrc, Change.Id id) private ChangeControl controlFor(RevisionResource rsrc, Change.Id id)
throws OrmException { throws OrmException, NoSuchChangeException {
if (rsrc.getChange().getId().equals(id)) { if (rsrc.getChange().getId().equals(id)) {
return rsrc.getControl(); return rsrc.getControl();
} }
ChangeNotes notes = ChangeNotes notes =
notesFactory.create(dbProvider.get(), rsrc.getProject(), id); notesFactory.createChecked(dbProvider.get(), rsrc.getProject(), id);
return rsrc.getControl().getProjectControl().controlFor(notes); return rsrc.getControl().getProjectControl().controlFor(notes);
} }

View File

@@ -46,6 +46,7 @@ import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeSuperSet; import com.google.gerrit.server.git.MergeSuperSet;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ChangeControl; 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.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery; import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -203,13 +204,14 @@ public class Submit implements RestModifyView<RevisionResource, SubmitInput>,
try (MergeOp op = mergeOpProvider.get()) { try (MergeOp op = mergeOpProvider.get()) {
ReviewDb db = dbProvider.get(); ReviewDb db = dbProvider.get();
op.merge(db, change, caller, true, input); op.merge(db, change, caller, true, input);
try {
change = changeNotesFactory change = changeNotesFactory
.create(db, change.getProject(), change.getId()).getChange(); .createChecked(db, change.getProject(), change.getId()).getChange();
} } catch (NoSuchChangeException e) {
if (change == null) {
throw new ResourceConflictException("change is deleted"); throw new ResourceConflictException("change is deleted");
} }
}
switch (change.getStatus()) { switch (change.getStatus()) {
case MERGED: case MERGED:
return new Output(change); return new Output(change);

View File

@@ -37,6 +37,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.PatchSetUtil;
import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectId;
@@ -103,7 +104,8 @@ public class GroupCollector {
} }
private static interface Lookup { private static interface Lookup {
List<String> lookup(PatchSet.Id psId) throws OrmException; List<String> lookup(PatchSet.Id psId)
throws OrmException, NoSuchChangeException;
} }
private final Multimap<ObjectId, PatchSet.Id> patchSetsBySha; private final Multimap<ObjectId, PatchSet.Id> patchSetsBySha;
@@ -120,10 +122,11 @@ public class GroupCollector {
transformRefs(changeRefsById), transformRefs(changeRefsById),
new Lookup() { new Lookup() {
@Override @Override
public List<String> lookup(PatchSet.Id psId) throws OrmException { public List<String> lookup(PatchSet.Id psId)
throws OrmException, NoSuchChangeException {
// TODO(dborowitz): Reuse open repository from caller. // TODO(dborowitz): Reuse open repository from caller.
ChangeNotes notes = ChangeNotes notes =
notesFactory.create(db, project, psId.getParentKey()); notesFactory.createChecked(db, project, psId.getParentKey());
PatchSet ps = psUtil.get(db, notes, psId); PatchSet ps = psUtil.get(db, notes, psId);
return ps != null ? ps.getGroups() : null; return ps != null ? ps.getGroups() : null;
} }
@@ -239,7 +242,8 @@ public class GroupCollector {
} }
} }
public SortedSetMultimap<ObjectId, String> getGroups() throws OrmException { public SortedSetMultimap<ObjectId, String> getGroups()
throws OrmException, NoSuchChangeException {
done = true; done = true;
SortedSetMultimap<ObjectId, String> result = MultimapBuilder SortedSetMultimap<ObjectId, String> result = MultimapBuilder
.hashKeys(groups.keySet().size()) .hashKeys(groups.keySet().size())
@@ -270,7 +274,8 @@ public class GroupCollector {
} }
private Set<String> resolveGroups(ObjectId forCommit, private Set<String> resolveGroups(ObjectId forCommit,
Collection<String> candidates) throws OrmException { Collection<String> candidates)
throws OrmException, NoSuchChangeException {
Set<String> actual = Sets.newTreeSet(); Set<String> actual = Sets.newTreeSet();
Set<String> done = Sets.newHashSetWithExpectedSize(candidates.size()); Set<String> done = Sets.newHashSetWithExpectedSize(candidates.size());
Set<String> seen = Sets.newHashSetWithExpectedSize(candidates.size()); Set<String> seen = Sets.newHashSetWithExpectedSize(candidates.size());
@@ -307,7 +312,7 @@ public class GroupCollector {
} }
private Iterable<String> resolveGroup(ObjectId forCommit, String group) private Iterable<String> resolveGroup(ObjectId forCommit, String group)
throws OrmException { throws OrmException, NoSuchChangeException {
ObjectId id = parseGroup(forCommit, group); ObjectId id = parseGroup(forCommit, group);
if (id != null) { if (id != null) {
PatchSet.Id psId = Iterables.getFirst(patchSetsBySha.get(id), null); PatchSet.Id psId = Iterables.getFirst(patchSetsBySha.get(id), null);

View File

@@ -113,6 +113,7 @@ import com.google.gerrit.server.notedb.ChangeUpdate;
import com.google.gerrit.server.notedb.NotesMigration; import com.google.gerrit.server.notedb.NotesMigration;
import com.google.gerrit.server.patch.PatchSetInfoFactory; import com.google.gerrit.server.patch.PatchSetInfoFactory;
import com.google.gerrit.server.project.ChangeControl; 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.ProjectCache;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
@@ -1460,14 +1461,14 @@ public class ReceiveCommits {
final Change changeEnt; final Change changeEnt;
try { try {
changeEnt = changeEnt = notesFactory.createChecked(db, project.getNameKey(), changeId)
notesFactory.create(db, project.getNameKey(), changeId).getChange(); .getChange();
} catch (OrmException e) { } catch (OrmException e) {
log.error("Cannot lookup existing change " + changeId, e); log.error("Cannot lookup existing change " + changeId, e);
reject(cmd, "database error"); reject(cmd, "database error");
return; return;
} } catch (NoSuchChangeException e) {
if (changeEnt == null) { log.error("Change not found " + changeId, e);
reject(cmd, "change " + changeId + " not found"); reject(cmd, "change " + changeId + " not found");
return; return;
} }
@@ -1671,7 +1672,7 @@ public class ReceiveCommits {
for (UpdateGroupsRequest update : updateGroups) { for (UpdateGroupsRequest update : updateGroups) {
update.groups = ImmutableList.copyOf((groups.get(update.commit))); update.groups = ImmutableList.copyOf((groups.get(update.commit)));
} }
} catch (OrmException e) { } catch (OrmException | NoSuchChangeException e) {
log.error("Error collecting groups for changes", e); log.error("Error collecting groups for changes", e);
reject(magicBranch.cmd, "internal server error"); reject(magicBranch.cmd, "internal server error");
return; return;
@@ -1736,7 +1737,8 @@ public class ReceiveCommits {
requestScopePropagator.wrap(new Callable<Void>() { requestScopePropagator.wrap(new Callable<Void>() {
@Override @Override
public Void call() throws OrmException, RestApiException, public Void call() throws OrmException, RestApiException,
UpdateException, RepositoryNotFoundException, IOException { UpdateException, RepositoryNotFoundException, IOException,
NoSuchChangeException {
try (RequestState state = requestState(caller)) { try (RequestState state = requestState(caller)) {
insertChange(state); insertChange(state);
} }
@@ -1747,8 +1749,8 @@ public class ReceiveCommits {
return Futures.makeChecked(future, INSERT_EXCEPTION); return Futures.makeChecked(future, INSERT_EXCEPTION);
} }
private void insertChange(RequestState state) private void insertChange(RequestState state) throws OrmException,
throws OrmException, IOException, RestApiException, UpdateException { IOException, RestApiException, UpdateException, NoSuchChangeException {
RevCommit commit = state.rw.parseCommit(commitId); RevCommit commit = state.rw.parseCommit(commitId);
state.rw.parseBody(commit); state.rw.parseBody(commit);
final PatchSet.Id psId = ins.setGroups(groups).getPatchSetId(); final PatchSet.Id psId = ins.setGroups(groups).getPatchSetId();
@@ -1801,7 +1803,7 @@ public class ReceiveCommits {
} }
private void submit(ChangeControl changeCtl, PatchSet ps) private void submit(ChangeControl changeCtl, PatchSet ps)
throws OrmException, RestApiException { throws OrmException, RestApiException, NoSuchChangeException {
Submit submit = submitProvider.get(); Submit submit = submitProvider.get();
RevisionResource rsrc = new RevisionResource(changes.parse(changeCtl), ps); RevisionResource rsrc = new RevisionResource(changes.parse(changeCtl), ps);
try (MergeOp op = mergeOpProvider.get()) { try (MergeOp op = mergeOpProvider.get()) {
@@ -1810,7 +1812,8 @@ public class ReceiveCommits {
} }
addMessage(""); addMessage("");
Change c = notesFactory Change c = notesFactory
.create(db, project.getNameKey(), rsrc.getChange().getId()).getChange(); .createChecked(db, project.getNameKey(), rsrc.getChange().getId())
.getChange();
switch (c.getStatus()) { switch (c.getStatus()) {
case MERGED: case MERGED:
addMessage("Change " + c.getChangeId() + " merged."); addMessage("Change " + c.getChangeId() + " merged.");
@@ -2128,7 +2131,7 @@ public class ReceiveCommits {
requestScopePropagator.wrap(new Callable<PatchSet.Id>() { requestScopePropagator.wrap(new Callable<PatchSet.Id>() {
@Override @Override
public PatchSet.Id call() throws OrmException, IOException, public PatchSet.Id call() throws OrmException, IOException,
RestApiException, UpdateException { RestApiException, UpdateException, NoSuchChangeException {
try { try {
if (magicBranch != null && magicBranch.edit) { if (magicBranch != null && magicBranch.edit) {
return upsertEdit(); return upsertEdit();
@@ -2154,8 +2157,8 @@ public class ReceiveCommits {
return psId; return psId;
} }
PatchSet.Id insertPatchSet(RequestState state) PatchSet.Id insertPatchSet(RequestState state) throws OrmException,
throws OrmException, IOException, RestApiException, UpdateException { IOException, RestApiException, UpdateException, NoSuchChangeException {
RevCommit newCommit = state.rw.parseCommit(newCommitId); RevCommit newCommit = state.rw.parseCommit(newCommitId);
state.rw.parseBody(newCommit); state.rw.parseBody(newCommit);
@@ -2475,9 +2478,11 @@ public class ReceiveCommits {
String refName = cmd.getRefName(); String refName = cmd.getRefName();
Change.Id cid = psi.getParentKey(); Change.Id cid = psi.getParentKey();
Change change = Change change;
notesFactory.create(db, project.getNameKey(), cid).getChange(); try {
if (change == null) { change =
notesFactory.createChecked(db, project.getNameKey(), cid).getChange();
} catch (NoSuchChangeException e) {
log.warn(project.getName() + " change " + cid + " is missing"); log.warn(project.getName() + " change " + cid + " is missing");
return null; return null;
} }

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.QueueProvider.QueueType; import com.google.gerrit.server.git.QueueProvider.QueueType;
import com.google.gerrit.server.notedb.ChangeNotes; 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.query.change.InternalChangeQuery;
import com.google.gerrit.server.util.ManualRequestContext; import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext; import com.google.gerrit.server.util.OneOffRequestContext;
@@ -146,11 +147,12 @@ public class ReindexAfterUpdate implements GitReferenceUpdatedListener {
} }
@Override @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. // Reload change, as some time may have passed since GetChanges.
ReviewDb db = ctx.getReviewDbProvider().get(); ReviewDb db = ctx.getReviewDbProvider().get();
Change c = notesFactory Change c = notesFactory
.create(db, new Project.NameKey(event.getProjectName()), id) .createChecked(db, new Project.NameKey(event.getProjectName()), id)
.getChange(); .getChange();
indexerFactory.create(executor, indexes).index(db, c); indexerFactory.create(executor, indexes).index(db, c);
return null; return null;

View File

@@ -30,6 +30,7 @@ import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.GroupCollector; import com.google.gerrit.server.git.GroupCollector;
import com.google.gerrit.server.project.NoSuchChangeException;
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;
@@ -75,7 +76,7 @@ public class Schema_108 extends SchemaVersion {
try (Repository repo = repoManager.openRepository(e.getKey()); try (Repository repo = repoManager.openRepository(e.getKey());
RevWalk rw = new RevWalk(repo)) { RevWalk rw = new RevWalk(repo)) {
updateProjectGroups(db, repo, rw, (Set<Change.Id>) e.getValue(), ui); updateProjectGroups(db, repo, rw, (Set<Change.Id>) e.getValue(), ui);
} catch (IOException err) { } catch (IOException | NoSuchChangeException err) {
throw new OrmException(err); throw new OrmException(err);
} }
if (++i % 100 == 0) { if (++i % 100 == 0) {
@@ -85,9 +86,9 @@ public class Schema_108 extends SchemaVersion {
ui.message("done"); ui.message("done");
} }
private void updateProjectGroups(ReviewDb db, Repository repo, private void updateProjectGroups(ReviewDb db, Repository repo, RevWalk rw,
RevWalk rw, Set<Change.Id> changes, UpdateUI ui) Set<Change.Id> changes, UpdateUI ui)
throws OrmException, IOException { throws OrmException, IOException, NoSuchChangeException {
// Match sorting in ReceiveCommits. // Match sorting in ReceiveCommits.
rw.reset(); rw.reset();
rw.sort(RevSort.TOPO); rw.sort(RevSort.TOPO);
@@ -131,7 +132,8 @@ public class Schema_108 extends SchemaVersion {
} }
private static void updateGroups(ReviewDb db, GroupCollector collector, private static void updateGroups(ReviewDb db, GroupCollector collector,
Multimap<ObjectId, PatchSet.Id> patchSetsBySha) throws OrmException { Multimap<ObjectId, PatchSet.Id> patchSetsBySha)
throws OrmException, NoSuchChangeException {
Map<PatchSet.Id, PatchSet> patchSets = Map<PatchSet.Id, PatchSet> patchSets =
db.patchSets().toMap(db.patchSets().get(patchSetsBySha.values())); db.patchSets().toMap(db.patchSets().get(patchSetsBySha.values()));
for (Map.Entry<ObjectId, Collection<String>> e for (Map.Entry<ObjectId, Collection<String>> e