Use secondary index for advertising extra haves during push

The current code uses ChangeAccess.byChangesOpenNext to list recent
changes; other indexes on that table don't have a specified sort
order. sortkey is going away, so we can't use that index anymore.
Rather than add another index just for this one query, just use the
normal query system to look up recent changes, which are naturally
ordered by updated time.

This does incur a slight performance penalty, as the query system
checks visibility of changes where the direct database scan did not.
However, this provides a security benefit (admittedly slight): SHA-1s
of patch sets that are not visible are no longer advertised.

Change-Id: I8c73cbf631ce074d6676afd35337579a0b7f5119

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

@@ -111,6 +111,7 @@ import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.RefControl;
 import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.QueryProcessor;
 import com.google.gerrit.server.ssh.SshInfo;
 import com.google.gerrit.server.util.LabelVote;
 import com.google.gerrit.server.util.MagicBranch;
@@ -336,6 +337,7 @@ public class ReceiveCommits {
 
   @Inject
   ReceiveCommits(final ReviewDb db,
+      final Provider<QueryProcessor> queryProcessor,
       final SchemaFactory<ReviewDb> schemaFactory,
       final ChangeData.Factory changeDataFactory,
       final ChangeUpdate.Factory updateFactory,
@@ -472,7 +474,7 @@ public class ReceiveCommits {
     });
     advHooks.add(rp.getAdvertiseRefsHook());
     advHooks.add(new ReceiveCommitsAdvertiseRefsHook(
-        db, projectControl.getProject().getNameKey()));
+        db, queryProcessor, projectControl.getProject().getNameKey()));
     rp.setAdvertiseRefsHook(AdvertiseRefsHookChain.newChain(advHooks));
   }
 

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

@@ -18,13 +18,18 @@ import static org.eclipse.jgit.lib.RefDatabase.ALL;
 
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
-import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
 import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.server.query.Predicate;
+import com.google.gerrit.server.query.QueryParseException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.gerrit.server.query.change.QueryProcessor;
 import com.google.gerrit.server.util.MagicBranch;
 import com.google.gwtorm.server.OrmException;
+import com.google.inject.Provider;
 
 import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.ObjectId;
@@ -48,11 +53,14 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
       .getLogger(ReceiveCommitsAdvertiseRefsHook.class);
 
   private final ReviewDb db;
+  private final Provider<QueryProcessor> queryProcessor;
   private final Project.NameKey projectName;
 
   public ReceiveCommitsAdvertiseRefsHook(ReviewDb db,
+      Provider<QueryProcessor> queryProcessor,
       Project.NameKey projectName) {
     this.db = db;
+    this.queryProcessor = queryProcessor;
     this.projectName = projectName;
   }
 
@@ -93,10 +101,11 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
     Set<ObjectId> toInclude = Sets.newHashSet();
 
     // Advertise some recent open changes, in case a commit is based one.
+    final int limit = 32;
     try {
-      Set<PatchSet.Id> toGet = Sets.newHashSetWithExpectedSize(32);
-      for (Change c : db.changes().byProjectOpenNext(projectName, "z", 32)) {
-        PatchSet.Id id = c.currentPatchSetId();
+      Set<PatchSet.Id> toGet = Sets.newHashSetWithExpectedSize(limit);
+      for (ChangeData cd : queryRecentChanges(limit)) {
+        PatchSet.Id id = cd.change().currentPatchSetId();
         if (id != null) {
           toGet.add(id);
         }
@@ -164,6 +173,20 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
     return toInclude;
   }
 
+  private Iterable<ChangeData> queryRecentChanges(int limit)
+      throws OrmException {
+    QueryProcessor qp = queryProcessor.get();
+    qp.setLimit(limit);
+    ChangeQueryBuilder qb = qp.getQueryBuilder();
+    Predicate<ChangeData> p =
+        Predicate.and(qb.project(projectName.get()), qb.status_open());
+    try {
+      return qp.queryChanges(p).changes();
+    } catch (QueryParseException e) {
+      throw new OrmException(e);
+    }
+  }
+
   private static boolean skip(String name) {
     return name.startsWith(RefNames.REFS_CHANGES)
         || name.startsWith(RefNames.REFS_CACHE_AUTOMERGE)

gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java

@@ -46,7 +46,11 @@ public class QueryProcessor {
       .getMax();
   }
 
-  void setLimit(int n) {
+  public ChangeQueryBuilder getQueryBuilder() {
+    return queryBuilder;
+  }
+
+  public void setLimit(int n) {
     limitFromCaller = n;
   }