Merge "Refactor RestoreChange code into gerrit-server"

gerrit-common/src/main/java/com/google/gerrit/common/data/ReviewResult.java

@@ -49,7 +49,10 @@ public class ReviewResult {
   public static class Error {
     public static enum Type {
       /** Not permitted to abandon this change. */
-      ABANDON_NOT_PERMITTED
+      ABANDON_NOT_PERMITTED,
+
+      /** Not permitted to restore this change. */
+      RESTORE_NOT_PERMITTED
     }
 
     protected Type type;

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeManageServiceImpl.java

@@ -24,7 +24,7 @@ import com.google.inject.Inject;
 class ChangeManageServiceImpl implements ChangeManageService {
   private final SubmitAction.Factory submitAction;
   private final AbandonChangeHandler.Factory abandonChangeHandlerFactory;
-  private final RestoreChange.Factory restoreChangeFactory;
+  private final RestoreChangeHandler.Factory restoreChangeHandlerFactory;
   private final RevertChange.Factory revertChangeFactory;
   private final PublishAction.Factory publishAction;
   private final DeleteDraftChange.Factory deleteDraftChangeFactory;
@@ -32,13 +32,13 @@ class ChangeManageServiceImpl implements ChangeManageService {
   @Inject
   ChangeManageServiceImpl(final SubmitAction.Factory patchSetAction,
       final AbandonChangeHandler.Factory abandonChangeHandlerFactory,
-      final RestoreChange.Factory restoreChangeFactory,
+      final RestoreChangeHandler.Factory restoreChangeHandlerFactory,
       final RevertChange.Factory revertChangeFactory,
       final PublishAction.Factory publishAction,
       final DeleteDraftChange.Factory deleteDraftChangeFactory) {
     this.submitAction = patchSetAction;
     this.abandonChangeHandlerFactory = abandonChangeHandlerFactory;
-    this.restoreChangeFactory = restoreChangeFactory;
+    this.restoreChangeHandlerFactory = restoreChangeHandlerFactory;
     this.revertChangeFactory = revertChangeFactory;
     this.publishAction = publishAction;
     this.deleteDraftChangeFactory = deleteDraftChangeFactory;
@@ -61,7 +61,7 @@ class ChangeManageServiceImpl implements ChangeManageService {
 
   public void restoreChange(final PatchSet.Id patchSetId, final String message,
       final AsyncCallback<ChangeDetail> callback) {
-    restoreChangeFactory.create(patchSetId, message).to(callback);
+    restoreChangeHandlerFactory.create(patchSetId, message).to(callback);
   }
 
   public void publish(final PatchSet.Id patchSetId,

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/ChangeModule.java

@@ -29,7 +29,7 @@ public class ChangeModule extends RpcServletModule {
       @Override
       protected void configure() {
         factory(AbandonChangeHandler.Factory.class);
-        factory(RestoreChange.Factory.class);
+        factory(RestoreChangeHandler.Factory.class);
         factory(RevertChange.Factory.class);
         factory(ChangeDetailFactory.Factory.class);
         factory(IncludedInDetailFactory.Factory.class);

gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/changedetail/RestoreChangeHandler.java

@@ -14,19 +14,14 @@
 
 package com.google.gerrit.httpd.rpc.changedetail;
 
-import com.google.gerrit.common.ChangeHooks;
 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.RestoreChange;
-import com.google.gerrit.server.ChangeUtil;
-import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.mail.EmailException;
-import com.google.gerrit.server.mail.RestoredSender;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
-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.client.OrmException;
@@ -35,55 +30,39 @@ import com.google.inject.assistedinject.Assisted;
 
 import javax.annotation.Nullable;
 
-class RestoreChange extends Handler<ChangeDetail> {
+class RestoreChangeHandler extends Handler<ChangeDetail> {
   interface Factory {
-    RestoreChange create(PatchSet.Id patchSetId, String message);
+    RestoreChangeHandler create(PatchSet.Id patchSetId, String message);
   }
 
-  private final ChangeControl.Factory changeControlFactory;
+  private final RestoreChange.Factory restoreChangeFactory;
-  private final ReviewDb db;
-  private final IdentifiedUser currentUser;
-  private final RestoredSender.Factory senderFactory;
   private final ChangeDetailFactory.Factory changeDetailFactory;
 
   private final PatchSet.Id patchSetId;
   @Nullable
   private final String message;
 
-  private final ChangeHooks hooks;
-
   @Inject
-  RestoreChange(final ChangeControl.Factory changeControlFactory,
+  RestoreChangeHandler(final RestoreChange.Factory restoreChangeFactory,
-      final ReviewDb db, final IdentifiedUser currentUser,
-      final RestoredSender.Factory senderFactory,
       final ChangeDetailFactory.Factory changeDetailFactory,
       @Assisted final PatchSet.Id patchSetId,
-      @Assisted @Nullable final String message, final ChangeHooks hooks) {
+      @Assisted @Nullable final String message) {
-    this.changeControlFactory = changeControlFactory;
+    this.restoreChangeFactory = restoreChangeFactory;
-    this.db = db;
-    this.currentUser = currentUser;
-    this.senderFactory = senderFactory;
     this.changeDetailFactory = changeDetailFactory;
 
     this.patchSetId = patchSetId;
     this.message = message;
-    this.hooks = hooks;
   }
 
   @Override
   public ChangeDetail call() throws NoSuchChangeException, OrmException,
       EmailException, NoSuchEntityException, InvalidChangeOperationException,
       PatchSetInfoNotAvailableException {
-
+    final ReviewResult result =
-    final Change.Id changeId = patchSetId.getParentKey();
+        restoreChangeFactory.create(patchSetId, message).call();
-    final ChangeControl control = changeControlFactory.validateFor(changeId);
+    if (result.getErrors().size() > 0) {
-    if (!control.canRestore()) {
+      throw new NoSuchChangeException(result.getChangeId());
-      throw new NoSuchChangeException(changeId);
     }
-
+    return changeDetailFactory.create(result.getChangeId()).call();
-    ChangeUtil.restore(patchSetId, currentUser, message, db, senderFactory,
-        hooks);
-
-    return changeDetailFactory.create(changeId).call();
   }
 }

gerrit-server/src/main/java/com/google/gerrit/server/ChangeUtil.java

@@ -33,7 +33,6 @@ import com.google.gerrit.server.git.MergeQueue;
 import com.google.gerrit.server.git.ReplicationQueue;
 import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.mail.ReplyToChangeSender;
-import com.google.gerrit.server.mail.RestoredSender;
 import com.google.gerrit.server.mail.RevertedSender;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
@@ -317,49 +316,6 @@ public class ChangeUtil {
     }
   }
 
-  public static void restore(final PatchSet.Id patchSetId,
-      final IdentifiedUser user, final String message, final ReviewDb db,
-      final RestoredSender.Factory senderFactory,
-      final ChangeHooks hooks) throws NoSuchChangeException,
-      InvalidChangeOperationException, EmailException, OrmException {
-    final Change.Id changeId = patchSetId.getParentKey();
-    final PatchSet patch = db.patchSets().get(patchSetId);
-    if (patch == null) {
-      throw new NoSuchChangeException(changeId);
-    }
-
-    final ChangeMessage cmsg =
-        new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil
-            .messageUUID(db)), user.getAccountId(), patchSetId);
-    final StringBuilder msgBuf =
-        new StringBuilder("Patch Set " + patchSetId.get() + ": Restored");
-    if (message != null && message.length() > 0) {
-      msgBuf.append("\n\n");
-      msgBuf.append(message);
-    }
-    cmsg.setMessage(msgBuf.toString());
-
-    final Change updatedChange = db.changes().atomicUpdate(changeId,
-        new AtomicUpdate<Change>() {
-      @Override
-      public Change update(Change change) {
-        if (change.getStatus() == Change.Status.ABANDONED
-            && change.currentPatchSetId().equals(patchSetId)) {
-          change.setStatus(Change.Status.NEW);
-          ChangeUtil.updated(change);
-          return change;
-        } else {
-          return null;
-        }
-      }
-    });
-
-    updatedChange(db, user, updatedChange, cmsg, senderFactory,
-       "Change is not abandoned or patchset is not latest");
-
-    hooks.doChangeRestoreHook(updatedChange, user.getAccount(), message, db);
-  }
-
   public static void publishDraftPatchSet(final ReviewDb db,
       final PatchSet.Id patchSetId) throws OrmException, NoSuchChangeException{
     final Change.Id changeId = patchSetId.getParentKey();

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

@@ -0,0 +1,123 @@
+// 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 com.google.gerrit.common.ChangeHooks;
+import com.google.gerrit.common.data.ReviewResult;
+import com.google.gerrit.reviewdb.Change;
+import com.google.gerrit.reviewdb.ChangeMessage;
+import com.google.gerrit.reviewdb.PatchSet;
+import com.google.gerrit.reviewdb.ReviewDb;
+import com.google.gerrit.server.ChangeUtil;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.mail.RestoredSender;
+import com.google.gerrit.server.mail.EmailException;
+import com.google.gerrit.server.project.InvalidChangeOperationException;
+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 RestoreChange implements Callable<ReviewResult> {
+
+  public interface Factory {
+    RestoreChange create(PatchSet.Id patchSetId, String changeComment);
+  }
+
+  private final RestoredSender.Factory restoredSenderFactory;
+  private final ChangeControl.Factory changeControlFactory;
+  private final ReviewDb db;
+  private final IdentifiedUser currentUser;
+  private final ChangeHooks hooks;
+
+  private final PatchSet.Id patchSetId;
+  private final String changeComment;
+
+  @Inject
+  RestoreChange(final RestoredSender.Factory restoredSenderFactory,
+      final ChangeControl.Factory changeControlFactory, final ReviewDb db,
+      final IdentifiedUser currentUser, final ChangeHooks hooks,
+      @Assisted final PatchSet.Id patchSetId,
+      @Assisted final String changeComment) {
+    this.restoredSenderFactory = restoredSenderFactory;
+    this.changeControlFactory = changeControlFactory;
+    this.db = db;
+    this.currentUser = currentUser;
+    this.hooks = hooks;
+
+    this.patchSetId = patchSetId;
+    this.changeComment = changeComment;
+  }
+
+  @Override
+  public ReviewResult call() throws EmailException,
+      InvalidChangeOperationException, 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 (!control.canRestore()) {
+      result.addError(new ReviewResult.Error(
+          ReviewResult.Error.Type.RESTORE_NOT_PERMITTED));
+    } else if (patch == null) {
+      throw new NoSuchChangeException(changeId);
+    } else {
+
+      // Create a message to accompany the restored change
+      final ChangeMessage cmsg =
+          new ChangeMessage(new ChangeMessage.Key(changeId, ChangeUtil
+              .messageUUID(db)), currentUser.getAccountId(), patchSetId);
+      final StringBuilder msgBuf =
+          new StringBuilder("Patch Set " + patchSetId.get() + ": Restored");
+      if (changeComment != null && changeComment.length() > 0) {
+        msgBuf.append("\n\n");
+        msgBuf.append(changeComment);
+      }
+      cmsg.setMessage(msgBuf.toString());
+
+      // Restore the change
+      final Change updatedChange = db.changes().atomicUpdate(changeId,
+          new AtomicUpdate<Change>() {
+        @Override
+        public Change update(Change change) {
+          if (change.getStatus() == Change.Status.ABANDONED
+              && change.currentPatchSetId().equals(patchSetId)) {
+            change.setStatus(Change.Status.NEW);
+            ChangeUtil.updated(change);
+            return change;
+          } else {
+            return null;
+          }
+        }
+      });
+
+      ChangeUtil.updatedChange(
+          db, currentUser, updatedChange, cmsg, restoredSenderFactory,
+         "Change is not abandoned or patchset is not latest");
+
+      hooks.doChangeRestoreHook(updatedChange, currentUser.getAccount(),
+                                changeComment, db);
+    }
+
+    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.RestoreChange;
 import com.google.gerrit.server.git.CreateCodeReviewNotes;
 import com.google.gerrit.server.git.MergeOp;
 import com.google.gerrit.server.git.MetaDataUpdate;
@@ -88,6 +89,7 @@ public class GerritRequestModule extends FactoryModule {
     factory(ReplacePatchSetSender.Factory.class);
     factory(AbandonedSender.Factory.class);
     factory(RemoveReviewer.Factory.class);
+    factory(RestoreChange.Factory.class);
     factory(RestoredSender.Factory.class);
     factory(RevertedSender.Factory.class);
     factory(CommentSender.Factory.class);

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

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.sshd.commands;
 
-import com.google.gerrit.common.ChangeHooks;
 import com.google.gerrit.common.data.ApprovalType;
 import com.google.gerrit.common.data.ApprovalTypes;
 import com.google.gerrit.common.data.ReviewResult;
@@ -30,13 +29,12 @@ 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.RestoreChange;
 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.git.ReplicationQueue;
-import com.google.gerrit.server.mail.AbandonedSender;
 import com.google.gerrit.server.mail.EmailException;
-import com.google.gerrit.server.mail.RestoredSender;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
 import com.google.gerrit.server.patch.PatchSetInfoNotAvailableException;
 import com.google.gerrit.server.patch.PublishComments;
@@ -137,9 +135,6 @@ public class ReviewCommand extends BaseCommand {
   @Inject
   private AbandonChange.Factory abandonChangeFactory;
 
-  @Inject
-  private AbandonedSender.Factory abandonedSenderFactory;
-
   @Inject
   private FunctionState.Factory functionStateFactory;
 
@@ -147,10 +142,7 @@ public class ReviewCommand extends BaseCommand {
   private PublishComments.Factory publishCommentsFactory;
 
   @Inject
-  private RestoredSender.Factory restoredSenderFactory;
+  private RestoreChange.Factory restoreChangeFactory;
-
-  @Inject
-  private ChangeHooks hooks;
 
   @Inject
   private GitRepositoryManager gitManager;
@@ -278,15 +270,8 @@ public class ReviewCommand extends BaseCommand {
 
       if (abandonChange) {
         result = abandonChangeFactory.create(patchSetId, changeComment).call();
-      }
+      } else if (restoreChange) {
-
+        result = restoreChangeFactory.create(patchSetId, changeComment).call();
-      if (restoreChange) {
-        if (changeControl.canRestore()) {
-          ChangeUtil.restore(patchSetId, currentUser, changeComment, db,
-              restoredSenderFactory, hooks);
-        } else {
-          throw error("Not permitted to restore change");
-        }
         if (submitChange) {
           changeControl = changeControlFactory.validateFor(changeId);
         }
@@ -379,6 +364,10 @@ public class ReviewCommand extends BaseCommand {
         switch (resultError.getType()) {
           case ABANDON_NOT_PERMITTED:
             writeError("error: not permitted to abandon change");
+            break;
+          case RESTORE_NOT_PERMITTED:
+            writeError("error: not permitted to restore change");
+            break;
           default:
             writeError("error: failure in review");
         }