Make AccountCache available to Prolog predicates

Some plugins implement Prolog predicates that need to get accounts.
E.g. the find-owners plugin has a Prolog predicate that loads the
accounts for all approvals on a change.

SubmitRuleEvaluator sets ReviewDb as a stored value in the Prolog
environment and hence Prolog predicates in plugins have access to it.
When accounts are moved to NoteDb accounts are no longer accessible
through ReviewDb. Hence the Prolog predicates need a different way to
get accounts. For this we make the AccountCache available by setting it
as a stored value in the Prolog environment.

Using the account cache instead of accessing the database should also
improve performance of Prolog predicates that need to access accounts.

Change-Id: I363a8ee0400622b4cc25d44f1d6fc876c951d412
Signed-off-by: Edwin Kempin <ekempin@google.com>
This commit is contained in:
Edwin Kempin
2017-06-13 14:31:08 +02:00
parent 8c36cc8805
commit c056334599
9 changed files with 66 additions and 11 deletions

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.AnonymousUser;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.patch.PatchList;
import com.google.gerrit.server.patch.PatchListCache;
@@ -46,6 +47,7 @@ import org.eclipse.jgit.lib.ObjectId;
import org.eclipse.jgit.lib.Repository;
public final class StoredValues {
public static final StoredValue<AccountCache> ACCOUNT_CACHE = create(AccountCache.class);
public static final StoredValue<ReviewDb> REVIEW_DB = create(ReviewDb.class);
public static final StoredValue<ChangeData> CHANGE_DATA = create(ChangeData.class);

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.git.BranchOrderSection;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
@@ -61,6 +62,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
private boolean otherBranches;
private final GitRepositoryManager gitManager;
private final AccountCache accountCache;
private final ProjectCache projectCache;
private final MergeUtil.Factory mergeUtilFactory;
private final ChangeData.Factory changeDataFactory;
@@ -71,6 +73,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
@Inject
Mergeable(
GitRepositoryManager gitManager,
AccountCache accountCache,
ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory,
ChangeData.Factory changeDataFactory,
@@ -78,6 +81,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
ChangeIndexer indexer,
MergeabilityCache cache) {
this.gitManager = gitManager;
this.accountCache = accountCache;
this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory;
this.changeDataFactory = changeDataFactory;
@@ -139,7 +143,8 @@ public class Mergeable implements RestReadView<RevisionResource> {
}
private SubmitType getSubmitType(ChangeData cd, PatchSet patchSet) throws OrmException {
SubmitTypeRecord rec = new SubmitRuleEvaluator(cd).setPatchSet(patchSet).getSubmitType();
SubmitTypeRecord rec =
new SubmitRuleEvaluator(accountCache, cd).setPatchSet(patchSet).getSubmitType();
if (rec.status != SubmitTypeRecord.Status.OK) {
throw new OrmException("Submit type rule failed: " + rec);
}

View File

@@ -28,6 +28,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.permissions.LabelPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
@@ -48,6 +49,7 @@ public class ReviewerJson {
private final Provider<ReviewDb> db;
private final PermissionBackend permissionBackend;
private final ChangeData.Factory changeDataFactory;
private final AccountCache accountCache;
private final ApprovalsUtil approvalsUtil;
private final AccountLoader.Factory accountLoaderFactory;
@@ -56,11 +58,13 @@ public class ReviewerJson {
Provider<ReviewDb> db,
PermissionBackend permissionBackend,
ChangeData.Factory changeDataFactory,
AccountCache accountCache,
ApprovalsUtil approvalsUtil,
AccountLoader.Factory accountLoaderFactory) {
this.db = db;
this.permissionBackend = permissionBackend;
this.changeDataFactory = changeDataFactory;
this.accountCache = accountCache;
this.approvalsUtil = approvalsUtil;
this.accountLoaderFactory = accountLoaderFactory;
}
@@ -128,7 +132,10 @@ public class ReviewerJson {
PatchSet ps = cd.currentPatchSet();
if (ps != null) {
for (SubmitRecord rec :
new SubmitRuleEvaluator(cd).setFastEvalLabels(true).setAllowDraft(true).evaluate()) {
new SubmitRuleEvaluator(accountCache, cd)
.setFastEvalLabels(true)
.setAllowDraft(true)
.evaluate()) {
if (rec.labels == null) {
continue;
}

View File

@@ -24,6 +24,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.RulesCache;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
@@ -39,6 +40,7 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
private final Provider<ReviewDb> db;
private final ChangeData.Factory changeDataFactory;
private final RulesCache rules;
private final AccountCache accountCache;
private final AccountLoader.Factory accountInfoFactory;
@Option(name = "--filters", usage = "impact of filters in parent projects")
@@ -49,10 +51,12 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory,
RulesCache rules,
AccountCache accountCache,
AccountLoader.Factory infoFactory) {
this.db = db;
this.changeDataFactory = changeDataFactory;
this.rules = rules;
this.accountCache = accountCache;
this.accountInfoFactory = infoFactory;
}
@@ -67,7 +71,8 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
}
input.filters = MoreObjects.firstNonNull(input.filters, filters);
SubmitRuleEvaluator evaluator =
new SubmitRuleEvaluator(changeDataFactory.create(db.get(), rsrc.getControl()));
new SubmitRuleEvaluator(
accountCache, changeDataFactory.create(db.get(), rsrc.getControl()));
List<SubmitRecord> records =
evaluator

View File

@@ -25,6 +25,7 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.RulesCache;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
@@ -34,6 +35,7 @@ import org.kohsuke.args4j.Option;
public class TestSubmitType implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
private final Provider<ReviewDb> db;
private final AccountCache accountCache;
private final ChangeData.Factory changeDataFactory;
private final RulesCache rules;
@@ -41,8 +43,13 @@ public class TestSubmitType implements RestModifyView<RevisionResource, TestSubm
private Filters filters = Filters.RUN;
@Inject
TestSubmitType(Provider<ReviewDb> db, ChangeData.Factory changeDataFactory, RulesCache rules) {
TestSubmitType(
Provider<ReviewDb> db,
AccountCache accountCache,
ChangeData.Factory changeDataFactory,
RulesCache rules) {
this.db = db;
this.accountCache = accountCache;
this.changeDataFactory = changeDataFactory;
this.rules = rules;
}
@@ -58,7 +65,8 @@ public class TestSubmitType implements RestModifyView<RevisionResource, TestSubm
}
input.filters = MoreObjects.firstNonNull(input.filters, filters);
SubmitRuleEvaluator evaluator =
new SubmitRuleEvaluator(changeDataFactory.create(db.get(), rsrc.getControl()));
new SubmitRuleEvaluator(
accountCache, changeDataFactory.create(db.get(), rsrc.getControl()));
SubmitTypeRecord rec =
evaluator

View File

@@ -31,6 +31,7 @@ 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.CurrentUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
@@ -94,6 +95,7 @@ public class MergeSuperSet {
abstract ImmutableSet<String> hashes();
}
private final AccountCache accountCache;
private final ChangeData.Factory changeDataFactory;
private final Provider<InternalChangeQuery> queryProvider;
private final Provider<MergeOpRepoManager> repoManagerProvider;
@@ -107,10 +109,12 @@ public class MergeSuperSet {
@Inject
MergeSuperSet(
@GerritServerConfig Config cfg,
AccountCache accountCache,
ChangeData.Factory changeDataFactory,
Provider<InternalChangeQuery> queryProvider,
Provider<MergeOpRepoManager> repoManagerProvider) {
this.cfg = cfg;
this.accountCache = accountCache;
this.changeDataFactory = changeDataFactory;
this.queryProvider = queryProvider;
this.repoManagerProvider = repoManagerProvider;
@@ -161,7 +165,7 @@ public class MergeSuperSet {
SubmitTypeRecord str =
ps == cd.currentPatchSet()
? cd.submitTypeRecord()
: new SubmitRuleEvaluator(cd).setPatchSet(ps).getSubmitType();
: new SubmitRuleEvaluator(accountCache, cd).setPatchSet(ps).getSubmitType();
if (!str.isOk()) {
logErrorAndThrow("Failed to get submit type for " + cd.getId() + ": " + str.errorMessage);
}

View File

@@ -27,6 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.rules.PrologEnvironment;
import com.google.gerrit.rules.StoredValues;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gwtorm.server.OrmException;
import com.googlecode.prolog_cafe.exceptions.CompileException;
@@ -81,6 +82,7 @@ public class SubmitRuleEvaluator {
}
}
private final AccountCache accountCache;
private final ChangeData cd;
private final ChangeControl control;
@@ -92,7 +94,8 @@ public class SubmitRuleEvaluator {
private Term submitRule;
public SubmitRuleEvaluator(ChangeData cd) throws OrmException {
public SubmitRuleEvaluator(AccountCache accountCache, ChangeData cd) throws OrmException {
this.accountCache = accountCache;
this.cd = cd;
this.control = cd.changeControl();
}
@@ -564,6 +567,7 @@ public class SubmitRuleEvaluator {
}
throw new RuleEvalException(msg, err);
}
env.set(StoredValues.ACCOUNT_CACHE, accountCache);
env.set(StoredValues.REVIEW_DB, cd.db());
env.set(StoredValues.CHANGE_DATA, cd);
env.set(StoredValues.CHANGE_CONTROL, control);

View File

@@ -58,6 +58,7 @@ import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.StarRef;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.change.MergeabilityCache;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil;
@@ -302,7 +303,7 @@ public class ChangeData {
ChangeData cd =
new ChangeData(
null, null, null, null, null, null, null, null, null, null, null, null, null, null,
null, project, id);
null, null, project, id);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd;
}
@@ -312,6 +313,7 @@ public class ChangeData {
private final GitRepositoryManager repoManager;
private final ChangeControl.GenericFactory changeControlFactory;
private final IdentifiedUser.GenericFactory userFactory;
private final AccountCache accountCache;
private final ProjectCache projectCache;
private final MergeUtil.Factory mergeUtilFactory;
private final ChangeNotes.Factory notesFactory;
@@ -368,6 +370,7 @@ public class ChangeData {
GitRepositoryManager repoManager,
ChangeControl.GenericFactory changeControlFactory,
IdentifiedUser.GenericFactory userFactory,
AccountCache accountCache,
ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory,
ChangeNotes.Factory notesFactory,
@@ -386,6 +389,7 @@ public class ChangeData {
this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
this.userFactory = userFactory;
this.accountCache = accountCache;
this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory;
this.notesFactory = notesFactory;
@@ -406,6 +410,7 @@ public class ChangeData {
GitRepositoryManager repoManager,
ChangeControl.GenericFactory changeControlFactory,
IdentifiedUser.GenericFactory userFactory,
AccountCache accountCache,
ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory,
ChangeNotes.Factory notesFactory,
@@ -423,6 +428,7 @@ public class ChangeData {
this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
this.userFactory = userFactory;
this.accountCache = accountCache;
this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory;
this.notesFactory = notesFactory;
@@ -444,6 +450,7 @@ public class ChangeData {
GitRepositoryManager repoManager,
ChangeControl.GenericFactory changeControlFactory,
IdentifiedUser.GenericFactory userFactory,
AccountCache accountCache,
ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory,
ChangeNotes.Factory notesFactory,
@@ -461,6 +468,7 @@ public class ChangeData {
this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
this.userFactory = userFactory;
this.accountCache = accountCache;
this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory;
this.notesFactory = notesFactory;
@@ -483,6 +491,7 @@ public class ChangeData {
GitRepositoryManager repoManager,
ChangeControl.GenericFactory changeControlFactory,
IdentifiedUser.GenericFactory userFactory,
AccountCache accountCache,
ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory,
ChangeNotes.Factory notesFactory,
@@ -500,6 +509,7 @@ public class ChangeData {
this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
this.userFactory = userFactory;
this.accountCache = accountCache;
this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory;
this.notesFactory = notesFactory;
@@ -523,6 +533,7 @@ public class ChangeData {
GitRepositoryManager repoManager,
ChangeControl.GenericFactory changeControlFactory,
IdentifiedUser.GenericFactory userFactory,
AccountCache accountCache,
ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory,
ChangeNotes.Factory notesFactory,
@@ -543,6 +554,7 @@ public class ChangeData {
this.repoManager = repoManager;
this.changeControlFactory = changeControlFactory;
this.userFactory = userFactory;
this.accountCache = accountCache;
this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory;
this.notesFactory = notesFactory;
@@ -1054,7 +1066,7 @@ public class ChangeData {
if (!lazyLoad) {
return Collections.emptyList();
}
records = new SubmitRuleEvaluator(this).setOptions(options).evaluate();
records = new SubmitRuleEvaluator(accountCache, this).setOptions(options).evaluate();
submitRecords.put(options, records);
}
return records;
@@ -1071,7 +1083,7 @@ public class ChangeData {
public SubmitTypeRecord submitTypeRecord() throws OrmException {
if (submitTypeRecord == null) {
submitTypeRecord = new SubmitRuleEvaluator(this).getSubmitType();
submitTypeRecord = new SubmitRuleEvaluator(accountCache, this).getSubmitType();
}
return submitTypeRecord;
}

View File

@@ -23,6 +23,7 @@ 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.CurrentUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.data.ChangeAttribute;
import com.google.gerrit.server.data.PatchSetAttribute;
@@ -68,6 +69,7 @@ public class OutputStreamQuery {
}
private final ReviewDb db;
private final AccountCache accountCache;
private final GitRepositoryManager repoManager;
private final ChangeQueryBuilder queryBuilder;
private final ChangeQueryProcessor queryProcessor;
@@ -92,6 +94,7 @@ public class OutputStreamQuery {
@Inject
OutputStreamQuery(
ReviewDb db,
AccountCache accountCache,
GitRepositoryManager repoManager,
ChangeQueryBuilder queryBuilder,
ChangeQueryProcessor queryProcessor,
@@ -99,6 +102,7 @@ public class OutputStreamQuery {
TrackingFooters trackingFooters,
CurrentUser user) {
this.db = db;
this.accountCache = accountCache;
this.repoManager = repoManager;
this.queryBuilder = queryBuilder;
this.queryProcessor = queryProcessor;
@@ -244,7 +248,11 @@ public class OutputStreamQuery {
if (includeSubmitRecords) {
eventFactory.addSubmitRecords(
c, new SubmitRuleEvaluator(d).setAllowClosed(true).setAllowDraft(true).evaluate());
c,
new SubmitRuleEvaluator(accountCache, d)
.setAllowClosed(true)
.setAllowDraft(true)
.evaluate());
}
if (includeCommitMessage) {