Merge "Move ReceiveCommits.advertiseHistory into hook"

gerrit-httpd/src/main/java/com/google/gerrit/httpd/GitOverHttpServlet.java

@@ -309,9 +309,6 @@ public class GitOverHttpServlet extends GitServlet {
       }
 
       if (!rp.isCheckReferencedObjectsAreReachable()) {
-        if (isGet) {
-          rc.advertiseHistory();
-        }
         chain.doFilter(request, response);
         return;
       }
@@ -326,7 +323,6 @@ public class GitOverHttpServlet extends GitServlet {
           projectName);
 
       if (isGet) {
-        rc.advertiseHistory();
         cache.invalidate(cacheKey);
       } else {
         Set<ObjectId> ids = cache.getIfPresent(cacheKey);

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

@@ -404,7 +404,8 @@ public class ReceiveCommits {
       }
     });
     advHooks.add(rp.getAdvertiseRefsHook());
-    advHooks.add(new ReceiveCommitsAdvertiseRefsHook());
+    advHooks.add(new ReceiveCommitsAdvertiseRefsHook(
+        db, projectControl.getProject().getNameKey()));
     rp.setAdvertiseRefsHook(AdvertiseRefsHookChain.newChain(advHooks));
   }
 
@@ -439,80 +440,6 @@ public class ReceiveCommits {
     return rp;
   }
 
-  /** Scan part of history and include it in the advertisement. */
-  public void advertiseHistory() {
-    Set<ObjectId> toInclude = new HashSet<ObjectId>();
-
-    // Advertise some recent open changes, in case a commit is based one.
-    try {
-      Set<PatchSet.Id> toGet = new HashSet<PatchSet.Id>();
-      for (Change change : db.changes()
-          .byProjectOpenNext(project.getNameKey(), "z", 32)) {
-        PatchSet.Id id = change.currentPatchSetId();
-        if (id != null) {
-          toGet.add(id);
-        }
-      }
-      for (PatchSet ps : db.patchSets().get(toGet)) {
-        if (ps.getRevision() != null && ps.getRevision().get() != null) {
-          toInclude.add(ObjectId.fromString(ps.getRevision().get()));
-        }
-      }
-    } catch (OrmException err) {
-      log.error("Cannot list open changes of " + project.getNameKey(), err);
-    }
-
-    // Size of an additional ".have" line.
-    final int haveLineLen = 4 + Constants.OBJECT_ID_STRING_LENGTH + 1 + 5 + 1;
-
-    // Maximum number of bytes to "waste" in the advertisement with
-    // a peek at this repository's current reachable history.
-    final int maxExtraSize = 8192;
-
-    // Number of recent commits to advertise immediately, hoping to
-    // show a client a nearby merge base.
-    final int base = 64;
-
-    // Number of commits to skip once base has already been shown.
-    final int step = 16;
-
-    // Total number of commits to extract from the history.
-    final int max = maxExtraSize / haveLineLen;
-
-    // Scan history until the advertisement is full.
-    Set<ObjectId> alreadySending = rp.getAdvertisedObjects();
-    RevWalk rw = rp.getRevWalk();
-    for (ObjectId haveId : alreadySending) {
-      try {
-        rw.markStart(rw.parseCommit(haveId));
-      } catch (IOException badCommit) {
-        continue;
-      }
-    }
-
-    int stepCnt = 0;
-    RevCommit c;
-    try {
-      while ((c = rw.next()) != null && toInclude.size() < max) {
-        if (alreadySending.contains(c)) {
-        } else if (toInclude.contains(c)) {
-        } else if (c.getParentCount() > 1) {
-        } else if (toInclude.size() < base) {
-          toInclude.add(c);
-        } else {
-          stepCnt = ++stepCnt % step;
-          if (stepCnt == 0) {
-            toInclude.add(c);
-          }
-        }
-      }
-    } catch (IOException err) {
-      log.error("Error trying to advertise history on " + project.getNameKey(), err);
-    }
-    rw.reset();
-    rp.getAdvertisedObjects().addAll(toInclude);
-  }
-
   /** Determine if the user can upload commits. */
   public Capable canUpload() {
     Capable result = projectControl.canPushToAtLeastOneRef();

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

@@ -15,17 +15,43 @@
 package com.google.gerrit.server.git;
 
 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.server.ReviewDb;
 import com.google.gerrit.server.util.MagicBranch;
+import com.google.gwtorm.server.OrmException;
 
+import org.eclipse.jgit.lib.Constants;
+import org.eclipse.jgit.lib.ObjectId;
 import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.revwalk.RevCommit;
+import org.eclipse.jgit.revwalk.RevWalk;
 import org.eclipse.jgit.transport.AdvertiseRefsHook;
 import org.eclipse.jgit.transport.BaseReceivePack;
 import org.eclipse.jgit.transport.UploadPack;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
+import java.io.IOException;
 import java.util.Map;
+import java.util.Set;
 
 /** Exposes only the non refs/changes/ reference names. */
 public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
+  private static final Logger log = LoggerFactory
+      .getLogger(ReceiveCommitsAdvertiseRefsHook.class);
+
+  private final ReviewDb db;
+  private final Project.NameKey projectName;
+
+  public ReceiveCommitsAdvertiseRefsHook(ReviewDb db,
+      Project.NameKey projectName) {
+    this.db = db;
+    this.projectName = projectName;
+  }
+
   @Override
   public void advertiseRefs(UploadPack us) {
     throw new UnsupportedOperationException(
@@ -45,7 +71,84 @@ public class ReceiveCommitsAdvertiseRefsHook implements AdvertiseRefsHook {
         r.put(name, e.getValue());
       }
     }
-    rp.setAdvertisedRefs(r, rp.getAdvertisedObjects());
+    rp.setAdvertisedRefs(r, advertiseHistory(r.values(), rp));
+  }
+
+  private Set<ObjectId> advertiseHistory(
+      Iterable<Ref> sending,
+      BaseReceivePack rp) {
+    Set<ObjectId> toInclude = Sets.newHashSet();
+
+    // Advertise some recent open changes, in case a commit is based one.
+    try {
+      Set<PatchSet.Id> toGet = Sets.newHashSetWithExpectedSize(32);
+      for (Change c : db.changes().byProjectOpenNext(projectName, "z", 32)) {
+        PatchSet.Id id = c.currentPatchSetId();
+        if (id != null) {
+          toGet.add(id);
+        }
+      }
+      for (PatchSet ps : db.patchSets().get(toGet)) {
+        if (ps.getRevision() != null && ps.getRevision().get() != null) {
+          toInclude.add(ObjectId.fromString(ps.getRevision().get()));
+        }
+      }
+    } catch (OrmException err) {
+      log.error("Cannot list open changes of " + projectName, err);
+    }
+
+    // Size of an additional ".have" line.
+    final int haveLineLen = 4 + Constants.OBJECT_ID_STRING_LENGTH + 1 + 5 + 1;
+
+    // Maximum number of bytes to "waste" in the advertisement with
+    // a peek at this repository's current reachable history.
+    final int maxExtraSize = 8192;
+
+    // Number of recent commits to advertise immediately, hoping to
+    // show a client a nearby merge base.
+    final int base = 64;
+
+    // Number of commits to skip once base has already been shown.
+    final int step = 16;
+
+    // Total number of commits to extract from the history.
+    final int max = maxExtraSize / haveLineLen;
+
+    // Scan history until the advertisement is full.
+    Set<ObjectId> alreadySending = Sets.newHashSet();
+    RevWalk rw = rp.getRevWalk();
+    for (Ref ref : sending) {
+      try {
+        if (ref.getObjectId() != null) {
+          alreadySending.add(ref.getObjectId());
+          rw.markStart(rw.parseCommit(ref.getObjectId()));
+        }
+      } catch (IOException badCommit) {
+        continue;
+      }
+    }
+
+    int stepCnt = 0;
+    RevCommit c;
+    try {
+      while ((c = rw.next()) != null && toInclude.size() < max) {
+        if (alreadySending.contains(c)) {
+        } else if (toInclude.contains(c)) {
+        } else if (c.getParentCount() > 1) {
+        } else if (toInclude.size() < base) {
+          toInclude.add(c);
+        } else {
+          stepCnt = ++stepCnt % step;
+          if (stepCnt == 0) {
+            toInclude.add(c);
+          }
+        }
+      }
+    } catch (IOException err) {
+      log.error("Error trying to advertise history on " + projectName, err);
+    }
+    rw.reset();
+    return toInclude;
   }
 
   private static boolean skip(String name) {

gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/Receive.java

@@ -97,7 +97,6 @@ final class Receive extends AbstractGitCommand {
     rp.setMaxObjectSizeLimit(config.getEffectiveMaxObjectSizeLimit(
         projectControl.getProjectState()));
     try {
-      receive.advertiseHistory();
       rp.receive(in, out, err);
     } catch (UnpackException badStream) {
       // In case this was caused by the user pushing an object whose size