Merge "Consolidate reachable commit in CommitsCollection"

* submodules:
* Update plugins/replication from branch 'master'
  - Migrate calls to ChangeControl#isVisible to PermissionBackend
    
    Change-Id: I740e70aa799b5cc5e74ed86054876d1baa753373

gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java

@@ -23,6 +23,7 @@ import com.google.gerrit.extensions.api.projects.CommentLinkInfo;
 import com.google.gerrit.extensions.config.FactoryModule;
 import com.google.gerrit.extensions.registration.DynamicMap;
 import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.extensions.restapi.RestView;
 import com.google.gerrit.reviewdb.client.AccountGroup;
 import com.google.gerrit.rules.PrologModule;
 import com.google.gerrit.server.CurrentUser;
@@ -63,6 +64,7 @@ import com.google.gerrit.server.notedb.NoteDbModule;
 import com.google.gerrit.server.patch.DiffExecutorModule;
 import com.google.gerrit.server.patch.PatchListCacheImpl;
 import com.google.gerrit.server.project.CommentLinkProvider;
+import com.google.gerrit.server.project.CommitResource;
 import com.google.gerrit.server.project.DefaultPermissionBackendModule;
 import com.google.gerrit.server.project.ProjectCacheImpl;
 import com.google.gerrit.server.project.ProjectState;
@@ -115,6 +117,8 @@ public class BatchProgramModule extends FactoryModule {
         .in(SINGLETON);
     bind(new TypeLiteral<DynamicMap<ChangeQueryProcessor.ChangeAttributeFactory>>() {})
         .toInstance(DynamicMap.<ChangeQueryProcessor.ChangeAttributeFactory>emptyMap());
+    bind(new TypeLiteral<DynamicMap<RestView<CommitResource>>>() {})
+        .toInstance(DynamicMap.<RestView<CommitResource>>emptyMap());
     bind(String.class)
         .annotatedWith(CanonicalWebUrl.class)
         .toProvider(CanonicalWebUrlProvider.class);

gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java

@@ -56,9 +56,11 @@ import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.RefPermission;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.CommitsCollection;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.ProjectControl;
 import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.project.ProjectsCollection;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.RetryHelper;
@@ -100,6 +102,7 @@ public class CreateChange
   private final PermissionBackend permissionBackend;
   private final Provider<CurrentUser> user;
   private final ProjectsCollection projectsCollection;
+  private final CommitsCollection commits;
   private final ChangeInserter.Factory changeInserterFactory;
   private final ChangeJson.Factory jsonFactory;
   private final ChangeFinder changeFinder;
@@ -121,6 +124,7 @@ public class CreateChange
       PermissionBackend permissionBackend,
       Provider<CurrentUser> user,
       ProjectsCollection projectsCollection,
+      CommitsCollection commits,
       ChangeInserter.Factory changeInserterFactory,
       ChangeJson.Factory json,
       ChangeFinder changeFinder,
@@ -139,6 +143,7 @@ public class CreateChange
     this.permissionBackend = permissionBackend;
     this.user = user;
     this.projectsCollection = projectsCollection;
+    this.commits = commits;
     this.changeInserterFactory = changeInserterFactory;
     this.jsonFactory = json;
     this.changeFinder = changeFinder;
@@ -311,12 +316,13 @@ public class CreateChange
       throw new BadRequestException("merge.source must be non-empty");
     }
 
+    ProjectState state = projectControl.getProjectState();
     RevCommit sourceCommit = MergeUtil.resolveCommit(repo, rw, merge.source);
-    if (!projectControl.canReadCommit(db.get(), repo, sourceCommit)) {
+    if (!commits.canRead(state, repo, sourceCommit)) {
       throw new BadRequestException("do not have read permission for: " + merge.source);
     }
 
-    MergeUtil mergeUtil = mergeUtilFactory.create(projectControl.getProjectState());
+    MergeUtil mergeUtil = mergeUtilFactory.create(state);
     // default merge strategy from project settings
     String mergeStrategy =
         MoreObjects.firstNonNull(

gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java

@@ -43,8 +43,10 @@ import com.google.gerrit.server.git.MergeUtil;
 import com.google.gerrit.server.permissions.ChangePermission;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ChangeControl;
+import com.google.gerrit.server.project.CommitsCollection;
 import com.google.gerrit.server.project.InvalidChangeOperationException;
 import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.server.update.BatchUpdate;
 import com.google.gerrit.server.update.RetryHelper;
 import com.google.gerrit.server.update.RetryingRestModifyView;
@@ -69,9 +71,9 @@ import org.eclipse.jgit.util.ChangeIdUtil;
 @Singleton
 public class CreateMergePatchSet
     extends RetryingRestModifyView<ChangeResource, MergePatchSetInput, Response<ChangeInfo>> {
-
   private final Provider<ReviewDb> db;
   private final GitRepositoryManager gitManager;
+  private final CommitsCollection commits;
   private final TimeZone serverTimeZone;
   private final Provider<CurrentUser> user;
   private final ChangeJson.Factory jsonFactory;
@@ -83,6 +85,7 @@ public class CreateMergePatchSet
   CreateMergePatchSet(
       Provider<ReviewDb> db,
       GitRepositoryManager gitManager,
+      CommitsCollection commits,
       @GerritPersonIdent PersonIdent myIdent,
       Provider<CurrentUser> user,
       ChangeJson.Factory json,
@@ -93,6 +96,7 @@ public class CreateMergePatchSet
     super(retryHelper);
     this.db = db;
     this.gitManager = gitManager;
+    this.commits = commits;
     this.serverTimeZone = myIdent.getTimeZone();
     this.user = user;
     this.jsonFactory = json;
@@ -116,6 +120,7 @@ public class CreateMergePatchSet
     ChangeControl ctl = rsrc.getControl();
     PatchSet ps = psUtil.current(db.get(), ctl.getNotes());
     ProjectControl projectControl = ctl.getProjectControl();
+    ProjectState state = projectControl.getProjectState();
     Change change = ctl.getChange();
     Project.NameKey project = change.getProject();
     Branch.NameKey dest = change.getDest();
@@ -125,7 +130,7 @@ public class CreateMergePatchSet
         RevWalk rw = new RevWalk(reader)) {
 
       RevCommit sourceCommit = MergeUtil.resolveCommit(git, rw, merge.source);
-      if (!projectControl.canReadCommit(db.get(), git, sourceCommit)) {
+      if (!commits.canRead(state, git, sourceCommit)) {
         throw new ResourceNotFoundException(
             "cannot find source commit: " + merge.source + " to merge.");
       }

gerrit-server/src/main/java/com/google/gerrit/server/project/CheckMergeability.java

@@ -19,13 +19,11 @@ import com.google.gerrit.extensions.common.MergeableInfo;
 import com.google.gerrit.extensions.restapi.BadRequestException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.git.InMemoryInserter;
 import com.google.gerrit.server.git.MergeUtil;
 import com.google.inject.Inject;
-import com.google.inject.Provider;
 import java.io.IOException;
 import org.eclipse.jgit.lib.Config;
 import org.eclipse.jgit.lib.ObjectInserter;
@@ -39,11 +37,9 @@ import org.kohsuke.args4j.Option;
 
 /** Check the mergeability at current branch for a git object references expression. */
 public class CheckMergeability implements RestReadView<BranchResource> {
-
   private String source;
   private String strategy;
   private SubmitType submitType;
-  private final Provider<ReviewDb> db;
 
   @Option(
     name = "--source",
@@ -68,14 +64,15 @@ public class CheckMergeability implements RestReadView<BranchResource> {
   }
 
   private final GitRepositoryManager gitManager;
+  private final CommitsCollection commits;
 
   @Inject
   CheckMergeability(
-      GitRepositoryManager gitManager, @GerritServerConfig Config cfg, Provider<ReviewDb> db) {
+      GitRepositoryManager gitManager, CommitsCollection commits, @GerritServerConfig Config cfg) {
     this.gitManager = gitManager;
+    this.commits = commits;
     this.strategy = MergeUtil.getMergeStrategy(cfg).getName();
     this.submitType = cfg.getEnum("project", null, "submitType", SubmitType.MERGE_IF_NECESSARY);
-    this.db = db;
   }
 
   @Override
@@ -102,7 +99,7 @@ public class CheckMergeability implements RestReadView<BranchResource> {
       RevCommit targetCommit = rw.parseCommit(destRef.getObjectId());
       RevCommit sourceCommit = MergeUtil.resolveCommit(git, rw, source);
 
-      if (!resource.getControl().canReadCommit(db.get(), git, sourceCommit)) {
+      if (!commits.canRead(resource.getProjectState(), git, sourceCommit)) {
         throw new BadRequestException("do not have read permission for: " + source);
       }
 

gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java

@@ -19,33 +19,48 @@ import com.google.gerrit.extensions.restapi.ChildCollection;
 import com.google.gerrit.extensions.restapi.IdString;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestView;
-import com.google.gerrit.reviewdb.server.ReviewDb;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.change.IncludedInResolver;
 import com.google.gerrit.server.git.GitRepositoryManager;
+import com.google.gerrit.server.git.VisibleRefFilter;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
+import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
+import java.util.List;
+import java.util.Map;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
 import org.eclipse.jgit.errors.MissingObjectException;
 import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 @Singleton
 public class CommitsCollection implements ChildCollection<ProjectResource, CommitResource> {
+  private static final Logger log = LoggerFactory.getLogger(CommitsCollection.class);
+
   private final DynamicMap<RestView<CommitResource>> views;
   private final GitRepositoryManager repoManager;
-  private final Provider<ReviewDb> db;
+  private final VisibleRefFilter.Factory refFilter;
+  private final Provider<InternalChangeQuery> queryProvider;
 
   @Inject
   public CommitsCollection(
       DynamicMap<RestView<CommitResource>> views,
       GitRepositoryManager repoManager,
-      Provider<ReviewDb> db) {
+      VisibleRefFilter.Factory refFilter,
+      Provider<InternalChangeQuery> queryProvider) {
     this.views = views;
     this.repoManager = repoManager;
-    this.db = db;
+    this.refFilter = refFilter;
+    this.queryProvider = queryProvider;
   }
 
   @Override
@@ -67,7 +82,7 @@ public class CommitsCollection implements ChildCollection<ProjectResource, Commi
         RevWalk rw = new RevWalk(repo)) {
       RevCommit commit = rw.parseCommit(objectId);
       rw.parseBody(commit);
-      if (!parent.getControl().canReadCommit(db.get(), repo, commit)) {
+      if (!canRead(parent.getProjectState(), repo, commit)) {
         throw new ResourceNotFoundException(id);
       }
       for (int i = 0; i < commit.getParentCount(); i++) {
@@ -83,4 +98,37 @@ public class CommitsCollection implements ChildCollection<ProjectResource, Commi
   public DynamicMap<RestView<CommitResource>> views() {
     return views;
   }
+
+  /** @return true if {@code commit} is visible to the caller. */
+  public boolean canRead(ProjectState state, Repository repo, RevCommit commit) {
+    Project.NameKey project = state.getProject().getNameKey();
+
+    // Look for changes associated with the commit.
+    try {
+      List<ChangeData> changes =
+          queryProvider.get().enforceVisibility(true).byProjectCommit(project, commit);
+      if (!changes.isEmpty()) {
+        return true;
+      }
+    } catch (OrmException e) {
+      log.error("Cannot look up change for commit " + commit.name() + " in " + project, e);
+    }
+
+    return isReachableFrom(state, repo, commit, repo.getAllRefs());
+  }
+
+  public boolean isReachableFrom(
+      ProjectState state, Repository repo, RevCommit commit, Map<String, Ref> refs) {
+    try (RevWalk rw = new RevWalk(repo)) {
+      refs = refFilter.create(state, repo).filter(refs, true);
+      return IncludedInResolver.includedInAny(repo, rw, commit, refs.values());
+    } catch (IOException e) {
+      log.error(
+          String.format(
+              "Cannot verify permissions to commit object %s in repository %s",
+              commit.name(), state.getProject().getNameKey()),
+          e);
+      return false;
+    }
+  }
 }

gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java

@@ -17,10 +17,8 @@ package com.google.gerrit.server.project;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
 import com.google.gerrit.extensions.restapi.RestReadView;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.inject.Inject;
-import com.google.inject.Provider;
 import com.google.inject.Singleton;
 import java.io.IOException;
 import org.eclipse.jgit.errors.IncorrectObjectTypeException;
@@ -34,13 +32,13 @@ import org.eclipse.jgit.revwalk.RevWalk;
 
 @Singleton
 public class GetHead implements RestReadView<ProjectResource> {
-  private GitRepositoryManager repoManager;
+  private final GitRepositoryManager repoManager;
-  private Provider<ReviewDb> db;
+  private final CommitsCollection commits;
 
   @Inject
-  GetHead(GitRepositoryManager repoManager, Provider<ReviewDb> db) {
+  GetHead(GitRepositoryManager repoManager, CommitsCollection commits) {
     this.repoManager = repoManager;
-    this.db = db;
+    this.commits = commits;
   }
 
   @Override
@@ -59,7 +57,7 @@ public class GetHead implements RestReadView<ProjectResource> {
       } else if (head.getObjectId() != null) {
         try (RevWalk rw = new RevWalk(repo)) {
           RevCommit commit = rw.parseCommit(head.getObjectId());
-          if (rsrc.getControl().canReadCommit(db.get(), repo, commit)) {
+          if (commits.canRead(rsrc.getProjectState(), repo, commit)) {
             return head.getObjectId().name();
           }
           throw new AuthException("not allowed to see HEAD");

gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.project;
 
 import static com.google.common.base.Preconditions.checkArgument;
 
+import com.google.common.collect.Iterables;
 import com.google.common.collect.Maps;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.PageLinks;
@@ -39,11 +40,9 @@ 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.GroupMembership;
-import com.google.gerrit.server.change.IncludedInResolver;
 import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.GitReceivePackGroups;
 import com.google.gerrit.server.config.GitUploadPackGroups;
-import com.google.gerrit.server.git.VisibleRefFilter;
 import com.google.gerrit.server.group.SystemGroupBackend;
 import com.google.gerrit.server.notedb.ChangeNotes;
 import com.google.gerrit.server.permissions.FailedPermissionBackend;
@@ -55,7 +54,6 @@ import com.google.gerrit.server.permissions.PermissionBackend.ForRef;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.query.change.ChangeData;
-import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gwtorm.server.OrmException;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
@@ -71,10 +69,11 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.Ref;
+import org.eclipse.jgit.lib.RefDatabase;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
-import org.eclipse.jgit.revwalk.RevWalk;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -132,16 +131,14 @@ public class ProjectControl {
 
   private final Set<AccountGroup.UUID> uploadGroups;
   private final Set<AccountGroup.UUID> receiveGroups;
-
   private final String canonicalWebUrl;
   private final PermissionBackend.WithUser perm;
   private final CurrentUser user;
   private final ProjectState state;
+  private final CommitsCollection commits;
   private final ChangeControl.Factory changeControlFactory;
   private final PermissionCollection.Factory permissionFilter;
-  private final VisibleRefFilter.Factory refFilter;
   private final Collection<ContributorAgreement> contributorAgreements;
-  private final Provider<InternalChangeQuery> queryProvider;
   private final Metrics metrics;
 
   private List<SectionMatcher> allSections;
@@ -156,22 +153,20 @@ public class ProjectControl {
       @GitReceivePackGroups Set<AccountGroup.UUID> receiveGroups,
       ProjectCache pc,
       PermissionCollection.Factory permissionFilter,
+      CommitsCollection commits,
       ChangeControl.Factory changeControlFactory,
-      VisibleRefFilter.Factory refFilter,
-      Provider<InternalChangeQuery> queryProvider,
       @CanonicalWebUrl @Nullable String canonicalWebUrl,
       PermissionBackend permissionBackend,
       @Assisted CurrentUser who,
       @Assisted ProjectState ps,
       Metrics metrics) {
     this.changeControlFactory = changeControlFactory;
-    this.refFilter = refFilter;
     this.uploadGroups = uploadGroups;
     this.receiveGroups = receiveGroups;
     this.permissionFilter = permissionFilter;
+    this.commits = commits;
     this.contributorAgreements = pc.getAllProjects().getConfig().getContributorAgreements();
     this.canonicalWebUrl = canonicalWebUrl;
-    this.queryProvider = queryProvider;
     this.metrics = metrics;
     this.perm = permissionBackend.user(who);
     user = who;
@@ -483,50 +478,26 @@ public class ProjectControl {
     return false;
   }
 
-  /** @return whether a commit is visible to user. */
+  boolean isReachableFromHeadsOrTags(Repository repo, RevCommit commit) {
-  public boolean canReadCommit(ReviewDb db, Repository repo, RevCommit commit) {
-    // Look for changes associated with the commit.
     try {
-      List<ChangeData> changes =
+      RefDatabase refdb = repo.getRefDatabase();
-          queryProvider.get().byProjectCommit(getProject().getNameKey(), commit);
+      Collection<Ref> heads = refdb.getRefs(Constants.R_HEADS).values();
-      for (ChangeData change : changes) {
+      Collection<Ref> tags = refdb.getRefs(Constants.R_TAGS).values();
-        if (controlFor(db, change.change()).isVisible(db)) {
+      Map<String, Ref> refs = Maps.newHashMapWithExpectedSize(heads.size() + tags.size());
-          return true;
+      for (Ref r : Iterables.concat(heads, tags)) {
-        }
+        refs.put(r.getName(), r);
       }
-    } catch (OrmException e) {
+      return commits.isReachableFrom(state, repo, commit, refs);
-      log.error(
-          "Cannot look up change for commit " + commit.name() + " in " + getProject().getName(), e);
-    }
-    // Scan all visible refs.
-    return canReadCommitFromVisibleRef(repo, commit);
-  }
-
-  private boolean canReadCommitFromVisibleRef(Repository repo, RevCommit commit) {
-    try (RevWalk rw = new RevWalk(repo)) {
-      return isMergedIntoVisibleRef(repo, rw, commit, repo.getAllRefs().values());
     } catch (IOException e) {
-      String msg =
+      log.error(
           String.format(
               "Cannot verify permissions to commit object %s in repository %s",
-              commit.name(), getProject().getNameKey());
+              commit.name(), getProject().getNameKey()),
-      log.error(msg, e);
+          e);
       return false;
     }
   }
 
-  boolean isMergedIntoVisibleRef(
-      Repository repo, RevWalk rw, RevCommit commit, Collection<Ref> unfilteredRefs)
-      throws IOException {
-    VisibleRefFilter filter = refFilter.create(state, repo);
-    Map<String, Ref> m = Maps.newHashMapWithExpectedSize(unfilteredRefs.size());
-    for (Ref r : unfilteredRefs) {
-      m.put(r.getName(), r);
-    }
-    Map<String, Ref> refs = filter.filter(m, true);
-    return IncludedInResolver.includedInAny(repo, rw, commit, refs.values());
-  }
-
   ForProject asForProject() {
     return new ForProjectImpl();
   }

gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java

@@ -45,9 +45,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
-import org.eclipse.jgit.lib.Constants;
 import org.eclipse.jgit.lib.PersonIdent;
-import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevObject;
@@ -347,7 +345,7 @@ public class RefControl {
       // If the user has push permissions, they can create the ref regardless
       // of whether they are pushing any new objects along with the create.
       return null;
-    } else if (isMergedIntoBranchOrTag(repo, commit)) {
+    } else if (projectControl.isReachableFromHeadsOrTags(repo, commit)) {
       // If the user has no push permissions, check whether the object is
       // merged into a branch or tag readable by this user. If so, they are
       // not effectively "pushing" more objects, so they can create the ref
@@ -357,21 +355,6 @@ public class RefControl {
     return userId + " lacks permission " + Permission.PUSH + " for creating new commit object";
   }
 
-  private boolean isMergedIntoBranchOrTag(Repository repo, RevCommit commit) {
-    try (RevWalk rw = new RevWalk(repo)) {
-      List<Ref> refs = new ArrayList<>(repo.getRefDatabase().getRefs(Constants.R_HEADS).values());
-      refs.addAll(repo.getRefDatabase().getRefs(Constants.R_TAGS).values());
-      return projectControl.isMergedIntoVisibleRef(repo, rw, commit, refs);
-    } catch (IOException e) {
-      String msg =
-          String.format(
-              "Cannot verify permissions to commit object %s in repository %s",
-              commit.name(), projectControl.getProject().getNameKey());
-      log.error(msg, e);
-    }
-    return false;
-  }
-
   /**
    * Determines whether the user can delete the Git ref controlled by this object.
    *

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

@@ -55,19 +55,19 @@ import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
 
-/** Unit tests for {@link ProjectControl}. */
+/** Unit tests for {@link CommitsCollection}. */
-public class ProjectControlTest {
+public class CommitsCollectionTest {
   @Inject private AccountManager accountManager;
   @Inject private IdentifiedUser.GenericFactory userFactory;
   @Inject private InMemoryDatabase schemaFactory;
   @Inject private InMemoryRepositoryManager repoManager;
-  @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;
+  @Inject private CommitsCollection commits;
 
   private LifecycleManager lifecycle;
   private ReviewDb db;
@@ -135,11 +135,11 @@ public class ProjectControlTest {
   public void canReadCommitWhenAllRefsVisible() throws Exception {
     allow(project, READ, REGISTERED_USERS, "refs/*");
     ObjectId id = repo.branch("master").commit().create();
-    ProjectControl pc = newProjectControl();
+    ProjectState state = readProjectState();
     RevWalk rw = repo.getRevWalk();
     Repository r = repo.getRepository();
 
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(id)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(id)));
   }
 
   @Test
@@ -150,12 +150,12 @@ public class ProjectControlTest {
     ObjectId id1 = repo.branch("branch1").commit().create();
     ObjectId id2 = repo.branch("branch2").commit().create();
 
-    ProjectControl pc = newProjectControl();
+    ProjectState state = readProjectState();
     RevWalk rw = repo.getRevWalk();
     Repository r = repo.getRepository();
 
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(id1)));
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(id2)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(id2)));
   }
 
   @Test
@@ -166,12 +166,12 @@ public class ProjectControlTest {
     ObjectId id1 = repo.branch("branch1").commit().create();
     ObjectId id2 = repo.branch("branch2").commit().create();
 
-    ProjectControl pc = newProjectControl();
+    ProjectState state = readProjectState();
     RevWalk rw = repo.getRevWalk();
     Repository r = repo.getRepository();
 
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(id1)));
-    assertFalse(pc.canReadCommit(db, r, rw.parseCommit(id2)));
+    assertFalse(commits.canRead(state, r, rw.parseCommit(id2)));
   }
 
   @Test
@@ -185,11 +185,11 @@ public class ProjectControlTest {
     RevCommit parent2 = repo.commit().create();
     repo.branch("branch2").commit().parent(parent2).create();
 
-    ProjectControl pc = newProjectControl();
+    ProjectState state = readProjectState();
     RevWalk rw = repo.getRevWalk();
     Repository r = repo.getRepository();
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(parent1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
-    assertFalse(pc.canReadCommit(db, r, rw.parseCommit(parent2)));
+    assertFalse(commits.canRead(state, r, rw.parseCommit(parent2)));
   }
 
   @Test
@@ -199,16 +199,16 @@ public class ProjectControlTest {
     RevCommit parent1 = repo.commit().create();
     ObjectId id1 = repo.branch("branch1").commit().parent(parent1).create();
 
-    ProjectControl pc = newProjectControl();
+    ProjectState state = readProjectState();
     RevWalk rw = repo.getRevWalk();
     Repository r = repo.getRepository();
 
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(parent1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(id1)));
 
     repo.branch("branch1").update(parent1);
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(parent1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
-    assertFalse(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+    assertFalse(commits.canRead(state, r, rw.parseCommit(id1)));
   }
 
   @Test
@@ -218,20 +218,20 @@ public class ProjectControlTest {
     RevCommit parent1 = repo.commit().create();
     ObjectId id1 = repo.branch("branch1").commit().parent(parent1).create();
 
-    ProjectControl pc = newProjectControl();
+    ProjectState state = readProjectState();
     RevWalk rw = repo.getRevWalk();
     Repository r = repo.getRepository();
 
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(parent1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(id1)));
 
     repo.branch("branch1").update(parent1);
-    assertTrue(pc.canReadCommit(db, r, rw.parseCommit(parent1)));
+    assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
-    assertFalse(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+    assertFalse(commits.canRead(state, r, rw.parseCommit(id1)));
   }
 
-  private ProjectControl newProjectControl() throws Exception {
+  private ProjectState readProjectState() throws Exception {
-    return projectControlFactory.controlFor(project.getName(), user);
+    return projectCache.get(project.getName());
   }
 
   protected void allow(ProjectConfig project, String permission, AccountGroup.UUID id, String ref)

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

@@ -60,7 +60,6 @@ import com.google.gerrit.server.index.SingleVersionModule.SingleVersionListener;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.server.permissions.RefPermission;
-import com.google.gerrit.server.query.change.InternalChangeQuery;
 import com.google.gerrit.server.schema.SchemaCreator;
 import com.google.gerrit.server.util.RequestContext;
 import com.google.gerrit.server.util.ThreadLocalRequestContext;
@@ -212,7 +211,6 @@ public class RefControlTest {
   @Inject private SingleVersionListener singleVersionListener;
   @Inject private InMemoryDatabase schemaFactory;
   @Inject private ThreadLocalRequestContext requestContext;
-  @Inject private Provider<InternalChangeQuery> queryProvider;
   @Inject private ProjectControl.Metrics metrics;
 
   @Before
@@ -899,17 +897,14 @@ public class RefControlTest {
   }
 
   private ProjectControl user(ProjectConfig local, String name, AccountGroup.UUID... memberOf) {
-    String canonicalWebUrl = "http://localhost";
-
     return new ProjectControl(
         Collections.<AccountGroup.UUID>emptySet(),
         Collections.<AccountGroup.UUID>emptySet(),
         projectCache,
         sectionSorter,
+        null, // commitsCollection
         changeControlFactory,
-        null, // refFilter
+        "http://localhost", // canonicalWebUrl
-        queryProvider,
-        canonicalWebUrl,
         permissionBackend,
         new MockUser(name, memberOf),
         newProjectState(local),

gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java

@@ -20,6 +20,7 @@ import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.GitRepositoryManager;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ProjectControl;
+import com.google.gerrit.server.project.ProjectState;
 import com.google.gerrit.sshd.SshScope.Context;
 import com.google.inject.Inject;
 import java.io.IOException;
@@ -45,6 +46,8 @@ public abstract class AbstractGitCommand extends BaseCommand {
   @Inject private IdentifiedUser.GenericFactory userFactory;
 
   protected Repository repo;
+  protected ProjectState state;
+  protected Project.NameKey projectName;
   protected Project project;
 
   @Override
@@ -86,10 +89,12 @@ public abstract class AbstractGitCommand extends BaseCommand {
   }
 
   private void service() throws IOException, PermissionBackendException, Failure {
-    project = projectControl.getProjectState().getProject();
+    state = projectControl.getProjectState();
+    project = state.getProject();
+    projectName = project.getNameKey();
 
     try {
-      repo = repoManager.openRepository(project.getNameKey());
+      repo = repoManager.openRepository(projectName);
     } catch (RepositoryNotFoundException e) {
       throw new Failure(1, "fatal: '" + project.getName() + "': not a git archive", e);
     }

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

@@ -18,13 +18,13 @@ import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.collect.ImmutableMap;
 import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.change.AllowedFormats;
 import com.google.gerrit.server.change.ArchiveFormat;
 import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.project.CommitsCollection;
 import com.google.gerrit.sshd.AbstractGitCommand;
 import com.google.inject.Inject;
 import java.io.IOException;
@@ -121,9 +121,9 @@ public class UploadArchive extends AbstractGitCommand {
   }
 
   @Inject private PermissionBackend permissionBackend;
+  @Inject private CommitsCollection commits;
   @Inject private IdentifiedUser user;
   @Inject private AllowedFormats allowedFormats;
-  @Inject private ReviewDb db;
   private Options options = new Options();
 
   /**
@@ -244,13 +244,13 @@ public class UploadArchive extends AbstractGitCommand {
 
   private boolean canRead(ObjectId revId) throws IOException, PermissionBackendException {
     try {
-      permissionBackend.user(user).project(project.getNameKey()).check(ProjectPermission.READ);
+      permissionBackend.user(user).project(projectName).check(ProjectPermission.READ);
       return true;
     } catch (AuthException e) {
       // Check reachability of the specific revision.
       try (RevWalk rw = new RevWalk(repo)) {
         RevCommit commit = rw.parseCommit(revId);
-        return projectControl.canReadCommit(db, repo, commit);
+        return commits.canRead(state, repo, commit);
       }
     }
   }