Move addition of refs/users/self symlink out of filtering logic

Gerrit moved to a model where it wraps around RefDatabase to tell JGit
which refs are visible to a user. This was done in I0bb85de2b. This
meant moving filtering code from a AdvertiseRefsHook into a wrapper of
RefDatabase.

There is one case in which the logic that got moved would not only
filter refs but add a ref in addition that is not present in the input:
when authenticated users interact with All-Users. In this case, Gerrit
adds a refs/users/self symbolic reference to make it easier to interact
with one's account data.

This led to a problem with the new inferface and smelled as a whole
since concerns weren't clearly separated. The problem with the new
interface would occur when using RefDatabase#exactRef - a method that
expects {0,1} refs but got two for All-Users since refs/users/self was
added.

This change fixes the issue by separating the addition of the symref
into an AdvertiseRefsHook and leaving ref filtering by just that.

RefAdvertisementIT has different tests for testing the advertisement of
refs/users/self and PushAccountIT covers push cases. The tests succeeded
before, because the problem could only be encountered when the in-memory
ref cache on the push path is not used. We add a config to run push tests
without in-memory ref cache to make sure the code path that failed
previosuly gets executed now.

Bug: Issue 12027
Change-Id: I07777b698ddb86ad5cec1407ac5671874866fee5

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

@@ -34,6 +34,7 @@ import com.google.gerrit.metrics.MetricMaker;
 import com.google.gerrit.metrics.Timer1;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.PublishCommentsOp;
+import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.config.ConfigUtil;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.config.ReceiveCommitsExecutor;
@@ -41,6 +42,7 @@ import com.google.gerrit.server.git.MultiProgressMonitor;
 import com.google.gerrit.server.git.PermissionAwareRepositoryManager;
 import com.google.gerrit.server.git.ProjectRunnable;
 import com.google.gerrit.server.git.TransferConfig;
+import com.google.gerrit.server.git.UsersSelfAdvertiseRefsHook;
 import com.google.gerrit.server.logging.Metadata;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
@@ -269,6 +271,8 @@ public class AsyncReceiveCommits implements PreReceiveHook {
       ContributorAgreementsChecker contributorAgreements,
       Metrics metrics,
       QuotaBackend quotaBackend,
+      UsersSelfAdvertiseRefsHook usersSelfAdvertiseRefsHook,
+      AllUsersName allUsersName,
       @Named(TIMEOUT_NAME) long timeoutMillis,
       @Assisted ProjectState projectState,
       @Assisted IdentifiedUser user,
@@ -313,7 +317,12 @@ public class AsyncReceiveCommits implements PreReceiveHook {
     allRefsWatcher = new AllRefsWatcher();
     receivePack.setAdvertiseRefsHook(
         ReceiveCommitsAdvertiseRefsHookChain.create(
-            allRefsWatcher, queryProvider, projectName, user.getAccountId()));
+            allRefsWatcher,
+            usersSelfAdvertiseRefsHook,
+            allUsersName,
+            queryProvider,
+            projectName,
+            user.getAccountId()));
     resultChangeIds = new ResultChangeIds();
     receiveCommits =
         factory.create(

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

@@ -17,8 +17,13 @@ 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.CurrentUser;
+import com.google.gerrit.server.config.AllUsersName;
+import com.google.gerrit.server.config.AllUsersNameProvider;
+import com.google.gerrit.server.git.UsersSelfAdvertiseRefsHook;
 import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.inject.Provider;
+import com.google.inject.util.Providers;
 import java.util.ArrayList;
 import java.util.List;
 import org.eclipse.jgit.transport.AdvertiseRefsHook;
@@ -35,10 +40,19 @@ public class ReceiveCommitsAdvertiseRefsHookChain {
    */
   public static AdvertiseRefsHook create(
       AllRefsWatcher allRefsWatcher,
+      UsersSelfAdvertiseRefsHook usersSelfAdvertiseRefsHook,
+      AllUsersName allUsersName,
       Provider<InternalChangeQuery> queryProvider,
       Project.NameKey projectName,
       Account.Id user) {
-    return create(allRefsWatcher, queryProvider, projectName, user, false);
+    return create(
+        allRefsWatcher,
+        usersSelfAdvertiseRefsHook,
+        allUsersName,
+        queryProvider,
+        projectName,
+        user,
+        false);
   }
 
   /**
@@ -49,12 +63,21 @@ public class ReceiveCommitsAdvertiseRefsHookChain {
    */
   @VisibleForTesting
   public static AdvertiseRefsHook createForTest(
-      Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName, Account.Id user) {
-    return create(new AllRefsWatcher(), queryProvider, projectName, user, true);
+      Provider<InternalChangeQuery> queryProvider, Project.NameKey projectName, CurrentUser user) {
+    return create(
+        new AllRefsWatcher(),
+        new UsersSelfAdvertiseRefsHook(Providers.of(user)),
+        new AllUsersName(AllUsersNameProvider.DEFAULT),
+        queryProvider,
+        projectName,
+        user.getAccountId(),
+        true);
   }
 
   private static AdvertiseRefsHook create(
       AllRefsWatcher allRefsWatcher,
+      UsersSelfAdvertiseRefsHook usersSelfAdvertiseRefsHook,
+      AllUsersName allUsersName,
       Provider<InternalChangeQuery> queryProvider,
       Project.NameKey projectName,
       Account.Id user,
@@ -65,6 +88,9 @@ public class ReceiveCommitsAdvertiseRefsHookChain {
     if (!skipHackPushNegotiateHook) {
       advHooks.add(new HackPushNegotiateHook());
     }
+    if (projectName.equals(allUsersName)) {
+      advHooks.add(usersSelfAdvertiseRefsHook);
+    }
     return AdvertiseRefsHookChain.newChain(advHooks);
   }
 }