Delete old PublishDraft

The only remaining caller was ReviewCommand; use the new-style API and
clean that up as well.

Change-Id: I2ee77c11ffada28b0edc7fba14f23d5d1b4313f6

gerrit-extension-api/src/main/java/com/google/gerrit/extensions/api/changes/RevisionApi.java

@@ -23,6 +23,7 @@ public interface RevisionApi {
   /** {@code submit} with {@link SubmitInput#waitForMerge} set to true. */
   void submit() throws RestApiException;
   void submit(SubmitInput in) throws RestApiException;
+  void publish() throws RestApiException;
   ChangeApi cherryPick(CherryPickInput in) throws RestApiException;
   ChangeApi rebase() throws RestApiException;
 }

gerrit-server/src/main/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java

@@ -25,6 +25,7 @@ import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.server.change.CherryPick;
 import com.google.gerrit.server.change.DeleteDraftPatchSet;
 import com.google.gerrit.server.change.PostReview;
+import com.google.gerrit.server.change.Publish;
 import com.google.gerrit.server.change.Rebase;
 import com.google.gerrit.server.change.RevisionResource;
 import com.google.gerrit.server.change.Submit;
@@ -46,6 +47,7 @@ class RevisionApiImpl implements RevisionApi {
   private final Provider<Rebase> rebase;
   private final Provider<PostReview> review;
   private final Provider<Submit> submit;
+  private final Provider<Publish> publish;
   private final RevisionResource revision;
 
   @Inject
@@ -55,6 +57,7 @@ class RevisionApiImpl implements RevisionApi {
       Provider<Rebase> rebase,
       Provider<PostReview> review,
       Provider<Submit> submit,
+      Provider<Publish> publish,
       @Assisted RevisionResource r) {
     this.changes = changes;
     this.cherryPick = cherryPick;
@@ -62,6 +65,7 @@ class RevisionApiImpl implements RevisionApi {
     this.rebase = rebase;
     this.review = review;
     this.submit = submit;
+    this.publish = publish;
     this.revision = r;
   }
 
@@ -90,6 +94,15 @@ class RevisionApiImpl implements RevisionApi {
     }
   }
 
+  @Override
+  public void publish() throws RestApiException {
+    try {
+      publish.get().apply(revision, new Publish.Input());
+    } catch (OrmException | IOException e) {
+      throw new RestApiException("Cannot publish draft patch set", e);
+    }
+  }
+
   @Override
   public void delete() throws RestApiException {
     try {

gerrit-server/src/main/java/com/google/gerrit/server/changedetail/PublishDraft.java

@@ -1,137 +0,0 @@
-// 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.common.util.concurrent.CheckedFuture;
-import com.google.gerrit.common.ChangeHooks;
-import com.google.gerrit.common.data.LabelTypes;
-import com.google.gerrit.common.data.ReviewResult;
-import com.google.gerrit.reviewdb.client.Change;
-import com.google.gerrit.reviewdb.client.PatchSet;
-import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.ApprovalsUtil;
-import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.account.AccountResolver;
-import com.google.gerrit.server.git.GitRepositoryManager;
-import com.google.gerrit.server.index.ChangeIndexer;
-import com.google.gerrit.server.mail.CreateChangeSender;
-import com.google.gerrit.server.mail.PatchSetNotificationSender;
-import com.google.gerrit.server.mail.ReplacePatchSetSender;
-import com.google.gerrit.server.patch.PatchSetInfoFactory;
-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.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.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 ChangeHooks hooks;
-  private final ChangeIndexer indexer;
-  private final PatchSetNotificationSender sender;
-
-  private final PatchSet.Id patchSetId;
-
-  @Inject
-  PublishDraft(final ChangeControl.Factory changeControlFactory,
-      final ReviewDb db, final ChangeHooks hooks,
-      final GitRepositoryManager repoManager,
-      final PatchSetInfoFactory patchSetInfoFactory,
-      final ApprovalsUtil approvalsUtil,
-      final AccountResolver accountResolver,
-      final CreateChangeSender.Factory createChangeSenderFactory,
-      final ReplacePatchSetSender.Factory replacePatchSetFactory,
-      final ChangeIndexer indexer,
-      final PatchSetNotificationSender sender,
-      @Assisted final PatchSet.Id patchSetId) {
-    this.changeControlFactory = changeControlFactory;
-    this.db = db;
-    this.hooks = hooks;
-    this.indexer = indexer;
-    this.sender = sender;
-
-    this.patchSetId = patchSetId;
-  }
-
-  @Override
-  public ReviewResult call() throws NoSuchChangeException, OrmException,
-      IOException, PatchSetInfoNotAvailableException {
-    final ReviewResult result = new ReviewResult();
-
-    final Change.Id changeId = patchSetId.getParentKey();
-    result.setChangeId(changeId);
-    final ChangeControl control = changeControlFactory.validateFor(changeId);
-    final LabelTypes labelTypes = control.getLabelTypes();
-    final PatchSet patch = db.patchSets().get(patchSetId);
-    if (patch == null) {
-      throw new NoSuchChangeException(changeId);
-    }
-    if (!patch.isDraft()) {
-      result.addError(new ReviewResult.Error(
-          ReviewResult.Error.Type.NOT_A_DRAFT));
-      return result;
-    }
-
-    if (!control.canPublish(db)) {
-      result.addError(new ReviewResult.Error(
-          ReviewResult.Error.Type.PUBLISH_NOT_PERMITTED));
-    } else {
-      final PatchSet updatedPatchSet = db.patchSets().atomicUpdate(patchSetId,
-          new AtomicUpdate<PatchSet>() {
-        @Override
-        public PatchSet update(PatchSet patchset) {
-          patchset.setDraft(false);
-          return patchset;
-        }
-      });
-
-      final Change updatedChange = db.changes().atomicUpdate(changeId,
-          new AtomicUpdate<Change>() {
-        @Override
-        public Change update(Change change) {
-          if (change.getStatus() == Change.Status.DRAFT) {
-            change.setStatus(Change.Status.NEW);
-            ChangeUtil.updated(change);
-          }
-          return change;
-        }
-      });
-
-      if (!updatedPatchSet.isDraft() || updatedChange.getStatus() == Change.Status.NEW) {
-        CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(updatedChange);
-        hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, db);
-
-        sender.send(control.getChange().getStatus() == Change.Status.DRAFT,
-            (IdentifiedUser) control.getCurrentUser(), updatedChange, updatedPatchSet,
-            labelTypes);
-        indexFuture.checkedGet();
-      }
-    }
-
-    return result;
-  }
-}

gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java

@@ -18,7 +18,6 @@ import static com.google.inject.Scopes.SINGLETON;
 
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.RequestCleanup;
-import com.google.gerrit.server.changedetail.PublishDraft;
 import com.google.gerrit.server.git.BanCommit;
 import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.SubmoduleOp;
@@ -47,7 +46,6 @@ public class GerritRequestModule extends FactoryModule {
     // Not really per-request, but dammit, I don't know where else to
     // easily park this stuff.
     //
-    factory(PublishDraft.Factory.class);
     factory(RemoveReviewer.Factory.class);
     factory(SuggestParentCandidates.Factory.class);
     factory(BanCommit.Factory.class);

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ReviewCommand.java

@@ -18,13 +18,13 @@ import com.google.common.base.Strings;
 import com.google.common.collect.Maps;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.LabelValue;
-import com.google.gerrit.common.data.ReviewResult;
-import com.google.gerrit.common.data.ReviewResult.Error.Type;
 import com.google.gerrit.extensions.api.GerritApi;
 import com.google.gerrit.extensions.api.changes.AbandonInput;
+import com.google.gerrit.extensions.api.changes.ChangeApi;
 import com.google.gerrit.extensions.api.changes.RestoreInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput;
 import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
+import com.google.gerrit.extensions.api.changes.RevisionApi;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -33,7 +33,6 @@ import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.RevId;
 import com.google.gerrit.reviewdb.server.ReviewDb;
-import com.google.gerrit.server.changedetail.PublishDraft;
 import com.google.gerrit.server.config.AllProjectsName;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.NoSuchChangeException;
@@ -132,9 +131,6 @@ public class ReviewCommand extends SshCommand {
   @Inject
   private Provider<GerritApi> gApi;
 
-  @Inject
-  private PublishDraft.Factory publishDraftFactory;
-
   private List<ApproveOption> optionList;
   private Map<String, Short> customLabels;
 
@@ -230,48 +226,24 @@ public class ReviewCommand extends SshCommand {
         AbandonInput input = new AbandonInput();
         input.message = changeComment;
         applyReview(patchSet, review);
-        try {
+        changeApi(patchSet).abandon(input);
-          gApi.get().changes()
-              .id(patchSet.getId().getParentKey().get())
-              .abandon(input);
-        } catch (AuthException e) {
-          writeError("error: " + parseError(Type.ABANDON_NOT_PERMITTED) + "\n");
-        } catch (ResourceConflictException e) {
-          writeError("error: " + parseError(Type.CHANGE_IS_CLOSED) + "\n");
-        }
       } else if (restoreChange) {
         RestoreInput input = new RestoreInput();
         input.message = changeComment;
-        try {
+        changeApi(patchSet).restore(input);
-          gApi.get().changes()
+        applyReview(patchSet, review);
-              .id(patchSet.getId().getParentKey().get())
-              .restore(input);
-          applyReview(patchSet, review);
-        } catch (AuthException e) {
-          writeError("error: " + parseError(Type.RESTORE_NOT_PERMITTED) + "\n");
-        } catch (ResourceConflictException e) {
-          writeError("error: " + parseError(Type.CHANGE_NOT_ABANDONED) + "\n");
-        }
       } else {
         applyReview(patchSet, review);
       }
 
       if (submitChange) {
-        gApi.get().changes()
+        revisionApi(patchSet).submit();
-            .id(patchSet.getId().getParentKey().get())
-            .revision(patchSet.getRevision().get())
-            .submit();
       }
 
       if (publishPatchSet) {
-        final ReviewResult result =
+        revisionApi(patchSet).publish();
-            publishDraftFactory.create(patchSet.getId()).call();
-        handleReviewResultErrors(result);
       } else if (deleteDraftPatchSet) {
-        gApi.get().changes()
+        revisionApi(patchSet).delete();
-            .id(patchSet.getId().getParentKey().get())
-            .revision(patchSet.getRevision().get())
-            .delete();
       }
     } catch (InvalidChangeOperationException e) {
       throw error(e.getMessage());
@@ -288,46 +260,12 @@ public class ReviewCommand extends SshCommand {
     }
   }
 
-  private void handleReviewResultErrors(final ReviewResult result) {
+  private ChangeApi changeApi(PatchSet patchSet) throws RestApiException {
-    for (ReviewResult.Error resultError : result.getErrors()) {
+    return gApi.get().changes().id(patchSet.getId().getParentKey().get());
-      String errMsg = "error: (change " + result.getChangeId() + ") ";
-      errMsg += parseError(resultError.getType());
-      if (resultError.getMessage() != null) {
-        errMsg += ": " + resultError.getMessage();
-      }
-      writeError(errMsg);
-    }
   }
 
-  private String parseError(Type type) {
+  private RevisionApi revisionApi(PatchSet patchSet) throws RestApiException {
-    switch (type) {
+    return changeApi(patchSet).revision(patchSet.getRevision().get());
-      case ABANDON_NOT_PERMITTED:
-        return "not permitted to abandon change";
-      case RESTORE_NOT_PERMITTED:
-        return "not permitted to restore change";
-      case SUBMIT_NOT_PERMITTED:
-        return "not permitted to submit change";
-      case SUBMIT_NOT_READY:
-        return "approvals or dependencies lacking";
-      case CHANGE_IS_CLOSED:
-        return "change is closed";
-      case CHANGE_NOT_ABANDONED:
-        return "change is not abandoned";
-      case PUBLISH_NOT_PERMITTED:
-        return "not permitted to publish change";
-      case DELETE_NOT_PERMITTED:
-        return "not permitted to delete change/patch set";
-      case RULE_ERROR:
-        return "rule error";
-      case NOT_A_DRAFT:
-        return "change/patch set is not a draft";
-      case GIT_ERROR:
-        return "error writing change to git repository";
-      case DEST_BRANCH_NOT_FOUND:
-        return "destination branch not found";
-      default:
-        return "failure in review";
-    }
   }
 
   private PatchSet parsePatchSet(final String patchIdentity)