From bc61b54f297abe263351989a8411b6ab4a3774cc Mon Sep 17 00:00:00 2001 From: Martin Fick Date: Tue, 15 Jun 2010 12:09:36 -0700 Subject: [PATCH] review: Refactor to reuse same code as PublishComments web UI This way the two interfaces to the same action have the same behavior, and stay in sync with each other if any changes are made in the future. Change-Id: Iad6bedbc445fbd102a53e593787b32ce9bc3942e --- .../gerrit/sshd/commands/ReviewCommand.java | 125 ++++-------------- 1 file changed, 27 insertions(+), 98 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 54b7196be7..4bc3686463 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 @@ -14,13 +14,11 @@ package com.google.gerrit.sshd.commands; -import com.google.gerrit.common.ChangeHookRunner; import com.google.gerrit.common.data.ApprovalType; import com.google.gerrit.common.data.ApprovalTypes; import com.google.gerrit.reviewdb.ApprovalCategory; import com.google.gerrit.reviewdb.ApprovalCategoryValue; import com.google.gerrit.reviewdb.Change; -import com.google.gerrit.reviewdb.ChangeMessage; import com.google.gerrit.reviewdb.PatchSet; import com.google.gerrit.reviewdb.PatchSetApproval; import com.google.gerrit.reviewdb.RevId; @@ -28,10 +26,7 @@ import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.git.MergeQueue; -import com.google.gerrit.server.mail.CommentSender; -import com.google.gerrit.server.mail.EmailException; -import com.google.gerrit.server.patch.PatchSetInfoFactory; -import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; +import com.google.gerrit.server.patch.PublishComments; import com.google.gerrit.server.project.CanSubmitResult; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; @@ -52,10 +47,8 @@ import org.slf4j.LoggerFactory; import java.io.IOException; import java.util.ArrayList; import java.util.Collections; -import java.util.HashMap; import java.util.HashSet; import java.util.List; -import java.util.Map; import java.util.Set; public class ReviewCommand extends BaseCommand { @@ -102,12 +95,6 @@ public class ReviewCommand extends BaseCommand { @Inject private MergeQueue merger; - @Inject - private CommentSender.Factory commentSenderFactory; - - @Inject - private PatchSetInfoFactory patchSetInfoFactory; - @Inject private ApprovalTypes approvalTypes; @@ -117,11 +104,11 @@ public class ReviewCommand extends BaseCommand { @Inject private FunctionState.Factory functionStateFactory; - @Inject - private ChangeHookRunner hooks; - private List optionList; + @Inject + private PublishComments.Factory publishCommentsFactory; + @Override public final void start(final Environment env) { startThread(new CommandRunnable() { @@ -153,70 +140,26 @@ public class ReviewCommand extends BaseCommand { } private void approveOne(final PatchSet.Id patchSetId) - throws NoSuchChangeException, UnloggedFailure, OrmException, - PatchSetInfoNotAvailableException, EmailException { + throws NoSuchChangeException, UnloggedFailure, OrmException { + final Change.Id changeId = patchSetId.getParentKey(); final ChangeControl changeControl = changeControlFactory.validateFor(changeId); - final Change change = changeControl.getChange(); - final StringBuffer msgBuf = new StringBuffer(); - msgBuf.append("Patch Set "); - msgBuf.append(patchSetId.get()); - msgBuf.append(": "); + if (changeComment == null) { + changeComment = ""; + } - final Map approvalsMap = - new HashMap(); - - if (change.getStatus().isOpen()) { - for (ApproveOption co : optionList) { - final ApprovalCategory.Id category = co.getCategoryId(); - PatchSetApproval.Key psaKey = - new PatchSetApproval.Key(patchSetId, currentUser.getAccountId(), - category); - PatchSetApproval psa = db.patchSetApprovals().get(psaKey); - - Short score = co.value(); - - if (score != null) { - addApproval(psaKey, score, change, co); - } else { - if (psa == null) { - score = 0; - addApproval(psaKey, score, change, co); - } else { - score = psa.getValue(); - } - } - - final ApprovalCategoryValue.Id val = - new ApprovalCategoryValue.Id(category, score); - - String message = db.approvalCategoryValues().get(val).getName(); - msgBuf.append(" " + message + ";"); - approvalsMap.put(category, val); + Set aps = new HashSet(); + for (ApproveOption ao : optionList) { + Short v = ao.value(); + if (v != null) { + assertScoreIsAllowed(patchSetId, changeControl, ao, v); + aps.add(new ApprovalCategoryValue.Id(ao.getCategoryId(), v)); } } - msgBuf.deleteCharAt(msgBuf.length() - 1); - msgBuf.append("\n\n"); - - if (changeComment != null) { - msgBuf.append(changeComment); - } - - String uuid = ChangeUtil.messageUUID(db); - ChangeMessage cm = - new ChangeMessage(new ChangeMessage.Key(changeId, uuid), currentUser - .getAccountId()); - cm.setMessage(msgBuf.toString()); - db.changeMessages().insert(Collections.singleton(cm)); - - ChangeUtil.touch(change, db); - sendMail(change, change.currentPatchSetId(), cm); - - hooks.doCommentAddedHook(change, currentUser.getAccount(), db.patchSets() - .get(patchSetId), changeComment, approvalsMap); + publishCommentsFactory.create(patchSetId, changeComment, aps).call(); if (submitChange) { CanSubmitResult result = @@ -294,34 +237,20 @@ public class ReviewCommand extends BaseCommand { return projectControl.getProject().getNameKey().equals(change.getProject()); } - private void sendMail(final Change c, final PatchSet.Id psid, - final ChangeMessage message) throws PatchSetInfoNotAvailableException, - EmailException, OrmException { - PatchSet ps = db.patchSets().get(psid); - final CommentSender cm; - cm = commentSenderFactory.create(c); - cm.setFrom(currentUser.getAccountId()); - cm.setPatchSet(ps, patchSetInfoFactory.get(psid)); - cm.setChangeMessage(message); - cm.setReviewDb(db); - cm.send(); - } - - private void addApproval(final PatchSetApproval.Key psaKey, - final Short score, final Change c, final ApproveOption co) - throws OrmException, UnloggedFailure { - final PatchSetApproval psa = new PatchSetApproval(psaKey, score); - final List approvals = Collections.emptyList(); + private void assertScoreIsAllowed(final PatchSet.Id patchSetId, + final ChangeControl changeControl, ApproveOption ao, Short v) + throws UnloggedFailure { + final PatchSetApproval psa = + new PatchSetApproval(new PatchSetApproval.Key(patchSetId, currentUser + .getAccountId(), ao.getCategoryId()), v); final FunctionState fs = - functionStateFactory.create(c, psaKey.getParentKey(), approvals); - psa.setValue(score); + functionStateFactory.create(changeControl.getChange(), patchSetId, + Collections. emptyList()); + psa.setValue(v); fs.normalize(approvalTypes.getApprovalType(psa.getCategoryId()), psa); - if (score != psa.getValue()) { - throw error(co.name() + "=" + co.value() + " not permitted"); + if (v != psa.getValue()) { + throw error(ao.name() + "=" + ao.value() + " not permitted"); } - - psa.setGranted(); - db.patchSetApprovals().upsert(Collections.singleton(psa)); } private void initOptionList() {