Stop reindexing changes for batch ref updates with only draft refs
See previous change I1e7277b3: we no longer keep draft comment fields in
the change index hence we don't need to reindex changes if
BatchRefUpdate contained draft comments only.
Google-Bug-Id: b/191226871
Release-Notes: skip
Change-Id: Iaea11e5ff68525f4bc0684cc81b90428d0d4e316
diff --git a/java/com/google/gerrit/server/update/BatchUpdate.java b/java/com/google/gerrit/server/update/BatchUpdate.java
index 3dfd9b4..92d614b 100644
--- a/java/com/google/gerrit/server/update/BatchUpdate.java
+++ b/java/com/google/gerrit/server/update/BatchUpdate.java
@@ -38,6 +38,7 @@
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
+import com.google.gerrit.entities.RefNames;
import com.google.gerrit.extensions.api.changes.NotifyHandling;
import com.google.gerrit.extensions.config.FactoryModule;
import com.google.gerrit.extensions.restapi.BadRequestException;
@@ -148,7 +149,9 @@
}
for (ChangesHandle h : changesHandles) {
h.execute();
- indexFutures.addAll(h.startIndexFutures());
+ if (h.requiresReindex()) {
+ indexFutures.addAll(h.startIndexFutures());
+ }
}
notifyAfterUpdateRefs(listeners);
notifyAfterUpdateChanges(listeners);
@@ -621,6 +624,16 @@
BatchUpdate.this.executed = manager.isExecuted();
}
+ boolean requiresReindex() {
+ // We do not need to reindex changes if there are no ref updates, or if updated refs
+ // are all draft comment refs (since draft fields are not stored in the change index).
+ BatchRefUpdate bru = BatchUpdate.this.batchRefUpdate;
+ return !(bru == null
+ || bru.getCommands().isEmpty()
+ || bru.getCommands().stream()
+ .allMatch(cmd -> RefNames.isRefsDraftsComments(cmd.getRefName())));
+ }
+
ImmutableList<ListenableFuture<ChangeData>> startIndexFutures() {
if (dryrun) {
return ImmutableList.of();
diff --git a/javatests/com/google/gerrit/acceptance/api/change/BUILD b/javatests/com/google/gerrit/acceptance/api/change/BUILD
index 94fb0dc..306852a 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/BUILD
+++ b/javatests/com/google/gerrit/acceptance/api/change/BUILD
@@ -6,5 +6,8 @@
labels = [
"api",
],
- deps = ["//java/com/google/gerrit/server/util/time"],
+ deps = [
+ "//java/com/google/gerrit/server/util/time",
+ "//javatests/com/google/gerrit/acceptance/server/change:util",
+ ],
) for f in glob(["*IT.java"])]
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index b795873..c89da5b 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -85,6 +85,7 @@
import com.google.gerrit.acceptance.VerifyNoPiiInChangeNotes;
import com.google.gerrit.acceptance.api.change.ChangeIT.TestAttentionSetListenerModule.TestAttentionSetListener;
import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.server.change.CommentsUtil;
import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
import com.google.gerrit.acceptance.testsuite.change.IndexOperations;
import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
@@ -4667,6 +4668,31 @@
}
@Test
+ public void createAndDeleteDraftCommentDoesNotTriggerChangeReindex() throws Exception {
+ ChangeIndexedCounter changeIndexedCounter = new ChangeIndexedCounter();
+ try (Registration registration =
+ extensionRegistry.newRegistration().add(changeIndexedCounter)) {
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+ String revId = r.getCommit().getName();
+ String triplet = project.get() + "~master~" + r.getChangeId();
+ changeIndexedCounter.clear();
+
+ // Create the draft. Change is not re-indexed
+ DraftInput draftInput =
+ CommentsUtil.newDraft("file1", Side.REVISION, /* line= */ 1, "comment 1");
+ CommentInfo draftInfo =
+ gApi.changes().id(changeId).revision(revId).createDraft(draftInput).get();
+ ChangeInfo change = info(triplet);
+ changeIndexedCounter.assertReindexOf(change, 0);
+
+ // Delete the draft comment. Change is not re-indexed
+ gApi.changes().id(changeId).revision(revId).draft(draftInfo.id).delete();
+ changeIndexedCounter.assertReindexOf(change, 0);
+ }
+ }
+
+ @Test
public void changeDetailsDoesNotRequireIndex() throws Exception {
// This set of options must be kept in sync with gr-rest-api-interface.js
Set<ListChangesOption> options =
diff --git a/javatests/com/google/gerrit/acceptance/server/change/BUILD b/javatests/com/google/gerrit/acceptance/server/change/BUILD
index 19ca946..4514ea3 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/BUILD
+++ b/javatests/com/google/gerrit/acceptance/server/change/BUILD
@@ -14,6 +14,7 @@
java_library(
name = "util",
srcs = ["CommentsUtil.java"],
+ visibility = ["//javatests/com/google/gerrit/acceptance/api/change:__subpackages__"],
deps = [
"//java/com/google/gerrit/acceptance:lib",
"//java/com/google/gerrit/entities",
diff --git a/javatests/com/google/gerrit/acceptance/server/change/CommentsUtil.java b/javatests/com/google/gerrit/acceptance/server/change/CommentsUtil.java
index c4927f0..f32cf32 100644
--- a/javatests/com/google/gerrit/acceptance/server/change/CommentsUtil.java
+++ b/javatests/com/google/gerrit/acceptance/server/change/CommentsUtil.java
@@ -34,7 +34,7 @@
* A utility class for creating {@link CommentInput} objects, publishing comments and creating draft
* comments. Used by tests that require dealing with comments.
*/
-class CommentsUtil {
+public class CommentsUtil {
static CommentInput addComment(GerritApi gApi, String changeId) throws Exception {
ReviewInput input = new ReviewInput();
CommentInput comment = CommentsUtil.newComment(FILE_NAME, Side.REVISION, 0, "a message", false);
@@ -88,7 +88,7 @@
return populate(c, path, Side.PARENT, parent, line, message);
}
- static DraftInput newDraft(String path, Side side, int line, String message) {
+ public static DraftInput newDraft(String path, Side side, int line, String message) {
DraftInput d = new DraftInput();
d.unresolved = false;
return populate(d, path, side, null, line, message);
