BatchUpdate: Prevent lastUpdatedOn from going backwards

We can add comments using historic timestamps in PostReview, which
also contained code to try and ensure the lastUpdatedOn timestamp
never went backwards. However, this wasn't working:
BatchUpdate#bumpLastUpdatedOn was going behind its back and using the
(older) timestamp from the Context.

Move this check from PostReview into BatchUpdate, so it always
applies. After all, one of the goals of BatchUpdate.Op is for callers
to not have to worry about handling timestamps themselves.

Change-Id: Ibd906641e8af43302b46df33fd8286afa187ad6f

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java

diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
index c7b2705..dd337c8 100644
--- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
+++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/server/change/CommentsIT.java
@@ -231,6 +231,8 @@
       PushOneCommit.Result r = push.to("refs/for/master");
       String changeId = r.getChangeId();
       String revId = r.getCommit().getName();
+      Timestamp origLastUpdated = r.getChange().change().getLastUpdatedOn();
+
       ReviewInput input = new ReviewInput();
       CommentInput comment = newComment(file, Side.REVISION, line, "comment 1");
       comment.updated = timestamp;
@@ -248,6 +250,10 @@
       assertCommentInfo(comment, actual);
       assertThat(actual.updated)
           .isEqualTo(gApi.changes().id(r.getChangeId()).info().created);
+
+      // Updating historic comments doesn't cause lastUpdatedOn to regress.
+      assertThat(r.getChange().change().getLastUpdatedOn())
+          .isEqualTo(origLastUpdated);
     }
   }
 

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

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 e869c1b..961b0bf 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
@@ -49,7 +49,6 @@
 import com.google.gerrit.extensions.restapi.RestModifyView;
 import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.extensions.restapi.Url;
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.CommentRange;
 import com.google.gerrit.reviewdb.client.Patch;
@@ -371,10 +370,6 @@
       dirty |= insertComments(ctx);
       dirty |= updateLabels(ctx);
       dirty |= insertMessage(ctx);
-      Change c = notes.getChange();
-      if (c.getLastUpdatedOn().before(ctx.getWhen())) {
-        c.setLastUpdatedOn(ctx.getWhen());
-      }
       if (dirty) {
         ctx.saveChange();
       }

gerrit-server/src/main/java/com/google/gerrit/server/git/BatchUpdate.java

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 e32e6ad..db1e97e 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
@@ -620,7 +620,9 @@
 
   private static Iterable<Change> bumpLastUpdatedOn(ChangeContext ctx) {
     Change c = ctx.getChange();
-    c.setLastUpdatedOn(ctx.getWhen());
+    if (c.getLastUpdatedOn().before(ctx.getWhen())) {
+      c.setLastUpdatedOn(ctx.getWhen());
+    }
     return Collections.singleton(c);
   }