VisibleRefFilter Avoid touching the database for drafts

This is an analogous problem to the one with search results fixed in
Ie54b9e2d. VisibleRefFilter is a little trickier because it was using
ChangeCache, which only returned Changes, not the reviewer set. Change
SearchingChangeCacheImpl to cache a different value type and return
ChangeDatas from its search method so we can use the cached
ReviewerSet where appropriate.

Unfortunately for the Reindex program we still need to support not
having a SearchingChangeCacheImpl, which means we have a completely
separate codepath for reading changes from the database.

Change-Id: Ic432a8e48a2bafc8d142b84b25101d95ffb674b7
This commit is contained in:
Dave Borowitz 2016-06-03 11:32:50 -04:00
parent fe99d302cc
commit a758640bc3
14 changed files with 267 additions and 57 deletions

View File

@ -29,13 +29,14 @@ import com.google.gerrit.server.RequestCleanup;
import com.google.gerrit.server.config.GerritRequestModule;
import com.google.gerrit.server.config.RequestScopedReviewDbProvider;
import com.google.gerrit.server.git.AsyncReceiveCommits;
import com.google.gerrit.server.git.ChangeCache;
import com.google.gerrit.server.git.ReceiveCommits;
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.VisibleRefFilter;
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.ProjectControl;
import com.google.gerrit.server.util.RequestContext;
@ -211,8 +212,9 @@ class InProcessProtocol extends TestProtocol<Context> {
private final Provider<ReviewDb> dbProvider;
private final Provider<CurrentUser> userProvider;
private final TagCache tagCache;
private final ChangeCache changeCache;
private final SearchingChangeCacheImpl changeCache;
private final ProjectControl.GenericFactory projectControlFactory;
private final ChangeNotes.Factory changeNotesFactory;
private final TransferConfig transferConfig;
private final DynamicSet<PreUploadHook> preUploadHooks;
private final UploadValidators.Factory uploadValidatorsFactory;
@ -223,8 +225,9 @@ class InProcessProtocol extends TestProtocol<Context> {
Provider<ReviewDb> dbProvider,
Provider<CurrentUser> userProvider,
TagCache tagCache,
ChangeCache changeCache,
SearchingChangeCacheImpl changeCache,
ProjectControl.GenericFactory projectControlFactory,
ChangeNotes.Factory changeNotesFactory,
TransferConfig transferConfig,
DynamicSet<PreUploadHook> preUploadHooks,
UploadValidators.Factory uploadValidatorsFactory,
@ -234,6 +237,7 @@ class InProcessProtocol extends TestProtocol<Context> {
this.tagCache = tagCache;
this.changeCache = changeCache;
this.projectControlFactory = projectControlFactory;
this.changeNotesFactory = changeNotesFactory;
this.transferConfig = transferConfig;
this.preUploadHooks = preUploadHooks;
this.uploadValidatorsFactory = uploadValidatorsFactory;
@ -260,7 +264,8 @@ class InProcessProtocol extends TestProtocol<Context> {
up.setPackConfig(transferConfig.getPackConfig());
up.setTimeout(transferConfig.getTimeout());
up.setAdvertiseRefsHook(new VisibleRefFilter(
tagCache, changeCache, repo, ctl, dbProvider.get(), true));
tagCache, changeNotesFactory, changeCache, repo, ctl,
dbProvider.get(), true));
List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks);
hooks.add(uploadValidatorsFactory.create(
ctl.getProject(), repo, "localhost-test"));

View File

@ -29,15 +29,17 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.edit.ChangeEditModifier;
import com.google.gerrit.server.git.ChangeCache;
import com.google.gerrit.server.git.ProjectConfig;
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.project.ProjectControl;
import com.google.gerrit.server.project.Util;
import com.google.gerrit.testutil.DisabledReviewDb;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Ref;
@ -59,11 +61,14 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
private ProjectControl.GenericFactory projectControlFactory;
@Inject
private ChangeCache changeCache;
private SearchingChangeCacheImpl changeCache;
@Inject
private TagCache tagCache;
@Inject
private Provider<CurrentUser> userProvider;
private AccountGroup.UUID admins;
private Change.Id c1;
@ -136,6 +141,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
Util.doNotInherit(cfg, Permission.READ, "refs/meta/config");
saveProjectConfig(project, cfg);
setApiUser(user);
assertRefs(
"HEAD",
r1 + "1",
@ -171,6 +177,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/master");
deny(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
setApiUser(user);
assertRefs(
"HEAD",
r1 + "1",
@ -184,6 +191,7 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
deny(Permission.READ, REGISTERED_USERS, "refs/heads/master");
allow(Permission.READ, REGISTERED_USERS, "refs/heads/branch");
setApiUser(user);
assertRefs(
r2 + "1",
r2 + "meta",
@ -252,6 +260,70 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
}
}
@Test
public void draftRefs() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
PushOneCommit.Result br = pushFactory.create(db, admin.getIdent(), testRepo)
.to("refs/drafts/master");
br.assertOkStatus();
Change.Id c3 = br.getChange().getId();
String r3 = changeRefPrefix(c3);
// Only admin can see admin's draft change.
setApiUser(admin);
assertRefs(
"HEAD",
r1 + "1",
r1 + "meta",
r2 + "1",
r2 + "meta",
r3 + "1",
r3 + "meta",
"refs/heads/branch",
"refs/heads/master",
"refs/meta/config",
"refs/tags/branch-tag",
"refs/tags/master-tag");
// user can't.
setApiUser(user);
assertRefs(
"HEAD",
r1 + "1",
r1 + "meta",
r2 + "1",
r2 + "meta",
"refs/heads/branch",
"refs/heads/master",
"refs/tags/branch-tag",
"refs/tags/master-tag");
}
@Test
public void noSearchingChangeCacheImpl() throws Exception {
allow(Permission.READ, REGISTERED_USERS, "refs/heads/*");
setApiUser(user);
try (Repository repo = repoManager.openRepository(project)) {
assertRefs(
repo,
new VisibleRefFilter(tagCache, notesFactory, null, repo,
projectControl(), db, true),
// Can't use stored values from the index so DB must be enabled.
false,
"HEAD",
r1 + "1",
r1 + "meta",
r2 + "1",
r2 + "meta",
"refs/heads/branch",
"refs/heads/master",
"refs/tags/branch-tag",
"refs/tags/master-tag");
}
}
/**
* Assert that refs seen by a non-admin user match expected.
*
@ -261,6 +333,18 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
* @throws Exception
*/
private void assertRefs(String... expectedWithMeta) throws Exception {
try (Repository repo = repoManager.openRepository(project)) {
assertRefs(
repo,
new VisibleRefFilter(tagCache, notesFactory, changeCache, repo,
projectControl(), new DisabledReviewDb(), true),
true,
expectedWithMeta);
}
}
private void assertRefs(Repository repo, VisibleRefFilter filter,
boolean disableDb, String... expectedWithMeta) throws Exception {
List<String> expected = new ArrayList<>(expectedWithMeta.length);
for (String r : expectedWithMeta) {
if (notesMigration.writeChanges() || !r.endsWith(RefNames.META_SUFFIX)) {
@ -268,17 +352,22 @@ public class VisibleRefFilterIT extends AbstractDaemonTest {
}
}
AcceptanceTestRequestScope.Context ctx = disableDb();
try (Repository repo = repoManager.openRepository(project)) {
ProjectControl ctl = projectControlFactory.controlFor(project,
identifiedUserFactory.create(user.getId()));
VisibleRefFilter filter = new VisibleRefFilter(
tagCache, changeCache, repo, ctl, new DisabledReviewDb(), true);
AcceptanceTestRequestScope.Context ctx = null;
if (disableDb) {
ctx = disableDb();
}
try {
Map<String, Ref> all = repo.getAllRefs();
assertThat(filter.filter(all, false).keySet())
.containsExactlyElementsIn(expected);
} finally {
enableDb(ctx);
if (disableDb) {
enableDb(ctx);
}
}
}
private ProjectControl projectControl() throws Exception {
return projectControlFactory.controlFor(project, userProvider.get());
}
}

View File

@ -25,14 +25,15 @@ import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.git.AsyncReceiveCommits;
import com.google.gerrit.server.git.ChangeCache;
import com.google.gerrit.server.git.GitRepositoryManager;
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.UploadPackMetricsHook;
import com.google.gerrit.server.git.VisibleRefFilter;
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.ProjectControl;
import com.google.inject.AbstractModule;
@ -223,14 +224,18 @@ public class GitOverHttpServlet extends GitServlet {
static class UploadFilter implements Filter {
private final Provider<ReviewDb> db;
private final TagCache tagCache;
private final ChangeCache changeCache;
private final ChangeNotes.Factory changeNotesFactory;
private final SearchingChangeCacheImpl changeCache;
private final UploadValidators.Factory uploadValidatorsFactory;
@Inject
UploadFilter(Provider<ReviewDb> db, TagCache tagCache, ChangeCache changeCache,
UploadFilter(Provider<ReviewDb> db, TagCache tagCache,
ChangeNotes.Factory changeNotesFactory,
SearchingChangeCacheImpl changeCache,
UploadValidators.Factory uploadValidatorsFactory) {
this.db = db;
this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache;
this.uploadValidatorsFactory = uploadValidatorsFactory;
}
@ -255,8 +260,8 @@ public class GitOverHttpServlet extends GitServlet {
uploadValidatorsFactory.create(pc.getProject(), repo, request.getRemoteHost());
up.setPreUploadHook(PreUploadHookChain.newChain(
Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache,
repo, pc, db.get(), true));
up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeNotesFactory,
changeCache, repo, pc, db.get(), true));
next.doFilter(request, response);
}

View File

@ -47,6 +47,7 @@ import com.google.gerrit.server.config.GitReceivePackGroups;
import com.google.gerrit.server.config.GitUploadPackGroups;
import com.google.gerrit.server.git.BatchUpdate;
import com.google.gerrit.server.git.MergeUtil;
import com.google.gerrit.server.git.SearchingChangeCacheImpl;
import com.google.gerrit.server.git.TagCache;
import com.google.gerrit.server.group.GroupModule;
import com.google.gerrit.server.mail.ReplacePatchSetSender;
@ -120,6 +121,11 @@ public class BatchProgramModule extends FactoryModule {
factory(PatchSetInserter.Factory.class);
factory(RebaseChangeOp.Factory.class);
// As Reindex is a batch program, don't assume the index is available for
// the change cache.
bind(SearchingChangeCacheImpl.class).toProvider(
Providers.<SearchingChangeCacheImpl>of(null));
bind(new TypeLiteral<Set<AccountGroup.UUID>>() {})
.annotatedWith(GitUploadPackGroups.class)
.toInstance(Collections.<AccountGroup.UUID> emptySet());

View File

@ -373,7 +373,7 @@ public class ReceiveCommits {
GitRepositoryManager repoManager,
TagCache tagCache,
AccountCache accountCache,
ChangeCache changeCache,
SearchingChangeCacheImpl changeCache,
ChangesCollection changes,
ChangeInserter.Factory changeInserterFactory,
CommitValidators.Factory commitValidatorsFactory,
@ -480,8 +480,8 @@ public class ReceiveCommits {
rp.setCheckReferencedObjectsAreReachable(
receiveConfig.checkReferencedObjectsAreReachable);
}
rp.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo,
projectControl, db, false));
rp.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, notesFactory,
changeCache, repo, projectControl, db, false));
List<AdvertiseRefsHook> advHooks = new ArrayList<>(3);
advHooks.add(new AdvertiseRefsHook() {
@Override

View File

@ -14,12 +14,18 @@
package com.google.gerrit.server.git;
import com.google.auto.value.AutoValue;
import com.google.common.base.Function;
import com.google.common.cache.CacheLoader;
import com.google.common.cache.LoadingCache;
import com.google.common.collect.Lists;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.extensions.events.GitReferenceUpdatedListener;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.RefNames;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.cache.CacheModule;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.InternalChangeQuery;
@ -34,6 +40,7 @@ import com.google.inject.name.Named;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collections;
import java.util.List;
import java.util.concurrent.ExecutionException;
@ -52,25 +59,62 @@ public class SearchingChangeCacheImpl
bind(ChangeCache.class).to(SearchingChangeCacheImpl.class);
cache(ID_CACHE,
Project.NameKey.class,
new TypeLiteral<List<Change>>() {})
new TypeLiteral<List<CachedChange>>() {})
.maximumWeight(0)
.loader(Loader.class);
}
};
}
private final LoadingCache<Project.NameKey, List<Change>> cache;
@AutoValue
static abstract class CachedChange {
// Subset of fields in ChangeData, specifically fields needed to serve
// VisibleRefFilter without touching the database. More can be added as
// necessary.
abstract Change change();
@Nullable abstract ReviewerSet reviewers();
}
private final LoadingCache<Project.NameKey, List<CachedChange>> cache;
private final ChangeData.Factory changeDataFactory;
@Inject
SearchingChangeCacheImpl(
@Named(ID_CACHE) LoadingCache<Project.NameKey, List<Change>> cache) {
@Named(ID_CACHE) LoadingCache<Project.NameKey, List<CachedChange>> cache,
ChangeData.Factory changeDataFactory) {
this.cache = cache;
this.changeDataFactory = changeDataFactory;
}
@Override
public List<Change> get(Project.NameKey name) {
try {
return cache.get(name);
return Lists.transform(
cache.get(name),
new Function<CachedChange, Change>() {
@Override
public Change apply(CachedChange in) {
return in.change();
}
});
} catch (ExecutionException e) {
log.warn("Cannot fetch changes for " + name, e);
return Collections.emptyList();
}
}
// TODO(dborowitz): I think VisibleRefFilter is the only user of ChangeCache
// at all; probably we can just stop implementing that interface entirely.
public List<ChangeData> getChangeData(ReviewDb db, Project.NameKey name) {
try {
List<CachedChange> cached = cache.get(name);
List<ChangeData> cds = new ArrayList<>(cached.size());
for (CachedChange cc : cached) {
ChangeData cd = changeDataFactory.create(db, cc.change());
cd.setReviewers(cc.reviewers());
cds.add(cd);
}
return Collections.unmodifiableList(cds);
} catch (ExecutionException e) {
log.warn("Cannot fetch changes for " + name, e);
return Collections.emptyList();
@ -84,7 +128,7 @@ public class SearchingChangeCacheImpl
}
}
static class Loader extends CacheLoader<Project.NameKey, List<Change>> {
static class Loader extends CacheLoader<Project.NameKey, List<CachedChange>> {
private final OneOffRequestContext requestContext;
private final Provider<InternalChangeQuery> queryProvider;
@ -96,10 +140,15 @@ public class SearchingChangeCacheImpl
}
@Override
public List<Change> load(Project.NameKey key) throws Exception {
public List<CachedChange> load(Project.NameKey key) throws Exception {
try (AutoCloseable ctx = requestContext.open()) {
return Collections.unmodifiableList(
ChangeData.asChanges(queryProvider.get().byProject(key)));
List<ChangeData> cds = queryProvider.get().byProject(key);
List<CachedChange> result = new ArrayList<>(cds.size());
for (ChangeData cd : cds) {
result.add(new AutoValue_SearchingChangeCacheImpl_CachedChange(
cd.change(), cd.getReviewers()));
}
return Collections.unmodifiableList(result);
}
}
}

View File

@ -16,13 +16,16 @@ package com.google.gerrit.server.git;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Maps;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
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.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import org.eclipse.jgit.lib.Constants;
@ -50,17 +53,24 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
LoggerFactory.getLogger(VisibleRefFilter.class);
private final TagCache tagCache;
private final ChangeCache changeCache;
private final ChangeNotes.Factory changeNotesFactory;
@Nullable private final SearchingChangeCacheImpl changeCache;
private final Repository db;
private final Project.NameKey projectName;
private final ProjectControl projectCtl;
private final ReviewDb reviewDb;
private final boolean showMetadata;
public VisibleRefFilter(TagCache tagCache, ChangeCache changeCache,
Repository db, ProjectControl projectControl, ReviewDb reviewDb,
public VisibleRefFilter(
TagCache tagCache,
ChangeNotes.Factory changeNotesFactory,
@Nullable SearchingChangeCacheImpl changeCache,
Repository db,
ProjectControl projectControl,
ReviewDb reviewDb,
boolean showMetadata) {
this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache;
this.db = db;
this.projectName = projectControl.getProject().getNameKey();
@ -195,14 +205,21 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
private Set<Change.Id> visibleChanges() {
if (!showMetadata) {
return Collections.emptySet();
} else if (changeCache == null) {
return visibleChangesByScan();
}
return visibleChangesBySearch();
}
final Project project = projectCtl.getProject();
private Set<Change.Id> visibleChangesBySearch() {
Project project = projectCtl.getProject();
try {
final Set<Change.Id> visibleChanges = new HashSet<>();
for (Change change : changeCache.get(project.getNameKey())) {
if (projectCtl.controlForIndexedChange(change).isVisible(reviewDb)) {
visibleChanges.add(change.getId());
Set<Change.Id> visibleChanges = new HashSet<>();
for (ChangeData cd : changeCache.getChangeData(
reviewDb, project.getNameKey())) {
if (projectCtl.controlForIndexedChange(cd.change())
.isVisible(reviewDb, cd)) {
visibleChanges.add(cd.getId());
}
}
return visibleChanges;
@ -213,6 +230,23 @@ public class VisibleRefFilter extends AbstractAdvertiseRefsHook {
}
}
private Set<Change.Id> visibleChangesByScan() {
Project.NameKey project = projectCtl.getProject().getNameKey();
try {
Set<Change.Id> visibleChanges = new HashSet<>();
for (ChangeNotes cn : changeNotesFactory.scan(db, reviewDb, project)) {
if (projectCtl.controlFor(cn).isVisible(reviewDb)) {
visibleChanges.add(cn.getChangeId());
}
}
return visibleChanges;
} catch (IOException | OrmException e) {
log.error("Cannot load changes for project " + project
+ ", assuming no changes are visible", e);
return Collections.emptySet();
}
}
private static boolean isTag(Ref ref) {
return ref.getLeaf().getName().startsWith(Constants.R_TAGS);
}

View File

@ -23,10 +23,11 @@ import com.google.gerrit.extensions.restapi.RestReadView;
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.git.ChangeCache;
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.notedb.ChangeNotes;
import com.google.inject.Inject;
import com.google.inject.Provider;
@ -52,7 +53,8 @@ public class ListTags implements RestReadView<ProjectResource> {
private final GitRepositoryManager repoManager;
private final Provider<ReviewDb> dbProvider;
private final TagCache tagCache;
private final ChangeCache changeCache;
private final ChangeNotes.Factory changeNotesFactory;
private final SearchingChangeCacheImpl changeCache;
@Option(name = "--limit", aliases = {"-n"}, metaVar = "CNT", usage = "maximum number of tags to list")
public void setLimit(int limit) {
@ -83,10 +85,12 @@ public class ListTags implements RestReadView<ProjectResource> {
public ListTags(GitRepositoryManager repoManager,
Provider<ReviewDb> dbProvider,
TagCache tagCache,
ChangeCache changeCache) {
ChangeNotes.Factory changeNotesFactory,
SearchingChangeCacheImpl changeCache) {
this.repoManager = repoManager;
this.dbProvider = dbProvider;
this.tagCache = tagCache;
this.changeNotesFactory = changeNotesFactory;
this.changeCache = changeCache;
}
@ -147,7 +151,7 @@ public class ListTags implements RestReadView<ProjectResource> {
private Map<String, Ref> visibleTags(ProjectControl control, Repository repo,
Map<String, Ref> tags) {
return new VisibleRefFilter(tagCache, changeCache, repo,
return new VisibleRefFilter(tagCache, changeNotesFactory, changeCache, repo,
control, dbProvider.get(), false).filter(tags, true);
}

View File

@ -37,8 +37,8 @@ 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.ChangeCache;
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.group.SystemGroupBackend;
@ -149,11 +149,12 @@ public class ProjectControl {
private final CurrentUser user;
private final ProjectState state;
private final GitRepositoryManager repoManager;
private final ChangeNotes.Factory changeNotesFactory;
private final ChangeControl.Factory changeControlFactory;
private final PermissionCollection.Factory permissionFilter;
private final Collection<ContributorAgreement> contributorAgreements;
private final TagCache tagCache;
private final ChangeCache changeCache;
private final SearchingChangeCacheImpl changeCache;
private List<SectionMatcher> allSections;
private List<SectionMatcher> localSections;
@ -167,13 +168,15 @@ public class ProjectControl {
ProjectCache pc,
PermissionCollection.Factory permissionFilter,
GitRepositoryManager repoManager,
ChangeNotes.Factory changeNotesFactory,
ChangeControl.Factory changeControlFactory,
TagCache tagCache,
ChangeCache changeCache,
SearchingChangeCacheImpl changeCache,
@CanonicalWebUrl @Nullable String canonicalWebUrl,
@Assisted CurrentUser who,
@Assisted ProjectState ps) {
this.repoManager = repoManager;
this.changeNotesFactory = changeNotesFactory;
this.changeControlFactory = changeControlFactory;
this.tagCache = tagCache;
this.changeCache = changeCache;
@ -528,8 +531,8 @@ public class ProjectControl {
boolean isMergedIntoVisibleRef(Repository repo, ReviewDb db, RevWalk rw,
RevCommit commit, Collection<Ref> unfilteredRefs) throws IOException {
VisibleRefFilter filter =
new VisibleRefFilter(tagCache, changeCache, repo, this, db, true);
VisibleRefFilter filter = new VisibleRefFilter(
tagCache, changeNotesFactory, changeCache, repo, this, db, true);
Map<String, Ref> m = Maps.newHashMapWithExpectedSize(unfilteredRefs.size());
for (Ref r : unfilteredRefs) {
m.put(r.getName(), r);

View File

@ -916,6 +916,10 @@ public void setPatchSets(Collection<PatchSet> patchSets) {
this.reviewers = reviewers;
}
public ReviewerSet getReviewers() {
return reviewers;
}
public Collection<PatchLineComment> publishedComments()
throws OrmException {
if (publishedComments == null) {

View File

@ -879,7 +879,7 @@ public class RefControlTest {
return new ProjectControl(Collections.<AccountGroup.UUID> emptySet(),
Collections.<AccountGroup.UUID> emptySet(), projectCache,
sectionSorter, repoManager, changeControlFactory, null, null,
sectionSorter, repoManager, null, changeControlFactory, null, null,
canonicalWebUrl, new MockUser(name, memberOf), newProjectState(local));
}

View File

@ -24,10 +24,11 @@ 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.account.AccountResolver;
import com.google.gerrit.server.git.ChangeCache;
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.notedb.ChangeNotes;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
@ -59,7 +60,10 @@ public class LsUserRefs extends SshCommand {
private TagCache tagCache;
@Inject
private ChangeCache changeCache;
private ChangeNotes.Factory changeNotesFactory;
@Inject
private SearchingChangeCacheImpl changeCache;
@Option(name = "--project", aliases = {"-p"}, metaVar = "PROJECT",
required = true, usage = "project for which the refs should be listed")
@ -95,9 +99,10 @@ public class LsUserRefs extends SshCommand {
try (Repository repo = repoManager.openRepository(
userProjectControl.getProject().getNameKey())) {
try {
Map<String, Ref> refsMap =
new VisibleRefFilter(tagCache, changeCache, repo, userProjectControl,
db, true).filter(repo.getRefDatabase().getRefs(ALL), false);
Map<String, Ref> refsMap = new VisibleRefFilter(
tagCache, changeNotesFactory, changeCache, repo,
userProjectControl, db, true)
.filter(repo.getRefDatabase().getRefs(ALL), false);
for (final String ref : refsMap.keySet()) {
if (!onlyRefsHeads || ref.startsWith(RefNames.REFS_HEADS)) {

View File

@ -17,13 +17,14 @@ package com.google.gerrit.sshd.commands;
import com.google.common.collect.Lists;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.git.ChangeCache;
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.UploadPackMetricsHook;
import com.google.gerrit.server.git.VisibleRefFilter;
import com.google.gerrit.server.git.validators.UploadValidationException;
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.SshSession;
import com.google.inject.Inject;
@ -47,7 +48,10 @@ final class Upload extends AbstractGitCommand {
private TagCache tagCache;
@Inject
private ChangeCache changeCache;
private ChangeNotes.Factory changeNotesFactory;
@Inject
private SearchingChangeCacheImpl changeCache;
@Inject
private DynamicSet<PreUploadHook> preUploadHooks;
@ -68,8 +72,10 @@ final class Upload extends AbstractGitCommand {
}
final UploadPack up = new UploadPack(repo);
up.setAdvertiseRefsHook(new VisibleRefFilter(tagCache, changeCache, repo,
projectControl, db, true));
up.setAdvertiseRefsHook(
new VisibleRefFilter(
tagCache, changeNotesFactory, changeCache, repo, projectControl, db,
true));
up.setPackConfig(config.getPackConfig());
up.setTimeout(config.getTimeout());
up.setPostUploadHook(uploadMetrics);

@ -1 +1 @@
Subproject commit adda82cb5cb2c2220bf7288922889d91829482b2
Subproject commit 939889fd63b55cb98eb89aab59493e5fe368544a