From 230343b94d0d69a89f9743f6f357e97de913f1df Mon Sep 17 00:00:00 2001 From: "Shawn O. Pearce" Date: Mon, 26 Nov 2012 17:00:33 -0800 Subject: [PATCH] Use /changes/{id}/revisions/{sha1}/submit to submit changes Replace the legacy JSON-RPC invocation with a new REST style API that names the patch set being submitted in the URI. Callers that don't care can use /changes/{id}/submit to submit whatever the current patch set is at the time the call starts at the server. The input is trivial, an optional boolean indicating if the caller wants to wait for the merge operation to execute now, or just have it schedule in the background to complete some time later. Web UI and SSH both set wait_for_merge true to match the old behavior. The logic to identify an error message written by the server is now part of the server rather than being buried in the client UI and is also now reported over SSH if there is an error. Change-Id: Ibade3bda3e716c789522da7ce14b284e07df08bc --- .../common/data/ChangeManageService.java | 4 - .../gerrit/client/changes/ChangeApi.java | 25 +- .../PatchSetComplexDisclosurePanel.java | 55 ++- .../client/changes/PublishCommentScreen.java | 15 +- .../client/changes/SubmitFailureDialog.java | 12 +- .../gerrit/client/changes/SubmitInfo.java | 29 ++ .../changedetail/ChangeManageServiceImpl.java | 9 +- .../httpd/rpc/changedetail/ChangeModule.java | 1 - .../httpd/rpc/changedetail/SubmitAction.java | 67 ---- .../google/gerrit/reviewdb/client/Change.java | 4 + .../google/gerrit/server/change/Module.java | 2 + .../google/gerrit/server/change/Submit.java | 328 ++++++++++++++++++ .../gerrit/server/changedetail/Submit.java | 207 ----------- .../server/config/GerritRequestModule.java | 2 - .../gerrit/server/git/ChangeMergeQueue.java | 14 +- .../google/gerrit/server/git/MergeQueue.java | 4 +- .../gerrit/server/project/ChangeControl.java | 4 + .../gerrit/sshd/commands/ReviewCommand.java | 15 +- 18 files changed, 448 insertions(+), 349 deletions(-) create mode 100644 gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/SubmitInfo.java delete mode 100644 gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java create mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java delete mode 100644 gerrit-server/src/main/java/com/google/gerrit/server/changedetail/Submit.java diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeManageService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeManageService.java index f01359bf1d..d0e5154776 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeManageService.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ChangeManageService.java @@ -25,10 +25,6 @@ import com.google.gwtjsonrpc.common.RpcImpl.Version; @RpcImpl(version = Version.V2_0) public interface ChangeManageService extends RemoteJsonService { - @Audit - @SignInRequired - void submit(PatchSet.Id patchSetId, AsyncCallback callback); - @Audit @SignInRequired void createNewPatchSet(final PatchSet.Id patchSetId, final String newCommitMessage, diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java index eba1028f74..7db32ad7d4 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/ChangeApi.java @@ -60,23 +60,46 @@ public class ChangeApi { } } + /** Submit a specific revision of a change. */ + public static void submit(int id, String commit, AsyncCallback cb) { + SubmitInput in = SubmitInput.create(); + in.wait_for_merge(true); + api(id, commit, "submit").data(in).post(cb); + } + private static class Input extends JavaScriptObject { final native void topic(String t) /*-{ if(t)this.topic=t; }-*/; final native void message(String m) /*-{ if(m)this.message=m; }-*/; static Input create() { - return (Input) JavaScriptObject.createObject(); + return (Input) createObject(); } protected Input() { } } + private static class SubmitInput extends JavaScriptObject { + final native void wait_for_merge(boolean b) /*-{ this.wait_for_merge=b; }-*/; + + static SubmitInput create() { + return (SubmitInput) createObject(); + } + + protected SubmitInput() { + } + } + private static RestApi api(int id, String action) { // TODO Switch to triplet project~branch~id format in URI. return new RestApi("/changes/" + id + "/" + action); } + private static RestApi api(int id, String commit, String action) { + // TODO Switch to triplet project~branch~id format in URI. + return new RestApi("/changes/" + id + "/revisions/" + commit + "/" + action); + } + public static String emptyToNull(String str) { return str == null || str.isEmpty() ? null : str; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java index cad27d5b3d..97891bf6e8 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PatchSetComplexDisclosurePanel.java @@ -30,13 +30,12 @@ import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.data.ChangeDetail; import com.google.gerrit.common.data.PatchSetDetail; import com.google.gerrit.reviewdb.client.AccountDiffPreference; +import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadCommand; +import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadScheme; import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSetInfo; import com.google.gerrit.reviewdb.client.UserIdentity; -import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadCommand; -import com.google.gerrit.reviewdb.client.AccountGeneralPreferences.DownloadScheme; import com.google.gwt.event.dom.client.ClickEvent; import com.google.gwt.event.dom.client.ClickHandler; import com.google.gwt.event.logical.shared.OpenEvent; @@ -308,11 +307,29 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel @Override public void onClick(final ClickEvent event) { b.setEnabled(false); - Util.MANAGE_SVC.submit(patchSet.getId(), - new ChangeDetailCache.GerritWidgetCallback(b) { - public void onSuccess(ChangeDetail result) { - onSubmitResult(result); - } + ChangeApi.submit( + patchSet.getId().getParentKey().get(), + patchSet.getRevision().get(), + new GerritCallback() { + public void onSuccess(SubmitInfo result) { + redisplay(); + } + + public void onFailure(Throwable err) { + if (SubmitFailureDialog.isConflict(err)) { + new SubmitFailureDialog(err.getMessage()).center(); + redisplay(); + } else { + b.setEnabled(true); + super.onFailure(err); + } + } + + private void redisplay() { + Gerrit.display( + PageLinks.toChange(patchSet.getId().getParentKey()), + new ChangeScreen(patchSet.getId().getParentKey())); + } }); } }); @@ -588,28 +605,6 @@ class PatchSetComplexDisclosurePanel extends ComplexDisclosurePanel Gerrit.RESOURCES.css().header()); } - private void onSubmitResult(final ChangeDetail result) { - if (result.getChange().getStatus() == Change.Status.NEW) { - // The submit failed. Try to locate the message and display - // it to the user, it should be the last one created by Gerrit. - // - ChangeMessage msg = null; - if (result.getMessages() != null && result.getMessages().size() > 0) { - for (int i = result.getMessages().size() - 1; i >= 0; i--) { - if (result.getMessages().get(i).getAuthor() == null) { - msg = result.getMessages().get(i); - break; - } - } - } - - if (msg != null) { - new SubmitFailureDialog(result, msg).center(); - } - } - detailCache.set(result); - } - public PatchSet getPatchSet() { return patchSet; } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java index 8806f5b030..97a33f2c4e 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/PublishCommentScreen.java @@ -26,7 +26,6 @@ import com.google.gerrit.client.ui.SmallHeading; import com.google.gerrit.common.PageLinks; import com.google.gerrit.common.data.ApprovalType; import com.google.gerrit.common.data.ApprovalTypes; -import com.google.gerrit.common.data.ChangeDetail; import com.google.gerrit.common.data.PatchSetPublishDetail; import com.google.gerrit.common.data.PermissionRange; import com.google.gerrit.reviewdb.client.ApprovalCategory; @@ -399,17 +398,21 @@ public class PublishCommentScreen extends AccountScreen implements } private void submit() { - Util.MANAGE_SVC.submit(patchSetId, - new GerritCallback() { - public void onSuccess(ChangeDetail result) { + ChangeApi.submit(patchSetId.getParentKey().get(), revision, + new GerritCallback() { + public void onSuccess(SubmitInfo result) { saveStateOnUnload = false; goChange(); } @Override - public void onFailure(Throwable caught) { + public void onFailure(Throwable err) { + if (SubmitFailureDialog.isConflict(err)) { + new SubmitFailureDialog(err.getMessage()).center(); + } else { + super.onFailure(err); + } goChange(); - super.onFailure(caught); } }); } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/SubmitFailureDialog.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/SubmitFailureDialog.java index bedfb74b2f..70bf4b6bbf 100644 --- a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/SubmitFailureDialog.java +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/SubmitFailureDialog.java @@ -15,13 +15,17 @@ package com.google.gerrit.client.changes; import com.google.gerrit.client.ErrorDialog; -import com.google.gerrit.common.data.ChangeDetail; -import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gwtexpui.safehtml.client.SafeHtmlBuilder; +import com.google.gwtjsonrpc.client.RemoteJsonException; class SubmitFailureDialog extends ErrorDialog { - SubmitFailureDialog(final ChangeDetail result, final ChangeMessage msg) { - super(new SafeHtmlBuilder().append(msg.getMessage().trim()).wikify()); + static boolean isConflict(Throwable err) { + return err instanceof RemoteJsonException + && 409 == ((RemoteJsonException) err).getCode(); + } + + SubmitFailureDialog(String msg) { + super(new SafeHtmlBuilder().append(msg.trim()).wikify()); setText(Util.C.submitFailed()); } } diff --git a/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/SubmitInfo.java b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/SubmitInfo.java new file mode 100644 index 0000000000..a9206b740f --- /dev/null +++ b/gerrit-gwtui/src/main/java/com/google/gerrit/client/changes/SubmitInfo.java @@ -0,0 +1,29 @@ +// 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.client.changes; + +import com.google.gerrit.reviewdb.client.Change; +import com.google.gwt.core.client.JavaScriptObject; + +class SubmitInfo extends JavaScriptObject { + final Change.Status status() { + return Change.Status.valueOf(statusRaw()); + } + + private final native String statusRaw() /*-{ return this.status; }-*/; + + protected SubmitInfo() { + } +} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java index c80002d617..53e0d9de87 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java @@ -23,30 +23,23 @@ import com.google.gwtjsonrpc.common.VoidResult; import com.google.inject.Inject; class ChangeManageServiceImpl implements ChangeManageService { - private final SubmitAction.Factory submitAction; private final RebaseChangeHandler.Factory rebaseChangeFactory; private final PublishAction.Factory publishAction; private final DeleteDraftChange.Factory deleteDraftChangeFactory; private final EditCommitMessageHandler.Factory editCommitMessageHandlerFactory; @Inject - ChangeManageServiceImpl(final SubmitAction.Factory patchSetAction, + ChangeManageServiceImpl( final RebaseChangeHandler.Factory rebaseChangeFactory, final PublishAction.Factory publishAction, final DeleteDraftChange.Factory deleteDraftChangeFactory, final EditCommitMessageHandler.Factory editCommitMessageHandler) { - this.submitAction = patchSetAction; this.rebaseChangeFactory = rebaseChangeFactory; this.publishAction = publishAction; this.deleteDraftChangeFactory = deleteDraftChangeFactory; this.editCommitMessageHandlerFactory = editCommitMessageHandler; } - public void submit(final PatchSet.Id patchSetId, - final AsyncCallback cb) { - submitAction.create(patchSetId).to(cb); - } - public void rebaseChange(final PatchSet.Id patchSetId, final AsyncCallback callback) { rebaseChangeFactory.create(patchSetId).to(callback); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java index e059cc3ebc..48c13c2698 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java @@ -34,7 +34,6 @@ public class ChangeModule extends RpcServletModule { factory(IncludedInDetailFactory.Factory.class); factory(PatchSetDetailFactory.Factory.class); factory(PatchSetPublishDetailFactory.Factory.class); - factory(SubmitAction.Factory.class); factory(PublishAction.Factory.class); factory(DeleteDraftChange.Factory.class); } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java deleted file mode 100644 index 23b21d5daf..0000000000 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/SubmitAction.java +++ /dev/null @@ -1,67 +0,0 @@ -// Copyright (C) 2009 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.httpd.rpc.changedetail; - -import com.google.gerrit.common.data.ChangeDetail; -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.client.PatchSet; -import com.google.gerrit.server.changedetail.Submit; -import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; -import com.google.gerrit.server.project.InvalidChangeOperationException; -import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; -import com.google.inject.assistedinject.Assisted; - -import org.eclipse.jgit.errors.RepositoryNotFoundException; - -import java.io.IOException; - -class SubmitAction extends Handler { - interface Factory { - SubmitAction create(PatchSet.Id patchSetId); - } - - private final Submit.Factory submitFactory; - private final ChangeDetailFactory.Factory changeDetailFactory; - - private final PatchSet.Id patchSetId; - - @Inject - SubmitAction(final Submit.Factory submitFactory, - final ChangeDetailFactory.Factory changeDetailFactory, - @Assisted final PatchSet.Id patchSetId) { - this.submitFactory = submitFactory; - this.changeDetailFactory = changeDetailFactory; - - this.patchSetId = patchSetId; - } - - @Override - public ChangeDetail call() throws OrmException, NoSuchEntityException, - IllegalStateException, InvalidChangeOperationException, - PatchSetInfoNotAvailableException, NoSuchChangeException, - RepositoryNotFoundException, IOException { - 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(); - } -} diff --git a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java index bfbacc0adb..cbd1157f2e 100644 --- a/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java +++ b/gerrit-reviewdb/src/main/java/com/google/gerrit/reviewdb/client/Change.java @@ -428,6 +428,10 @@ public final class Change { return lastUpdatedOn; } + public void setLastUpdatedOn(Timestamp now) { + lastUpdatedOn = now; + } + public void resetLastUpdatedOn() { lastUpdatedOn = new Timestamp(System.currentTimeMillis()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java index c03a0e4fbd..861dac72d7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Module.java @@ -43,11 +43,13 @@ public class Module extends RestApiModule { post(CHANGE_KIND, "restore").to(Restore.class); child(CHANGE_KIND, "reviewers").to(Reviewers.class); post(CHANGE_KIND, "revert").to(Revert.class); + post(CHANGE_KIND, "submit").to(Submit.CurrentRevision.class); get(REVIEWER_KIND).to(GetReviewer.class); child(CHANGE_KIND, "revisions").to(Revisions.class); post(REVISION_KIND, "review").to(PostReview.class); + post(REVISION_KIND, "submit").to(Submit.class); child(REVISION_KIND, "drafts").to(Drafts.class); put(REVISION_KIND, "drafts").to(CreateDraft.class); 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 new file mode 100644 index 0000000000..542d5165ed --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Submit.java @@ -0,0 +1,328 @@ +// 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.change; + +import static com.google.gerrit.common.data.SubmitRecord.Status.OK; + +import com.google.common.base.Optional; +import com.google.common.base.Predicate; +import com.google.common.collect.Iterables; +import com.google.common.collect.Lists; +import com.google.gerrit.common.data.SubmitRecord; +import com.google.gerrit.extensions.restapi.AuthException; +import com.google.gerrit.extensions.restapi.ResourceConflictException; +import com.google.gerrit.extensions.restapi.RestModifyView; +import com.google.gerrit.reviewdb.client.ApprovalCategory; +import com.google.gerrit.reviewdb.client.Change; +import com.google.gerrit.reviewdb.client.ChangeMessage; +import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.PatchSetApproval; +import com.google.gerrit.reviewdb.server.ReviewDb; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.ProjectUtil; +import com.google.gerrit.server.change.Submit.Input; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.gerrit.server.git.MergeQueue; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gwtorm.server.AtomicUpdate; +import com.google.gwtorm.server.OrmException; +import com.google.inject.Inject; +import com.google.inject.Provider; + +import org.eclipse.jgit.errors.RepositoryNotFoundException; + +import java.io.IOException; +import java.sql.Timestamp; +import java.util.Collection; +import java.util.Collections; +import java.util.List; + +public class Submit implements RestModifyView { + public static class Input { + public boolean waitForMerge; + } + + public enum Status { + SUBMITTED, MERGED; + } + + public static class Output { + public Status status; + transient Change change; + + private Output(Status s, Change c) { + status = s; + change = c; + } + } + + private final Provider dbProvider; + private final GitRepositoryManager repoManager; + private final MergeQueue mergeQueue; + + @Inject + Submit(Provider dbProvider, + GitRepositoryManager repoManager, + MergeQueue mergeQueue) { + this.dbProvider = dbProvider; + this.repoManager = repoManager; + this.mergeQueue = mergeQueue; + } + + @Override + public Class inputType() { + return Input.class; + } + + @Override + public Output apply(RevisionResource rsrc, Input input) throws AuthException, + ResourceConflictException, RepositoryNotFoundException, IOException, + OrmException { + ChangeControl control = rsrc.getControl(); + IdentifiedUser caller = (IdentifiedUser) control.getCurrentUser(); + Change change = rsrc.getChange(); + if (!control.canSubmit()) { + throw new AuthException("submit not permitted"); + } else if (!change.getStatus().isOpen()) { + throw new ResourceConflictException("change is " + status(change)); + } else if (!ProjectUtil.branchExists(repoManager, change.getDest())) { + throw new ResourceConflictException(String.format( + "destination branch \"%s\" not found.", + change.getDest().get())); + } else if (!rsrc.getPatchSet().getId().equals(change.currentPatchSetId())) { + // TODO Allow submitting non-current revision by changing the current. + throw new ResourceConflictException(String.format( + "revision %s is not current revision", + rsrc.getPatchSet().getRevision().get())); + } + + checkSubmitRule(rsrc); + change = submit(rsrc, caller); + + if (input.waitForMerge) { + mergeQueue.merge(change.getDest()); + change = dbProvider.get().changes().get(change.getId()); + } else { + mergeQueue.schedule(change.getDest()); + } + + if (change == null) { + throw new ResourceConflictException("change is deleted"); + } + switch (change.getStatus()) { + case SUBMITTED: + return new Output(Status.SUBMITTED, change); + case MERGED: + return new Output(Status.MERGED, change); + case NEW: + // If the merge was attempted and it failed the system usually + // writes a comment as a ChangeMessage and sets status to NEW. + // Find the relevant message and report that as the conflict. + final Timestamp before = rsrc.getChange().getLastUpdatedOn(); + ChangeMessage msg = Iterables.getFirst(Iterables.filter( + Lists.reverse(dbProvider.get().changeMessages() + .byChange(change.getId()) + .toList()), + new Predicate() { + @Override + public boolean apply(ChangeMessage input) { + return input.getAuthor() == null + && input.getWrittenOn().getTime() >= before.getTime(); + } + }), null); + if (msg != null) { + throw new ResourceConflictException(msg.getMessage()); + } + default: + throw new ResourceConflictException("change is " + status(change)); + } + } + + private Change submit(RevisionResource rsrc, IdentifiedUser caller) + throws OrmException, ResourceConflictException { + final Timestamp timestamp = new Timestamp(System.currentTimeMillis()); + Change change = rsrc.getChange(); + ReviewDb db = dbProvider.get(); + db.changes().beginTransaction(change.getId()); + try { + approve(rsrc.getPatchSet(), caller, timestamp); + change = db.changes().atomicUpdate( + change.getId(), + new AtomicUpdate() { + @Override + public Change update(Change change) { + if (change.getStatus().isOpen()) { + change.setStatus(Change.Status.SUBMITTED); + change.setLastUpdatedOn(timestamp); + ChangeUtil.computeSortKey(change); + return change; + } + return null; + } + }); + if (change == null) { + throw new ResourceConflictException("change is " + + status(db.changes().get(rsrc.getChange().getId()))); + } + db.commit(); + } finally { + db.rollback(); + } + return change; + } + + private void approve(PatchSet rev, IdentifiedUser caller, Timestamp timestamp) + throws OrmException { + PatchSetApproval submit = Iterables.getFirst(Iterables.filter( + dbProvider.get().patchSetApprovals() + .byPatchSetUser(rev.getId(), caller.getAccountId()), + new Predicate() { + @Override + public boolean apply(PatchSetApproval input) { + return ApprovalCategory.SUBMIT.equals(input.getCategoryId()); + } + }), null); + if (submit == null) { + submit = new PatchSetApproval( + new PatchSetApproval.Key( + rev.getId(), + caller.getAccountId(), + ApprovalCategory.SUBMIT), + (short) 1); + } + submit.setValue((short) 1); + submit.setGranted(timestamp); + dbProvider.get().patchSetApprovals().upsert(Collections.singleton(submit)); + } + + private void checkSubmitRule(RevisionResource rsrc) + throws ResourceConflictException { + List results = rsrc.getControl().canSubmit( + dbProvider.get(), + rsrc.getPatchSet()); + Optional ok = findOkRecord(results); + if (ok.isPresent()) { + // Rules supplied a valid solution. + return; + } else if (results.isEmpty()) { + throw new IllegalStateException(String.format( + "ChangeControl.canSubmit returned empty list for %s in %s", + rsrc.getPatchSet().getId(), + rsrc.getChange().getProject().get())); + } + + for (SubmitRecord record : results) { + switch (record.status) { + case CLOSED: + throw new ResourceConflictException("change is closed"); + + case RULE_ERROR: + throw new ResourceConflictException(String.format( + "rule error: %s", + record.errorMessage)); + + case NOT_READY: + StringBuilder msg = new StringBuilder(); + for (SubmitRecord.Label lbl : record.labels) { + switch (lbl.status) { + case OK: + case MAY: + continue; + + case REJECT: + if (msg.length() > 0) msg.append("; "); + msg.append("blocked by " + lbl.label); + continue; + + case NEED: + if (msg.length() > 0) msg.append("; "); + msg.append("needs " + lbl.label); + continue; + + case IMPOSSIBLE: + if (msg.length() > 0) msg.append("; "); + msg.append("needs " + lbl.label + " (check project access)"); + continue; + + default: + throw new IllegalStateException(String.format( + "Unsupported SubmitRecord.Label %s for %s in %s", + lbl.toString(), + rsrc.getPatchSet().getId(), + rsrc.getChange().getProject().get())); + } + } + throw new ResourceConflictException(msg.toString()); + + default: + throw new IllegalStateException(String.format( + "Unsupported SubmitRecord %s for %s in %s", + record, + rsrc.getPatchSet().getId(), + rsrc.getChange().getProject().get())); + } + } + } + + private static Optional findOkRecord(Collection in) { + return Iterables.tryFind(in, new Predicate() { + @Override + public boolean apply(SubmitRecord input) { + return input.status == OK; + } + }); + } + + private static String status(Change change) { + return change != null ? change.getStatus().name().toLowerCase() : "deleted"; + } + + public static class CurrentRevision implements + RestModifyView { + private final Provider dbProvider; + private final Submit submit; + private final ChangeJson json; + + @Inject + CurrentRevision(Provider dbProvider, + Submit submit, + ChangeJson json) { + this.dbProvider = dbProvider; + this.submit = submit; + this.json = json; + } + + @Override + public Class inputType() { + return Input.class; + } + + @Override + public Object apply(ChangeResource rsrc, Input input) throws AuthException, + ResourceConflictException, RepositoryNotFoundException, IOException, + OrmException { + PatchSet ps = dbProvider.get().patchSets() + .get(rsrc.getChange().currentPatchSetId()); + if (ps == null) { + throw new ResourceConflictException("current revision is missing"); + } else if (!rsrc.getControl().isPatchVisible(ps, dbProvider.get())) { + throw new AuthException("current revision not accessible"); + } + Output out = submit.apply(new RevisionResource(rsrc, ps), input); + return json.format(out.change); + } + } +} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/Submit.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/Submit.java deleted file mode 100644 index 3287aa1d77..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/Submit.java +++ /dev/null @@ -1,207 +0,0 @@ -// 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.client.ApprovalCategory.SUBMIT; - -import com.google.gerrit.common.data.ReviewResult; -import com.google.gerrit.common.data.SubmitRecord; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.PatchSetApproval; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.ChangeUtil; -import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.ProjectUtil; -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.project.ChangeControl; -import com.google.gerrit.server.project.InvalidChangeOperationException; -import com.google.gerrit.server.project.NoSuchChangeException; -import com.google.gwtorm.server.AtomicUpdate; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; -import com.google.inject.assistedinject.Assisted; - -import java.io.IOException; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; -import java.util.concurrent.Callable; - -public class Submit implements Callable { - - 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 GitRepositoryManager repoManager; - 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 GitRepositoryManager repoManager, - final IdentifiedUser currentUser, @Assisted final PatchSet.Id patchSetId) { - this.changeControlFactory = changeControlFactory; - this.opFactory = opFactory; - this.merger = merger; - this.db = db; - this.repoManager = repoManager; - this.currentUser = currentUser; - - this.patchSetId = patchSetId; - } - - @Override - public ReviewResult call() throws IllegalStateException, - InvalidChangeOperationException, NoSuchChangeException, OrmException, - IOException { - 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 submitResult = control.canSubmit(db, patch); - 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 MAY: - // The MAY label didn't cause the NOT_READY status - 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 - + ")"); - } - } - - if (!ProjectUtil.branchExists(repoManager, control.getChange().getDest())) { - result.addError(new ReviewResult.Error( - ReviewResult.Error.Type.DEST_BRANCH_NOT_FOUND, - "Destination branch \"" + control.getChange().getDest().get() - + "\" not found.")); - return result; - } - - // Submit the change if we can - if (result.getErrors().isEmpty()) { - final List allApprovals = - new ArrayList(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() { - @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; - } -} diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java index 71e4de765a..f30259d872 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java @@ -28,7 +28,6 @@ import com.google.gerrit.server.account.VisibleGroups; import com.google.gerrit.server.changedetail.DeleteDraftPatchSet; import com.google.gerrit.server.changedetail.PublishDraft; import com.google.gerrit.server.changedetail.RebaseChange; -import com.google.gerrit.server.changedetail.Submit; import com.google.gerrit.server.git.AsyncReceiveCommits; import com.google.gerrit.server.git.BanCommit; import com.google.gerrit.server.git.CreateCodeReviewNotes; @@ -92,7 +91,6 @@ public class GerritRequestModule extends FactoryModule { factory(GroupDetailFactory.Factory.class); factory(GroupMembers.Factory.class); factory(CreateProject.Factory.class); - factory(Submit.Factory.class); factory(SuggestParentCandidates.Factory.class); factory(BanCommit.Factory.class); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java index 90d51e224c..12219e23cf 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ChangeMergeQueue.java @@ -119,9 +119,9 @@ public class ChangeMergeQueue implements MergeQueue { } @Override - public void merge(MergeOp.Factory mof, Branch.NameKey branch) { + public void merge(Branch.NameKey branch) { if (start(branch)) { - mergeImpl(mof, branch); + mergeImpl(branch); } } @@ -199,16 +199,6 @@ public class ChangeMergeQueue implements MergeQueue { e.needMerge = false; } - private void mergeImpl(MergeOp.Factory opFactory, Branch.NameKey branch) { - try { - opFactory.create(branch).merge(); - } catch (Throwable e) { - log.error("Merge attempt for " + branch + " failed", e); - } finally { - finish(branch); - } - } - private void mergeImpl(final Branch.NameKey branch) { try { threadScoper.scope(new Callable(){ diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeQueue.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeQueue.java index 1071768c2b..a53b04c97b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeQueue.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/MergeQueue.java @@ -19,9 +19,7 @@ import com.google.gerrit.reviewdb.client.Branch; import java.util.concurrent.TimeUnit; public interface MergeQueue { - void merge(MergeOp.Factory mof, Branch.NameKey branch); - + void merge(Branch.NameKey branch); void schedule(Branch.NameKey branch); - void recheckAfter(Branch.NameKey branch, long delay, TimeUnit delayUnit); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java index 644594e795..7f66b51694 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java @@ -307,6 +307,10 @@ public class ChangeControl { return canSubmit(db, patchSet, null, false, true); } + public boolean canSubmit() { + return getRefControl().canSubmit(); + } + public List canSubmit(ReviewDb db, PatchSet patchSet) { return canSubmit(db, patchSet, null, false, false); } 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 4b6b8e44e0..396b307787 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 @@ -36,7 +36,7 @@ import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.change.Restore; import com.google.gerrit.server.changedetail.DeleteDraftPatchSet; import com.google.gerrit.server.changedetail.PublishDraft; -import com.google.gerrit.server.changedetail.Submit; +import com.google.gerrit.server.change.Submit; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.InvalidChangeOperationException; import com.google.gerrit.server.project.NoSuchChangeException; @@ -137,7 +137,7 @@ public class ReviewCommand extends SshCommand { private Provider restoreProvider; @Inject - private Submit.Factory submitFactory; + private Provider submitProvider; private List optionList; @@ -242,8 +242,13 @@ public class ReviewCommand extends SshCommand { } } if (submitChange) { - final ReviewResult result = submitFactory.create(patchSetId).call(); - handleReviewResultErrors(result); + Submit submit = submitProvider.get(); + Submit.Input input = new Submit.Input(); + input.waitForMerge = true; + submit.apply(new RevisionResource( + new ChangeResource(ctl), + db.patchSets().get(patchSetId)), + input); } } catch (InvalidChangeOperationException e) { throw error(e.getMessage()); @@ -253,6 +258,8 @@ public class ReviewCommand extends SshCommand { throw error(e.getMessage()); } catch (BadRequestException e) { throw error(e.getMessage()); + } catch (ResourceConflictException e) { + throw error(e.getMessage()); } if (publishPatchSet) {