Make TestSubmitRule compliant with docs
The docs of TestSubmitRule say: "Tests the submit_rule Prolog rule in the project, or the one given.". However, we were calling the SubmitRuleEvaluator which did not adhere to this specification. Since this endpoint is only about Prolog rules, the desired behavior is: Use the provided rule, if present. Use rules.pl otherwise. Use the default rules (previously in Prolog, now in Java) as fallback. This commits changes the behavior to adhere to this specification. The previously added tests are adapted to assert the correct behavior. Change-Id: I4dab0a6ce8d2300633471e8403c33e1e24d3c0ba
This commit is contained in:
parent
4cec171465
commit
d7c89a8dbc
@ -15,6 +15,7 @@
|
||||
package com.google.gerrit.server.restapi.change;
|
||||
|
||||
import com.google.common.base.MoreObjects;
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.Lists;
|
||||
import com.google.gerrit.common.data.SubmitRecord;
|
||||
import com.google.gerrit.extensions.common.AccountInfo;
|
||||
@ -22,14 +23,18 @@ import com.google.gerrit.extensions.common.TestSubmitRuleInfo;
|
||||
import com.google.gerrit.extensions.common.TestSubmitRuleInput;
|
||||
import com.google.gerrit.extensions.common.TestSubmitRuleInput.Filters;
|
||||
import com.google.gerrit.extensions.restapi.AuthException;
|
||||
import com.google.gerrit.extensions.restapi.BadRequestException;
|
||||
import com.google.gerrit.extensions.restapi.RestModifyView;
|
||||
import com.google.gerrit.reviewdb.server.ReviewDb;
|
||||
import com.google.gerrit.server.account.AccountLoader;
|
||||
import com.google.gerrit.server.change.RevisionResource;
|
||||
import com.google.gerrit.server.permissions.PermissionBackendException;
|
||||
import com.google.gerrit.server.project.SubmitRuleEvaluator;
|
||||
import com.google.gerrit.server.project.ProjectCache;
|
||||
import com.google.gerrit.server.project.ProjectState;
|
||||
import com.google.gerrit.server.project.SubmitRuleOptions;
|
||||
import com.google.gerrit.server.query.change.ChangeData;
|
||||
import com.google.gerrit.server.rules.DefaultSubmitRule;
|
||||
import com.google.gerrit.server.rules.PrologRule;
|
||||
import com.google.gerrit.server.rules.RulesCache;
|
||||
import com.google.gwtorm.server.OrmException;
|
||||
import com.google.inject.Inject;
|
||||
@ -43,7 +48,9 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
|
||||
private final ChangeData.Factory changeDataFactory;
|
||||
private final RulesCache rules;
|
||||
private final AccountLoader.Factory accountInfoFactory;
|
||||
private final SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory;
|
||||
private final ProjectCache projectCache;
|
||||
private final DefaultSubmitRule defaultSubmitRule;
|
||||
private final PrologRule prologRule;
|
||||
|
||||
@Option(name = "--filters", usage = "impact of filters in parent projects")
|
||||
private Filters filters = Filters.RUN;
|
||||
@ -54,17 +61,21 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
|
||||
ChangeData.Factory changeDataFactory,
|
||||
RulesCache rules,
|
||||
AccountLoader.Factory infoFactory,
|
||||
SubmitRuleEvaluator.Factory submitRuleEvaluatorFactory) {
|
||||
ProjectCache projectCache,
|
||||
DefaultSubmitRule defaultSubmitRule,
|
||||
PrologRule prologRule) {
|
||||
this.db = db;
|
||||
this.changeDataFactory = changeDataFactory;
|
||||
this.rules = rules;
|
||||
this.accountInfoFactory = infoFactory;
|
||||
this.submitRuleEvaluatorFactory = submitRuleEvaluatorFactory;
|
||||
this.projectCache = projectCache;
|
||||
this.defaultSubmitRule = defaultSubmitRule;
|
||||
this.prologRule = prologRule;
|
||||
}
|
||||
|
||||
@Override
|
||||
public List<TestSubmitRuleInfo> apply(RevisionResource rsrc, TestSubmitRuleInput input)
|
||||
throws AuthException, OrmException, PermissionBackendException {
|
||||
throws AuthException, OrmException, PermissionBackendException, BadRequestException {
|
||||
if (input == null) {
|
||||
input = new TestSubmitRuleInput();
|
||||
}
|
||||
@ -80,8 +91,20 @@ public class TestSubmitRule implements RestModifyView<RevisionResource, TestSubm
|
||||
.logErrors(false)
|
||||
.build();
|
||||
|
||||
ProjectState projectState = projectCache.get(rsrc.getProject());
|
||||
if (projectState == null) {
|
||||
throw new BadRequestException("project not found");
|
||||
}
|
||||
ChangeData cd = changeDataFactory.create(db.get(), rsrc.getNotes());
|
||||
List<SubmitRecord> records = submitRuleEvaluatorFactory.create(opts).evaluate(cd);
|
||||
List<SubmitRecord> records;
|
||||
if (projectState.hasPrologRules() || input.rule != null) {
|
||||
records = ImmutableList.copyOf(prologRule.evaluate(cd, opts));
|
||||
} else {
|
||||
// No rules were provided as input and we have no rules.pl. This means we are supposed to run
|
||||
// the default rules. Nowadays, the default rules are implemented in Java, not Prolog.
|
||||
// Therefore, we call the DefaultRuleEvaluator instead.
|
||||
records = ImmutableList.copyOf(defaultSubmitRule.evaluate(cd, opts));
|
||||
}
|
||||
|
||||
List<TestSubmitRuleInfo> out = Lists.newArrayListWithCapacity(records.size());
|
||||
AccountLoader accounts = accountInfoFactory.create(true);
|
||||
|
@ -23,7 +23,6 @@ import static com.google.gerrit.extensions.client.SubmitType.REBASE_ALWAYS;
|
||||
import static com.google.gerrit.extensions.client.SubmitType.REBASE_IF_NECESSARY;
|
||||
|
||||
import com.google.common.collect.ImmutableList;
|
||||
import com.google.common.collect.ImmutableMap;
|
||||
import com.google.gerrit.acceptance.AbstractDaemonTest;
|
||||
import com.google.gerrit.acceptance.NoHttpd;
|
||||
import com.google.gerrit.acceptance.PushOneCommit;
|
||||
@ -265,7 +264,7 @@ public class SubmitTypeRuleIT extends AbstractDaemonTest {
|
||||
in.rule = "invalid prolog rule";
|
||||
// We have no rules.pl by default. The fact that the default rules are showing up here is a bug.
|
||||
List<TestSubmitRuleInfo> response = gApi.changes().id(changeId).current().testSubmitRule(in);
|
||||
assertThat(response).containsExactly(defaultUnsatisfiedRuleInfo(), invalidPrologRuleInfo());
|
||||
assertThat(response).containsExactly(invalidPrologRuleInfo());
|
||||
}
|
||||
|
||||
@Test
|
||||
@ -287,13 +286,6 @@ public class SubmitTypeRuleIT extends AbstractDaemonTest {
|
||||
return info;
|
||||
}
|
||||
|
||||
private static TestSubmitRuleInfo defaultUnsatisfiedRuleInfo() {
|
||||
TestSubmitRuleInfo info = new TestSubmitRuleInfo();
|
||||
info.status = "NOT_READY";
|
||||
info.need = ImmutableMap.of("Code-Review", TestSubmitRuleInfo.None.INSTANCE);
|
||||
return info;
|
||||
}
|
||||
|
||||
private List<RevCommit> log(String commitish, int n) throws Exception {
|
||||
try (Repository repo = repoManager.openRepository(project);
|
||||
Git git = new Git(repo)) {
|
||||
|
Loading…
Reference in New Issue
Block a user