Pass only Change.Id for async background indexing

The background worker thread creates its own database connection
to read the record. The change could have been modified between
the time it is enqueued, and the time the indexing is processed.
Always read from the database in the background thread to ensure
the index gets the most recent possible data.

This may fix some index state errors on gerrit-review (and friends).

Change-Id: Ifab8d0ba7c9dcac54b7e168b86e9a82a3c681ea5

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

@@ -107,7 +107,8 @@ public class Abandon implements RestModifyView<ChangeResource, AbandonInput>,
       db.rollback();
     }
 
-    CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(change);
+    CheckedFuture<?, IOException> indexFuture =
+        indexer.indexAsync(change.getId());
     try {
       ReplyToChangeSender cm = abandonedSenderFactory.create(change);
       cm.setFrom(caller.getAccountId());

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

@@ -185,7 +185,8 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
    *         reindexes the change (whether the mergeability flag was updated or
    *         not)
    */
-  public CheckedFuture<?, IOException> updateAndIndexAsync(final Change change) {
+  public CheckedFuture<?, IOException> updateAndIndexAsync(Change change) {
+    final Change.Id id = change.getId();
     return Futures.makeChecked(
         Futures.transform(updateAsync(change),
           new AsyncFunction<Boolean, Object>() {
@@ -194,7 +195,7 @@ public class MergeabilityChecker implements GitReferenceUpdatedListener {
             public ListenableFuture<Object> apply(Boolean indexUpdated)
                 throws Exception {
               if (!indexUpdated) {
-                return (ListenableFuture<Object>) indexer.indexAsync(change);
+                return (ListenableFuture<Object>) indexer.indexAsync(id);
               }
               return Futures.immediateFuture(null);
             }

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

@@ -139,7 +139,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
 
     CheckedFuture<?, IOException> indexWrite;
     if (dirty) {
-      indexWrite = indexer.indexAsync(change);
+      indexWrite = indexer.indexAsync(change.getId());
     } else {
       indexWrite = Futures.<Void, IOException> immediateCheckedFuture(null);
     }

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

@@ -228,7 +228,8 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
       db.rollback();
     }
 
-    CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(rsrc.getChange());
+    CheckedFuture<?, IOException> indexFuture =
+        indexer.indexAsync(rsrc.getChange().getId());
     result.reviewers = Lists.newArrayListWithCapacity(added.size());
     for (PatchSetApproval psa : added) {
       result.reviewers.add(json.format(

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

@@ -77,7 +77,7 @@ public class Publish implements RestModifyView<RevisionResource, Input>,
       if (!updatedPatchSet.isDraft()
           || updatedChange.getStatus() == Change.Status.NEW) {
         CheckedFuture<?, IOException> indexFuture =
-            indexer.indexAsync(updatedChange);
+            indexer.indexAsync(updatedChange.getId());
         hooks.doDraftPublishedHook(updatedChange, updatedPatchSet, dbProvider.get());
         sender.send(rsrc.getChange().getStatus() == Change.Status.DRAFT,
             rsrc.getUser(), updatedChange, updatedPatchSet,

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

@@ -115,7 +115,8 @@ class PutTopic implements RestModifyView<ChangeResource, Input>,
       } finally {
         db.rollback();
       }
-      CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(change);
+      CheckedFuture<?, IOException> indexFuture =
+          indexer.indexAsync(change.getId());
       hooks.doTopicChangedHook(change, currentUser.getAccount(),
           oldTopicName, db);
       indexFuture.checkedGet();

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

@@ -108,7 +108,8 @@ public class Restore implements RestModifyView<ChangeResource, RestoreInput>,
       db.rollback();
     }
 
-    CheckedFuture<?, IOException> indexFuture = indexer.indexAsync(change);
+    CheckedFuture<?, IOException> indexFuture =
+        indexer.indexAsync(change.getId());
     try {
       ReplyToChangeSender cm = restoredSenderFactory.create(change);
       cm.setFrom(caller.getAccountId());

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

@@ -1043,7 +1043,7 @@ public class MergeOp {
 
     CheckedFuture<?, IOException> indexFuture;
     if (change != null) {
-      indexFuture = indexer.indexAsync(change);
+      indexFuture = indexer.indexAsync(change.getId());
     } else {
       indexFuture = null;
     }

gerrit-server/src/main/java/com/google/gerrit/server/index/ChangeIndexer.java

@@ -112,12 +112,12 @@ public class ChangeIndexer {
   /**
    * Start indexing a change.
    *
-   * @param change change to index.
+   * @param id change to index.
    * @return future for the indexing task.
    */
-  public CheckedFuture<?, IOException> indexAsync(Change change) {
+  public CheckedFuture<?, IOException> indexAsync(Change.Id id) {
     return executor != null
-        ? submit(new Task(change, false))
+        ? submit(new Task(id, false))
         : Futures.<Object, IOException> immediateCheckedFuture(null);
   }
 
@@ -145,12 +145,12 @@ public class ChangeIndexer {
   /**
    * Start deleting a change.
    *
-   * @param change change to delete.
+   * @param id change to delete.
    * @return future for the deleting task.
    */
-  public CheckedFuture<?, IOException> deleteAsync(Change change) {
+  public CheckedFuture<?, IOException> deleteAsync(Change.Id id) {
     return executor != null
-        ? submit(new Task(change, true))
+        ? submit(new Task(id, true))
         : Futures.<Object, IOException> immediateCheckedFuture(null);
   }
 
@@ -186,11 +186,11 @@ public class ChangeIndexer {
   }
 
   private class Task implements Callable<Void> {
-    private final Change change;
+    private final Change.Id id;
     private final boolean delete;
 
-    private Task(Change change, boolean delete) {
+    private Task(Change.Id id, boolean delete) {
-      this.change = change;
+      this.id = id;
       this.delete = delete;
     }
 
@@ -225,7 +225,7 @@ public class ChangeIndexer {
         RequestContext oldCtx = context.setContext(newCtx);
         try {
           ChangeData cd = changeDataFactory.create(
-              newCtx.getReviewDbProvider().get(), change);
+              newCtx.getReviewDbProvider().get(), id);
           if (delete) {
             for (ChangeIndex i : getWriteIndexes()) {
               i.delete(cd);
@@ -244,16 +244,14 @@ public class ChangeIndexer {
           }
         }
       } catch (Exception e) {
-        log.error(String.format(
+        log.error(String.format("Failed to index change %d", id), e);
-            "Failed to index change %d in %s",
-            change.getId().get(), change.getProject().get()), e);
         throw e;
       }
     }
 
     @Override
     public String toString() {
-      return "index-change-" + change.getId().get();
+      return "index-change-" + id.get();
     }
   }
 }