Fix ProjectControlTest setup and reset RevWalk before calling IncludedInResolver#includedIn()

There are two bugs in this fix:
1. The permission rules are not setup correctly in ProjectControlTest
2. RevWalk does not reset before calling includedIn method

The two bugs are covering each other, so the test was passed even before this
fix. Add one more test to unveil the two bugs at the same time and have to fix
them together.

Two more things need to be fixed in ProjectControlTest:
1. Need remove all READ permission in allProjects except for
Administration Group
2. Have to create an admin account before creating a regular registered
user account, because the first account is always assumed to be admin
account in AccountManager#create().

Bug: Issue 4161
Change-Id: Idbac9a904ecca48d4d501713d4b0a43ea69c7360

gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java

@@ -16,19 +16,23 @@ package com.google.gerrit.server.project;
 
 import static com.google.gerrit.common.data.Permission.READ;
 import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
-import static com.google.gerrit.server.project.Util.allow;
-import static com.google.gerrit.server.project.Util.deny;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
 
+import com.google.gerrit.common.data.AccessSection;
+import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.lifecycle.LifecycleManager;
 import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountManager;
 import com.google.gerrit.server.account.AuthRequest;
+import com.google.gerrit.server.account.GroupCache;
+import com.google.gerrit.server.config.AllProjectsName;
+import com.google.gerrit.server.git.MetaDataUpdate;
 import com.google.gerrit.server.git.ProjectConfig;
 import com.google.gerrit.server.schema.SchemaCreator;
 import com.google.gerrit.server.util.RequestContext;
@@ -60,12 +64,17 @@ public class ProjectControlTest {
   @Inject private ProjectControl.GenericFactory projectControlFactory;
   @Inject private SchemaCreator schemaCreator;
   @Inject private ThreadLocalRequestContext requestContext;
+  @Inject protected ProjectCache projectCache;
+  @Inject protected MetaDataUpdate.Server metaDataUpdateFactory;
+  @Inject protected AllProjectsName allProjects;
+  @Inject protected GroupCache groupCache;
 
   private LifecycleManager lifecycle;
   private ReviewDb db;
   private TestRepository<InMemoryRepository> repo;
   private ProjectConfig project;
   private IdentifiedUser user;
+  private AccountGroup.UUID admins;
 
   @Before
   public void setUp() throws Exception {
@@ -77,6 +86,13 @@ public class ProjectControlTest {
 
     db = schemaFactory.open();
     schemaCreator.create(db);
+    // Need to create at least one user to be admin before creating a "normal"
+    // registered user.
+    // See AccountManager#create().
+    accountManager.authenticate(AuthRequest.forUser("admin")).getAccountId();
+    admins = groupCache.get(new AccountGroup.NameKey("Administrators")).getGroupUUID();
+    setUpPermissions();
+
     Account.Id userId = accountManager.authenticate(AuthRequest.forUser("user"))
         .getAccountId();
     user = userFactory.create(userId);
@@ -124,6 +140,20 @@ public class ProjectControlTest {
     assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(id)));
   }
 
+  @Test
+  public void canReadCommitIfTwoRefsVisible() throws Exception {
+    allow(project, READ, REGISTERED_USERS, "refs/heads/branch1");
+    allow(project, READ, REGISTERED_USERS, "refs/heads/branch2");
+
+    ObjectId id1 = repo.branch("branch1").commit().create();
+    ObjectId id2 = repo.branch("branch2").commit().create();
+
+    ProjectControl pc = newProjectControl();
+    RevWalk rw = repo.getRevWalk();
+    assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(id1)));
+    assertTrue(pc.canReadCommit(db, rw, rw.parseCommit(id2)));
+  }
+
   @Test
   public void canReadCommitIfRefVisible() throws Exception {
     allow(project, READ, REGISTERED_USERS, "refs/heads/branch1");
@@ -192,4 +222,36 @@ public class ProjectControlTest {
   private ProjectControl newProjectControl() throws Exception {
     return projectControlFactory.controlFor(project.getName(), user);
   }
+
+  protected void allow(ProjectConfig project, String permission,
+      AccountGroup.UUID id, String ref)
+      throws Exception {
+    Util.allow(project, permission, id, ref);
+    saveProjectConfig(project);
+  }
+
+  protected void deny(ProjectConfig project, String permission,
+      AccountGroup.UUID id, String ref)
+      throws Exception {
+    Util.deny(project, permission, id, ref);
+    saveProjectConfig(project);
+  }
+
+  protected void saveProjectConfig(ProjectConfig cfg) throws Exception {
+    try (MetaDataUpdate md = metaDataUpdateFactory.create(cfg.getName())) {
+      cfg.commit(md);
+    }
+    projectCache.evict(cfg.getProject());
+  }
+
+  private void setUpPermissions() throws Exception {
+    // Remove read permissions for all users besides admin, because by default
+    // Anonymous user group has ALLOW READ permission in refs/*.
+    // This method is idempotent, so is safe to call on every test setup.
+    ProjectConfig pc = projectCache.checkedGet(allProjects).getConfig();
+    for (AccessSection sec : pc.getAccessSections()) {
+      sec.removePermission(Permission.READ);
+    }
+    allow(pc, Permission.READ, admins, "refs/*");
+  }
 }