commit 725b9b3be653ce0517b473d2c682bfe7e1ddadb5
author Youssef Elghareeb <ghareeb@google.com> Tue Jan 31 13:57:51 2023 +0100
committer Youssef Elghareeb <ghareeb@google.com> Tue Jan 31 14:01:27 2023 +0100
tree 74bad8386ebaa792250c05b2365f8d8661d4233c
parent bd05f014fefdab61a7bc87273489e8615f37397e

ChecksUpdate: use ChangeIndexer to index changes

We saw in the logs thrown exceptions on this code path with message:
  * AuthException: maintain server not permitted

This was because the current code delegated to ChangeApiImpl which calls
Index (see [1]), which in turn checks that the calling user has the
'Maintain Server' global capability and fails. This failed the POST
request to update checks.

Updating the code to use ChangeIndexer instead.

[1] https://gerrit.googlesource.com/gerrit/+/88f26343b13560d4c75b25a77a59427972c1c91a/java/com/google/gerrit/server/restapi/change/Index.java#44

Google-Bug-Id: b/250877007
Change-Id: Iea638a383ecfc7ddf6a3b46ccc6a005eb5cd3aea

java/com/google/gerrit/plugins/checks/ChecksUpdate.java

diff --git a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
index 336a961..0dae9a7 100644
--- a/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
+++ b/java/com/google/gerrit/plugins/checks/ChecksUpdate.java
@@ -21,12 +21,10 @@
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.exceptions.DuplicateKeyException;
-import com.google.gerrit.extensions.api.changes.Changes;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.api.changes.NotifyInfo;
 import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.extensions.restapi.BadRequestException;
-import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.plugins.checks.Checks.GetCheckOptions;
 import com.google.gerrit.plugins.checks.api.CombinedCheckState;
 import com.google.gerrit.plugins.checks.email.CombinedCheckStateUpdatedSender;
@@ -35,6 +33,7 @@
 import com.google.gerrit.server.ServerInitiated;
 import com.google.gerrit.server.UserInitiated;
 import com.google.gerrit.server.change.NotifyResolver;
+import com.google.gerrit.server.index.change.ChangeIndexer;
 import com.google.gerrit.server.mail.send.MessageIdGenerator;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.inject.assistedinject.Assisted;
@@ -70,7 +69,8 @@
   private final Checkers checkers;
   private final NotifyResolver notifyResolver;
   private final MessageIdGenerator messageIdGenerator;
-  private final Changes changes;
+
+  private final ChangeIndexer changeIndexer;
   private final Optional<IdentifiedUser> currentUser;
 
   @AssistedInject
@@ -84,7 +84,7 @@
       Checkers checkers,
       NotifyResolver notifyResolver,
       MessageIdGenerator messageIdGenerator,
-      Changes changes,
+      ChangeIndexer changeIndexer,
       @Assisted IdentifiedUser currentUser) {
     this.checksStorageUpdate = checksStorageUpdate;
     this.combinedCheckStateCache = combinedCheckStateCache;
@@ -95,7 +95,7 @@
     this.checkers = checkers;
     this.notifyResolver = notifyResolver;
     this.messageIdGenerator = messageIdGenerator;
-    this.changes = changes;
+    this.changeIndexer = changeIndexer;
     this.currentUser = Optional.of(currentUser);
   }
 
@@ -110,7 +110,7 @@
       Checkers checkers,
       NotifyResolver notifyResolver,
       MessageIdGenerator messageIdGenerator,
-      Changes changes) {
+      ChangeIndexer changeIndexer) {
     this.checksStorageUpdate = checksStorageUpdate;
     this.combinedCheckStateCache = combinedCheckStateCache;
     this.combinedCheckStateUpdatedSenderFactory = combinedCheckStateUpdatedSenderFactory;
@@ -120,7 +120,7 @@
     this.checkers = checkers;
     this.notifyResolver = notifyResolver;
     this.messageIdGenerator = messageIdGenerator;
-    this.changes = changes;
+    this.changeIndexer = changeIndexer;
     this.currentUser = Optional.empty();
   }
 
@@ -172,11 +172,7 @@
       Project.NameKey project,
       Change.Id changeId) {
     if (oldState != newState) {
-      try {
-        changes.id(project.get(), changeId.get()).index();
-      } catch (RestApiException e) {
-        logger.atSevere().withCause(e).log("Cannot index change: %s after check update.", changeId);
-      }
+      changeIndexer.index(project, changeId);
     }
   }