Consolidate reachable commit in CommitsCollection
Moving the logic to determine if a commit is reachable into
CommitsCollection pulls most of it out of the legacy ProjectControl
and RefControl.
Improve the heads or tags code slightly by determining the size of the
two collections and presizing the map for that count. This avoids an
intermediate ArrayList copy.
Rename the related tests into CommitsCollectionTest.
Change-Id: I4c4a7624a4b50335509034a968c31da90e981795
diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
index 788f7ce..de1fda69 100644
--- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
+++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/util/BatchProgramModule.java
@@ -23,6 +23,7 @@
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.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 @@
.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);
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
index 92ad46d..10c4539 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateChange.java
@@ -56,9 +56,11 @@
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 @@
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 @@
PermissionBackend permissionBackend,
Provider<CurrentUser> user,
ProjectsCollection projectsCollection,
+ CommitsCollection commits,
ChangeInserter.Factory changeInserterFactory,
ChangeJson.Factory json,
ChangeFinder changeFinder,
@@ -139,6 +143,7 @@
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 @@
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(
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java
index b53bdd9..0b7d495 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/CreateMergePatchSet.java
@@ -43,8 +43,10 @@
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 @@
@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 @@
CreateMergePatchSet(
Provider<ReviewDb> db,
GitRepositoryManager gitManager,
+ CommitsCollection commits,
@GerritPersonIdent PersonIdent myIdent,
Provider<CurrentUser> user,
ChangeJson.Factory json,
@@ -93,6 +96,7 @@
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 @@
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 @@
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.");
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CheckMergeability.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CheckMergeability.java
index 1a81726..1ab2dbd 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CheckMergeability.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CheckMergeability.java
@@ -19,13 +19,11 @@
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 @@
/** 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 @@
}
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 @@
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);
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java
index d481c014..e38f442 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/CommitsCollection.java
@@ -19,33 +19,48 @@
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 @@
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 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;
+ }
+ }
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java
index 03db4f6..58a8987 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/GetHead.java
@@ -17,10 +17,8 @@
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 @@
@Singleton
public class GetHead implements RestReadView<ProjectResource> {
- private GitRepositoryManager repoManager;
- private Provider<ReviewDb> db;
+ private final GitRepositoryManager repoManager;
+ 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 @@
} 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");
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
index a1ab177..248d471 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
@@ -16,6 +16,7 @@
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.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.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.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 @@
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 @@
@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 @@
return false;
}
- /** @return whether a commit is visible to user. */
- public boolean canReadCommit(ReviewDb db, Repository repo, RevCommit commit) {
- // Look for changes associated with the commit.
+ boolean isReachableFromHeadsOrTags(Repository repo, RevCommit commit) {
try {
- List<ChangeData> changes =
- queryProvider.get().byProjectCommit(getProject().getNameKey(), commit);
- for (ChangeData change : changes) {
- if (controlFor(db, change.change()).isVisible(db)) {
- return true;
- }
+ RefDatabase refdb = repo.getRefDatabase();
+ Collection<Ref> heads = refdb.getRefs(Constants.R_HEADS).values();
+ Collection<Ref> tags = refdb.getRefs(Constants.R_TAGS).values();
+ Map<String, Ref> refs = Maps.newHashMapWithExpectedSize(heads.size() + tags.size());
+ for (Ref r : Iterables.concat(heads, tags)) {
+ refs.put(r.getName(), r);
}
- } catch (OrmException e) {
- 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());
+ return commits.isReachableFrom(state, repo, commit, refs);
} catch (IOException e) {
- String msg =
+ log.error(
String.format(
"Cannot verify permissions to commit object %s in repository %s",
- commit.name(), getProject().getNameKey());
- log.error(msg, e);
+ commit.name(), getProject().getNameKey()),
+ 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();
}
diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
index 4bb823e..dd4571f 100644
--- a/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
+++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/RefControl.java
@@ -45,9 +45,7 @@
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 @@
// 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 @@
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.
*
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/CommitsCollectionTest.java
similarity index 84%
rename from gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java
rename to gerrit-server/src/test/java/com/google/gerrit/server/project/CommitsCollectionTest.java
index 92d7a52..0d8080f 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/ProjectControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/CommitsCollectionTest.java
@@ -55,19 +55,19 @@
import org.junit.Before;
import org.junit.Test;
-/** Unit tests for {@link ProjectControl}. */
-public class ProjectControlTest {
+/** Unit tests for {@link CommitsCollection}. */
+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 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 @@
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(pc.canReadCommit(db, r, rw.parseCommit(id2)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(id1)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(id2)));
}
@Test
@@ -166,12 +166,12 @@
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)));
- assertFalse(pc.canReadCommit(db, r, rw.parseCommit(id2)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(id1)));
+ assertFalse(commits.canRead(state, r, rw.parseCommit(id2)));
}
@Test
@@ -185,11 +185,11 @@
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)));
- assertFalse(pc.canReadCommit(db, r, rw.parseCommit(parent2)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
+ assertFalse(commits.canRead(state, r, rw.parseCommit(parent2)));
}
@Test
@@ -199,16 +199,16 @@
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(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(id1)));
repo.branch("branch1").update(parent1);
- assertTrue(pc.canReadCommit(db, r, rw.parseCommit(parent1)));
- assertFalse(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
+ assertFalse(commits.canRead(state, r, rw.parseCommit(id1)));
}
@Test
@@ -218,20 +218,20 @@
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(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(id1)));
repo.branch("branch1").update(parent1);
- assertTrue(pc.canReadCommit(db, r, rw.parseCommit(parent1)));
- assertFalse(pc.canReadCommit(db, r, rw.parseCommit(id1)));
+ assertTrue(commits.canRead(state, r, rw.parseCommit(parent1)));
+ assertFalse(commits.canRead(state, r, rw.parseCommit(id1)));
}
- private ProjectControl newProjectControl() throws Exception {
- return projectControlFactory.controlFor(project.getName(), user);
+ private ProjectState readProjectState() throws Exception {
+ return projectCache.get(project.getName());
}
protected void allow(ProjectConfig project, String permission, AccountGroup.UUID id, String ref)
diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
index 9b9cfff..f15227e 100644
--- a/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
+++ b/gerrit-server/src/test/java/com/google/gerrit/server/project/RefControlTest.java
@@ -60,7 +60,6 @@
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 @@
@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 @@
}
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
- queryProvider,
- canonicalWebUrl,
+ "http://localhost", // canonicalWebUrl
permissionBackend,
new MockUser(name, memberOf),
newProjectState(local),
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
index 318e67f..58492b2 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/AbstractGitCommand.java
@@ -20,6 +20,7 @@
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 @@
@Inject private IdentifiedUser.GenericFactory userFactory;
protected Repository repo;
+ protected ProjectState state;
+ protected Project.NameKey projectName;
protected Project project;
@Override
@@ -86,10 +89,12 @@
}
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);
}
diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java
index 093808c..9a3e6ab 100644
--- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java
+++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/UploadArchive.java
@@ -18,13 +18,13 @@
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 @@
}
@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 @@
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);
}
}
}
