diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java index bf71e51d45..648b7ddcc9 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java @@ -63,6 +63,9 @@ public class ReviewResult { /** Review operation invalid because change is closed. */ CHANGE_IS_CLOSED, + /** Not permitted to publish this draft patch set */ + PUBLISH_NOT_PERMITTED, + /** Review operation not permitted by rule. */ RULE_ERROR } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java index bd0f40778a..f6241fa038 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/PublishAction.java @@ -15,14 +15,12 @@ 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.Change; import com.google.gerrit.reviewdb.PatchSet; -import com.google.gerrit.reviewdb.ReviewDb; -import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.changedetail.PublishDraft; import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException; -import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gwtorm.client.OrmException; import com.google.inject.Inject; @@ -33,19 +31,16 @@ class PublishAction extends Handler { PublishAction create(PatchSet.Id patchSetId); } - private final ReviewDb db; + private final PublishDraft.Factory publishFactory; private final ChangeDetailFactory.Factory changeDetailFactory; - private final ChangeControl.Factory changeControlFactory; private final PatchSet.Id patchSetId; @Inject - PublishAction(final ReviewDb db, + PublishAction(final PublishDraft.Factory publishFactory, final ChangeDetailFactory.Factory changeDetailFactory, - final ChangeControl.Factory changeControlFactory, @Assisted final PatchSet.Id patchSetId) { - this.db = db; - this.changeControlFactory = changeControlFactory; + this.publishFactory = publishFactory; this.changeDetailFactory = changeDetailFactory; this.patchSetId = patchSetId; @@ -55,16 +50,10 @@ class PublishAction extends Handler { public ChangeDetail call() throws OrmException, NoSuchEntityException, IllegalStateException, PatchSetInfoNotAvailableException, NoSuchChangeException { - - final Change.Id changeId = patchSetId.getParentKey(); - final ChangeControl changeControl = - changeControlFactory.validateFor(changeId); - - if (!changeControl.canPublish(db)) { + final ReviewResult result = publishFactory.create(patchSetId).call(); + if (result.getErrors().size() > 0) { throw new IllegalStateException("Cannot publish patchset"); } - - ChangeUtil.publishDraftPatchSet(db, patchSetId); - return changeDetailFactory.create(changeId).call(); + return changeDetailFactory.create(result.getChangeId()).call(); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java index b4ce6f5c43..0c17b256ba 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java @@ -35,7 +35,6 @@ import com.google.gerrit.server.patch.PatchSetInfoFactory; 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.client.AtomicUpdate; import com.google.gwtorm.client.OrmConcurrencyException; import com.google.gwtorm.client.OrmException; @@ -266,43 +265,6 @@ public class ChangeUtil { } } - public static void publishDraftPatchSet(final ReviewDb db, - final PatchSet.Id patchSetId) throws OrmException, NoSuchChangeException{ - final Change.Id changeId = patchSetId.getParentKey(); - final PatchSet patch = db.patchSets().get(patchSetId); - if (patch == null || !patch.isDraft()) { - throw new NoSuchChangeException(changeId); - } - - db.patchSets().atomicUpdate(patchSetId, new AtomicUpdate() { - @Override - public PatchSet update(PatchSet patchset) { - if (patchset.isDraft()) { - patchset.setDraft(false); - } - return null; - } - }); - - final Change change = db.changes().get(changeId); - if (change.getStatus() == Change.Status.DRAFT) { - db.changes().atomicUpdate(changeId, - new AtomicUpdate() { - @Override - public Change update(Change change) { - if (change.getStatus() == Change.Status.DRAFT - && change.currentPatchSetId().equals(patchSetId)) { - change.setStatus(Change.Status.NEW); - ChangeUtil.updated(change); - return change; - } else { - return null; - } - } - }); - } - } - public static void deleteDraftChange(final PatchSet.Id patchSetId, GitRepositoryManager gitManager, final ReplicationQueue replication, final ReviewDb db) diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PublishDraft.java b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PublishDraft.java new file mode 100644 index 0000000000..9f60ccbec6 --- /dev/null +++ b/gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PublishDraft.java @@ -0,0 +1,99 @@ +// Copyright (C) 2011 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 com.google.gerrit.common.data.ReviewResult; +import com.google.gerrit.reviewdb.Change; +import com.google.gerrit.reviewdb.PatchSet; +import com.google.gerrit.reviewdb.ReviewDb; +import com.google.gerrit.server.ChangeUtil; +import com.google.gerrit.server.project.NoSuchChangeException; +import com.google.gerrit.server.project.ChangeControl; +import com.google.gwtorm.client.AtomicUpdate; +import com.google.gwtorm.client.OrmException; +import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; + +import java.util.concurrent.Callable; + +public class PublishDraft implements Callable { + + public interface Factory { + PublishDraft create(PatchSet.Id patchSetId); + } + + private final ChangeControl.Factory changeControlFactory; + private final ReviewDb db; + + private final PatchSet.Id patchSetId; + + @Inject + PublishDraft(ChangeControl.Factory changeControlFactory, + ReviewDb db, @Assisted final PatchSet.Id patchSetId) { + this.changeControlFactory = changeControlFactory; + this.db = db; + + this.patchSetId = patchSetId; + } + + @Override + public ReviewResult call() throws NoSuchChangeException, OrmException { + final ReviewResult result = new ReviewResult(); + + final Change.Id changeId = patchSetId.getParentKey(); + result.setChangeId(changeId); + final ChangeControl control = changeControlFactory.validateFor(changeId); + final PatchSet patch = db.patchSets().get(patchSetId); + if (patch == null || !patch.isDraft()) { + throw new NoSuchChangeException(changeId); + } + + if (!control.canPublish(db)) { + result.addError(new ReviewResult.Error( + ReviewResult.Error.Type.PUBLISH_NOT_PERMITTED)); + } else { + db.patchSets().atomicUpdate(patchSetId, new AtomicUpdate() { + @Override + public PatchSet update(PatchSet patchset) { + if (patchset.isDraft()) { + patchset.setDraft(false); + } + return null; + } + }); + + final Change change = db.changes().get(changeId); + if (change.getStatus() == Change.Status.DRAFT) { + db.changes().atomicUpdate(changeId, + new AtomicUpdate() { + @Override + public Change update(Change change) { + if (change.getStatus() == Change.Status.DRAFT + && change.currentPatchSetId().equals(patchSetId)) { + change.setStatus(Change.Status.NEW); + ChangeUtil.updated(change); + return change; + } else { + return null; + } + } + }); + } + } + + 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 c232e3624e..9e95dfeeed 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,6 +28,7 @@ import com.google.gerrit.server.account.PerformCreateGroup; import com.google.gerrit.server.account.PerformRenameGroup; import com.google.gerrit.server.account.VisibleGroups; import com.google.gerrit.server.changedetail.AbandonChange; +import com.google.gerrit.server.changedetail.PublishDraft; import com.google.gerrit.server.changedetail.RestoreChange; import com.google.gerrit.server.changedetail.Submit; import com.google.gerrit.server.git.CreateCodeReviewNotes; @@ -88,6 +89,7 @@ public class GerritRequestModule extends FactoryModule { factory(AddReviewerSender.Factory.class); factory(CreateChangeSender.Factory.class); factory(PublishComments.Factory.class); + factory(PublishDraft.Factory.class); factory(ReplacePatchSetSender.Factory.class); factory(AbandonedSender.Factory.class); factory(RemoveReviewer.Factory.class); 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 7fdaf9fa57..96dc7cace5 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 @@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.ReviewDb; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.changedetail.AbandonChange; +import com.google.gerrit.server.changedetail.PublishDraft; import com.google.gerrit.server.changedetail.RestoreChange; import com.google.gerrit.server.changedetail.Submit; import com.google.gerrit.server.git.GitRepositoryManager; @@ -131,6 +132,9 @@ public class ReviewCommand extends BaseCommand { @Inject private PublishComments.Factory publishCommentsFactory; + @Inject + private PublishDraft.Factory publishDraftFactory; + @Inject private RestoreChange.Factory restoreChangeFactory; @@ -248,13 +252,9 @@ public class ReviewCommand extends BaseCommand { } if (publishPatchSet) { - if (changeControl.canPublish(db)) { - ChangeUtil.publishDraftPatchSet(db, patchSetId); - } else { - throw error("Not permitted to publish draft patchset"); - } - } - if (deleteDraftPatchSet) { + ReviewResult result = publishDraftFactory.create(patchSetId).call(); + handleReviewResultErrors(result); + } else if (deleteDraftPatchSet) { if (changeControl.isOwner() && changeControl.isVisible(db)) { try { ChangeUtil.deleteDraftPatchSet(patchSetId, gitManager, replication, patchSetInfoFactory, db); @@ -288,6 +288,9 @@ public class ReviewCommand extends BaseCommand { case CHANGE_IS_CLOSED: errMsg += "change is closed"; break; + case PUBLISH_NOT_PERMITTED: + errMsg += "not permitted to publish change"; + break; case RULE_ERROR: errMsg += "rule error"; break;