Convert VisibleRefFilter to assisted factory

Callers have to pass a number of arguments to build and configure the
VisibleRefFilter.  Instead of forcing callers to pass around many
arguments, use an assisted injection factory to create the instance.

Rely on the context Provider<ReviewDb> and Provider<CurrentUser> to
gain database access and user identity within the filter. Given all
current call sites, these should already be populated.

Change-Id: I8197ee773c94f16472d53162fb70791c45899c1b
This commit is contained in:
Shawn Pearce 2017-04-29 13:20:11 -07:00 committed by David Pursehouse
parent a9297614ed
commit ab841f6946
15 changed files with 113 additions and 219 deletions

View File

@ -16,7 +16,6 @@ package com.google.gerrit.acceptance;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.acceptance.InProcessProtocol.Context; import com.google.gerrit.acceptance.InProcessProtocol.Context;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Capable; import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
@ -32,12 +31,9 @@ import com.google.gerrit.server.config.RequestScopedReviewDbProvider;
import com.google.gerrit.server.git.AsyncReceiveCommits; import com.google.gerrit.server.git.AsyncReceiveCommits;
import com.google.gerrit.server.git.ReceiveCommits; import com.google.gerrit.server.git.ReceiveCommits;
import com.google.gerrit.server.git.ReceivePackInitializer; import com.google.gerrit.server.git.ReceivePackInitializer;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TransferConfig; import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.validators.UploadValidators; import com.google.gerrit.server.git.validators.UploadValidators;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.NoSuchProjectException; import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.RequestContext;
@ -206,12 +202,9 @@ class InProcessProtocol extends TestProtocol<Context> {
} }
private static class Upload implements UploadPackFactory<Context> { private static class Upload implements UploadPackFactory<Context> {
private final Provider<ReviewDb> dbProvider;
private final Provider<CurrentUser> userProvider; private final Provider<CurrentUser> userProvider;
private final TagCache tagCache;
@Nullable private final SearchingChangeCacheImpl changeCache;
private final ProjectControl.GenericFactory projectControlFactory; private final ProjectControl.GenericFactory projectControlFactory;
private final ChangeNotes.Factory changeNotesFactory; private final VisibleRefFilter.Factory refFilterFactory;
private final TransferConfig transferConfig; private final TransferConfig transferConfig;
private final DynamicSet<PreUploadHook> preUploadHooks; private final DynamicSet<PreUploadHook> preUploadHooks;
private final UploadValidators.Factory uploadValidatorsFactory; private final UploadValidators.Factory uploadValidatorsFactory;
@ -219,22 +212,16 @@ class InProcessProtocol extends TestProtocol<Context> {
@Inject @Inject
Upload( Upload(
Provider<ReviewDb> dbProvider,
Provider<CurrentUser> userProvider, Provider<CurrentUser> userProvider,
TagCache tagCache,
@Nullable SearchingChangeCacheImpl changeCache,
ProjectControl.GenericFactory projectControlFactory, ProjectControl.GenericFactory projectControlFactory,
ChangeNotes.Factory changeNotesFactory, VisibleRefFilter.Factory refFilterFactory,
TransferConfig transferConfig, TransferConfig transferConfig,
DynamicSet<PreUploadHook> preUploadHooks, DynamicSet<PreUploadHook> preUploadHooks,
UploadValidators.Factory uploadValidatorsFactory, UploadValidators.Factory uploadValidatorsFactory,
ThreadLocalRequestContext threadContext) { ThreadLocalRequestContext threadContext) {
this.dbProvider = dbProvider;
this.userProvider = userProvider; this.userProvider = userProvider;
this.tagCache = tagCache;
this.changeCache = changeCache;
this.projectControlFactory = projectControlFactory; this.projectControlFactory = projectControlFactory;
this.changeNotesFactory = changeNotesFactory; this.refFilterFactory = refFilterFactory;
this.transferConfig = transferConfig; this.transferConfig = transferConfig;
this.preUploadHooks = preUploadHooks; this.preUploadHooks = preUploadHooks;
this.uploadValidatorsFactory = uploadValidatorsFactory; this.uploadValidatorsFactory = uploadValidatorsFactory;
@ -258,9 +245,7 @@ class InProcessProtocol extends TestProtocol<Context> {
UploadPack up = new UploadPack(repo); UploadPack up = new UploadPack(repo);
up.setPackConfig(transferConfig.getPackConfig()); up.setPackConfig(transferConfig.getPackConfig());
up.setTimeout(transferConfig.getTimeout()); up.setTimeout(transferConfig.getTimeout());
up.setAdvertiseRefsHook( up.setAdvertiseRefsHook(refFilterFactory.create(ctl.getProjectState(), repo));
new VisibleRefFilter(
tagCache, changeNotesFactory, changeCache, repo, ctl, dbProvider.get(), true));
List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks); List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks);
hooks.add(uploadValidatorsFactory.create(ctl.getProject(), repo, "localhost-test")); hooks.add(uploadValidatorsFactory.create(ctl.getProject(), repo, "localhost-test"));
up.setPreUploadHook(PreUploadHookChain.newChain(hooks)); up.setPreUploadHook(PreUploadHookChain.newChain(hooks));

View File

@ -24,7 +24,6 @@ import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.AcceptanceTestRequestScope; import com.google.gerrit.acceptance.AcceptanceTestRequestScope;
import com.google.gerrit.acceptance.NoHttpd; import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.acceptance.PushOneCommit;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.AccessSection; import com.google.gerrit.common.data.AccessSection;
import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.common.data.Permission; import com.google.gerrit.common.data.Permission;
@ -34,23 +33,16 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.config.AnonymousCowardName; import com.google.gerrit.server.config.AnonymousCowardName;
import com.google.gerrit.server.git.ProjectConfig; import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.git.ReceiveCommitsAdvertiseRefsHook; import com.google.gerrit.server.git.ReceiveCommitsAdvertiseRefsHook;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.notedb.ChangeNoteUtil; import com.google.gerrit.server.notedb.ChangeNoteUtil;
import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage; import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.Util; import com.google.gerrit.server.project.Util;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testutil.DisabledReviewDb;
import com.google.gerrit.testutil.TestChanges; import com.google.gerrit.testutil.TestChanges;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
import java.util.List; import java.util.List;
@ -68,16 +60,8 @@ import org.junit.Test;
@NoHttpd @NoHttpd
public class RefAdvertisementIT extends AbstractDaemonTest { public class RefAdvertisementIT extends AbstractDaemonTest {
@Inject private ProjectControl.GenericFactory projectControlFactory; @Inject private VisibleRefFilter.Factory refFilterFactory;
@Inject @Nullable private SearchingChangeCacheImpl changeCache;
@Inject private TagCache tagCache;
@Inject private Provider<CurrentUser> userProvider;
@Inject private ChangeNoteUtil noteUtil; @Inject private ChangeNoteUtil noteUtil;
@Inject @AnonymousCowardName private String anonymousCowardName; @Inject @AnonymousCowardName private String anonymousCowardName;
private AccountGroup.UUID admins; private AccountGroup.UUID admins;
@ -386,7 +370,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project)) {
assertRefs( assertRefs(
repo, repo,
new VisibleRefFilter(tagCache, notesFactory, null, repo, projectControl(), db, true), refFilterFactory.create(projectCache.get(project), repo),
// Can't use stored values from the index so DB must be enabled. // Can't use stored values from the index so DB must be enabled.
false, false,
"HEAD", "HEAD",
@ -410,12 +394,12 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
assume().that(notesMigration.readChangeSequence()).isTrue(); assume().that(notesMigration.readChangeSequence()).isTrue();
try (Repository repo = repoManager.openRepository(allProjects)) { try (Repository repo = repoManager.openRepository(allProjects)) {
setApiUser(user); setApiUser(user);
assertRefs(repo, newFilter(db, repo, allProjects), true); assertRefs(repo, newFilter(repo, allProjects), true);
allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE); allowGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
try { try {
setApiUser(user); setApiUser(user);
assertRefs(repo, newFilter(db, repo, allProjects), true, "refs/sequences/changes"); assertRefs(repo, newFilter(repo, allProjects), true, "refs/sequences/changes");
} finally { } finally {
removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE); removeGlobalCapabilities(REGISTERED_USERS, GlobalCapability.ACCESS_DATABASE);
} }
@ -556,17 +540,7 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
private void assertUploadPackRefs(String... expectedWithMeta) throws Exception { private void assertUploadPackRefs(String... expectedWithMeta) throws Exception {
try (Repository repo = repoManager.openRepository(project)) { try (Repository repo = repoManager.openRepository(project)) {
assertRefs( assertRefs(
repo, repo, refFilterFactory.create(projectCache.get(project), repo), true, expectedWithMeta);
new VisibleRefFilter(
tagCache,
notesFactory,
changeCache,
repo,
projectControl(),
new DisabledReviewDb(),
true),
true,
expectedWithMeta);
} }
} }
@ -602,20 +576,8 @@ public class RefAdvertisementIT extends AbstractDaemonTest {
} }
} }
private ProjectControl projectControl() throws Exception { private VisibleRefFilter newFilter(Repository repo, Project.NameKey project) {
return projectControlFactory.controlFor(project, userProvider.get()); return refFilterFactory.create(projectCache.get(project), repo);
}
private VisibleRefFilter newFilter(ReviewDb db, Repository repo, Project.NameKey project)
throws Exception {
return new VisibleRefFilter(
tagCache,
notesFactory,
null,
repo,
projectControlFactory.controlFor(project, userProvider.get()),
db,
true);
} }
private static ObjectId obj(ChangeData cd, int psNum) throws Exception { private static ObjectId obj(ChangeData cd, int psNum) throws Exception {

View File

@ -16,12 +16,10 @@ package com.google.gerrit.httpd;
import com.google.common.cache.Cache; import com.google.common.cache.Cache;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.Capable; import com.google.gerrit.common.data.Capable;
import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AccessPath; import com.google.gerrit.server.AccessPath;
import com.google.gerrit.server.AnonymousUser; import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
@ -29,12 +27,9 @@ import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.git.AsyncReceiveCommits; import com.google.gerrit.server.git.AsyncReceiveCommits;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.ReceiveCommits; import com.google.gerrit.server.git.ReceiveCommits;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TransferConfig; import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.validators.UploadValidators; import com.google.gerrit.server.git.validators.UploadValidators;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission; import com.google.gerrit.server.permissions.ProjectPermission;
@ -237,23 +232,14 @@ public class GitOverHttpServlet extends GitServlet {
} }
static class UploadFilter implements Filter { static class UploadFilter implements Filter {
private final Provider<ReviewDb> db; private final VisibleRefFilter.Factory refFilterFactory;
private final TagCache tagCache;
private final ChangeNotes.Factory changeNotesFactory;
@Nullable private final SearchingChangeCacheImpl changeCache;
private final UploadValidators.Factory uploadValidatorsFactory; private final UploadValidators.Factory uploadValidatorsFactory;
@Inject @Inject
UploadFilter( UploadFilter(
Provider<ReviewDb> db, VisibleRefFilter.Factory refFilterFactory,
TagCache tagCache,
ChangeNotes.Factory changeNotesFactory,
@Nullable SearchingChangeCacheImpl changeCache,
UploadValidators.Factory uploadValidatorsFactory) { UploadValidators.Factory uploadValidatorsFactory) {
this.db = db; this.refFilterFactory = refFilterFactory;
this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache;
this.uploadValidatorsFactory = uploadValidatorsFactory; this.uploadValidatorsFactory = uploadValidatorsFactory;
} }
@ -279,9 +265,7 @@ public class GitOverHttpServlet extends GitServlet {
uploadValidatorsFactory.create(pc.getProject(), repo, request.getRemoteHost()); uploadValidatorsFactory.create(pc.getProject(), repo, request.getRemoteHost());
up.setPreUploadHook( up.setPreUploadHook(
PreUploadHookChain.newChain(Lists.newArrayList(up.getPreUploadHook(), uploadValidators))); PreUploadHookChain.newChain(Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
up.setAdvertiseRefsHook( up.setAdvertiseRefsHook(refFilterFactory.create(pc.getProjectState(), repo));
new VisibleRefFilter(
tagCache, changeNotesFactory, changeCache, repo, pc, db.get(), true));
next.doFilter(request, response); next.doFilter(request, response);
} }

View File

@ -57,6 +57,7 @@ import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.ReceiveCommitsExecutorModule; import com.google.gerrit.server.git.ReceiveCommitsExecutorModule;
import com.google.gerrit.server.git.SearchingChangeCacheImpl; import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache; import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.group.GroupModule; import com.google.gerrit.server.group.GroupModule;
import com.google.gerrit.server.mail.send.ReplacePatchSetSender; import com.google.gerrit.server.mail.send.ReplacePatchSetSender;
import com.google.gerrit.server.notedb.NoteDbModule; import com.google.gerrit.server.notedb.NoteDbModule;
@ -130,6 +131,7 @@ public class BatchProgramModule extends FactoryModule {
factory(MergeUtil.Factory.class); factory(MergeUtil.Factory.class);
factory(PatchSetInserter.Factory.class); factory(PatchSetInserter.Factory.class);
factory(RebaseChangeOp.Factory.class); factory(RebaseChangeOp.Factory.class);
factory(VisibleRefFilter.Factory.class);
// As Reindex is a batch program, don't assume the index is available for // As Reindex is a batch program, don't assume the index is available for
// the change cache. // the change cache.

View File

@ -123,6 +123,7 @@ import com.google.gerrit.server.git.ReceivePackInitializer;
import com.google.gerrit.server.git.ReplaceOp; import com.google.gerrit.server.git.ReplaceOp;
import com.google.gerrit.server.git.TagCache; import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TransferConfig; import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.strategy.SubmitStrategy; import com.google.gerrit.server.git.strategy.SubmitStrategy;
import com.google.gerrit.server.git.validators.CommitValidationListener; import com.google.gerrit.server.git.validators.CommitValidationListener;
import com.google.gerrit.server.git.validators.MergeValidationListener; import com.google.gerrit.server.git.validators.MergeValidationListener;
@ -261,6 +262,7 @@ public class GerritGlobalModule extends FactoryModule {
factory(RegisterNewEmailSender.Factory.class); factory(RegisterNewEmailSender.Factory.class);
factory(ReplacePatchSetSender.Factory.class); factory(ReplacePatchSetSender.Factory.class);
factory(SetAssigneeSender.Factory.class); factory(SetAssigneeSender.Factory.class);
factory(VisibleRefFilter.Factory.class);
bind(PermissionCollection.Factory.class); bind(PermissionCollection.Factory.class);
bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON); bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON);
factory(ProjectOwnerGroupsProvider.Factory.class); factory(ProjectOwnerGroupsProvider.Factory.class);

View File

@ -384,7 +384,7 @@ public class ReceiveCommits {
PatchSetUtil psUtil, PatchSetUtil psUtil,
ProjectCache projectCache, ProjectCache projectCache,
TagCache tagCache, TagCache tagCache,
@Nullable SearchingChangeCacheImpl changeCache, VisibleRefFilter.Factory refFilterFactory,
ChangeInserter.Factory changeInserterFactory, ChangeInserter.Factory changeInserterFactory,
CommitValidators.Factory commitValidatorsFactory, CommitValidators.Factory commitValidatorsFactory,
RefOperationValidators.Factory refValidatorsFactory, RefOperationValidators.Factory refValidatorsFactory,
@ -494,7 +494,7 @@ public class ReceiveCommits {
} }
rp.setAdvertiseRefsHook( rp.setAdvertiseRefsHook(
new VisibleRefFilter(tagCache, notesFactory, changeCache, repo, projectControl, db, false)); refFilterFactory.create(projectControl.getProjectState(), repo).setShowMetadata(false));
List<AdvertiseRefsHook> advHooks = new ArrayList<>(3); List<AdvertiseRefsHook> advHooks = new ArrayList<>(3);
advHooks.add( advHooks.add(
new AdvertiseRefsHook() { new AdvertiseRefsHook() {
@ -1076,7 +1076,7 @@ public class ReceiveCommits {
} catch (AuthException err) { } catch (AuthException err) {
ok = false; ok = false;
} }
if (ok && ctl.canCreate(db, rp.getRepository(), obj)) { if (ok && ctl.canCreate(rp.getRepository(), obj)) {
if (!validRefOperation(cmd)) { if (!validRefOperation(cmd)) {
return; return;
} }

View File

@ -27,11 +27,16 @@ import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException; import java.io.IOException;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collections; import java.util.Collections;
@ -52,50 +57,62 @@ import org.slf4j.LoggerFactory;
public class VisibleRefFilter extends AbstractAdvertiseRefsHook { public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
private static final Logger log = LoggerFactory.getLogger(VisibleRefFilter.class); private static final Logger log = LoggerFactory.getLogger(VisibleRefFilter.class);
public interface Factory {
VisibleRefFilter create(ProjectState projectState, Repository git);
}
private final TagCache tagCache; private final TagCache tagCache;
private final ChangeNotes.Factory changeNotesFactory; private final ChangeNotes.Factory changeNotesFactory;
@Nullable private final SearchingChangeCacheImpl changeCache; @Nullable private final SearchingChangeCacheImpl changeCache;
private final Repository db; private final Provider<ReviewDb> db;
private final Project.NameKey projectName; private final Provider<CurrentUser> user;
private final ProjectControl projectCtl; private final ProjectState projectState;
private final ReviewDb reviewDb; private final Repository git;
private final boolean showMetadata; private ProjectControl projectCtl;
private boolean showMetadata = true;
private String userEditPrefix; private String userEditPrefix;
private Map<Change.Id, Branch.NameKey> visibleChanges; private Map<Change.Id, Branch.NameKey> visibleChanges;
public VisibleRefFilter( @Inject
VisibleRefFilter(
TagCache tagCache, TagCache tagCache,
ChangeNotes.Factory changeNotesFactory, ChangeNotes.Factory changeNotesFactory,
@Nullable SearchingChangeCacheImpl changeCache, @Nullable SearchingChangeCacheImpl changeCache,
Repository db, Provider<ReviewDb> db,
ProjectControl projectControl, Provider<CurrentUser> user,
ReviewDb reviewDb, @Assisted ProjectState projectState,
boolean showMetadata) { @Assisted Repository git) {
this.tagCache = tagCache; this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory; this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache; this.changeCache = changeCache;
this.db = db; this.db = db;
this.projectName = projectControl.getProject().getNameKey(); this.user = user;
this.projectCtl = projectControl; this.projectState = projectState;
this.reviewDb = reviewDb; this.git = git;
this.showMetadata = showMetadata; }
/** Show change references. Default is {@code true}. */
public VisibleRefFilter setShowMetadata(boolean show) {
showMetadata = show;
return this;
} }
public Map<String, Ref> filter(Map<String, Ref> refs, boolean filterTagsSeparately) { public Map<String, Ref> filter(Map<String, Ref> refs, boolean filterTagsSeparately) {
if (projectCtl.getProjectState().isAllUsers()) { if (projectState.isAllUsers()) {
refs = addUsersSelfSymref(refs); refs = addUsersSelfSymref(refs);
} }
projectCtl = projectState.controlFor(user.get());
if (projectCtl.allRefsAreVisible(ImmutableSet.of(REFS_CONFIG))) { if (projectCtl.allRefsAreVisible(ImmutableSet.of(REFS_CONFIG))) {
return fastHideRefsMetaConfig(refs); return fastHideRefsMetaConfig(refs);
} }
Account.Id userId; Account.Id userId;
boolean viewMetadata; boolean viewMetadata;
if (projectCtl.getUser().isIdentifiedUser()) { if (user.get().isIdentifiedUser()) {
IdentifiedUser user = projectCtl.getUser().asIdentifiedUser(); IdentifiedUser u = user.get().asIdentifiedUser();
userId = user.getAccountId(); userId = u.getAccountId();
viewMetadata = user.getCapabilities().canAccessDatabase(); viewMetadata = u.getCapabilities().canAccessDatabase();
userEditPrefix = RefNames.refsEditPrefix(userId); userEditPrefix = RefNames.refsEditPrefix(userId);
} else { } else {
userId = null; userId = null;
@ -109,8 +126,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
String name = ref.getName(); String name = ref.getName();
Change.Id changeId; Change.Id changeId;
Account.Id accountId; Account.Id accountId;
if (name.startsWith(REFS_CACHE_AUTOMERGE) if (name.startsWith(REFS_CACHE_AUTOMERGE) || (!showMetadata && isMetadata(name))) {
|| (!showMetadata && isMetadata(projectCtl, name))) {
continue; continue;
} else if (RefNames.isRefsEdit(name)) { } else if (RefNames.isRefsEdit(name)) {
// Edits are visible only to the owning user, if change is visible. // Edits are visible only to the owning user, if change is visible.
@ -138,8 +154,7 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
if (viewMetadata) { if (viewMetadata) {
result.put(name, ref); result.put(name, ref);
} }
} else if (projectCtl.getProjectState().isAllUsers() } else if (projectState.isAllUsers() && name.equals(RefNames.REFS_EXTERNAL_IDS)) {
&& name.equals(RefNames.REFS_EXTERNAL_IDS)) {
// The notes branch with the external IDs of all users must not be exposed to normal users. // The notes branch with the external IDs of all users must not be exposed to normal users.
if (viewMetadata) { if (viewMetadata) {
result.put(name, ref); result.put(name, ref);
@ -158,11 +173,11 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
if (!deferredTags.isEmpty() && (!result.isEmpty() || filterTagsSeparately)) { if (!deferredTags.isEmpty() && (!result.isEmpty() || filterTagsSeparately)) {
TagMatcher tags = TagMatcher tags =
tagCache tagCache
.get(projectName) .get(projectState.getProject().getNameKey())
.matcher( .matcher(
tagCache, tagCache,
db, git,
filterTagsSeparately ? filter(db.getAllRefs()).values() : result.values()); filterTagsSeparately ? filter(git.getAllRefs()).values() : result.values());
for (Ref tag : deferredTags) { for (Ref tag : deferredTags) {
if (tags.isReachable(tag)) { if (tags.isReachable(tag)) {
result.put(tag.getName(), tag); result.put(tag.getName(), tag);
@ -183,8 +198,8 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
} }
private Map<String, Ref> addUsersSelfSymref(Map<String, Ref> refs) { private Map<String, Ref> addUsersSelfSymref(Map<String, Ref> refs) {
if (projectCtl.getUser().isIdentifiedUser()) { if (user.get().isIdentifiedUser()) {
Ref r = refs.get(RefNames.refsUsers(projectCtl.getUser().getAccountId())); Ref r = refs.get(RefNames.refsUsers(user.get().getAccountId()));
if (r != null) { if (r != null) {
SymbolicRef s = new SymbolicRef(REFS_USERS_SELF, r); SymbolicRef s = new SymbolicRef(REFS_USERS_SELF, r);
refs = new HashMap<>(refs); refs = new HashMap<>(refs);
@ -241,8 +256,8 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
Project project = projectCtl.getProject(); Project project = projectCtl.getProject();
try { try {
Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>(); Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>();
for (ChangeData cd : changeCache.getChangeData(reviewDb, project.getNameKey())) { for (ChangeData cd : changeCache.getChangeData(db.get(), project.getNameKey())) {
if (projectCtl.controlForIndexedChange(cd.change()).isVisible(reviewDb, cd)) { if (projectCtl.controlForIndexedChange(cd.change()).isVisible(db.get(), cd)) {
visibleChanges.put(cd.getId(), cd.change().getDest()); visibleChanges.put(cd.getId(), cd.change().getDest());
} }
} }
@ -261,8 +276,8 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
Project.NameKey project = projectCtl.getProject().getNameKey(); Project.NameKey project = projectCtl.getProject().getNameKey();
try { try {
Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>(); Map<Change.Id, Branch.NameKey> visibleChanges = new HashMap<>();
for (ChangeNotes cn : changeNotesFactory.scan(db, reviewDb, project)) { for (ChangeNotes cn : changeNotesFactory.scan(git, db.get(), project)) {
if (projectCtl.controlFor(cn).isVisible(reviewDb)) { if (projectCtl.controlFor(cn).isVisible(db.get())) {
visibleChanges.put(cn.getChangeId(), cn.getChange().getDest()); visibleChanges.put(cn.getChangeId(), cn.getChange().getDest());
} }
} }
@ -274,10 +289,10 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
} }
} }
private static boolean isMetadata(ProjectControl projectCtl, String name) { private boolean isMetadata(String name) {
return name.startsWith(REFS_CHANGES) return name.startsWith(REFS_CHANGES)
|| RefNames.isRefsEdit(name) || RefNames.isRefsEdit(name)
|| (projectCtl.getProjectState().isAllUsers() && name.equals(RefNames.REFS_EXTERNAL_IDS)); || (projectState.isAllUsers() && name.equals(RefNames.REFS_EXTERNAL_IDS));
} }
private static boolean isTag(Ref ref) { private static boolean isTag(Ref ref) {

View File

@ -22,7 +22,6 @@ import com.google.gerrit.extensions.restapi.ResourceConflictException;
import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.client.Branch; import com.google.gerrit.reviewdb.client.Branch;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.extensions.events.GitReferenceUpdated; import com.google.gerrit.server.extensions.events.GitReferenceUpdated;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
@ -54,7 +53,6 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
private final Provider<IdentifiedUser> identifiedUser; private final Provider<IdentifiedUser> identifiedUser;
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
private final Provider<ReviewDb> db;
private final GitReferenceUpdated referenceUpdated; private final GitReferenceUpdated referenceUpdated;
private final RefValidationHelper refCreationValidator; private final RefValidationHelper refCreationValidator;
private String ref; private String ref;
@ -64,14 +62,12 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
Provider<IdentifiedUser> identifiedUser, Provider<IdentifiedUser> identifiedUser,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
GitRepositoryManager repoManager, GitRepositoryManager repoManager,
Provider<ReviewDb> db,
GitReferenceUpdated referenceUpdated, GitReferenceUpdated referenceUpdated,
RefValidationHelper.Factory refHelperFactory, RefValidationHelper.Factory refHelperFactory,
@Assisted String ref) { @Assisted String ref) {
this.identifiedUser = identifiedUser; this.identifiedUser = identifiedUser;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.repoManager = repoManager; this.repoManager = repoManager;
this.db = db;
this.referenceUpdated = referenceUpdated; this.referenceUpdated = referenceUpdated;
this.refCreationValidator = refHelperFactory.create(ReceiveCommand.Type.CREATE); this.refCreationValidator = refHelperFactory.create(ReceiveCommand.Type.CREATE);
this.ref = ref; this.ref = ref;
@ -121,7 +117,7 @@ public class CreateBranch implements RestModifyView<ProjectResource, BranchInput
} }
} }
if (!refControl.canCreate(db.get(), repo, object)) { if (!refControl.canCreate(repo, object)) {
throw new AuthException("Cannot create \"" + ref + "\""); throw new AuthException("Cannot create \"" + ref + "\"");
} }

View File

@ -22,14 +22,11 @@ import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.ResourceNotFoundException;
import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CommonConverters; import com.google.gerrit.server.CommonConverters;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.SearchingChangeCacheImpl; import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.RefPermission; import com.google.gerrit.server.permissions.RefPermission;
import com.google.inject.Inject; import com.google.inject.Inject;
@ -55,9 +52,7 @@ public class ListTags implements RestReadView<ProjectResource> {
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
private final Provider<CurrentUser> user; private final Provider<CurrentUser> user;
private final Provider<ReviewDb> dbProvider; private final VisibleRefFilter.Factory refFilterFactory;
private final TagCache tagCache;
private final ChangeNotes.Factory changeNotesFactory;
@Nullable private final SearchingChangeCacheImpl changeCache; @Nullable private final SearchingChangeCacheImpl changeCache;
@Option( @Option(
@ -110,16 +105,12 @@ public class ListTags implements RestReadView<ProjectResource> {
GitRepositoryManager repoManager, GitRepositoryManager repoManager,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
Provider<CurrentUser> user, Provider<CurrentUser> user,
Provider<ReviewDb> dbProvider, VisibleRefFilter.Factory refFilterFactory,
TagCache tagCache,
ChangeNotes.Factory changeNotesFactory,
@Nullable SearchingChangeCacheImpl changeCache) { @Nullable SearchingChangeCacheImpl changeCache) {
this.repoManager = repoManager; this.repoManager = repoManager;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.user = user; this.user = user;
this.dbProvider = dbProvider; this.refFilterFactory = refFilterFactory;
this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache; this.changeCache = changeCache;
} }
@ -211,8 +202,9 @@ public class ListTags implements RestReadView<ProjectResource> {
private Map<String, Ref> visibleTags( private Map<String, Ref> visibleTags(
ProjectControl control, Repository repo, Map<String, Ref> tags) { ProjectControl control, Repository repo, Map<String, Ref> tags) {
return new VisibleRefFilter( return refFilterFactory
tagCache, changeNotesFactory, changeCache, repo, control, dbProvider.get(), false) .create(control.getProjectState(), repo)
.setShowMetadata(false)
.filter(tags, true); .filter(tags, true);
} }
} }

View File

@ -43,8 +43,6 @@ import com.google.gerrit.server.change.IncludedInResolver;
import com.google.gerrit.server.config.CanonicalWebUrl; import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GitReceivePackGroups; import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups; import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
@ -136,12 +134,10 @@ public class ProjectControl {
private final String canonicalWebUrl; private final String canonicalWebUrl;
private final CurrentUser user; private final CurrentUser user;
private final ProjectState state; private final ProjectState state;
private final ChangeNotes.Factory changeNotesFactory;
private final ChangeControl.Factory changeControlFactory; private final ChangeControl.Factory changeControlFactory;
private final PermissionCollection.Factory permissionFilter; private final PermissionCollection.Factory permissionFilter;
private final VisibleRefFilter.Factory refFilter;
private final Collection<ContributorAgreement> contributorAgreements; private final Collection<ContributorAgreement> contributorAgreements;
private final TagCache tagCache;
@Nullable private final SearchingChangeCacheImpl changeCache;
private final Provider<InternalChangeQuery> queryProvider; private final Provider<InternalChangeQuery> queryProvider;
private final Metrics metrics; private final Metrics metrics;
@ -157,19 +153,15 @@ public class ProjectControl {
@GitReceivePackGroups Set<AccountGroup.UUID> receiveGroups, @GitReceivePackGroups Set<AccountGroup.UUID> receiveGroups,
ProjectCache pc, ProjectCache pc,
PermissionCollection.Factory permissionFilter, PermissionCollection.Factory permissionFilter,
ChangeNotes.Factory changeNotesFactory,
ChangeControl.Factory changeControlFactory, ChangeControl.Factory changeControlFactory,
TagCache tagCache, VisibleRefFilter.Factory refFilter,
Provider<InternalChangeQuery> queryProvider, Provider<InternalChangeQuery> queryProvider,
@Nullable SearchingChangeCacheImpl changeCache,
@CanonicalWebUrl @Nullable String canonicalWebUrl, @CanonicalWebUrl @Nullable String canonicalWebUrl,
@Assisted CurrentUser who, @Assisted CurrentUser who,
@Assisted ProjectState ps, @Assisted ProjectState ps,
Metrics metrics) { Metrics metrics) {
this.changeNotesFactory = changeNotesFactory;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.tagCache = tagCache; this.refFilter = refFilter;
this.changeCache = changeCache;
this.uploadGroups = uploadGroups; this.uploadGroups = uploadGroups;
this.receiveGroups = receiveGroups; this.receiveGroups = receiveGroups;
this.permissionFilter = permissionFilter; this.permissionFilter = permissionFilter;
@ -492,12 +484,12 @@ public class ProjectControl {
"Cannot look up change for commit " + commit.name() + " in " + getProject().getName(), e); "Cannot look up change for commit " + commit.name() + " in " + getProject().getName(), e);
} }
// Scan all visible refs. // Scan all visible refs.
return canReadCommitFromVisibleRef(db, repo, commit); return canReadCommitFromVisibleRef(repo, commit);
} }
private boolean canReadCommitFromVisibleRef(ReviewDb db, Repository repo, RevCommit commit) { private boolean canReadCommitFromVisibleRef(Repository repo, RevCommit commit) {
try (RevWalk rw = new RevWalk(repo)) { try (RevWalk rw = new RevWalk(repo)) {
return isMergedIntoVisibleRef(repo, db, rw, commit, repo.getAllRefs().values()); return isMergedIntoVisibleRef(repo, rw, commit, repo.getAllRefs().values());
} catch (IOException e) { } catch (IOException e) {
String msg = String msg =
String.format( String.format(
@ -509,10 +501,9 @@ public class ProjectControl {
} }
boolean isMergedIntoVisibleRef( boolean isMergedIntoVisibleRef(
Repository repo, ReviewDb db, RevWalk rw, RevCommit commit, Collection<Ref> unfilteredRefs) Repository repo, RevWalk rw, RevCommit commit, Collection<Ref> unfilteredRefs)
throws IOException { throws IOException {
VisibleRefFilter filter = VisibleRefFilter filter = refFilter.create(state, repo);
new VisibleRefFilter(tagCache, changeNotesFactory, changeCache, repo, this, db, true);
Map<String, Ref> m = Maps.newHashMapWithExpectedSize(unfilteredRefs.size()); Map<String, Ref> m = Maps.newHashMapWithExpectedSize(unfilteredRefs.size());
for (Ref r : unfilteredRefs) { for (Ref r : unfilteredRefs) {
m.put(r.getName(), r); m.put(r.getName(), r);

View File

@ -24,7 +24,6 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.group.SystemGroupBackend; import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.notedb.ChangeNotes;
@ -260,12 +259,11 @@ public class RefControl {
/** /**
* Determines whether the user can create a new Git ref. * Determines whether the user can create a new Git ref.
* *
* @param db db for checking change visibility.
* @param repo repository on which user want to create * @param repo repository on which user want to create
* @param object the object the user will start the reference with. * @param object the object the user will start the reference with.
* @return {@code true} if the user specified can create a new Git ref * @return {@code true} if the user specified can create a new Git ref
*/ */
public boolean canCreate(ReviewDb db, Repository repo, RevObject object) { public boolean canCreate(Repository repo, RevObject object) {
if (!isProjectStatePermittingWrite()) { if (!isProjectStatePermittingWrite()) {
return false; return false;
} }
@ -275,7 +273,7 @@ public class RefControl {
// No create permissions. // No create permissions.
return false; return false;
} }
return canCreateCommit(db, repo, (RevCommit) object); return canCreateCommit(repo, (RevCommit) object);
} else if (object instanceof RevTag) { } else if (object instanceof RevTag) {
final RevTag tag = (RevTag) object; final RevTag tag = (RevTag) object;
try (RevWalk rw = new RevWalk(repo)) { try (RevWalk rw = new RevWalk(repo)) {
@ -302,11 +300,11 @@ public class RefControl {
RevObject tagObject = tag.getObject(); RevObject tagObject = tag.getObject();
if (tagObject instanceof RevCommit) { if (tagObject instanceof RevCommit) {
if (!canCreateCommit(db, repo, (RevCommit) tagObject)) { if (!canCreateCommit(repo, (RevCommit) tagObject)) {
return false; return false;
} }
} else { } else {
if (!canCreate(db, repo, tagObject)) { if (!canCreate(repo, tagObject)) {
return false; return false;
} }
} }
@ -323,12 +321,12 @@ public class RefControl {
} }
} }
private boolean canCreateCommit(ReviewDb db, Repository repo, RevCommit commit) { private boolean canCreateCommit(Repository repo, RevCommit commit) {
if (canUpdate()) { if (canUpdate()) {
// If the user has push permissions, they can create the ref regardless // If the user has push permissions, they can create the ref regardless
// of whether they are pushing any new objects along with the create. // of whether they are pushing any new objects along with the create.
return true; return true;
} else if (isMergedIntoBranchOrTag(db, repo, commit)) { } else if (isMergedIntoBranchOrTag(repo, commit)) {
// If the user has no push permissions, check whether the object is // 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 // 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 // not effectively "pushing" more objects, so they can create the ref
@ -338,11 +336,11 @@ public class RefControl {
return false; return false;
} }
private boolean isMergedIntoBranchOrTag(ReviewDb db, Repository repo, RevCommit commit) { private boolean isMergedIntoBranchOrTag(Repository repo, RevCommit commit) {
try (RevWalk rw = new RevWalk(repo)) { try (RevWalk rw = new RevWalk(repo)) {
List<Ref> refs = new ArrayList<>(repo.getRefDatabase().getRefs(Constants.R_HEADS).values()); List<Ref> refs = new ArrayList<>(repo.getRefDatabase().getRefs(Constants.R_HEADS).values());
refs.addAll(repo.getRefDatabase().getRefs(Constants.R_TAGS).values()); refs.addAll(repo.getRefDatabase().getRefs(Constants.R_TAGS).values());
return projectControl.isMergedIntoVisibleRef(repo, db, rw, commit, refs); return projectControl.isMergedIntoVisibleRef(repo, rw, commit, refs);
} catch (IOException e) { } catch (IOException e) {
String msg = String msg =
String.format( String.format(

View File

@ -906,11 +906,9 @@ public class RefControlTest {
Collections.<AccountGroup.UUID>emptySet(), Collections.<AccountGroup.UUID>emptySet(),
projectCache, projectCache,
sectionSorter, sectionSorter,
null,
changeControlFactory, changeControlFactory,
null, null, // refFilter
queryProvider, queryProvider,
null,
canonicalWebUrl, canonicalWebUrl,
new MockUser(name, memberOf), new MockUser(name, memberOf),
newProjectState(local), newProjectState(local),

View File

@ -17,20 +17,18 @@ package com.google.gerrit.sshd.commands;
import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER_OR_SLAVE; import static com.google.gerrit.sshd.CommandMetaData.Mode.MASTER_OR_SLAVE;
import static org.eclipse.jgit.lib.RefDatabase.ALL; import static org.eclipse.jgit.lib.RefDatabase.ALL;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GlobalCapability; import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.annotations.RequiresCapability; import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountResolver; import com.google.gerrit.server.account.AccountResolver;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectControl; import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.util.ManualRequestContext;
import com.google.gerrit.server.util.OneOffRequestContext;
import com.google.gerrit.sshd.CommandMetaData; import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand; import com.google.gerrit.sshd.SshCommand;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@ -50,16 +48,10 @@ import org.kohsuke.args4j.Option;
) )
public class LsUserRefs extends SshCommand { public class LsUserRefs extends SshCommand {
@Inject private AccountResolver accountResolver; @Inject private AccountResolver accountResolver;
@Inject private OneOffRequestContext requestContext;
@Inject private IdentifiedUser.GenericFactory userFactory; @Inject private VisibleRefFilter.Factory refFilterFactory;
@Inject private ReviewDb db; @Inject private ReviewDb db;
@Inject private GitRepositoryManager repoManager;
@Inject private TagCache tagCache;
@Inject private ChangeNotes.Factory changeNotesFactory;
@Inject @Nullable private SearchingChangeCacheImpl changeCache;
@Option( @Option(
name = "--project", name = "--project",
@ -82,8 +74,6 @@ public class LsUserRefs extends SshCommand {
@Option(name = "--only-refs-heads", usage = "list only refs under refs/heads") @Option(name = "--only-refs-heads", usage = "list only refs under refs/heads")
private boolean onlyRefsHeads; private boolean onlyRefsHeads;
@Inject private GitRepositoryManager repoManager;
@Override @Override
protected void run() throws Failure { protected void run() throws Failure {
Account userAccount; Account userAccount;
@ -92,21 +82,19 @@ public class LsUserRefs extends SshCommand {
} catch (OrmException e) { } catch (OrmException e) {
throw die(e); throw die(e);
} }
if (userAccount == null) { if (userAccount == null) {
stdout.print("No single user could be found when searching for: " + userName + '\n'); stdout.print("No single user could be found when searching for: " + userName + '\n');
stdout.flush(); stdout.flush();
return; return;
} }
IdentifiedUser user = userFactory.create(userAccount.getId()); Project.NameKey projectName = projectControl.getProject().getNameKey();
ProjectControl userProjectControl = projectControl.forUser(user); try (Repository repo = repoManager.openRepository(projectName);
try (Repository repo = ManualRequestContext ctx = requestContext.openAs(userAccount.getId())) {
repoManager.openRepository(userProjectControl.getProject().getNameKey())) {
try { try {
Map<String, Ref> refsMap = Map<String, Ref> refsMap =
new VisibleRefFilter( refFilterFactory
tagCache, changeNotesFactory, changeCache, repo, userProjectControl, db, true) .create(projectControl.getProjectState(), repo)
.filter(repo.getRefDatabase().getRefs(ALL), false); .filter(repo.getRefDatabase().getRefs(ALL), false);
for (String ref : refsMap.keySet()) { for (String ref : refsMap.keySet()) {
@ -115,13 +103,12 @@ public class LsUserRefs extends SshCommand {
} }
} }
} catch (IOException e) { } catch (IOException e) {
throw new Failure( throw new Failure(1, "fatal: Error reading refs: '" + projectName, e);
1, "fatal: Error reading refs: '" + projectControl.getProject().getNameKey(), e);
} }
} catch (RepositoryNotFoundException e) { } catch (RepositoryNotFoundException e) {
throw die("'" + projectControl.getProject().getNameKey() + "': not a git archive"); throw die("'" + projectName + "': not a git archive");
} catch (IOException e) { } catch (IOException | OrmException e) {
throw die("Error opening: '" + projectControl.getProject().getNameKey()); throw die("Error opening: '" + projectName);
} }
} }
} }

View File

@ -15,16 +15,11 @@
package com.google.gerrit.sshd.commands; package com.google.gerrit.sshd.commands;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.git.TransferConfig; import com.google.gerrit.server.git.TransferConfig;
import com.google.gerrit.server.git.VisibleRefFilter; import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.validators.UploadValidationException; import com.google.gerrit.server.git.validators.UploadValidationException;
import com.google.gerrit.server.git.validators.UploadValidators; import com.google.gerrit.server.git.validators.UploadValidators;
import com.google.gerrit.server.notedb.ChangeNotes;
import com.google.gerrit.sshd.AbstractGitCommand; import com.google.gerrit.sshd.AbstractGitCommand;
import com.google.gerrit.sshd.SshSession; import com.google.gerrit.sshd.SshSession;
import com.google.inject.Inject; import com.google.inject.Inject;
@ -38,22 +33,11 @@ import org.eclipse.jgit.transport.UploadPack;
/** Publishes Git repositories over SSH using the Git upload-pack protocol. */ /** Publishes Git repositories over SSH using the Git upload-pack protocol. */
final class Upload extends AbstractGitCommand { final class Upload extends AbstractGitCommand {
@Inject private ReviewDb db;
@Inject private TransferConfig config; @Inject private TransferConfig config;
@Inject private VisibleRefFilter.Factory refFilterFactory;
@Inject private TagCache tagCache;
@Inject private ChangeNotes.Factory changeNotesFactory;
@Inject @Nullable private SearchingChangeCacheImpl changeCache;
@Inject private DynamicSet<PreUploadHook> preUploadHooks; @Inject private DynamicSet<PreUploadHook> preUploadHooks;
@Inject private DynamicSet<PostUploadHook> postUploadHooks; @Inject private DynamicSet<PostUploadHook> postUploadHooks;
@Inject private UploadValidators.Factory uploadValidatorsFactory; @Inject private UploadValidators.Factory uploadValidatorsFactory;
@Inject private SshSession session; @Inject private SshSession session;
@Override @Override
@ -63,9 +47,7 @@ final class Upload extends AbstractGitCommand {
} }
final UploadPack up = new UploadPack(repo); final UploadPack up = new UploadPack(repo);
up.setAdvertiseRefsHook( up.setAdvertiseRefsHook(refFilterFactory.create(projectControl.getProjectState(), repo));
new VisibleRefFilter(
tagCache, changeNotesFactory, changeCache, repo, projectControl, db, true));
up.setPackConfig(config.getPackConfig()); up.setPackConfig(config.getPackConfig());
up.setTimeout(config.getTimeout()); up.setTimeout(config.getTimeout());
up.setPostUploadHook(PostUploadHookChain.newChain(Lists.newArrayList(postUploadHooks))); up.setPostUploadHook(PostUploadHookChain.newChain(Lists.newArrayList(postUploadHooks)));

@ -1 +1 @@
Subproject commit f9e2ef3f01d8b4a0e4951ff7195d2989f75a20dc Subproject commit 2a4d0bfe10c63c79ca0d47be21756377703e46c0