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
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 f01359b..d0e5154 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
@@ -27,10 +27,6 @@
public interface ChangeManageService extends RemoteJsonService {
@Audit
@SignInRequired
- void submit(PatchSet.Id patchSetId, AsyncCallback<ChangeDetail> callback);
-
- @Audit
- @SignInRequired
void createNewPatchSet(final PatchSet.Id patchSetId, final String newCommitMessage,
final AsyncCallback<ChangeDetail> callback);
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 eba1028..7db32ad 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 @@
}
}
+ /** Submit a specific revision of a change. */
+ public static void submit(int id, String commit, AsyncCallback<SubmitInfo> 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 cad27d5..97891bf 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.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 @@
@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<SubmitInfo>() {
+ 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 @@
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 8806f5b..97a33f2 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.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 @@
}
private void submit() {
- Util.MANAGE_SVC.submit(patchSetId,
- new GerritCallback<ChangeDetail>() {
- public void onSuccess(ChangeDetail result) {
+ ChangeApi.submit(patchSetId.getParentKey().get(), revision,
+ new GerritCallback<SubmitInfo>() {
+ 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 bedfb74..70bf4b6 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 0000000..a9206b7
--- /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 c80002d..53e0d9d 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.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<ChangeDetail> cb) {
- submitAction.create(patchSetId).to(cb);
- }
-
public void rebaseChange(final PatchSet.Id patchSetId,
final AsyncCallback<ChangeDetail> 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 e059cc3..48c13c2 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 @@
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 23b21d5..0000000
--- 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<ChangeDetail> {
- 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 bfbacc0..cbd1157 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 @@
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 c03a0e4..861dac7 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 @@
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 0000000..542d516
--- /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<RevisionResource, Input> {
+ 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<ReviewDb> dbProvider;
+ private final GitRepositoryManager repoManager;
+ private final MergeQueue mergeQueue;
+
+ @Inject
+ Submit(Provider<ReviewDb> dbProvider,
+ GitRepositoryManager repoManager,
+ MergeQueue mergeQueue) {
+ this.dbProvider = dbProvider;
+ this.repoManager = repoManager;
+ this.mergeQueue = mergeQueue;
+ }
+
+ @Override
+ public Class<Input> 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<ChangeMessage>() {
+ @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<Change>() {
+ @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<PatchSetApproval>() {
+ @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<SubmitRecord> results = rsrc.getControl().canSubmit(
+ dbProvider.get(),
+ rsrc.getPatchSet());
+ Optional<SubmitRecord> 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<SubmitRecord> findOkRecord(Collection<SubmitRecord> in) {
+ return Iterables.tryFind(in, new Predicate<SubmitRecord>() {
+ @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<ChangeResource, Input> {
+ private final Provider<ReviewDb> dbProvider;
+ private final Submit submit;
+ private final ChangeJson json;
+
+ @Inject
+ CurrentRevision(Provider<ReviewDb> dbProvider,
+ Submit submit,
+ ChangeJson json) {
+ this.dbProvider = dbProvider;
+ this.submit = submit;
+ this.json = json;
+ }
+
+ @Override
+ public Class<Input> 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 3287aa1..0000000
--- 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<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 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<SubmitRecord> 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<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;
- }
-}
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 71e4de7..f30259d 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.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 @@
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 90d51e2..12219e2 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 @@
}
@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 @@
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<Void>(){
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 1071768..a53b04c 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 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 644594e..7f66b51 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 @@
return canSubmit(db, patchSet, null, false, true);
}
+ public boolean canSubmit() {
+ return getRefControl().canSubmit();
+ }
+
public List<SubmitRecord> 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 4b6b8e4..396b307 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.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 @@
private Provider<Restore> restoreProvider;
@Inject
- private Submit.Factory submitFactory;
+ private Provider<Submit> submitProvider;
private List<ApproveOption> optionList;
@@ -242,8 +242,13 @@
}
}
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 @@
throw error(e.getMessage());
} catch (BadRequestException e) {
throw error(e.getMessage());
+ } catch (ResourceConflictException e) {
+ throw error(e.getMessage());
}
if (publishPatchSet) {
