From 68152ee280e5bf048edd8e546a72e23daae73564 Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 22 Jan 2013 10:08:31 +0100 Subject: [PATCH] Avoid loading patch sets multiple times from database in review command On parsing of the patch set ids we are already loading the patch sets from the database. Keep them so that they don't need to be loaded again during the command execution. Change-Id: I8a6238ae281f4f28694b8b325d1bd3bedd4099a1 Signed-off-by: Edwin Kempin --- .../gerrit/sshd/commands/ReviewCommand.java | 51 +++++++++---------- 1 file changed, 25 insertions(+), 26 deletions(-) diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java index 9f4fcb72a6..7960559dbf 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java @@ -55,7 +55,6 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -73,13 +72,13 @@ public class ReviewCommand extends SshCommand { return parser; } - private final Set patchSetIds = new HashSet(); + private final Set patchSets = new HashSet(); @Argument(index = 0, required = true, multiValued = true, metaVar = "{COMMIT | CHANGE,PATCHSET}", usage = "list of commits or patch sets to review") void addPatchSetId(final String token) { try { - patchSetIds.addAll(parsePatchSetId(token)); + patchSets.add(parsePatchSet(token)); } catch (UnloggedFailure e) { throw new IllegalArgumentException(e.getMessage(), e); } catch (OrmException e) { @@ -170,20 +169,20 @@ public class ReviewCommand extends SshCommand { } boolean ok = true; - for (final PatchSet.Id patchSetId : patchSetIds) { + for (final PatchSet patchSet : patchSets) { try { - approveOne(patchSetId); + approveOne(patchSet); } catch (UnloggedFailure e) { ok = false; writeError("error: " + e.getMessage() + "\n"); } catch (NoSuchChangeException e) { ok = false; - writeError("no such change " + patchSetId.getParentKey().get()); + writeError("no such change " + patchSet.getId().getParentKey().get()); } catch (Exception e) { ok = false; writeError("fatal: internal server error while approving " - + patchSetId + "\n"); - log.error("internal error while approving " + patchSetId, e); + + patchSet.getId() + "\n"); + log.error("internal error while approving " + patchSet.getId(), e); } } @@ -193,16 +192,15 @@ public class ReviewCommand extends SshCommand { } } - private void applyReview(final ChangeControl ctl, final PatchSet.Id patchSetId, + private void applyReview(final ChangeControl ctl, final PatchSet patchSet, final PostReview.Input review) throws Exception { if (!review.labels.isEmpty()) { reviewProvider.get().apply(new RevisionResource( - new ChangeResource(ctl), - db.patchSets().get(patchSetId)), review); + new ChangeResource(ctl), patchSet), review); } } - private void approveOne(final PatchSet.Id patchSetId) throws Exception { + private void approveOne(final PatchSet patchSet) throws Exception { if (changeComment == null) { changeComment = ""; @@ -229,13 +227,13 @@ public class ReviewCommand extends SshCommand { try { ChangeControl ctl = - changeControlFactory.controlFor(patchSetId.getParentKey()); + changeControlFactory.controlFor(patchSet.getId().getParentKey()); if (abandonChange) { final Abandon abandon = abandonProvider.get(); final Abandon.Input input = new Abandon.Input(); input.message = changeComment; - applyReview(ctl, patchSetId, review); + applyReview(ctl, patchSet, review); try { abandon.apply(new ChangeResource(ctl), input); } catch(AuthException e) { @@ -249,14 +247,14 @@ public class ReviewCommand extends SshCommand { input.message = changeComment; try { restore.apply(new ChangeResource(ctl), input); - applyReview(ctl, patchSetId, review); + applyReview(ctl, patchSet, review); } catch(AuthException e) { writeError("error: " + parseError(Type.RESTORE_NOT_PERMITTED) + "\n"); } catch(ResourceConflictException e) { writeError("error: " + parseError(Type.CHANGE_NOT_ABANDONED) + "\n"); } } else { - applyReview(ctl, patchSetId, review); + applyReview(ctl, patchSet, review); } if (submitChange) { @@ -264,8 +262,7 @@ public class ReviewCommand extends SshCommand { Submit.Input input = new Submit.Input(); input.waitForMerge = true; submit.apply(new RevisionResource( - new ChangeResource(ctl), - db.patchSets().get(patchSetId)), + new ChangeResource(ctl), patchSet), input); } } catch (InvalidChangeOperationException e) { @@ -281,11 +278,12 @@ public class ReviewCommand extends SshCommand { } if (publishPatchSet) { - final ReviewResult result = publishDraftFactory.create(patchSetId).call(); + final ReviewResult result = + publishDraftFactory.create(patchSet.getId()).call(); handleReviewResultErrors(result); } else if (deleteDraftPatchSet) { final ReviewResult result = - deleteDraftPatchSetFactory.create(patchSetId).call(); + deleteDraftPatchSetFactory.create(patchSet.getId()).call(); handleReviewResultErrors(result); } } @@ -332,7 +330,7 @@ public class ReviewCommand extends SshCommand { } } - private Set parsePatchSetId(final String patchIdentity) + private PatchSet parsePatchSet(final String patchIdentity) throws UnloggedFailure, OrmException { // By commit? // @@ -345,17 +343,17 @@ public class ReviewCommand extends SshCommand { patches = db.patchSets().byRevisionRange(id, id.max()); } - final Set matches = new HashSet(); + final Set matches = new HashSet(); for (final PatchSet ps : patches) { final Change change = db.changes().get(ps.getId().getParentKey()); if (inProject(change)) { - matches.add(ps.getId()); + matches.add(ps); } } switch (matches.size()) { case 1: - return matches; + return matches.iterator().next(); case 0: throw error("\"" + patchIdentity + "\" no such patch set"); default: @@ -372,7 +370,8 @@ public class ReviewCommand extends SshCommand { } catch (IllegalArgumentException e) { throw error("\"" + patchIdentity + "\" is not a valid patch set"); } - if (db.patchSets().get(patchSetId) == null) { + final PatchSet patchSet = db.patchSets().get(patchSetId); + if (patchSet == null) { throw error("\"" + patchIdentity + "\" no such patch set"); } if (projectControl != null) { @@ -382,7 +381,7 @@ public class ReviewCommand extends SshCommand { + projectControl.getProject().getName()); } } - return Collections.singleton(patchSetId); + return patchSet; } throw error("\"" + patchIdentity + "\" is not a valid patch set");