No longer throw InvalidChangeOp in changeUpdated

InvalidChangeOperationExceptions are only thrown when we pass a null
change to this method.  It is better that we just don't pass the change
and add a more useful error to the ReviewResult.

We no longer perform a check to see if the patchset we have is the most
current one.  restore/abandon happens at the change level and the
patchset is just fetched to append a message to the change.

Change-Id: I3b3af1e3982fcd9111214a7c66a28476189c923b

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,
 
+      /** Review operation invalid because change is not abandoned. */
+      CHANGE_NOT_ABANDONED,
+
       /** Not permitted to publish this draft patch set */
       PUBLISH_NOT_PERMITTED,
 

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

@@ -22,7 +22,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.server.changedetail.AbandonChange;
 import com.google.gerrit.server.mail.EmailException;
 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;
@@ -61,9 +60,8 @@ class AbandonChangeHandler extends Handler<ChangeDetail> {
 
   @Override
   public ChangeDetail call() throws NoSuchChangeException, OrmException,
-      EmailException, NoSuchEntityException, InvalidChangeOperationException,
-      PatchSetInfoNotAvailableException, RepositoryNotFoundException,
-      IOException {
+      EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException,
+      RepositoryNotFoundException, IOException {
     final ReviewResult result =
         abandonChangeFactory.create(patchSetId.getParentKey(), message).call();
     if (result.getErrors().size() > 0) {

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

@@ -22,7 +22,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.server.changedetail.RestoreChange;
 import com.google.gerrit.server.mail.EmailException;
 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;
@@ -60,9 +59,8 @@ class RestoreChangeHandler extends Handler<ChangeDetail> {
 
   @Override
   public ChangeDetail call() throws NoSuchChangeException, OrmException,
-      EmailException, NoSuchEntityException, InvalidChangeOperationException,
-      PatchSetInfoNotAvailableException, RepositoryNotFoundException,
-      IOException {
+      EmailException, NoSuchEntityException, PatchSetInfoNotAvailableException,
+      RepositoryNotFoundException, IOException {
     final ReviewResult result =
         restoreChangeFactory.create(patchSetId.getParentKey(), message).call();
     if (result.getErrors().size() > 0) {

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

@@ -585,12 +585,8 @@ public class ChangeUtil {
 
   public static <T extends ReplyToChangeSender> void updatedChange(
       final ReviewDb db, final IdentifiedUser user, final Change change,
-      final ChangeMessage cmsg, ReplyToChangeSender.Factory<T> senderFactory,
-      final String err) throws NoSuchChangeException,
-      InvalidChangeOperationException, EmailException, OrmException {
-    if (change == null) {
-      throw new InvalidChangeOperationException(err);
-    }
+      final ChangeMessage cmsg, ReplyToChangeSender.Factory<T> senderFactory)
+      throws NoSuchChangeException, EmailException, OrmException {
     db.changeMessages().insert(Collections.singleton(cmsg));
 
     new ApprovalsUtil(db, null).syncChangeStatus(change);

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

@@ -26,7 +26,6 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.mail.AbandonedSender;
 import com.google.gerrit.server.mail.EmailException;
 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;
@@ -69,8 +68,8 @@ public class AbandonChange implements Callable<ReviewResult> {
   }
 
   @Override
-  public ReviewResult call() throws EmailException,
-      InvalidChangeOperationException, NoSuchChangeException, OrmException {
+  public ReviewResult call() throws EmailException, NoSuchChangeException,
+      OrmException {
     final ReviewResult result = new ReviewResult();
     result.setChangeId(changeId);
 
@@ -102,8 +101,7 @@ public class AbandonChange implements Callable<ReviewResult> {
           new AtomicUpdate<Change>() {
         @Override
         public Change update(Change change) {
-          if (change.getStatus().isOpen()
-              && change.currentPatchSetId().equals(patchSetId)) {
+          if (change.getStatus().isOpen()) {
             change.setStatus(Change.Status.ABANDONED);
             ChangeUtil.updated(change);
             return change;
@@ -112,9 +110,15 @@ public class AbandonChange implements Callable<ReviewResult> {
           }
         }
       });
-      ChangeUtil.updatedChange(
-          db, currentUser, updatedChange, cmsg, abandonedSenderFactory,
-          "Change is no longer open or patchset is not latest");
+
+      if (updatedChange == null) {
+        result.addError(new ReviewResult.Error(
+            ReviewResult.Error.Type.CHANGE_IS_CLOSED));
+        return result;
+      }
+
+      ChangeUtil.updatedChange(db, currentUser, updatedChange, cmsg,
+                               abandonedSenderFactory);
       hooks.doChangeAbandonedHook(updatedChange, currentUser.getAccount(),
                                   changeComment, db);
     }

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

@@ -29,7 +29,6 @@ import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.mail.EmailException;
 import com.google.gerrit.server.mail.RestoredSender;
 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;
@@ -77,9 +76,8 @@ public class RestoreChange implements Callable<ReviewResult> {
   }
 
   @Override
-  public ReviewResult call() throws EmailException,
-      InvalidChangeOperationException, NoSuchChangeException, OrmException,
-      RepositoryNotFoundException, IOException {
+  public ReviewResult call() throws EmailException, NoSuchChangeException,
+      OrmException, RepositoryNotFoundException, IOException {
     final ReviewResult result = new ReviewResult();
     result.setChangeId(changeId);
 
@@ -121,8 +119,7 @@ public class RestoreChange implements Callable<ReviewResult> {
         new AtomicUpdate<Change>() {
           @Override
           public Change update(Change change) {
-            if (change.getStatus() == Change.Status.ABANDONED
-                && change.currentPatchSetId().equals(patchSetId)) {
+            if (change.getStatus() == Change.Status.ABANDONED) {
               change.setStatus(Change.Status.NEW);
               ChangeUtil.updated(change);
               return change;
@@ -132,10 +129,14 @@ public class RestoreChange implements Callable<ReviewResult> {
           }
         });
 
-    ChangeUtil.updatedChange(
-        db, currentUser, updatedChange, cmsg, restoredSenderFactory,
-        "Change is not abandoned or patchset is not latest");
+    if (updatedChange == null) {
+      result.addError(new ReviewResult.Error(
+          ReviewResult.Error.Type.CHANGE_NOT_ABANDONED));
+      return result;
+    }
 
+    ChangeUtil.updatedChange(db, currentUser, updatedChange, cmsg,
+                             restoredSenderFactory);
     hooks.doChangeRestoreHook(updatedChange, currentUser.getAccount(),
                               changeComment, db);
 

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

@@ -248,6 +248,9 @@ public class ReviewCommand extends SshCommand {
         case CHANGE_IS_CLOSED:
           errMsg += "change is closed";
           break;
+        case CHANGE_NOT_ABANDONED:
+          errMsg += "change is not abandoned";
+          break;
         case PUBLISH_NOT_PERMITTED:
           errMsg += "not permitted to publish change";
           break;