ReceiveCommitsAdvertiseRefsHook: Advertise the user's open changes

ReceiveCommitsAdvertiseRefsHook works around the fact that git is lacking
negotiation upon push. It adds the last 32 open changes for the repo
to .haves.

This is a heuristical approach. The user is much more likely to have
changes in their local repo that they own rather than the last 32
changes open changes. This commit changes the logic to account for
that.

Change-Id: Ie4261fab9365c2df78d73d5e8d0952f64aa7be38

java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java

@@ -310,7 +310,8 @@ public class AsyncReceiveCommits implements PreReceiveHook {
 
     allRefsWatcher = new AllRefsWatcher();
     receivePack.setAdvertiseRefsHook(
-        ReceiveCommitsAdvertiseRefsHookChain.create(allRefsWatcher, queryProvider, projectName));
+        ReceiveCommitsAdvertiseRefsHookChain.create(
+            allRefsWatcher, queryProvider, projectName, user.getAccountId()));
     resultChangeIds = new ResultChangeIds();
     receiveCommits =
         factory.create(

java/com/google/gerrit/server/git/receive/BUILD

@@ -15,6 +15,7 @@ java_library(
         "//java/com/google/gerrit/exceptions",
         "//java/com/google/gerrit/extensions:api",
         "//java/com/google/gerrit/git",
+        "//java/com/google/gerrit/index",
         "//java/com/google/gerrit/metrics",
         "//java/com/google/gerrit/server",
         "//java/com/google/gerrit/server/logging",

java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHook.java

@@ -18,14 +18,19 @@ import static com.google.common.collect.ImmutableList.toImmutableList;
 
 import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.PatchSet;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.server.git.HookUtil;
 import com.google.gerrit.server.index.change.ChangeField;
 import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeStatusPredicate;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gerrit.server.query.change.OwnerPredicate;
+import com.google.gerrit.server.query.change.ProjectPredicate;
 import com.google.gerrit.server.util.MagicBranch;
 import com.google.inject.Provider;
 import java.io.IOException;
@@ -65,11 +70,13 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
 
   private final Provider<InternalChangeQuery> queryProvider;
   private final Project.NameKey projectName;
+  private final Account.Id user;
 
   public ReceiveCommitsAdvertiseRefsHook(
-      Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName) {
+      Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName, Account.Id user) {
     this.queryProvider = queryProvider;
     this.projectName = projectName;
+    this.user = user;
   }
 
   @Override
@@ -90,7 +97,9 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
 
   private Set<ObjectId> advertiseOpenChanges(Repository repo)
       throws ServiceMayNotContinueException {
-    // Advertise some recent open changes, in case a commit is based on one.
+    // Advertise the user's most recent open changes. It's likely that the user has one of these in
+    // their local repo and they can serve as starting points to figure out the common ancestor of
+    // what the client and server have in common.
     int limit = 32;
     try {
       Set<ObjectId> r = Sets.newHashSetWithExpectedSize(limit);
@@ -105,7 +114,11 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
                   ChangeField.PATCH_SET)
               .enforceVisibility(true)
               .setLimit(limit)
-              .byProjectOpen(projectName)) {
+              .query(
+                  Predicate.and(
+                      new ProjectPredicate(projectName.get()),
+                      ChangeStatusPredicate.open(),
+                      new OwnerPredicate(user)))) {
         PatchSet ps = cd.currentPatchSet();
         if (ps != null) {
           // Ensure we actually observed a patch set ref pointing to this

java/com/google/gerrit/server/git/receive/ReceiveCommitsAdvertiseRefsHookChain.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.git.receive;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.gerrit.entities.Account;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.inject.Provider;
@@ -35,8 +36,9 @@ public class ReceiveCommitsAdvertiseRefsHookChain {
   public static AdvertiseRefsHook create(
       AllRefsWatcher allRefsWatcher,
       Provider<InternalChangeQuery> queryProvider,
-      Project.NameKey projectName) {
+      Project.NameKey projectName,
-    return create(allRefsWatcher, queryProvider, projectName, false);
+      Account.Id user) {
+    return create(allRefsWatcher, queryProvider, projectName, user, false);
   }
 
   /**
@@ -47,18 +49,19 @@ public class ReceiveCommitsAdvertiseRefsHookChain {
    */
   @VisibleForTesting
   public static AdvertiseRefsHook createForTest(
-      Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName) {
+      Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName, Account.Id user) {
-    return create(new AllRefsWatcher(), queryProvider, projectName, true);
+    return create(new AllRefsWatcher(), queryProvider, projectName, user, true);
   }
 
   private static AdvertiseRefsHook create(
       AllRefsWatcher allRefsWatcher,
       Provider<InternalChangeQuery> queryProvider,
       Project.NameKey projectName,
+      Account.Id user,
       boolean skipHackPushNegotiateHook) {
     List<AdvertiseRefsHook> advHooks = new ArrayList<>();
     advHooks.add(allRefsWatcher);
-    advHooks.add(new ReceiveCommitsAdvertiseRefsHook(queryProvider, projectName));
+    advHooks.add(new ReceiveCommitsAdvertiseRefsHook(queryProvider, projectName, user));
     if (!skipHackPushNegotiateHook) {
       advHooks.add(new HackPushNegotiateHook());
     }

javatests/com/google/gerrit/acceptance/git/RefAdvertisementIT.java

@@ -1446,7 +1446,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
   private TestRefAdvertiser.Result getReceivePackRefs() throws Exception {
     try (Repository repo = repoManager.openRepository(project)) {
       AdvertiseRefsHook adv =
-          ReceiveCommitsAdvertiseRefsHookChain.createForTest(queryProvider, project);
+          ReceiveCommitsAdvertiseRefsHookChain.createForTest(queryProvider, project, admin.id());
       ReceivePack rp = new ReceivePack(repo);
       rp.setAdvertiseRefsHook(adv);
       TestRefAdvertiser advertiser = new TestRefAdvertiser(repo);