Adapt SubmitRuleEvaluator for the new SubmitRule interface

By removing the ChangeData parameter from the constructor and
passing it to the evaluate method, we mimic the future SubmitRule
interface and get closer to a working implementation.

Remove the methods to configure SubmitRule options, leading to
a cleaner class with an immutable configuration that can be reused
for multiple evaluations.

Change-Id: Ic72a40a2281fb7ef172b9ef364ea543d4e80938b
This commit is contained in:
Maxime Guerreiro
2018-03-06 14:10:00 +01:00
parent 74b08b2090
commit 806e9db26f
10 changed files with 137 additions and 169 deletions

View File

@@ -14,13 +14,14 @@
package com.google.gerrit.server.project; package com.google.gerrit.server.project;
@SuppressWarnings("serial")
public class RuleEvalException extends Exception { public class RuleEvalException extends Exception {
private static final long serialVersionUID = 1L;
public RuleEvalException(String message) { public RuleEvalException(String message) {
super(message); super(message);
} }
RuleEvalException(String message, Throwable cause) { public RuleEvalException(String message, Throwable cause) {
super(message, cause); super(message, cause);
} }
} }

View File

@@ -16,7 +16,6 @@ package com.google.gerrit.server.project;
import static com.google.common.base.Preconditions.checkState; import static com.google.common.base.Preconditions.checkState;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.common.data.SubmitTypeRecord; import com.google.gerrit.common.data.SubmitTypeRecord;
import com.google.gerrit.extensions.client.SubmitType; import com.google.gerrit.extensions.client.SubmitType;
@@ -84,97 +83,31 @@ public class SubmitRuleEvaluator {
} }
public interface Factory { public interface Factory {
SubmitRuleEvaluator create(ChangeData cd); /** Returns a new {@link SubmitRuleEvaluator} with the specified options */
SubmitRuleEvaluator create(SubmitRuleOptions options);
} }
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 ProjectCache projectCache; private final ProjectCache projectCache;
private final ChangeData cd; private final SubmitRuleOptions opts;
private SubmitRuleOptions.Builder optsBuilder = SubmitRuleOptions.builder();
private SubmitRuleOptions opts;
private Change change;
private boolean logErrors = true;
private ProjectState projectState;
private Term submitRule; private Term submitRule;
@Inject @Inject
SubmitRuleEvaluator( private SubmitRuleEvaluator(
AccountCache accountCache, AccountCache accountCache,
Accounts accounts, Accounts accounts,
Emails emails, Emails emails,
ProjectCache projectCache, ProjectCache projectCache,
@Assisted ChangeData cd) { @Assisted SubmitRuleOptions options) {
this.accountCache = accountCache; this.accountCache = accountCache;
this.accounts = accounts; this.accounts = accounts;
this.emails = emails; this.emails = emails;
this.projectCache = projectCache; this.projectCache = projectCache;
this.cd = cd;
}
/** this.opts = options;
* @return immutable snapshot of options configured so far. If neither {@link #getSubmitRule()}
* nor {@link #getSubmitType()} have been called yet, state within this instance is still
* mutable, so may change before evaluation. The instance's options are frozen at evaluation
* time.
*/
public SubmitRuleOptions getOptions() {
if (opts != null) {
return opts;
}
return optsBuilder.build();
}
public SubmitRuleEvaluator setOptions(SubmitRuleOptions opts) {
checkNotStarted();
if (opts != null) {
optsBuilder = opts.toBuilder();
} else {
optsBuilder = SubmitRuleOptions.builder();
}
return this;
}
/**
* @param allow whether to allow {@link #evaluate()} on closed changes.
* @return this
*/
public SubmitRuleEvaluator setAllowClosed(boolean allow) {
checkNotStarted();
optsBuilder.allowClosed(allow);
return this;
}
/**
* @param skip if true, submit filter will not be applied.
* @return this
*/
public SubmitRuleEvaluator setSkipSubmitFilters(boolean skip) {
checkNotStarted();
optsBuilder.skipFilters(skip);
return this;
}
/**
* @param rule custom rule to use, or null to use refs/meta/config:rules.pl.
* @return this
*/
public SubmitRuleEvaluator setRule(@Nullable String rule) {
checkNotStarted();
optsBuilder.rule(rule);
return this;
}
/**
* @param log whether to log error messages in addition to returning error records. If true, error
* record messages will be less descriptive.
*/
public SubmitRuleEvaluator setLogErrors(boolean log) {
logErrors = log;
return this;
} }
/** /**
@@ -182,11 +115,21 @@ public class SubmitRuleEvaluator {
* *
* @return List of {@link SubmitRecord} objects returned from the evaluated rules, including any * @return List of {@link SubmitRecord} objects returned from the evaluated rules, including any
* errors. * errors.
* @param cd ChangeData to evaluate
*/ */
public List<SubmitRecord> evaluate() { public List<SubmitRecord> evaluate(ChangeData cd) {
initOptions(); Change change;
ProjectState projectState;
try { try {
init(); change = cd.change();
if (change == null) {
throw new OrmException("Change not found");
}
projectState = projectCache.get(cd.project());
if (projectState == null) {
throw new NoSuchProjectException(cd.project());
}
} catch (OrmException | NoSuchProjectException e) { } catch (OrmException | NoSuchProjectException e) {
return ruleError("Error looking up change " + cd.getId(), e); return ruleError("Error looking up change " + cd.getId(), e);
} }
@@ -201,7 +144,12 @@ public class SubmitRuleEvaluator {
try { try {
results = results =
evaluateImpl( evaluateImpl(
"locate_submit_rule", "can_submit", "locate_submit_filter", "filter_submit_results"); "locate_submit_rule",
"can_submit",
"locate_submit_filter",
"filter_submit_results",
cd,
projectState);
} catch (RuleEvalException e) { } catch (RuleEvalException e) {
return ruleError(e.getMessage(), e); return ruleError(e.getMessage(), e);
} }
@@ -214,10 +162,10 @@ public class SubmitRuleEvaluator {
return ruleError( return ruleError(
String.format( String.format(
"Submit rule '%s' for change %s of %s has no solution.", "Submit rule '%s' for change %s of %s has no solution.",
getSubmitRuleName(), cd.getId(), getProjectName())); getSubmitRuleName(), cd.getId(), projectState.getName()));
} }
return resultsToSubmitRecord(getSubmitRule(), results); return resultsToSubmitRecord(getSubmitRule(), results, cd);
} }
/** /**
@@ -228,7 +176,8 @@ public class SubmitRuleEvaluator {
* output. Later after the loop the out collection is reversed to restore it to the original * output. Later after the loop the out collection is reversed to restore it to the original
* ordering. * ordering.
*/ */
public List<SubmitRecord> resultsToSubmitRecord(Term submitRule, List<Term> results) { public List<SubmitRecord> resultsToSubmitRecord(
Term submitRule, List<Term> results, ChangeData cd) {
boolean foundOk = false; boolean foundOk = false;
List<SubmitRecord> out = new ArrayList<>(results.size()); List<SubmitRecord> out = new ArrayList<>(results.size());
for (int resultIdx = results.size() - 1; 0 <= resultIdx; resultIdx--) { for (int resultIdx = results.size() - 1; 0 <= resultIdx; resultIdx--) {
@@ -237,7 +186,7 @@ public class SubmitRuleEvaluator {
out.add(rec); out.add(rec);
if (!(submitRecord instanceof StructureTerm) || 1 != submitRecord.arity()) { if (!(submitRecord instanceof StructureTerm) || 1 != submitRecord.arity()) {
return invalidResult(submitRule, submitRecord); return invalidResult(submitRule, submitRecord, cd);
} }
if ("ok".equals(submitRecord.name())) { if ("ok".equals(submitRecord.name())) {
@@ -247,7 +196,7 @@ public class SubmitRuleEvaluator {
rec.status = SubmitRecord.Status.NOT_READY; rec.status = SubmitRecord.Status.NOT_READY;
} else { } else {
return invalidResult(submitRule, submitRecord); return invalidResult(submitRule, submitRecord, cd);
} }
// Unpack the one argument. This should also be a structure with one // Unpack the one argument. This should also be a structure with one
@@ -256,7 +205,7 @@ public class SubmitRuleEvaluator {
submitRecord = submitRecord.arg(0); submitRecord = submitRecord.arg(0);
if (!(submitRecord instanceof StructureTerm)) { if (!(submitRecord instanceof StructureTerm)) {
return invalidResult(submitRule, submitRecord); return invalidResult(submitRule, submitRecord, cd);
} }
rec.labels = new ArrayList<>(submitRecord.arity()); rec.labels = new ArrayList<>(submitRecord.arity());
@@ -265,7 +214,7 @@ public class SubmitRuleEvaluator {
if (!(state instanceof StructureTerm) if (!(state instanceof StructureTerm)
|| 2 != state.arity() || 2 != state.arity()
|| !"label".equals(state.name())) { || !"label".equals(state.name())) {
return invalidResult(submitRule, submitRecord); return invalidResult(submitRule, submitRecord, cd);
} }
SubmitRecord.Label lbl = new SubmitRecord.Label(); SubmitRecord.Label lbl = new SubmitRecord.Label();
@@ -293,10 +242,10 @@ public class SubmitRuleEvaluator {
lbl.status = SubmitRecord.Label.Status.IMPOSSIBLE; lbl.status = SubmitRecord.Label.Status.IMPOSSIBLE;
} else { } else {
return invalidResult(submitRule, submitRecord); return invalidResult(submitRule, submitRecord, cd);
} }
} catch (UserTermExpected e) { } catch (UserTermExpected e) {
return invalidResult(submitRule, submitRecord, e.getMessage()); return invalidResult(submitRule, submitRecord, e.getMessage(), cd);
} }
} }
@@ -320,19 +269,19 @@ public class SubmitRuleEvaluator {
return out; return out;
} }
private List<SubmitRecord> invalidResult(Term rule, Term record, String reason) { private List<SubmitRecord> invalidResult(Term rule, Term record, String reason, ChangeData cd) {
return ruleError( return ruleError(
String.format( String.format(
"Submit rule %s for change %s of %s output invalid result: %s%s", "Submit rule %s for change %s of %s output invalid result: %s%s",
rule, rule,
cd.getId(), cd.getId(),
getProjectName(), cd.project().get(),
record, record,
(reason == null ? "" : ". Reason: " + reason))); (reason == null ? "" : ". Reason: " + reason)));
} }
private List<SubmitRecord> invalidResult(Term rule, Term record) { private List<SubmitRecord> invalidResult(Term rule, Term record, ChangeData cd) {
return invalidResult(rule, record, null); return invalidResult(rule, record, null, cd);
} }
private List<SubmitRecord> ruleError(String err) { private List<SubmitRecord> ruleError(String err) {
@@ -340,7 +289,7 @@ public class SubmitRuleEvaluator {
} }
private List<SubmitRecord> ruleError(String err, Exception e) { private List<SubmitRecord> ruleError(String err, Exception e) {
if (logErrors) { if (opts.logErrors()) {
if (e == null) { if (e == null) {
log.error(err); log.error(err);
} else { } else {
@@ -355,12 +304,16 @@ public class SubmitRuleEvaluator {
* Evaluate the submit type rules to get the submit type. * Evaluate the submit type rules to get the submit type.
* *
* @return record from the evaluated rules. * @return record from the evaluated rules.
* @param cd
*/ */
public SubmitTypeRecord getSubmitType() { public SubmitTypeRecord getSubmitType(ChangeData cd) {
initOptions(); ProjectState projectState;
try { try {
init(); projectState = projectCache.get(cd.project());
} catch (OrmException | NoSuchProjectException e) { if (projectState == null) {
throw new NoSuchProjectException(cd.project());
}
} catch (NoSuchProjectException e) {
return typeError("Error looking up change " + cd.getId(), e); return typeError("Error looking up change " + cd.getId(), e);
} }
@@ -371,7 +324,9 @@ public class SubmitRuleEvaluator {
"locate_submit_type", "locate_submit_type",
"get_submit_type", "get_submit_type",
"locate_submit_type_filter", "locate_submit_type_filter",
"filter_submit_type_results"); "filter_submit_type_results",
cd,
projectState);
} catch (RuleEvalException e) { } catch (RuleEvalException e) {
return typeError(e.getMessage(), e); return typeError(e.getMessage(), e);
} }
@@ -384,7 +339,7 @@ public class SubmitRuleEvaluator {
+ "' for change " + "' for change "
+ cd.getId() + cd.getId()
+ " of " + " of "
+ getProjectName() + projectState.getName()
+ " has no solution."); + " has no solution.");
} }
@@ -396,7 +351,7 @@ public class SubmitRuleEvaluator {
+ "' for change " + "' for change "
+ cd.getId() + cd.getId()
+ " of " + " of "
+ getProjectName() + projectState.getName()
+ " did not return a symbol."); + " did not return a symbol.");
} }
@@ -410,7 +365,7 @@ public class SubmitRuleEvaluator {
+ " for change " + " for change "
+ cd.getId() + cd.getId()
+ " of " + " of "
+ getProjectName() + projectState.getName()
+ " output invalid result: " + " output invalid result: "
+ typeName); + typeName);
} }
@@ -421,7 +376,7 @@ public class SubmitRuleEvaluator {
} }
private SubmitTypeRecord typeError(String err, Exception e) { private SubmitTypeRecord typeError(String err, Exception e) {
if (logErrors) { if (opts.logErrors()) {
if (e == null) { if (e == null) {
log.error(err); log.error(err);
} else { } else {
@@ -436,9 +391,11 @@ public class SubmitRuleEvaluator {
String userRuleLocatorName, String userRuleLocatorName,
String userRuleWrapperName, String userRuleWrapperName,
String filterRuleLocatorName, String filterRuleLocatorName,
String filterRuleWrapperName) String filterRuleWrapperName,
ChangeData cd,
ProjectState projectState)
throws RuleEvalException { throws RuleEvalException {
PrologEnvironment env = getPrologEnvironment(); PrologEnvironment env = getPrologEnvironment(cd, projectState);
try { try {
Term sr = env.once("gerrit", userRuleLocatorName, new VariableTerm()); Term sr = env.once("gerrit", userRuleLocatorName, new VariableTerm());
List<Term> results = new ArrayList<>(); List<Term> results = new ArrayList<>();
@@ -449,11 +406,13 @@ public class SubmitRuleEvaluator {
} catch (ReductionLimitException err) { } catch (ReductionLimitException err) {
throw new RuleEvalException( throw new RuleEvalException(
String.format( String.format(
"%s on change %d of %s", err.getMessage(), cd.getId().get(), getProjectName())); "%s on change %d of %s",
err.getMessage(), cd.getId().get(), projectState.getName()));
} catch (RuntimeException err) { } catch (RuntimeException err) {
throw new RuleEvalException( throw new RuleEvalException(
String.format( String.format(
"Exception calling %s on change %d of %s", sr, cd.getId().get(), getProjectName()), "Exception calling %s on change %d of %s",
sr, cd.getId().get(), projectState.getName()),
err); err);
} }
@@ -480,7 +439,8 @@ public class SubmitRuleEvaluator {
} }
} }
private PrologEnvironment getPrologEnvironment() throws RuleEvalException { private PrologEnvironment getPrologEnvironment(ChangeData cd, ProjectState projectState)
throws RuleEvalException {
PrologEnvironment env; PrologEnvironment env;
try { try {
if (opts.rule() == null) { if (opts.rule() == null) {
@@ -491,7 +451,9 @@ public class SubmitRuleEvaluator {
} catch (CompileException err) { } catch (CompileException err) {
String msg; String msg;
if (opts.rule() == null) { if (opts.rule() == null) {
msg = String.format("Cannot load rules.pl for %s: %s", getProjectName(), err.getMessage()); msg =
String.format(
"Cannot load rules.pl for %s: %s", projectState.getName(), err.getMessage());
} else { } else {
msg = err.getMessage(); msg = err.getMessage();
} }
@@ -513,6 +475,8 @@ public class SubmitRuleEvaluator {
String filterRuleWrapperName) String filterRuleWrapperName)
throws RuleEvalException { throws RuleEvalException {
PrologEnvironment childEnv = env; PrologEnvironment childEnv = env;
ChangeData cd = env.get(StoredValues.CHANGE_DATA);
ProjectState projectState = env.get(StoredValues.PROJECT_STATE);
for (ProjectState parentState : projectState.parents()) { for (ProjectState parentState : projectState.parents()) {
PrologEnvironment parentEnv; PrologEnvironment parentEnv;
try { try {
@@ -583,31 +547,4 @@ public class SubmitRuleEvaluator {
private void checkNotStarted() { private void checkNotStarted() {
checkState(opts == null, "cannot set options after starting evaluation"); checkState(opts == null, "cannot set options after starting evaluation");
} }
private void initOptions() {
if (opts == null) {
opts = optsBuilder.build();
optsBuilder = null;
}
}
private void init() throws OrmException, NoSuchProjectException {
if (change == null) {
change = cd.change();
if (change == null) {
throw new OrmException("No change found");
}
}
if (projectState == null) {
projectState = projectCache.get(change.getProject());
if (projectState == null) {
throw new NoSuchProjectException(change.getProject());
}
}
}
private String getProjectName() {
return projectState.getName();
}
} }

View File

@@ -25,17 +25,28 @@ import com.google.gerrit.common.Nullable;
*/ */
@AutoValue @AutoValue
public abstract class SubmitRuleOptions { public abstract class SubmitRuleOptions {
public static Builder builder() { private static final SubmitRuleOptions defaults =
return new AutoValue_SubmitRuleOptions.Builder() new AutoValue_SubmitRuleOptions.Builder()
.allowClosed(false) .allowClosed(false)
.skipFilters(false) .skipFilters(false)
.rule(null); .logErrors(true)
.rule(null)
.build();
public static SubmitRuleOptions defaults() {
return defaults;
}
public static Builder builder() {
return defaults.toBuilder();
} }
public abstract boolean allowClosed(); public abstract boolean allowClosed();
public abstract boolean skipFilters(); public abstract boolean skipFilters();
public abstract boolean logErrors();
@Nullable @Nullable
public abstract String rule(); public abstract String rule();
@@ -49,6 +60,8 @@ public abstract class SubmitRuleOptions {
public abstract SubmitRuleOptions.Builder rule(@Nullable String rule); public abstract SubmitRuleOptions.Builder rule(@Nullable String rule);
public abstract SubmitRuleOptions.Builder logErrors(boolean logErrors);
public abstract SubmitRuleOptions build(); public abstract SubmitRuleOptions build();
} }
} }

View File

@@ -943,7 +943,7 @@ public class ChangeData {
if (!lazyLoad) { if (!lazyLoad) {
return Collections.emptyList(); return Collections.emptyList();
} }
records = submitRuleEvaluatorFactory.create(this).setOptions(options).evaluate(); records = submitRuleEvaluatorFactory.create(options).evaluate(this);
submitRecords.put(options, records); submitRecords.put(options, records);
} }
return records; return records;
@@ -960,7 +960,8 @@ public class ChangeData {
public SubmitTypeRecord submitTypeRecord() { public SubmitTypeRecord submitTypeRecord() {
if (submitTypeRecord == null) { if (submitTypeRecord == null) {
submitTypeRecord = submitRuleEvaluatorFactory.create(this).getSubmitType(); submitTypeRecord =
submitRuleEvaluatorFactory.create(SubmitRuleOptions.defaults()).getSubmitType(this);
} }
return submitTypeRecord; return submitTypeRecord;
} }

View File

@@ -31,6 +31,7 @@ import com.google.gerrit.server.data.QueryStatsAttribute;
import com.google.gerrit.server.events.EventFactory; import com.google.gerrit.server.events.EventFactory;
import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gson.Gson; import com.google.gson.Gson;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -249,8 +250,8 @@ public class OutputStreamQuery {
} }
if (includeSubmitRecords) { if (includeSubmitRecords) {
eventFactory.addSubmitRecords( SubmitRuleOptions options = SubmitRuleOptions.builder().allowClosed(true).build();
c, submitRuleEvaluatorFactory.create(d).setAllowClosed(true).evaluate()); eventFactory.addSubmitRecords(c, submitRuleEvaluatorFactory.create(options).evaluate(d));
} }
if (includeCommitMessage) { if (includeCommitMessage) {

View File

@@ -35,6 +35,7 @@ import com.google.gerrit.server.index.change.ChangeIndexer;
import com.google.gerrit.server.project.ProjectCache; import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
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.Inject;
@@ -70,7 +71,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
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; private final SubmitRuleEvaluator submitRuleEvaluator;
@Inject @Inject
Mergeable( Mergeable(
@@ -89,7 +90,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
this.db = db; this.db = db;
this.indexer = indexer; this.indexer = indexer;
this.cache = cache; this.cache = cache;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory; submitRuleEvaluator = submitRuleEvaluatorFactory.create(SubmitRuleOptions.defaults());
} }
public void setOtherBranches(boolean otherBranches) { public void setOtherBranches(boolean otherBranches) {
@@ -145,7 +146,7 @@ public class Mergeable implements RestReadView<RevisionResource> {
} }
private SubmitType getSubmitType(ChangeData cd) throws OrmException { private SubmitType getSubmitType(ChangeData cd) throws OrmException {
SubmitTypeRecord rec = submitRuleEvaluatorFactory.create(cd).getSubmitType(); SubmitTypeRecord rec = submitRuleEvaluator.getSubmitType(cd);
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

@@ -33,6 +33,7 @@ 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;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
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.Inject;
@@ -49,7 +50,7 @@ public class ReviewerJson {
private final ChangeData.Factory changeDataFactory; private final ChangeData.Factory changeDataFactory;
private final ApprovalsUtil approvalsUtil; private final ApprovalsUtil approvalsUtil;
private final AccountLoader.Factory accountLoaderFactory; private final AccountLoader.Factory accountLoaderFactory;
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory; private final SubmitRuleEvaluator submitRuleEvaluator;
@Inject @Inject
ReviewerJson( ReviewerJson(
@@ -64,7 +65,7 @@ public class ReviewerJson {
this.changeDataFactory = changeDataFactory; this.changeDataFactory = changeDataFactory;
this.approvalsUtil = approvalsUtil; this.approvalsUtil = approvalsUtil;
this.accountLoaderFactory = accountLoaderFactory; this.accountLoaderFactory = accountLoaderFactory;
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory; submitRuleEvaluator = submitRuleEvaluatorFactory.create(SubmitRuleOptions.defaults());
} }
public List<ReviewerInfo> format(Collection<ReviewerResource> rsrcs) public List<ReviewerInfo> format(Collection<ReviewerResource> rsrcs)
@@ -124,7 +125,7 @@ public class ReviewerJson {
// do not exist in the DB. // do not exist in the DB.
PatchSet ps = cd.currentPatchSet(); PatchSet ps = cd.currentPatchSet();
if (ps != null) { if (ps != null) {
for (SubmitRecord rec : submitRuleEvaluatorFactory.create(cd).evaluate()) { for (SubmitRecord rec : submitRuleEvaluator.evaluate(cd)) {
if (rec.labels == null) { if (rec.labels == null) {
continue; continue;
} }

View File

@@ -26,6 +26,7 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountLoader; import com.google.gerrit.server.account.AccountLoader;
import com.google.gerrit.server.change.RevisionResource; import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.RulesCache; import com.google.gerrit.server.rules.RulesCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -70,15 +71,17 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
throw new AuthException("project rules are disabled"); throw new AuthException("project rules are disabled");
} }
input.filters = MoreObjects.firstNonNull(input.filters, filters); input.filters = MoreObjects.firstNonNull(input.filters, filters);
SubmitRuleEvaluator evaluator =
submitRuleEvaluatorFactory.create(changeDataFactory.create(db.get(), rsrc.getNotes()));
List<SubmitRecord> records = SubmitRuleOptions opts =
evaluator SubmitRuleOptions.builder()
.setLogErrors(false) .skipFilters(input.filters == Filters.SKIP)
.setSkipSubmitFilters(input.filters == Filters.SKIP) .rule(input.rule)
.setRule(input.rule) .logErrors(false)
.evaluate(); .build();
ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes());
List<SubmitRecord> records = submitRuleEvaluatorFactory.create(opts).evaluate(cd);
List<Record> out = Lists.newArrayListWithCapacity(records.size()); List<Record> out = Lists.newArrayListWithCapacity(records.size());
AccountLoader accounts = accountInfoFactory.create(true); AccountLoader accounts = accountInfoFactory.create(true);
for (SubmitRecord r : records) { for (SubmitRecord r : records) {

View File

@@ -26,6 +26,7 @@ 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.server.change.RevisionResource; import com.google.gerrit.server.change.RevisionResource;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.rules.RulesCache; import com.google.gerrit.server.rules.RulesCache;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
@@ -64,15 +65,18 @@ public class TestSubmitType implements RestModifyView<RevisionResource, TestSubm
throw new AuthException("project rules are disabled"); throw new AuthException("project rules are disabled");
} }
input.filters = MoreObjects.firstNonNull(input.filters, filters); input.filters = MoreObjects.firstNonNull(input.filters, filters);
SubmitRuleEvaluator evaluator =
submitRuleEvaluatorFactory.create(changeDataFactory.create(db.get(), rsrc.getNotes()));
SubmitTypeRecord rec = SubmitRuleOptions opts =
evaluator SubmitRuleOptions.builder()
.setLogErrors(false) .logErrors(false)
.setSkipSubmitFilters(input.filters == Filters.SKIP) .skipFilters(input.filters == Filters.SKIP)
.setRule(input.rule) .rule(input.rule)
.getSubmitType(); .build();
SubmitRuleEvaluator evaluator = submitRuleEvaluatorFactory.create(opts);
ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes());
SubmitTypeRecord rec = evaluator.getSubmitType(cd);
if (rec.status != SubmitTypeRecord.Status.OK) { if (rec.status != SubmitTypeRecord.Status.OK) {
throw new BadRequestException( throw new BadRequestException(
String.format("rule %s produced invalid result: %s", evaluator.getSubmitRuleName(), rec)); String.format("rule %s produced invalid result: %s", evaluator.getSubmitRuleName(), rec));

View File

@@ -22,6 +22,7 @@ import com.google.gerrit.acceptance.TestAccount;
import com.google.gerrit.common.data.SubmitRecord; import com.google.gerrit.common.data.SubmitRecord;
import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.project.SubmitRuleEvaluator; import com.google.gerrit.server.project.SubmitRuleEvaluator;
import com.google.gerrit.server.project.SubmitRuleOptions;
import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.testing.TestChanges; import com.google.gerrit.testing.TestChanges;
import com.google.inject.Inject; import com.google.inject.Inject;
@@ -39,12 +40,13 @@ public class SubmitRulesEvaluatorIT extends AbstractDaemonTest {
@Test @Test
public void convertsPrologToSubmitRecord() { public void convertsPrologToSubmitRecord() {
SubmitRuleEvaluator evaluator = makeEvaluator(); SubmitRuleEvaluator evaluator = makeEvaluator();
ChangeData cd = makeChangeData();
StructureTerm verifiedLabel = makeLabel("Verified", "may"); StructureTerm verifiedLabel = makeLabel("Verified", "may");
StructureTerm labels = new StructureTerm("label", verifiedLabel); StructureTerm labels = new StructureTerm("label", verifiedLabel);
List<Term> terms = ImmutableList.of(makeTerm("ok", labels)); List<Term> terms = ImmutableList.of(makeTerm("ok", labels));
Collection<SubmitRecord> records = evaluator.resultsToSubmitRecord(null, terms); Collection<SubmitRecord> records = evaluator.resultsToSubmitRecord(null, terms, cd);
assertThat(records).hasSize(1); assertThat(records).hasSize(1);
} }
@@ -82,6 +84,7 @@ public class SubmitRulesEvaluatorIT extends AbstractDaemonTest {
@Test @Test
public void abortsEarlyWithOkayRecord() { public void abortsEarlyWithOkayRecord() {
SubmitRuleEvaluator evaluator = makeEvaluator(); SubmitRuleEvaluator evaluator = makeEvaluator();
ChangeData cd = makeChangeData();
SubmitRecord.Label submitRecordLabel1 = new SubmitRecord.Label(); SubmitRecord.Label submitRecordLabel1 = new SubmitRecord.Label();
submitRecordLabel1.label = "Verified"; submitRecordLabel1.label = "Verified";
@@ -111,7 +114,7 @@ public class SubmitRulesEvaluatorIT extends AbstractDaemonTest {
terms.add(makeTerm("not_ready", makeLabels(label3))); terms.add(makeTerm("not_ready", makeLabels(label3)));
// When // When
List<SubmitRecord> records = evaluator.resultsToSubmitRecord(null, terms); List<SubmitRecord> records = evaluator.resultsToSubmitRecord(null, terms, cd);
// assert that // assert that
SubmitRecord record1Expected = new SubmitRecord(); SubmitRecord record1Expected = new SubmitRecord();
@@ -147,10 +150,13 @@ public class SubmitRulesEvaluatorIT extends AbstractDaemonTest {
return new StructureTerm("label", labels); return new StructureTerm("label", labels);
} }
private SubmitRuleEvaluator makeEvaluator() { private ChangeData makeChangeData() {
ChangeData cd = ChangeData.createForTest(project, new Change.Id(1), 1); ChangeData cd = ChangeData.createForTest(project, new Change.Id(1), 1);
cd.setChange(TestChanges.newChange(project, admin.id)); cd.setChange(TestChanges.newChange(project, admin.id));
return cd;
}
return evaluatorFactory.create(cd); private SubmitRuleEvaluator makeEvaluator() {
return evaluatorFactory.create(SubmitRuleOptions.defaults());
} }
} }