opendev/gerrit
Code Issues Proposed changes

Refactor submit logic into gerrit-server

Browse Source 
This change moves logic that was duplicated over the ssh command and the rpc
handler into a Submit class in gerrit-server.

Change-Id: I70cf5aa04d091e5652d8bc3e5a254339a80bd6fd
This commit is contained in:
Conley Owens
2012-01-20 17:15:05 -08:00
committed by Edwin Kempin
parent c5635f2131
commit 6ada0d28c8
6 changed files with 286 additions and 246 deletions
Download Patch File Download Diff File Expand all files Collapse all files

38
gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java
View File

@@ -52,25 +52,59 @@ public class ReviewResult {
ABANDON_NOT_PERMITTED,
/** Not permitted to restore this change. */
RESTORE_NOT_PERMITTED
RESTORE_NOT_PERMITTED,
/** Not permitted to submit this change. */
SUBMIT_NOT_PERMITTED,
/** Approvals or dependencies are lacking for submission. */
SUBMIT_NOT_READY,
/** Review operation invalid because change is closed. */
CHANGE_IS_CLOSED,
/** Review operation not permitted by rule. */
RULE_ERROR
}
protected Type type;
protected String message;
protected Error() {
}
public Error(final Type type) {
this.type = type;
this.message = null;
}
public Error(final Type type, final String message) {
this.type = type;
this.message = message;
}
public Type getType() {
return type;
}
public String getMessage() {
return message;
}
public String getMessageOrType() {
if (message != null) {
return message;
}
return "" + type;
}
@Override
public String toString() {
return type + "";
String ret = type + "";
if (message != null) {
ret += " " + message;
}
return ret;
}
}
}

89
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java
View File

@@ -15,109 +15,48 @@
package com.google.gerrit.httpd.rpc.changedetail;
import com.google.gerrit.common.data.ChangeDetail;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.common.errors.NoSuchEntityException;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.PatchSet;
import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeQueue;
import com.google.gerrit.server.changedetail.Submit;
import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gwtorm.client.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.List;
class SubmitAction extends Handler<ChangeDetail> {
interface Factory {
SubmitAction create(PatchSet.Id patchSetId);
}
private final ReviewDb db;
private final MergeQueue merger;
private final IdentifiedUser user;
private final Submit.Factory submitFactory;
private final ChangeDetailFactory.Factory changeDetailFactory;
private final ChangeControl.Factory changeControlFactory;
private final MergeOp.Factory opFactory;
private final PatchSet.Id patchSetId;
@Inject
SubmitAction(final ReviewDb db, final MergeQueue mq,
final IdentifiedUser user,
SubmitAction(final Submit.Factory submitFactory,
final ChangeDetailFactory.Factory changeDetailFactory,
final ChangeControl.Factory changeControlFactory,
final MergeOp.Factory opFactory,
@Assisted final PatchSet.Id patchSetId) {
this.db = db;
this.merger = mq;
this.user = user;
this.changeControlFactory = changeControlFactory;
this.submitFactory = submitFactory;
this.changeDetailFactory = changeDetailFactory;
this.opFactory = opFactory;
this.patchSetId = patchSetId;
}
@Override
public ChangeDetail call() throws OrmException, NoSuchEntityException,
IllegalStateException, PatchSetInfoNotAvailableException,
NoSuchChangeException {
final Change.Id changeId = patchSetId.getParentKey();
final ChangeControl changeControl =
changeControlFactory.validateFor(changeId);
List<SubmitRecord> result = changeControl.canSubmit(db, patchSetId);
if (result.isEmpty()) {
throw new IllegalStateException("Cannot submit");
}
switch (result.get(0).status) {
case OK:
ChangeUtil.submit(patchSetId, user, db, opFactory, merger);
return changeDetailFactory.create(changeId).call();
case NOT_READY: {
for (SubmitRecord.Label lbl : result.get(0).labels) {
switch (lbl.status) {
case OK:
break;
case REJECT:
throw new IllegalStateException("Blocked by " + lbl.label);
case NEED:
throw new IllegalStateException("Needs " + lbl.label);
case IMPOSSIBLE:
throw new IllegalStateException("Cannnot submit, check project access");
default:
throw new IllegalArgumentException("Unknown status " + lbl.status);
}
}
throw new IllegalStateException("Cannot submit");
}
case CLOSED:
throw new IllegalStateException("Change is closed");
case RULE_ERROR:
if (result.get(0).errorMessage != null) {
throw new IllegalStateException(result.get(0).errorMessage);
} else {
throw new IllegalStateException("Internal rule error");
}
default:
throw new IllegalStateException("Uknown status " + result.get(0).status);
IllegalStateException, InvalidChangeOperationException,
PatchSetInfoNotAvailableException, NoSuchChangeException {
final ReviewResult result =
submitFactory.create(patchSetId).call();
if (result.getErrors().size() > 0) {
throw new IllegalStateException(
"Cannot submit " + result.getErrors().get(0).getMessageOrType());
}
return changeDetailFactory.create(result.getChangeId()).call();
}
}

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

@@ -14,8 +14,6 @@
package com.google.gerrit.server;
import static com.google.gerrit.reviewdb.ApprovalCategory.SUBMIT;
import com.google.gerrit.common.ChangeHooks;
import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.ChangeMessage;
@@ -29,7 +27,6 @@ import com.google.gerrit.server.config.TrackingFooter;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeQueue;
import com.google.gerrit.server.git.ReplicationQueue;
import com.google.gerrit.server.mail.EmailException;
import com.google.gerrit.server.mail.ReplyToChangeSender;
@@ -59,7 +56,6 @@ import org.eclipse.jgit.util.Base64;
import org.eclipse.jgit.util.NB;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
@@ -166,52 +162,6 @@ public class ChangeUtil {
opFactory.create(change.getDest()).verifyMergeability(change);
}
public static void submit(final PatchSet.Id patchSetId,
final IdentifiedUser user, final ReviewDb db,
final MergeOp.Factory opFactory, final MergeQueue merger)
throws OrmException {
final Change.Id changeId = patchSetId.getParentKey();
final PatchSetApproval approval = createSubmitApproval(patchSetId, user, db);
db.patchSetApprovals().upsert(Collections.singleton(approval));
final Change updatedChange = db.changes().atomicUpdate(changeId,
new AtomicUpdate<Change>() {
@Override
public Change update(Change change) {
if (change.getStatus() == Change.Status.NEW) {
change.setStatus(Change.Status.SUBMITTED);
ChangeUtil.updated(change);
}
return change;
}
});
if (updatedChange.getStatus() == Change.Status.SUBMITTED) {
merger.merge(opFactory, updatedChange.getDest());
}
}
public static PatchSetApproval createSubmitApproval(
final PatchSet.Id patchSetId, final IdentifiedUser user, final ReviewDb db
) throws OrmException {
final List<PatchSetApproval> allApprovals =
new ArrayList<PatchSetApproval>(db.patchSetApprovals().byPatchSet(
patchSetId).toList());
final PatchSetApproval.Key akey =
new PatchSetApproval.Key(patchSetId, user.getAccountId(), SUBMIT);
for (final PatchSetApproval approval : allApprovals) {
if (akey.equals(approval.getKey())) {
approval.setValue((short) 1);
approval.setGranted();
return approval;
}
}
return new PatchSetApproval(akey, (short) 1);
}
public static Change.Id revert(final PatchSet.Id patchSetId,
final IdentifiedUser user, final String message, final ReviewDb db,
final RevertedSender.Factory revertedSenderFactory,

189
gerrit-server/src/main/java/com/google/gerrit/server/changedetail/Submit.java Normal file
View File

@@ -0,0 +1,189 @@
// Copyright (C) 2012 The Android Open Source Project
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.server.changedetail;
import static com.google.gerrit.reviewdb.ApprovalCategory.SUBMIT;
import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.PatchSet;
import com.google.gerrit.reviewdb.PatchSetApproval;
import com.google.gerrit.reviewdb.ReviewDb;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeQueue;
import com.google.gerrit.server.project.InvalidChangeOperationException;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.project.ChangeControl;
import com.google.gwtorm.client.AtomicUpdate;
import com.google.gwtorm.client.OrmException;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.concurrent.Callable;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
public class Submit implements Callable<ReviewResult> {
public interface Factory {
Submit create(PatchSet.Id patchSetId);
}
private final ChangeControl.Factory changeControlFactory;
private final MergeOp.Factory opFactory;
private final MergeQueue merger;
private final ReviewDb db;
private final IdentifiedUser currentUser;
private final PatchSet.Id patchSetId;
@Inject
Submit(final ChangeControl.Factory changeControlFactory,
final MergeOp.Factory opFactory, final MergeQueue merger,
final ReviewDb db, final IdentifiedUser currentUser,
@Assisted final PatchSet.Id patchSetId) {
this.changeControlFactory = changeControlFactory;
this.opFactory = opFactory;
this.merger = merger;
this.db = db;
this.currentUser = currentUser;
this.patchSetId = patchSetId;
}
@Override
public ReviewResult call() throws IllegalStateException,
InvalidChangeOperationException, NoSuchChangeException, OrmException {
final ReviewResult result = new ReviewResult();
final PatchSet patch = db.patchSets().get(patchSetId);
final Change.Id changeId = patchSetId.getParentKey();
final ChangeControl control = changeControlFactory.validateFor(changeId);
result.setChangeId(changeId);
if (patch == null) {
throw new NoSuchChangeException(changeId);
}
List<SubmitRecord> submitResult = control.canSubmit(db, patchSetId);
if (submitResult.isEmpty()) {
throw new IllegalStateException(
"ChangeControl.canSubmit returned empty list");
}
for (SubmitRecord submitRecord : submitResult) {
switch (submitRecord.status) {
case OK:
if (!control.getRefControl().canSubmit()) {
result.addError(new ReviewResult.Error(
ReviewResult.Error.Type.SUBMIT_NOT_PERMITTED));
}
break;
case NOT_READY:
StringBuilder errMsg = new StringBuilder();
for (SubmitRecord.Label lbl : submitRecord.labels) {
switch (lbl.status) {
case OK:
break;
case REJECT:
if (errMsg.length() > 0) errMsg.append("; ");
errMsg.append("change " + changeId + ": blocked by "
+ lbl.label);
break;
case NEED:
if (errMsg.length() > 0) errMsg.append("; ");
errMsg.append("change " + changeId + ": needs " + lbl.label);
break;
case IMPOSSIBLE:
if (errMsg.length() > 0) errMsg.append("; ");
errMsg.append("change " + changeId + ": needs " + lbl.label
+ " (check project access)");
break;
default:
throw new IllegalArgumentException(
"Unsupported SubmitRecord.Label.status (" + lbl.status
+ ")");
}
}
result.addError(new ReviewResult.Error(
ReviewResult.Error.Type.SUBMIT_NOT_READY, errMsg.toString()));
break;
case CLOSED:
result.addError(new ReviewResult.Error(
ReviewResult.Error.Type.CHANGE_IS_CLOSED));
break;
case RULE_ERROR:
result.addError(new ReviewResult.Error(
ReviewResult.Error.Type.RULE_ERROR,
submitResult.get(0).errorMessage));
break;
default:
throw new IllegalStateException(
"Unsupported SubmitRecord.status + (" + submitRecord.status
+ ")");
}
}
// Submit the change if we can
if (result.getErrors().isEmpty()) {
final List<PatchSetApproval> allApprovals =
new ArrayList<PatchSetApproval>(db.patchSetApprovals().byPatchSet(
patchSetId).toList());
final PatchSetApproval.Key akey =
new PatchSetApproval.Key(patchSetId, currentUser.getAccountId(),
SUBMIT);
PatchSetApproval approval = new PatchSetApproval(akey, (short) 1);
for (final PatchSetApproval candidateApproval : allApprovals) {
if (akey.equals(candidateApproval.getKey())) {
candidateApproval.setValue((short) 1);
candidateApproval.setGranted();
approval = candidateApproval;
break;
}
}
db.patchSetApprovals().upsert(Collections.singleton(approval));
final Change updatedChange = db.changes().atomicUpdate(changeId,
new AtomicUpdate<Change>() {
@Override
public Change update(Change change) {
if (change.getStatus() == Change.Status.NEW) {
change.setStatus(Change.Status.SUBMITTED);
ChangeUtil.updated(change);
}
return change;
}
});
if (updatedChange.getStatus() == Change.Status.SUBMITTED) {
merger.merge(opFactory, updatedChange.getDest());
}
}
return result;
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java
View File

@@ -29,6 +29,7 @@ import com.google.gerrit.server.account.PerformRenameGroup;
import com.google.gerrit.server.account.VisibleGroups;
import com.google.gerrit.server.changedetail.AbandonChange;
import com.google.gerrit.server.changedetail.RestoreChange;
import com.google.gerrit.server.changedetail.Submit;
import com.google.gerrit.server.git.CreateCodeReviewNotes;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MetaDataUpdate;
@@ -101,5 +102,6 @@ public class GerritRequestModule extends FactoryModule {
factory(GroupDetailFactory.Factory.class);
factory(GroupMembers.Factory.class);
factory(CreateProject.Factory.class);
factory(Submit.Factory.class);
}
}

146
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java
View File

@@ -17,10 +17,8 @@ package com.google.gerrit.sshd.commands;
import com.google.gerrit.common.data.ApprovalType;
import com.google.gerrit.common.data.ApprovalTypes;
import com.google.gerrit.common.data.ReviewResult;
import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.ApprovalCategory;
import com.google.gerrit.reviewdb.ApprovalCategoryValue;
import com.google.gerrit.reviewdb.Branch;
import com.google.gerrit.reviewdb.Change;
import com.google.gerrit.reviewdb.PatchSet;
import com.google.gerrit.reviewdb.PatchSetApproval;
@@ -30,9 +28,8 @@ import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.changedetail.AbandonChange;
import com.google.gerrit.server.changedetail.RestoreChange;
import com.google.gerrit.server.changedetail.Submit;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeOp;
import com.google.gerrit.server.git.MergeQueue;
import com.google.gerrit.server.git.ReplicationQueue;
import com.google.gerrit.server.mail.EmailException;
import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -61,7 +58,6 @@ import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Set;
import java.util.concurrent.TimeUnit;
public class ReviewCommand extends BaseCommand {
private static final Logger log =
@@ -120,12 +116,6 @@ public class ReviewCommand extends BaseCommand {
@Inject
private IdentifiedUser currentUser;
@Inject
private MergeQueue merger;
@Inject
private MergeOp.Factory opFactory;
@Inject
private ApprovalTypes approvalTypes;
@@ -150,13 +140,14 @@ public class ReviewCommand extends BaseCommand {
@Inject
private ReplicationQueue replication;
@Inject
private Submit.Factory submitFactory;
@Inject
private PatchSetInfoFactory patchSetInfoFactory;
private List<ApproveOption> optionList;
private Set<PatchSet.Id> toSubmit = new HashSet<PatchSet.Id>();
@Override
public final void start(final Environment env) {
startThread(new CommandRunnable() {
@@ -210,36 +201,6 @@ public class ReviewCommand extends BaseCommand {
+ " review output above");
}
if (!toSubmit.isEmpty()) {
final Set<Branch.NameKey> toMerge = new HashSet<Branch.NameKey>();
try {
for (PatchSet.Id patchSetId : toSubmit) {
ChangeUtil.submit(patchSetId, currentUser, db, opFactory,
new MergeQueue() {
@Override
public void merge(MergeOp.Factory mof, Branch.NameKey branch) {
toMerge.add(branch);
}
@Override
public void schedule(Branch.NameKey branch) {
toMerge.add(branch);
}
@Override
public void recheckAfter(Branch.NameKey branch, long delay,
TimeUnit delayUnit) {
toMerge.add(branch);
}
});
}
for (Branch.NameKey branch : toMerge) {
merger.merge(opFactory, branch);
}
} catch (OrmException updateError) {
throw new Failure(1, "one or more submits failed", updateError);
}
}
}
});
}
@@ -249,7 +210,6 @@ public class ReviewCommand extends BaseCommand {
final Change.Id changeId = patchSetId.getParentKey();
ReviewResult result = null;
ChangeControl changeControl = changeControlFactory.validateFor(changeId);
if (changeComment == null) {
@@ -269,75 +229,24 @@ public class ReviewCommand extends BaseCommand {
publishCommentsFactory.create(patchSetId, changeComment, aps, forceMessage).call();
if (abandonChange) {
result = abandonChangeFactory.create(patchSetId, changeComment).call();
ReviewResult result = abandonChangeFactory.create(
patchSetId, changeComment).call();
handleReviewResultErrors(result);
} else if (restoreChange) {
result = restoreChangeFactory.create(patchSetId, changeComment).call();
if (submitChange) {
changeControl = changeControlFactory.validateFor(changeId);
ReviewResult result = restoreChangeFactory.create(
patchSetId, changeComment).call();
handleReviewResultErrors(result);
}
if (submitChange) {
ReviewResult result = submitFactory.create(patchSetId).call();
handleReviewResultErrors(result);
}
} catch (InvalidChangeOperationException e) {
throw error(e.getMessage());
} catch (IllegalStateException e) {
throw error(e.getMessage());
}
if (submitChange) {
List<SubmitRecord> submitResult = changeControl.canSubmit(db, patchSetId);
if (submitResult.isEmpty()) {
throw new Failure(1, "ChangeControl.canSubmit returned empty list");
}
switch (submitResult.get(0).status) {
case OK:
if (changeControl.getRefControl().canSubmit()) {
toSubmit.add(patchSetId);
} else {
throw error("change " + changeId + ": you do not have submit permission");
}
break;
case NOT_READY: {
StringBuilder msg = new StringBuilder();
for (SubmitRecord.Label lbl : submitResult.get(0).labels) {
switch (lbl.status) {
case OK:
break;
case REJECT:
if (msg.length() > 0) msg.append("\n");
msg.append("change " + changeId + ": blocked by " + lbl.label);
break;
case NEED:
if (msg.length() > 0) msg.append("\n");
msg.append("change " + changeId + ": needs " + lbl.label);
break;
case IMPOSSIBLE:
if (msg.length() > 0) msg.append("\n");
msg.append("change " + changeId + ": needs " + lbl.label
+ " (check project access)");
break;
default:
throw new Failure(1, "Unsupported label status " + lbl.status);
}
}
throw error(msg.toString());
}
case CLOSED:
throw error("change " + changeId + " is closed");
case RULE_ERROR:
if (submitResult.get(0).errorMessage != null) {
throw error("change " + changeId + ": " + submitResult.get(0).errorMessage);
} else {
throw error("change " + changeId + ": internal rule error");
}
default:
throw new Failure(1, "Unsupported status " + submitResult.get(0).status);
}
}
if (publishPatchSet) {
if (changeControl.isOwner() && changeControl.isVisible(db)) {
ChangeUtil.publishDraftPatchSet(db, patchSetId);
@@ -358,20 +267,37 @@ public class ReviewCommand extends BaseCommand {
throw error("Not permitted to delete draft patchset");
}
}
}
if (result != null) {
private void handleReviewResultErrors(final ReviewResult result) {
for (ReviewResult.Error resultError : result.getErrors()) {
String errMsg = "error: (change " + result.getChangeId() + ") ";
switch (resultError.getType()) {
case ABANDON_NOT_PERMITTED:
writeError("error: not permitted to abandon change");
errMsg += "not permitted to abandon change";
break;
case RESTORE_NOT_PERMITTED:
writeError("error: not permitted to restore change");
errMsg += "not permitted to restore change";
break;
case SUBMIT_NOT_PERMITTED:
errMsg += "not permitted to submit change";
break;
case SUBMIT_NOT_READY:
errMsg += "approvals or dependencies lacking";
break;
case CHANGE_IS_CLOSED:
errMsg += "change is closed";
break;
case RULE_ERROR:
errMsg += "rule error";
break;
default:
writeError("error: failure in review");
errMsg += "failure in review";
}
if (resultError.getMessage() != null) {
errMsg += ": " + resultError.getMessage();
}
writeError(errMsg);
}
}