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 deb2253e0a..2330d340de 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,12 +43,14 @@ 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); delete(REVIEWER_KIND).to(DeleteReviewer.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) {