Refactor PublishDraft into gerrit-server

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

Change-Id: I1569b8272fcf1e488e532332061e6224bc728cb8

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
     }

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.changedetail.PublishDraft;
-import com.google.gerrit.server.ChangeUtil;
 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<ChangeDetail> {
     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.publishFactory = publishFactory;
-    this.changeControlFactory = changeControlFactory;
     this.changeDetailFactory = changeDetailFactory;
 
     this.patchSetId = patchSetId;
@@ -55,16 +50,10 @@ class PublishAction extends Handler<ChangeDetail> {
   public ChangeDetail call() throws OrmException, NoSuchEntityException,
       IllegalStateException, PatchSetInfoNotAvailableException,
       NoSuchChangeException {
-
+    final ReviewResult result = publishFactory.create(patchSetId).call();
-    final Change.Id changeId = patchSetId.getParentKey();
+    if (result.getErrors().size() > 0) {
-    final ChangeControl changeControl =
-        changeControlFactory.validateFor(changeId);
-
-    if (!changeControl.canPublish(db)) {
       throw new IllegalStateException("Cannot publish patchset");
     }
-
+    return changeDetailFactory.create(result.getChangeId()).call();
-    ChangeUtil.publishDraftPatchSet(db, patchSetId);
-    return changeDetailFactory.create(changeId).call();
   }
 }

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<PatchSet>() {
-      @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<Change>() {
-        @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)

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<ReviewResult> {
+
+  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<PatchSet>() {
+        @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<Change>() {
+          @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;
+  }
+}

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);

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)) {
+      ReviewResult result = publishDraftFactory.create(patchSetId).call();
-        ChangeUtil.publishDraftPatchSet(db, patchSetId);
+      handleReviewResultErrors(result);
-      } else {
+    } else if (deleteDraftPatchSet) {
-        throw error("Not permitted to publish draft patchset");
-      }
-    }
-    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;