From c4365ffe8b3e21e1a9aabec06d82aa18b37a0d87 Mon Sep 17 00:00:00 2001
From: Dave Borowitz <dborowitz@google.com>
Date: Wed, 20 Jan 2016 13:44:48 -0500
Subject: [PATCH] BatchUpdate: Don't update change if there was nothing to do

Committing the transaction and indexing the change can be expensive,
so allow ops to skip that if they did nothing. Similar to how
updateRepo can be a no-op if no commands were added, except we don't
communicate this through the context and instead through a return
value. This is distinct from the insert/save/deleteChange methods on
ChangeContext, which refer specifically to the Change entity; a method
could update a non-Change entity and return true without calling
saveChange.

Change-Id: Id51eec8f9dfa35c80f062a365195f25777f60006
---
 .../google/gerrit/server/change/Abandon.java  |  3 +-
 .../gerrit/server/change/ChangeInserter.java  |  3 +-
 .../server/change/CreateDraftComment.java     |  3 +-
 .../server/change/DeleteDraftChangeOp.java    |  3 +-
 .../server/change/DeleteDraftComment.java     |  5 ++-
 .../server/change/DeleteDraftPatchSet.java    |  5 ++-
 .../gerrit/server/change/DeleteVote.java      |  3 +-
 .../server/change/PatchSetInserter.java       |  3 +-
 .../gerrit/server/change/PostReview.java      |  3 +-
 .../server/change/PublishDraftPatchSet.java   |  3 +-
 .../gerrit/server/change/PutDraftComment.java |  3 +-
 .../google/gerrit/server/change/PutTopic.java |  5 ++-
 .../gerrit/server/change/RebaseChangeOp.java  |  5 ++-
 .../google/gerrit/server/change/Restore.java  |  3 +-
 .../gerrit/server/change/SetHashtagsOp.java   |  5 ++-
 .../google/gerrit/server/git/BatchUpdate.java | 38 +++++++++++--------
 .../gerrit/server/git/ReceiveCommits.java     |  3 +-
 .../server/git/strategy/CherryPick.java       |  5 ++-
 .../git/strategy/RebaseIfNecessary.java       |  5 ++-
 19 files changed, 65 insertions(+), 41 deletions(-)

diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
index 5175f9765d..0a421f8732 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Abandon.java
@@ -118,7 +118,7 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
     }
 
     @Override
-    public void updateChange(ChangeContext ctx) throws OrmException,
+    public boolean updateChange(ChangeContext ctx) throws OrmException,
         ResourceConflictException {
       change = ctx.getChange();
       PatchSet.Id psId = change.currentPatchSetId();
@@ -137,6 +137,7 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
       update.setStatus(change.getStatus());
       message = newMessage(ctx.getDb());
       cmUtil.addChangeMessage(ctx.getDb(), update, message);
+      return true;
     }
 
     private ChangeMessage newMessage(ReviewDb db) throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
index ef35045fa8..31c88e049f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ChangeInserter.java
@@ -292,7 +292,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
   }
 
   @Override
-  public void updateChange(ChangeContext ctx) throws OrmException, IOException {
+  public boolean updateChange(ChangeContext ctx) throws OrmException, IOException {
     change = ctx.getChange(); // Use defensive copy created by ChangeControl.
     ReviewDb db = ctx.getDb();
     ChangeControl ctl = ctx.getControl();
@@ -335,6 +335,7 @@ public class ChangeInserter extends BatchUpdate.InsertChangeOp {
       changeMessage.setMessage(message);
       cmUtil.addChangeMessage(db, update, changeMessage);
     }
+    return true;
   }
 
   @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java
index 6461303de7..6b8d8db58d 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateDraftComment.java
@@ -104,7 +104,7 @@ public class CreateDraftComment implements RestModifyView<RevisionResource, Draf
     }
 
     @Override
-    public void updateChange(ChangeContext ctx)
+    public boolean updateChange(ChangeContext ctx)
         throws ResourceNotFoundException, OrmException {
       PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
       if (ps == null) {
@@ -126,6 +126,7 @@ public class CreateDraftComment implements RestModifyView<RevisionResource, Draf
           comment, patchListCache, ctx.getChange(), ps);
       plcUtil.insertComments(
           ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(comment));
+      return true;
     }
   }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java
index 74a8d5ab0d..1f0e75521f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftChangeOp.java
@@ -62,7 +62,7 @@ class DeleteDraftChangeOp extends BatchUpdate.Op {
   }
 
   @Override
-  public void updateChange(ChangeContext ctx)
+  public boolean updateChange(ChangeContext ctx)
       throws RestApiException, OrmException {
     checkState(ctx.getOrder() == BatchUpdate.Order.DB_BEFORE_REPO,
         "must use DeleteDraftChangeOp with DB_BEFORE_REPO");
@@ -100,6 +100,7 @@ class DeleteDraftChangeOp extends BatchUpdate.Op {
     db.changeMessages().delete(db.changeMessages().byChange(id));
     starredChangesUtil.unstarAll(id);
     ctx.deleteChange();
+    return true;
   }
 
   @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java
index 4f084e52e3..5c2acb2994 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftComment.java
@@ -86,12 +86,12 @@ public class DeleteDraftComment
     }
 
     @Override
-    public void updateChange(ChangeContext ctx)
+    public boolean updateChange(ChangeContext ctx)
         throws ResourceNotFoundException, OrmException {
       Optional<PatchLineComment> maybeComment =
           plcUtil.get(ctx.getDb(), ctx.getNotes(), key);
       if (!maybeComment.isPresent()) {
-        return; // Nothing to do.
+        return false; // Nothing to do.
       }
       PatchSet.Id psId = key.getParentKey().getParentKey();
       PatchSet ps = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
@@ -102,6 +102,7 @@ public class DeleteDraftComment
       setCommentRevId(c, patchListCache, ctx.getChange(), ps);
       plcUtil.deleteComments(
           ctx.getDb(), ctx.getUpdate(psId), Collections.singleton(c));
+      return true;
     }
   }
 }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
index 9d4b91745f..34c25e57e7 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteDraftPatchSet.java
@@ -100,11 +100,11 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
     }
 
     @Override
-    public void updateChange(ChangeContext ctx)
+    public boolean updateChange(ChangeContext ctx)
         throws RestApiException, OrmException, IOException {
       patchSet = psUtil.get(ctx.getDb(), ctx.getNotes(), psId);
       if (patchSet == null) {
-        return; // Nothing to do.
+        return false; // Nothing to do.
       }
       if (!patchSet.isDraft()) {
         throw new ResourceConflictException("Patch set is not a draft");
@@ -118,6 +118,7 @@ public class DeleteDraftPatchSet implements RestModifyView<RevisionResource, Inp
 
       deleteDraftPatchSet(patchSet, ctx);
       deleteOrUpdateDraftChange(ctx);
+      return true;
     }
 
     @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
index 702ad249c8..44f2a14d2b 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteVote.java
@@ -91,7 +91,7 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
     }
 
     @Override
-    public void updateChange(ChangeContext ctx)
+    public boolean updateChange(ChangeContext ctx)
         throws OrmException, AuthException, ResourceNotFoundException {
       IdentifiedUser user = ctx.getUser().asIdentifiedUser();
       Change change = ctx.getChange();
@@ -135,6 +135,7 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
         cmUtil.addChangeMessage(ctx.getDb(), ctx.getUpdate(psId),
             changeMessage);
       }
+      return true;
     }
   }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
index e278a9219b..51aaedbfe0 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PatchSetInserter.java
@@ -204,7 +204,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
   }
 
   @Override
-  public void updateChange(ChangeContext ctx) throws OrmException,
+  public boolean updateChange(ChangeContext ctx) throws OrmException,
       InvalidChangeOperationException, IOException {
     ChangeControl ctl = ctx.getControl();
 
@@ -249,6 +249,7 @@ public class PatchSetInserter extends BatchUpdate.Op {
     if (changeMessage != null) {
       cmUtil.addChangeMessage(db, update, changeMessage);
     }
+    return true;
   }
 
   @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
index a4381a596a..ab795f4a97 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java
@@ -354,7 +354,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
     }
 
     @Override
-    public void updateChange(ChangeContext ctx)
+    public boolean updateChange(ChangeContext ctx)
         throws OrmException, ResourceConflictException {
       user = ctx.getUser().asIdentifiedUser();
       change = ctx.getChange();
@@ -369,6 +369,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
       if (dirty) {
         ctx.saveChange();
       }
+      return dirty;
     }
 
     @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
index e93a9d825c..cdc6daabce 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PublishDraftPatchSet.java
@@ -168,7 +168,7 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
     }
 
     @Override
-    public void updateChange(ChangeContext ctx)
+    public boolean updateChange(ChangeContext ctx)
         throws RestApiException, OrmException, IOException {
       if (!ctx.getControl().canPublish(ctx.getDb())) {
         throw new AuthException("Cannot publish this draft patch set");
@@ -182,6 +182,7 @@ public class PublishDraftPatchSet implements RestModifyView<RevisionResource, In
       saveChange(ctx);
       savePatchSet(ctx);
       addReviewers(ctx);
+      return true;
     }
 
     private void saveChange(ChangeContext ctx) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
index e2244f9f10..00246c6f26 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutDraftComment.java
@@ -109,7 +109,7 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
     }
 
     @Override
-    public void updateChange(ChangeContext ctx)
+    public boolean updateChange(ChangeContext ctx)
         throws ResourceNotFoundException, OrmException {
       Optional<PatchLineComment> maybeComment =
           plcUtil.get(ctx.getDb(), ctx.getNotes(), key);
@@ -152,6 +152,7 @@ public class PutDraftComment implements RestModifyView<DraftCommentResource, Dra
         plcUtil.updateComments(ctx.getDb(), update,
             Collections.singleton(update(comment, in)));
       }
+      return true;
     }
   }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
index 30bff61203..7bb7cbd03f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PutTopic.java
@@ -98,13 +98,13 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>,
     }
 
     @Override
-    public void updateChange(ChangeContext ctx) throws OrmException {
+    public boolean updateChange(ChangeContext ctx) throws OrmException {
       change = ctx.getChange();
       ChangeUpdate update = ctx.getUpdate(change.currentPatchSetId());
       newTopicName = Strings.nullToEmpty(input.topic);
       oldTopicName = Strings.nullToEmpty(change.getTopic());
       if (oldTopicName.equals(newTopicName)) {
-        return;
+        return false;
       }
       String summary;
       if (oldTopicName.isEmpty()) {
@@ -128,6 +128,7 @@ public class PutTopic implements RestModifyView<ChangeResource, Input>,
           change.currentPatchSetId());
       cmsg.setMessage(summary);
       cmUtil.addChangeMessage(ctx.getDb(), update, cmsg);
+      return true;
     }
 
     @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
index 346d9492ae..b4ad49f6cf 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/RebaseChangeOp.java
@@ -152,10 +152,11 @@ public class RebaseChangeOp extends BatchUpdate.Op {
   }
 
   @Override
-  public void updateChange(ChangeContext ctx)
+  public boolean updateChange(ChangeContext ctx)
       throws OrmException, InvalidChangeOperationException, IOException {
-    patchSetInserter.updateChange(ctx);
+    boolean ret = patchSetInserter.updateChange(ctx);
     rebasedPatchSet = patchSetInserter.getPatchSet();
+    return ret;
   }
 
   @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
index be2d9af03a..a195e462fe 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/Restore.java
@@ -108,7 +108,7 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
     }
 
     @Override
-    public void updateChange(ChangeContext ctx) throws OrmException,
+    public boolean updateChange(ChangeContext ctx) throws OrmException,
         ResourceConflictException {
       caller = ctx.getUser().asIdentifiedUser();
       change = ctx.getChange();
@@ -125,6 +125,7 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
 
       message = newMessage(ctx.getDb());
       cmUtil.addChangeMessage(ctx.getDb(), update, message);
+      return true;
     }
 
     private ChangeMessage newMessage(ReviewDb db) throws OrmException {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
index 48a17b4277..5e75f96bc2 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/SetHashtagsOp.java
@@ -73,12 +73,12 @@ public class SetHashtagsOp extends BatchUpdate.Op {
   }
 
   @Override
-  public void updateChange(ChangeContext ctx)
+  public boolean updateChange(ChangeContext ctx)
       throws AuthException, BadRequestException, OrmException, IOException {
     if (input == null
         || (input.add == null && input.remove == null)) {
       updatedHashtags = ImmutableSortedSet.of();
-      return;
+      return false;
     }
     if (!ctx.getControl().canEditHashtags()) {
       throw new AuthException("Editing hashtags not permitted");
@@ -112,6 +112,7 @@ public class SetHashtagsOp extends BatchUpdate.Op {
     }
 
     updatedHashtags = ImmutableSortedSet.copyOf(updated);
+    return true;
   }
 
   @Override
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
index 3e0c67cd81..909a453663 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java
@@ -221,7 +221,8 @@ public class BatchUpdate implements AutoCloseable {
     }
 
     @SuppressWarnings("unused")
-    public void updateChange(ChangeContext ctx) throws Exception {
+    public boolean updateChange(ChangeContext ctx) throws Exception {
+      return false;
     }
 
     // TODO(dborowitz): Support async operations?
@@ -542,10 +543,11 @@ public class BatchUpdate implements AutoCloseable {
         Change.Id id = e.getKey();
         db.changes().beginTransaction(id);
         ChangeContext ctx;
+        boolean dirty = false;
         try {
           ctx = newChangeContext(id);
           for (Op op : e.getValue()) {
-            op.updateChange(ctx);
+            dirty |= op.updateChange(ctx);
           }
           Iterable<Change> changes = Collections.singleton(ctx.getChange());
           if (newChanges.containsKey(id)) {
@@ -555,26 +557,30 @@ public class BatchUpdate implements AutoCloseable {
           } else if (ctx.deleted) {
             db.changes().delete(changes);
           }
-          db.commit();
+          if (dirty) {
+            db.commit();
+          }
         } finally {
           db.rollback();
         }
 
-        BatchMetaDataUpdate bmdu = null;
-        for (ChangeUpdate u : ctx.updates.values()) {
-          if (bmdu == null) {
-            bmdu = u.openUpdate();
+        if (dirty) {
+          BatchMetaDataUpdate bmdu = null;
+          for (ChangeUpdate u : ctx.updates.values()) {
+            if (bmdu == null) {
+              bmdu = u.openUpdate();
+            }
+            u.writeCommit(bmdu);
+          }
+          if (bmdu != null) {
+            bmdu.commit();
           }
-          u.writeCommit(bmdu);
-        }
-        if (bmdu != null) {
-          bmdu.commit();
-        }
 
-        if (ctx.deleted) {
-          indexFutures.add(indexer.deleteAsync(id));
-        } else {
-          indexFutures.add(indexer.indexAsync(id));
+          if (ctx.deleted) {
+            indexFutures.add(indexer.deleteAsync(id));
+          } else {
+            indexFutures.add(indexer.indexAsync(id));
+          }
         }
       }
     } catch (Exception e) {
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
index f809f75062..037367d6b8 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java
@@ -1810,8 +1810,9 @@ public class ReceiveCommits {
                 changeId,
                 new BatchUpdate.Op() {
                   @Override
-                  public void updateChange(ChangeContext ctx) throws Exception {
+                  public boolean updateChange(ChangeContext ctx) {
                     ctx.getUpdate(psId).setTopic(magicBranch.topic);
+                    return true;
                   }
                 });
           }
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
index e3d98437f3..3d527a513a 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/CherryPick.java
@@ -162,11 +162,11 @@ public class CherryPick extends SubmitStrategy {
     }
 
     @Override
-    public void updateChange(ChangeContext ctx) throws OrmException,
+    public boolean updateChange(ChangeContext ctx) throws OrmException,
          NoSuchChangeException {
       if (newCommit == null) {
         // Merge conflict; don't update change.
-        return;
+        return false;
       }
       PatchSet prevPs = args.psUtil.current(ctx.getDb(), ctx.getNotes());
 
@@ -185,6 +185,7 @@ public class CherryPick extends SubmitStrategy {
 
       newCommit.setControl(
           args.changeControlFactory.controlFor(toMerge.change(), args.caller));
+      return true;
     }
   }
 
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
index ec6e1f258b..a3b7c11643 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/strategy/RebaseIfNecessary.java
@@ -170,11 +170,11 @@ public class RebaseIfNecessary extends SubmitStrategy {
     }
 
     @Override
-    public void updateChange(ChangeContext ctx) throws NoSuchChangeException,
+    public boolean updateChange(ChangeContext ctx) throws NoSuchChangeException,
         InvalidChangeOperationException, OrmException, IOException  {
       if (rebaseOp == null) {
         // Took the fast-forward option, nothing to do.
-        return;
+        return false;
       }
 
       rebaseOp.updateChange(ctx);
@@ -191,6 +191,7 @@ public class RebaseIfNecessary extends SubmitStrategy {
               newPatchSetId));
       newCommit.setControl(
           args.changeControlFactory.controlFor(toMerge.change(), args.caller));
+      return true;
     }
 
     @Override