From c89c21cd6d0537e4e81807fbb93da07905d64f0b Mon Sep 17 00:00:00 2001 From: Edwin Kempin Date: Tue, 2 Feb 2016 09:51:36 +0100 Subject: [PATCH] Load change in ChangeNotes.Factory ChangeNotes.Factory now loads the change from the database. This should become the only place where changes are loaded from the database, but for this more work is required in follow-up changes. We need to load the change for the database also when notedb is enabled. This is because for now we still want to write updates to the database when notedb is enabled. In order to write update to the database the change instance must be created by loading it from the database because otherwise its row version is not set and then all updates to the database fail with OrmConcurrencyException. The signature of the ChangeNotes.Factory#create is changed to require project name and a change ID instead of a change. Project name is not needed yet, since the change is always loaded by change ID from the database, but later when reviewdb is gone we will need the project name to instantiate the change from notedb. For changes that have been loaded from the secondary index we don't want to reload them from the database. This is why a new ChangeNotes.Factory#createFromIndexedChange method is added that still takes a change as input. Change-Id: Idc274d6938f989b4d7fa49ee496d7ac84c749455 Signed-off-by: Edwin Kempin --- .../gerrit/acceptance/PushOneCommit.java | 5 +-- .../gerrit/acceptance/git/SubmitOnPushIT.java | 4 +-- .../rest/change/AbstractSubmit.java | 10 +++--- .../gerrit/common/ChangeHookRunner.java | 5 +-- .../server/change/ConsistencyChecker.java | 2 +- .../gerrit/server/git/GroupCollector.java | 13 +++----- .../gerrit/server/git/ReceiveCommits.java | 4 +-- .../server/notedb/AbstractChangeNotes.java | 2 +- .../gerrit/server/notedb/ChangeNotes.java | 33 +++++++++++++------ .../gerrit/server/project/ChangeControl.java | 8 ++--- .../gerrit/server/project/ProjectControl.java | 3 +- .../server/query/change/ChangeData.java | 3 +- .../query/change/ChangeQueryBuilder.java | 19 ++++++----- .../query/change/IsVisibleToPredicate.java | 8 +++-- .../server/query/change/QueryProcessor.java | 7 +++- .../gerrit/server/index/FakeQueryBuilder.java | 2 +- .../notedb/AbstractChangeNotesTest.java | 3 +- .../google/gerrit/testutil/TestChanges.java | 5 +-- .../gerrit/sshd/commands/PatchSetParser.java | 2 +- 19 files changed, 84 insertions(+), 54 deletions(-) diff --git a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java index 3686e9d3ca..af3e5eb135 100644 --- a/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java +++ b/gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/PushOneCommit.java @@ -273,8 +273,9 @@ public class PushOneCommit { private void assertReviewers(Change c, TestAccount... expectedReviewers) throws OrmException { - Iterable actualIds = - approvalsUtil.getReviewers(db, notesFactory.create(db, c)).values(); + Iterable actualIds = approvalsUtil + .getReviewers(db, notesFactory.create(db, c.getProject(), c.getId())) + .values(); assertThat(actualIds).containsExactlyElementsIn( Sets.newHashSet(TestAccount.ids(expectedReviewers))); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index 3ea17af999..747c0ad2d1 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -223,8 +223,8 @@ public class SubmitOnPushIT extends AbstractDaemonTest { private PatchSetApproval getSubmitter(PatchSet.Id patchSetId) throws OrmException { - Change c = db.changes().get(patchSetId.getParentKey()); - ChangeNotes notes = changeNotesFactory.create(db, c).load(); + ChangeNotes notes = changeNotesFactory + .create(db, project, patchSetId.getParentKey()).load(); return approvalsUtil.getSubmitter(db, notes, patchSetId); } diff --git a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java index 1d5d0b604f..2e59e841f7 100644 --- a/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java +++ b/gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/AbstractSubmit.java @@ -313,8 +313,9 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { protected void assertSubmitter(String changeId, int psId) throws OrmException { - ChangeNotes cn = notesFactory.create(db, - getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change()); + Change c = + getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change(); + ChangeNotes cn = notesFactory.create(db, c.getProject(), c.getId()); PatchSetApproval submitter = approvalsUtil.getSubmitter( db, cn, new PatchSet.Id(cn.getChangeId(), psId)); assertThat(submitter).isNotNull(); @@ -324,8 +325,9 @@ public abstract class AbstractSubmit extends AbstractDaemonTest { protected void assertNoSubmitter(String changeId, int psId) throws OrmException { - ChangeNotes cn = notesFactory.create(db, - getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change()); + Change c = + getOnlyElement(queryProvider.get().byKeyPrefix(changeId)).change(); + ChangeNotes cn = notesFactory.create(db, c.getProject(), c.getId()); PatchSetApproval submitter = approvalsUtil.getSubmitter( db, cn, new PatchSet.Id(cn.getChangeId(), psId)); assertThat(submitter).isNull(); diff --git a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java index daf7eba328..af1344e1c1 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java +++ b/gerrit-server/src/main/java/com/google/gerrit/common/ChangeHookRunner.java @@ -309,8 +309,9 @@ public class ChangeHookRunner implements ChangeHooks, EventDispatcher, .build()); } - private ChangeNotes newNotes(ReviewDb db, Change change) { - return notesFactory.create(db, change); + private ChangeNotes newNotes(ReviewDb db, Change change) + throws OrmException { + return notesFactory.create(db, change.getProject(), change.getId()); } private static Optional hook(Config config, Path path, String name) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java index 7d2758c24b..2c65e32da2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ConsistencyChecker.java @@ -587,7 +587,7 @@ public class ConsistencyChecker { if (c == null) { throw new OrmException("Change missing: " + cid); } - ChangeNotes notes = notesFactory.create(db, c); + ChangeNotes notes = notesFactory.create(db, c.getProject(), cid); if (psId.equals(c.currentPatchSetId())) { List all = Lists.newArrayList(db.patchSets().byChange(cid)); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java index 0051e2f60f..8d13f7e8ca 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/GroupCollector.java @@ -31,8 +31,8 @@ import com.google.common.collect.Multimaps; import com.google.common.collect.SetMultimap; import com.google.common.collect.Sets; import com.google.common.collect.SortedSetMultimap; -import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; +import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.PatchSetUtil; import com.google.gerrit.server.change.RevisionResource; @@ -115,18 +115,15 @@ public class GroupCollector { public static GroupCollector create(Multimap changeRefsById, final ReviewDb db, final PatchSetUtil psUtil, - final ChangeNotes.Factory notesFactory) { + final ChangeNotes.Factory notesFactory, final Project.NameKey project) { return new GroupCollector( transformRefs(changeRefsById), new Lookup() { @Override public List lookup(PatchSet.Id psId) throws OrmException { - // TODO(dborowitz): Shouldn't have to look up Change. - Change c = db.changes().get(psId.getParentKey()); - if (c == null) { - return null; - } - ChangeNotes notes = notesFactory.create(db, c); + // TODO(dborowitz): Reuse open repository from caller. + ChangeNotes notes = + notesFactory.create(db, project, psId.getParentKey()); PatchSet ps = psUtil.get(db, notes, psId); return ps != null ? ps.getGroups() : null; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java index 3a24db788e..86ae1ab286 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/git/ReceiveCommits.java @@ -1526,8 +1526,8 @@ public class ReceiveCommits { newChanges = Lists.newArrayList(); SetMultimap existing = changeRefsById(); - GroupCollector groupCollector = - GroupCollector.create(refsById, db, psUtil, notesFactory); + GroupCollector groupCollector = GroupCollector.create(refsById, db, psUtil, + notesFactory, project.getNameKey()); rp.getRevWalk().reset(); rp.getRevWalk().sort(RevSort.TOPO); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java index 88f795ddbb..c22d4f8abd 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/AbstractChangeNotes.java @@ -50,7 +50,7 @@ public abstract class AbstractChangeNotes extends VersionedMetaData { if (loaded) { return self(); } - if (!migration.enabled()) { + if (!migration.enabled() || changeId == null) { loadDefaults(); return self(); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java index aa8028a607..b5e8c48b26 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/notedb/ChangeNotes.java @@ -116,16 +116,27 @@ public class ChangeNotes extends AbstractChangeNotes { this.allUsersProvider = allUsersProvider; } - public ChangeNotes create(@SuppressWarnings("unused") ReviewDb db, - Change change) { - return new ChangeNotes(repoManager, migration, allUsersProvider, change); + public ChangeNotes create(ReviewDb db, Project.NameKey project, + Change.Id changeId) throws OrmException { + Change change = db.changes().get(changeId); + // TODO: Throw NoSuchChangeException when the change is not found in the + // database + return new ChangeNotes(repoManager, migration, allUsersProvider, project, + change); + } + + public ChangeNotes createFromIndexedChange(Change change) { + return new ChangeNotes(repoManager, migration, allUsersProvider, + change.getProject(), change); } public ChangeNotes createForNew(Change change) { - return new ChangeNotes(repoManager, migration, allUsersProvider, change); + return new ChangeNotes(repoManager, migration, allUsersProvider, + change.getProject(), change); } } + private final Project.NameKey project; private final Change change; private ImmutableSortedMap patchSets; private ImmutableListMultimap approvals; @@ -147,10 +158,12 @@ public class ChangeNotes extends AbstractChangeNotes { @VisibleForTesting public ChangeNotes(GitRepositoryManager repoManager, NotesMigration migration, - AllUsersNameProvider allUsersProvider, Change change) { - super(repoManager, migration, change.getId()); + AllUsersNameProvider allUsersProvider, Project.NameKey project, + Change change) { + super(repoManager, migration, change != null ? change.getId() : null); this.allUsers = allUsersProvider.get(); - this.change = new Change(change); + this.project = project; + this.change = change != null ? new Change(change) : null; } public Change getChange() { @@ -283,7 +296,7 @@ public class ChangeNotes extends AbstractChangeNotes { return; } try (RevWalk walk = new RevWalk(reader); - ChangeNotesParser parser = new ChangeNotesParser(change.getProject(), + ChangeNotesParser parser = new ChangeNotesParser(project, change.getId(), rev, walk, repoManager)) { parser.parseAll(); @@ -297,7 +310,7 @@ public class ChangeNotes extends AbstractChangeNotes { noteMap = parser.noteMap; revisionNotes = parser.revisionNotes; change.setKey(new Change.Key(parser.changeId)); - change.setDest(new Branch.NameKey(getProjectName(), parser.branch)); + change.setDest(new Branch.NameKey(project, parser.branch)); change.setTopic(Strings.emptyToNull(parser.topic)); change.setCreatedOn(parser.createdOn); change.setLastUpdatedOn(parser.lastUpdatedOn); @@ -352,6 +365,6 @@ public class ChangeNotes extends AbstractChangeNotes { @Override public Project.NameKey getProjectName() { - return getChange().getProject(); + return project; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java index 8b5695e725..0fd0ca4c50 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/project/ChangeControl.java @@ -121,10 +121,10 @@ public class ChangeControl { this.approvalsUtil = approvalsUtil; } - @SuppressWarnings("unused") - ChangeControl create(RefControl refControl, Change change) - throws OrmException { - return create(refControl, notesFactory.create(db, change)); + ChangeControl create(RefControl refControl, Project.NameKey project, + Change.Id changeId) throws OrmException { + return create(refControl, + notesFactory.create(db, project, changeId)); } ChangeControl create(RefControl refControl, ChangeNotes notes) { 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 367dc1464e..e51bf98755 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 @@ -195,7 +195,8 @@ public class ProjectControl { } public ChangeControl controlFor(Change change) throws OrmException { - return changeControlFactory.create(controlForRef(change.getDest()), change); + return changeControlFactory.create(controlForRef(change.getDest()), + change.getProject(), change.getId()); } public ChangeControl controlFor(ChangeNotes notes) { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java index 65c7295151..431852d6b2 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeData.java @@ -597,7 +597,8 @@ public class ChangeData { public ChangeNotes notes() throws OrmException { if (notes == null) { - notes = notesFactory.create(db, change()); + Change c = change(); + notes = notesFactory.create(db, c.getProject(), c.getId()); } return notes; } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java index eaed46614d..c502faf1e9 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java @@ -57,6 +57,7 @@ import com.google.gerrit.server.index.IndexCollection; import com.google.gerrit.server.index.IndexConfig; import com.google.gerrit.server.index.IndexRewriter; import com.google.gerrit.server.index.Schema; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.patch.PatchListCache; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ListChildProjects; @@ -161,6 +162,7 @@ public class ChangeQueryBuilder extends QueryBuilder { final IdentifiedUser.GenericFactory userFactory; final CapabilityControl.Factory capabilityControlFactory; final ChangeControl.GenericFactory changeControlGenericFactory; + final ChangeNotes.Factory notesFactory; final ChangeData.Factory changeDataFactory; final FieldDef.FillArgs fillArgs; final PatchLineCommentsUtil plcUtil; @@ -192,6 +194,7 @@ public class ChangeQueryBuilder extends QueryBuilder { Provider self, CapabilityControl.Factory capabilityControlFactory, ChangeControl.GenericFactory changeControlGenericFactory, + ChangeNotes.Factory notesFactory, ChangeData.Factory changeDataFactory, FieldDef.FillArgs fillArgs, PatchLineCommentsUtil plcUtil, @@ -211,12 +214,11 @@ public class ChangeQueryBuilder extends QueryBuilder { Provider listMembers, @GerritServerConfig Config cfg) { this(db, queryProvider, rewriter, opFactories, userFactory, self, - capabilityControlFactory, changeControlGenericFactory, + capabilityControlFactory, changeControlGenericFactory, notesFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, allUsersName, patchListCache, repoManager, - projectCache, listChildProjects, submitDryRun, - conflictsCache, trackingFooters, - indexes != null ? indexes.getSearchIndex() : null, + projectCache, listChildProjects, submitDryRun, conflictsCache, + trackingFooters, indexes != null ? indexes.getSearchIndex() : null, indexConfig, listMembers, cfg == null ? true : cfg.getBoolean("change", "allowDrafts", true)); } @@ -230,6 +232,7 @@ public class ChangeQueryBuilder extends QueryBuilder { Provider self, CapabilityControl.Factory capabilityControlFactory, ChangeControl.GenericFactory changeControlGenericFactory, + ChangeNotes.Factory notesFactory, ChangeData.Factory changeDataFactory, FieldDef.FillArgs fillArgs, PatchLineCommentsUtil plcUtil, @@ -255,6 +258,7 @@ public class ChangeQueryBuilder extends QueryBuilder { this.userFactory = userFactory; this.self = self; this.capabilityControlFactory = capabilityControlFactory; + this.notesFactory = notesFactory; this.changeControlGenericFactory = changeControlGenericFactory; this.changeDataFactory = changeDataFactory; this.fillArgs = fillArgs; @@ -279,7 +283,7 @@ public class ChangeQueryBuilder extends QueryBuilder { Arguments asUser(CurrentUser otherUser) { return new Arguments(db, queryProvider, rewriter, opFactories, userFactory, Providers.of(otherUser), - capabilityControlFactory, changeControlGenericFactory, + capabilityControlFactory, changeControlGenericFactory, notesFactory, changeDataFactory, fillArgs, plcUtil, accountResolver, groupBackend, allProjectsName, allUsersName, patchListCache, repoManager, projectCache, listChildProjects, submitDryRun, @@ -729,9 +733,8 @@ public class ChangeQueryBuilder extends QueryBuilder { } public Predicate visibleto(CurrentUser user) { - return new IsVisibleToPredicate(args.db, // - args.changeControlGenericFactory, // - user); + return new IsVisibleToPredicate(args.db, args.notesFactory, + args.changeControlGenericFactory, user); } public Predicate is_visible() throws QueryParseException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsVisibleToPredicate.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsVisibleToPredicate.java index 856a559b38..a99a85c621 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsVisibleToPredicate.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/IsVisibleToPredicate.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.query.change; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.query.OperatorPredicate; @@ -36,13 +37,15 @@ class IsVisibleToPredicate extends OperatorPredicate { } private final Provider db; + private final ChangeNotes.Factory notesFactory; private final ChangeControl.GenericFactory changeControl; private final CurrentUser user; - IsVisibleToPredicate(Provider db, + IsVisibleToPredicate(Provider db, ChangeNotes.Factory notesFactory, ChangeControl.GenericFactory changeControlFactory, CurrentUser user) { super(ChangeQueryBuilder.FIELD_VISIBLETO, describe(user)); this.db = db; + this.notesFactory = notesFactory; this.changeControl = changeControlFactory; this.user = user; } @@ -58,7 +61,8 @@ class IsVisibleToPredicate extends OperatorPredicate { return false; } - ChangeControl cc = changeControl.controlFor(c, user); + ChangeNotes notes = notesFactory.createFromIndexedChange(c); + ChangeControl cc = changeControl.controlFor(notes, user); if (cc.isVisible(db.get())) { cd.cacheVisibleTo(cc); return true; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java index 1a6ae02a0f..12882f3878 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/change/QueryProcessor.java @@ -31,6 +31,7 @@ import com.google.gerrit.server.index.IndexCollection; import com.google.gerrit.server.index.IndexConfig; import com.google.gerrit.server.index.IndexPredicate; import com.google.gerrit.server.index.IndexRewriter; +import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.query.Predicate; import com.google.gerrit.server.query.QueryParseException; @@ -48,6 +49,7 @@ public class QueryProcessor { private final Provider db; private final Provider userProvider; private final ChangeControl.GenericFactory changeControlFactory; + private final ChangeNotes.Factory notesFactory; private final IndexCollection indexes; private final IndexRewriter rewriter; private final IndexConfig indexConfig; @@ -62,6 +64,7 @@ public class QueryProcessor { QueryProcessor(Provider db, Provider userProvider, ChangeControl.GenericFactory changeControlFactory, + ChangeNotes.Factory notesFactory, IndexCollection indexes, IndexRewriter rewriter, IndexConfig indexConfig, @@ -69,6 +72,7 @@ public class QueryProcessor { this.db = db; this.userProvider = userProvider; this.changeControlFactory = changeControlFactory; + this.notesFactory = notesFactory; this.indexes = indexes; this.rewriter = rewriter; this.indexConfig = indexConfig; @@ -138,7 +142,8 @@ public class QueryProcessor { Timer0.Context context = metrics.executionTime.start(); Predicate visibleToMe = enforceVisibility - ? new IsVisibleToPredicate(db, changeControlFactory, userProvider.get()) + ? new IsVisibleToPredicate(db, notesFactory, changeControlFactory, + userProvider.get()) : null; int cnt = queries.size(); diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java index dd03bb043a..a6bcac2977 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/index/FakeQueryBuilder.java @@ -27,7 +27,7 @@ public class FakeQueryBuilder extends ChangeQueryBuilder { FakeQueryBuilder.class), new ChangeQueryBuilder.Arguments(null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, - null, null, indexes, null, null, null, null, null, null)); + null, null, null, indexes, null, null, null, null, null, null)); } @Operator diff --git a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java index 7ba42ae2ab..f8660d4a61 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java +++ b/gerrit-server/src/test/java/com/google/gerrit/server/notedb/AbstractChangeNotesTest.java @@ -186,7 +186,8 @@ public class AbstractChangeNotesTest extends GerritBaseTests { } protected ChangeNotes newNotes(Change c) throws OrmException { - return new ChangeNotes(repoManager, MIGRATION, allUsers, c).load(); + return new ChangeNotes(repoManager, MIGRATION, allUsers, c.getProject(), c) + .load(); } protected static SubmitRecord submitRecord(String status, diff --git a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java index c7e3c4b7e8..300b06779c 100644 --- a/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java +++ b/gerrit-server/src/test/java/com/google/gerrit/testutil/TestChanges.java @@ -139,8 +139,9 @@ public class TestChanges { expect(ctl.getChange()).andStubReturn(c); expect(ctl.getProject()).andStubReturn(new Project(c.getProject())); expect(ctl.getUser()).andStubReturn(user); - ChangeNotes notes = new ChangeNotes(repoManager, migration, allUsers, c) - .load(); + ChangeNotes notes = + new ChangeNotes(repoManager, migration, allUsers, c.getProject(), c) + .load(); expect(ctl.getNotes()).andStubReturn(notes); expect(ctl.getId()).andStubReturn(c.getId()); EasyMock.replay(ctl); diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PatchSetParser.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PatchSetParser.java index 18fed7e34c..ffc4e0559e 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PatchSetParser.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/PatchSetParser.java @@ -127,7 +127,7 @@ public class PatchSetParser { if (c == null) { throw error("\"" + changeId + "\" no such change"); } - return notesFactory.create(db.get(), c); + return notesFactory.create(db.get(), c.getProject(), changeId); } private static boolean inProject(Change change,