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
diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
index 9b845b4..c7eb781 100644
--- a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
+++ b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java
@@ -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.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 @@
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 @@
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 @@
.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);
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java
index 982613d..507205d 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java
@@ -23,7 +23,6 @@
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 @@
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 @@
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)) {
