Use injector and factory to instantiate SubmitRuleEvaluator

All callers of SubmitRuleEvaluator get AccountCache, Accounts and
Emails injected just to pass them to SubmitRuleEvaluator manually.

This commit adds a Factory to SubmitRuleEvaluator and migrates callers
to use the factory instead to remove boilerplate.

In addition, this commit moves the instantiation of ChangeControl from
the constructor closer to where it is needed to prevent the constructor
from throwing an OrmException.

Change-Id: I429e7a67e4f1a45be1cf2c2f05692661c086cabe
This commit is contained in:
Patrick Hiesel
2017-09-19 11:16:53 +02:00
parent 29d0dd0cb3
commit 6942c1ba48
10 changed files with 63 additions and 129 deletions

View File

@@ -68,6 +68,7 @@ import com.google.gerrit.server.project.DefaultPermissionBackendModule;
import com.google.gerrit.server.project.ProjectCacheImpl; import com.google.gerrit.server.project.ProjectCacheImpl;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SectionSortCache; import com.google.gerrit.server.project.SectionSortCache;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryProcessor; import com.google.gerrit.server.query.change.ChangeQueryProcessor;
import com.google.gerrit.server.update.BatchUpdate; import com.google.gerrit.server.update.BatchUpdate;
@@ -167,6 +168,7 @@ public class BatchProgramModule extends FactoryModule {
factory(CapabilityCollection.Factory.class); factory(CapabilityCollection.Factory.class);
factory(ChangeData.AssistedFactory.class); factory(ChangeData.AssistedFactory.class);
factory(ProjectState.Factory.class); factory(ProjectState.Factory.class);
factory(SubmitRuleEvaluator.Factory.class);
bind(ChangeJson.Factory.class).toProvider(Providers.<ChangeJson.Factory>of(null)); bind(ChangeJson.Factory.class).toProvider(Providers.<ChangeJson.Factory>of(null));
bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON); bind(AccountVisibility.class).toProvider(AccountVisibilityProvider.class).in(SINGLETON);

View File

@@ -26,9 +26,6 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.reviewdb.server.ReviewDbUtil; import com.google.gerrit.reviewdb.server.ReviewDbUtil;
import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.ChangeUtil;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.git.BranchOrderSection; import com.google.gerrit.server.git.BranchOrderSection;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MergeUtil; import com.google.gerrit.server.git.MergeUtil;
@@ -64,38 +61,32 @@ public class Mergeable implements RestReadView<RevisionResource> {
private boolean otherBranches; private boolean otherBranches;
private final GitRepositoryManager gitManager; private final GitRepositoryManager gitManager;
private final AccountCache accountCache;
private final Accounts accounts;
private final Emails emails;
private final ProjectCache projectCache; private final ProjectCache projectCache;
private final MergeUtil.Factory mergeUtilFactory; private final MergeUtil.Factory mergeUtilFactory;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final ChangeIndexer indexer; private final ChangeIndexer indexer;
private final MergeabilityCache cache; private final MergeabilityCache cache;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Inject @Inject
Mergeable( Mergeable(
GitRepositoryManager gitManager, GitRepositoryManager gitManager,
AccountCache accountCache,
Accounts accounts,
Emails emails,
ProjectCache projectCache, ProjectCache projectCache,
MergeUtil.Factory mergeUtilFactory, MergeUtil.Factory mergeUtilFactory,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
Provider<ReviewDb> db, Provider<ReviewDb> db,
ChangeIndexer indexer, ChangeIndexer indexer,
MergeabilityCache cache) { MergeabilityCache cache,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.gitManager = gitManager; this.gitManager = gitManager;
this.accountCache = accountCache;
this.accounts = accounts;
this.emails = emails;
this.projectCache = projectCache; this.projectCache = projectCache;
this.mergeUtilFactory = mergeUtilFactory; this.mergeUtilFactory = mergeUtilFactory;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.db = db; this.db = db;
this.indexer = indexer; this.indexer = indexer;
this.cache = cache; this.cache = cache;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
} }
public void setOtherBranches(boolean otherBranches) { public void setOtherBranches(boolean otherBranches) {
@@ -152,9 +143,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
private SubmitType getSubmitType(ChangeData cd, PatchSet patchSet) throws OrmException { private SubmitType getSubmitType(ChangeData cd, PatchSet patchSet) throws OrmException {
SubmitTypeRecord rec = SubmitTypeRecord rec =
new SubmitRuleEvaluator(accountCache, accounts, emails, cd) submitRuleEvaluatorFactory.create(cd).setPatchSet(patchSet).getSubmitType();
.setPatchSet(patchSet)
.getSubmitType();
if (rec.status != SubmitTypeRecord.Status.OK) { if (rec.status != SubmitTypeRecord.Status.OK) {
throw new OrmException("Submit type rule failed: " + rec); throw new OrmException("Submit type rule failed: " + rec);
} }

View File

@@ -27,10 +27,7 @@ import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.PatchSetApproval;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.ApprovalsUtil; 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.account.AccountLoader;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.permissions.LabelPermission; import com.google.gerrit.server.permissions.LabelPermission;
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;
@@ -49,30 +46,24 @@ public class ReviewerJson {
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final PermissionBackend permissionBackend; private final PermissionBackend permissionBackend;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final AccountCache accountCache;
private final Accounts accounts;
private final Emails emails;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final AccountLoader.Factory accountLoaderFactory; private final AccountLoader.Factory accountLoaderFactory;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Inject @Inject
ReviewerJson( ReviewerJson(
Provider<ReviewDb> db, Provider<ReviewDb> db,
PermissionBackend permissionBackend, PermissionBackend permissionBackend,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
AccountCache accountCache,
Accounts accounts,
Emails emails,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
AccountLoader.Factory accountLoaderFactory) { AccountLoader.Factory accountLoaderFactory,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db; this.db = db;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.accountCache = accountCache;
this.accounts = accounts;
this.emails = emails;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.accountLoaderFactory = accountLoaderFactory; this.accountLoaderFactory = accountLoaderFactory;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
} }
public List<ReviewerInfo> format(Collection<ReviewerResource> rsrcs) public List<ReviewerInfo> format(Collection<ReviewerResource> rsrcs)
@@ -133,7 +124,8 @@ public class ReviewerJson {
PatchSet ps = cd.currentPatchSet(); PatchSet ps = cd.currentPatchSet();
if (ps != null) { if (ps != null) {
for (SubmitRecord rec : for (SubmitRecord rec :
new SubmitRuleEvaluator(accountCache, accounts, emails, cd) submitRuleEvaluatorFactory
.create(cd)
.setFastEvalLabels(true) .setFastEvalLabels(true)
.setAllowDraft(true) .setAllowDraft(true)
.evaluate()) { .evaluate()) {

View File

@@ -24,10 +24,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
import com.google.gerrit.extensions.restapi.RestModifyView; import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.RulesCache; 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.account.AccountLoader;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
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;
@@ -42,10 +39,8 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final RulesCache rules; private final RulesCache rules;
private final AccountCache accountCache;
private final AccountLoader.Factory accountInfoFactory; private final AccountLoader.Factory accountInfoFactory;
private final Accounts accounts; private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private final Emails emails;
@Option(name = "--filters", usage = "impact of filters in parent projects") @Option(name = "--filters", usage = "impact of filters in parent projects")
private Filters filters = Filters.RUN; private Filters filters = Filters.RUN;
@@ -55,17 +50,13 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
Provider<ReviewDb> db, Provider<ReviewDb> db,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
RulesCache rules, RulesCache rules,
AccountCache accountCache,
AccountLoader.Factory infoFactory, AccountLoader.Factory infoFactory,
Accounts accounts, SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
Emails emails) {
this.db = db; this.db = db;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.rules = rules; this.rules = rules;
this.accountCache = accountCache;
this.accountInfoFactory = infoFactory; this.accountInfoFactory = infoFactory;
this.accounts = accounts; this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
this.emails = emails;
} }
@Override @Override
@@ -79,10 +70,7 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
} }
input.filters = MoreObjects.firstNonNull(input.filters, filters); input.filters = MoreObjects.firstNonNull(input.filters, filters);
SubmitRuleEvaluator evaluator = SubmitRuleEvaluator evaluator =
new SubmitRuleEvaluator( submitRuleEvaluatorFactory.create(
accountCache,
accounts,
emails,
changeDataFactory.create(db.get(), rsrc.getChangeResource())); changeDataFactory.create(db.get(), rsrc.getChangeResource()));
List<SubmitRecord> records = List<SubmitRecord> records =

View File

@@ -25,9 +25,6 @@ import com.google.gerrit.extensions.restapi.RestModifyView;
import com.google.gerrit.extensions.restapi.RestReadView; import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.RulesCache; import com.google.gerrit.rules.RulesCache;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
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;
@@ -37,11 +34,9 @@ import org.kohsuke.args4j.Option;
public class TestSubmitType implements RestModifyView<RevisionResource, TestSubmitRuleInput> { public class TestSubmitType implements RestModifyView<RevisionResource, TestSubmitRuleInput> {
private final Provider<ReviewDb> db; private final Provider<ReviewDb> db;
private final AccountCache accountCache;
private final Accounts accounts;
private final Emails emails;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final RulesCache rules; private final RulesCache rules;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
@Option(name = "--filters", usage = "impact of filters in parent projects") @Option(name = "--filters", usage = "impact of filters in parent projects")
private Filters filters = Filters.RUN; private Filters filters = Filters.RUN;
@@ -49,17 +44,13 @@ public class TestSubmitType implements RestModifyView<RevisionResource, TestSubm
@Inject @Inject
TestSubmitType( TestSubmitType(
Provider<ReviewDb> db, Provider<ReviewDb> db,
AccountCache accountCache,
Accounts accounts,
Emails emails,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
RulesCache rules) { RulesCache rules,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db; this.db = db;
this.accountCache = accountCache;
this.accounts = accounts;
this.emails = emails;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.rules = rules; this.rules = rules;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
} }
@Override @Override
@@ -73,10 +64,7 @@ public class TestSubmitType implements RestModifyView<RevisionResource, TestSubm
} }
input.filters = MoreObjects.firstNonNull(input.filters, filters); input.filters = MoreObjects.firstNonNull(input.filters, filters);
SubmitRuleEvaluator evaluator = SubmitRuleEvaluator evaluator =
new SubmitRuleEvaluator( submitRuleEvaluatorFactory.create(
accountCache,
accounts,
emails,
changeDataFactory.create(db.get(), rsrc.getChangeResource())); changeDataFactory.create(db.get(), rsrc.getChangeResource()));
SubmitTypeRecord rec = SubmitTypeRecord rec =

View File

@@ -166,6 +166,7 @@ import com.google.gerrit.server.project.ProjectCacheImpl;
import com.google.gerrit.server.project.ProjectNode; import com.google.gerrit.server.project.ProjectNode;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SectionSortCache; import com.google.gerrit.server.project.SectionSortCache;
import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder; import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeQueryProcessor; import com.google.gerrit.server.query.change.ChangeQueryProcessor;
@@ -264,6 +265,7 @@ public class GerritGlobalModule extends FactoryModule {
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);
factory(SubmitRuleEvaluator.Factory.class);
bind(AuthBackend.class).to(UniversalAuthBackend.class).in(SINGLETON); bind(AuthBackend.class).to(UniversalAuthBackend.class).in(SINGLETON);
DynamicSet.setOf(binder(), AuthBackend.class); DynamicSet.setOf(binder(), AuthBackend.class);

View File

@@ -31,9 +31,6 @@ 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.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.change.Submit; import com.google.gerrit.server.change.Submit;
import com.google.gerrit.server.config.GerritServerConfig; import com.google.gerrit.server.config.GerritServerConfig;
import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo; import com.google.gerrit.server.git.MergeOpRepoManager.OpenRepo;
@@ -100,9 +97,6 @@ public class MergeSuperSet {
abstract ImmutableSet<String> hashes(); abstract ImmutableSet<String> hashes();
} }
private final AccountCache accountCache;
private final Accounts accounts;
private final Emails emails;
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final Provider<InternalChangeQuery> queryProvider; private final Provider<InternalChangeQuery> queryProvider;
private final Provider<MergeOpRepoManager> repoManagerProvider; private final Provider<MergeOpRepoManager> repoManagerProvider;
@@ -110,6 +104,7 @@ public class MergeSuperSet {
private final Config cfg; private final Config cfg;
private final Map<QueryKey, List<ChangeData>> queryCache; private final Map<QueryKey, List<ChangeData>> queryCache;
private final Map<Branch.NameKey, Optional<RevCommit>> heads; private final Map<Branch.NameKey, Optional<RevCommit>> heads;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private MergeOpRepoManager orm; private MergeOpRepoManager orm;
private boolean closeOrm; private boolean closeOrm;
@@ -117,21 +112,17 @@ public class MergeSuperSet {
@Inject @Inject
MergeSuperSet( MergeSuperSet(
@GerritServerConfig Config cfg, @GerritServerConfig Config cfg,
AccountCache accountCache,
Accounts accounts,
Emails emails,
ChangeData.Factory changeDataFactory, ChangeData.Factory changeDataFactory,
Provider<InternalChangeQuery> queryProvider, Provider<InternalChangeQuery> queryProvider,
Provider<MergeOpRepoManager> repoManagerProvider, Provider<MergeOpRepoManager> repoManagerProvider,
PermissionBackend permissionBackend) { PermissionBackend permissionBackend,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.cfg = cfg; this.cfg = cfg;
this.accountCache = accountCache;
this.accounts = accounts;
this.emails = emails;
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.queryProvider = queryProvider; this.queryProvider = queryProvider;
this.repoManagerProvider = repoManagerProvider; this.repoManagerProvider = repoManagerProvider;
this.permissionBackend = permissionBackend; this.permissionBackend = permissionBackend;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
queryCache = new HashMap<>(); queryCache = new HashMap<>();
heads = new HashMap<>(); heads = new HashMap<>();
} }
@@ -181,9 +172,7 @@ public class MergeSuperSet {
SubmitTypeRecord str = SubmitTypeRecord str =
ps == cd.currentPatchSet() ps == cd.currentPatchSet()
? cd.submitTypeRecord() ? cd.submitTypeRecord()
: new SubmitRuleEvaluator(accountCache, accounts, emails, cd) : submitRuleEvaluatorFactory.create(cd).setPatchSet(ps).getSubmitType();
.setPatchSet(ps)
.getSubmitType();
if (!str.isOk()) { if (!str.isOk()) {
logErrorAndThrow("Failed to get submit type for " + cd.getId() + ": " + str.errorMessage); logErrorAndThrow("Failed to get submit type for " + cd.getId() + ": " + str.errorMessage);
} }

View File

@@ -32,6 +32,8 @@ import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails; import com.google.gerrit.server.account.Emails;
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.assistedinject.Assisted;
import com.googlecode.prolog_cafe.exceptions.CompileException; import com.googlecode.prolog_cafe.exceptions.CompileException;
import com.googlecode.prolog_cafe.exceptions.ReductionLimitException; import com.googlecode.prolog_cafe.exceptions.ReductionLimitException;
import com.googlecode.prolog_cafe.lang.IntegerTerm; import com.googlecode.prolog_cafe.lang.IntegerTerm;
@@ -84,28 +86,31 @@ public class SubmitRuleEvaluator {
} }
} }
public interface Factory {
SubmitRuleEvaluator create(ChangeData cd);
}
private final AccountCache accountCache; private final AccountCache accountCache;
private final Accounts accounts; private final Accounts accounts;
private final Emails emails; private final Emails emails;
private final ChangeData cd; private final ChangeData cd;
private final ChangeControl control;
private SubmitRuleOptions.Builder optsBuilder = SubmitRuleOptions.defaults(); private SubmitRuleOptions.Builder optsBuilder = SubmitRuleOptions.defaults();
private SubmitRuleOptions opts; private SubmitRuleOptions opts;
private PatchSet patchSet; private PatchSet patchSet;
private boolean logErrors = true; private boolean logErrors = true;
private long reductionsConsumed; private long reductionsConsumed;
private ChangeControl control;
private Term submitRule; private Term submitRule;
public SubmitRuleEvaluator( @Inject
AccountCache accountCache, Accounts accounts, Emails emails, ChangeData cd) SubmitRuleEvaluator(
throws OrmException { AccountCache accountCache, Accounts accounts, Emails emails, @Assisted ChangeData cd) {
this.accountCache = accountCache; this.accountCache = accountCache;
this.accounts = accounts; this.accounts = accounts;
this.emails = emails; this.emails = emails;
this.cd = cd; this.cd = cd;
this.control = cd.changeControl();
} }
/** /**
@@ -219,6 +224,12 @@ public class SubmitRuleEvaluator {
*/ */
public List<SubmitRecord> evaluate() { public List<SubmitRecord> evaluate() {
initOptions(); initOptions();
try {
initChange();
} catch (OrmException e) {
return ruleError("Error looking up change " + cd.getId(), e);
}
Change c = control.getChange(); Change c = control.getChange();
if (!opts.allowClosed() && c.getStatus().isClosed()) { if (!opts.allowClosed() && c.getStatus().isClosed()) {
SubmitRecord rec = new SubmitRecord(); SubmitRecord rec = new SubmitRecord();
@@ -226,12 +237,6 @@ public class SubmitRuleEvaluator {
return Collections.singletonList(rec); return Collections.singletonList(rec);
} }
if (!opts.allowDraft()) { if (!opts.allowDraft()) {
try {
initPatchSet();
} catch (OrmException e) {
return ruleError(
"Error looking up patch set " + control.getChange().currentPatchSetId(), e);
}
if (c.getStatus() == Change.Status.DRAFT || patchSet.isDraft()) { if (c.getStatus() == Change.Status.DRAFT || patchSet.isDraft()) {
return cannotSubmitDraft(); return cannotSubmitDraft();
} }
@@ -409,9 +414,9 @@ public class SubmitRuleEvaluator {
public SubmitTypeRecord getSubmitType() { public SubmitTypeRecord getSubmitType() {
initOptions(); initOptions();
try { try {
initPatchSet(); initChange();
} catch (OrmException e) { } catch (OrmException e) {
return typeError("Error looking up patch set " + control.getChange().currentPatchSetId(), e); return typeError("Error looking up change " + cd.getId(), e);
} }
try { try {
@@ -679,7 +684,11 @@ public class SubmitRuleEvaluator {
} }
} }
private void initPatchSet() throws OrmException { private void initChange() throws OrmException {
if (control == null) {
control = cd.changeControl();
}
if (patchSet == null) { if (patchSet == null) {
patchSet = cd.currentPatchSet(); patchSet = cd.currentPatchSet();
if (patchSet == null) { if (patchSet == null) {

View File

@@ -59,9 +59,6 @@ import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.ReviewerStatusUpdate; import com.google.gerrit.server.ReviewerStatusUpdate;
import com.google.gerrit.server.StarredChangesUtil; import com.google.gerrit.server.StarredChangesUtil;
import com.google.gerrit.server.StarredChangesUtil.StarRef; import com.google.gerrit.server.StarredChangesUtil.StarRef;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.change.ChangeResource;
import com.google.gerrit.server.change.GetPureRevert; import com.google.gerrit.server.change.GetPureRevert;
import com.google.gerrit.server.change.MergeabilityCache; import com.google.gerrit.server.change.MergeabilityCache;
@@ -352,22 +349,19 @@ public class ChangeData {
ChangeData cd = ChangeData cd =
new ChangeData( new ChangeData(
null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null, null,
null, null, null, null, null, null, null, project, id, null, null, null); null, null, null, null, null, project, id, null, null, null);
cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId)); cd.currentPatchSet = new PatchSet(new PatchSet.Id(id, currentPatchSetId));
return cd; return cd;
} }
// Injected fields. // Injected fields.
private @Nullable final StarredChangesUtil starredChangesUtil; private @Nullable final StarredChangesUtil starredChangesUtil;
private final AccountCache accountCache;
private final Accounts accounts;
private final AllUsersName allUsersName; private final AllUsersName allUsersName;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final ChangeControl.GenericFactory changeControlFactory; private final ChangeControl.GenericFactory changeControlFactory;
private final ChangeMessagesUtil cmUtil; private final ChangeMessagesUtil cmUtil;
private final ChangeNotes.Factory notesFactory; private final ChangeNotes.Factory notesFactory;
private final CommentsUtil commentsUtil; private final CommentsUtil commentsUtil;
private final Emails emails;
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
private final IdentifiedUser.GenericFactory userFactory; private final IdentifiedUser.GenericFactory userFactory;
private final MergeUtil.Factory mergeUtilFactory; private final MergeUtil.Factory mergeUtilFactory;
@@ -378,6 +372,7 @@ public class ChangeData {
private final ProjectCache projectCache; private final ProjectCache projectCache;
private final TrackingFooters trackingFooters; private final TrackingFooters trackingFooters;
private final GetPureRevert pureRevert; private final GetPureRevert pureRevert;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
// Required assisted injected fields. // Required assisted injected fields.
private final ReviewDb db; private final ReviewDb db;
@@ -431,15 +426,12 @@ public class ChangeData {
@Inject @Inject
private ChangeData( private ChangeData(
@Nullable StarredChangesUtil starredChangesUtil, @Nullable StarredChangesUtil starredChangesUtil,
AccountCache accountCache,
Accounts accounts,
ApprovalsUtil approvalsUtil, ApprovalsUtil approvalsUtil,
AllUsersName allUsersName, AllUsersName allUsersName,
ChangeControl.GenericFactory changeControlFactory, ChangeControl.GenericFactory changeControlFactory,
ChangeMessagesUtil cmUtil, ChangeMessagesUtil cmUtil,
ChangeNotes.Factory notesFactory, ChangeNotes.Factory notesFactory,
CommentsUtil commentsUtil, CommentsUtil commentsUtil,
Emails emails,
GitRepositoryManager repoManager, GitRepositoryManager repoManager,
IdentifiedUser.GenericFactory userFactory, IdentifiedUser.GenericFactory userFactory,
MergeUtil.Factory mergeUtilFactory, MergeUtil.Factory mergeUtilFactory,
@@ -450,21 +442,19 @@ public class ChangeData {
ProjectCache projectCache, ProjectCache projectCache,
TrackingFooters trackingFooters, TrackingFooters trackingFooters,
GetPureRevert pureRevert, GetPureRevert pureRevert,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory,
@Assisted ReviewDb db, @Assisted ReviewDb db,
@Assisted Project.NameKey project, @Assisted Project.NameKey project,
@Assisted Change.Id id, @Assisted Change.Id id,
@Assisted @Nullable Change change, @Assisted @Nullable Change change,
@Assisted @Nullable ChangeNotes notes, @Assisted @Nullable ChangeNotes notes,
@Assisted @Nullable ChangeControl control) { @Assisted @Nullable ChangeControl control) {
this.accountCache = accountCache;
this.accounts = accounts;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.allUsersName = allUsersName; this.allUsersName = allUsersName;
this.changeControlFactory = changeControlFactory; this.changeControlFactory = changeControlFactory;
this.cmUtil = cmUtil; this.cmUtil = cmUtil;
this.notesFactory = notesFactory; this.notesFactory = notesFactory;
this.commentsUtil = commentsUtil; this.commentsUtil = commentsUtil;
this.emails = emails;
this.repoManager = repoManager; this.repoManager = repoManager;
this.userFactory = userFactory; this.userFactory = userFactory;
this.mergeUtilFactory = mergeUtilFactory; this.mergeUtilFactory = mergeUtilFactory;
@@ -476,6 +466,7 @@ public class ChangeData {
this.starredChangesUtil = starredChangesUtil; this.starredChangesUtil = starredChangesUtil;
this.trackingFooters = trackingFooters; this.trackingFooters = trackingFooters;
this.pureRevert = pureRevert; this.pureRevert = pureRevert;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
// May be null in tests when created via createForTest above, in which case lazy-loading will // May be null in tests when created via createForTest above, in which case lazy-loading will
// intentionally fail with NPE. Still not marked @Nullable in the constructor, to force callers // intentionally fail with NPE. Still not marked @Nullable in the constructor, to force callers
@@ -1032,10 +1023,7 @@ public class ChangeData {
if (!lazyLoad) { if (!lazyLoad) {
return Collections.emptyList(); return Collections.emptyList();
} }
records = records = submitRuleEvaluatorFactory.create(this).setOptions(options).evaluate();
new SubmitRuleEvaluator(accountCache, accounts, emails, this)
.setOptions(options)
.evaluate();
submitRecords.put(options, records); submitRecords.put(options, records);
} }
return records; return records;
@@ -1052,8 +1040,7 @@ public class ChangeData {
public SubmitTypeRecord submitTypeRecord() throws OrmException { public SubmitTypeRecord submitTypeRecord() throws OrmException {
if (submitTypeRecord == null) { if (submitTypeRecord == null) {
submitTypeRecord = submitTypeRecord = submitRuleEvaluatorFactory.create(this).getSubmitType();
new SubmitRuleEvaluator(accountCache, accounts, emails, this).getSubmitType();
} }
return submitTypeRecord; return submitTypeRecord;
} }

View File

@@ -25,9 +25,6 @@ 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.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.Accounts;
import com.google.gerrit.server.account.Emails;
import com.google.gerrit.server.config.TrackingFooters; import com.google.gerrit.server.config.TrackingFooters;
import com.google.gerrit.server.data.ChangeAttribute; import com.google.gerrit.server.data.ChangeAttribute;
import com.google.gerrit.server.data.PatchSetAttribute; import com.google.gerrit.server.data.PatchSetAttribute;
@@ -75,15 +72,13 @@ public class OutputStreamQuery {
} }
private final ReviewDb db; private final ReviewDb db;
private final AccountCache accountCache;
private final Accounts accounts;
private final Emails emails;
private final GitRepositoryManager repoManager; private final GitRepositoryManager repoManager;
private final ChangeQueryBuilder queryBuilder; private final ChangeQueryBuilder queryBuilder;
private final ChangeQueryProcessor queryProcessor; private final ChangeQueryProcessor queryProcessor;
private final EventFactory eventFactory; private final EventFactory eventFactory;
private final TrackingFooters trackingFooters; private final TrackingFooters trackingFooters;
private final CurrentUser user; private final CurrentUser user;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
private OutputFormat outputFormat = OutputFormat.TEXT; private OutputFormat outputFormat = OutputFormat.TEXT;
private boolean includePatchSets; private boolean includePatchSets;
@@ -102,25 +97,21 @@ public class OutputStreamQuery {
@Inject @Inject
OutputStreamQuery( OutputStreamQuery(
ReviewDb db, ReviewDb db,
AccountCache accountCache,
Accounts accounts,
Emails emails,
GitRepositoryManager repoManager, GitRepositoryManager repoManager,
ChangeQueryBuilder queryBuilder, ChangeQueryBuilder queryBuilder,
ChangeQueryProcessor queryProcessor, ChangeQueryProcessor queryProcessor,
EventFactory eventFactory, EventFactory eventFactory,
TrackingFooters trackingFooters, TrackingFooters trackingFooters,
CurrentUser user) { CurrentUser user,
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
this.db = db; this.db = db;
this.accountCache = accountCache;
this.accounts = accounts;
this.emails = emails;
this.repoManager = repoManager; this.repoManager = repoManager;
this.queryBuilder = queryBuilder; this.queryBuilder = queryBuilder;
this.queryProcessor = queryProcessor; this.queryProcessor = queryProcessor;
this.eventFactory = eventFactory; this.eventFactory = eventFactory;
this.trackingFooters = trackingFooters; this.trackingFooters = trackingFooters;
this.user = user; this.user = user;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
} }
void setLimit(int n) { void setLimit(int n) {
@@ -259,10 +250,7 @@ public class OutputStreamQuery {
if (includeSubmitRecords) { if (includeSubmitRecords) {
eventFactory.addSubmitRecords( eventFactory.addSubmitRecords(
c, c,
new SubmitRuleEvaluator(accountCache, accounts, emails, d) submitRuleEvaluatorFactory.create(d).setAllowClosed(true).setAllowDraft(true).evaluate());
.setAllowClosed(true)
.setAllowDraft(true)
.evaluate());
} }
if (includeCommitMessage) { if (includeCommitMessage) {