From d7c89a8dbce2400107624a43298b93090cc74e8f Mon Sep 17 00:00:00 2001 From: Patrick Hiesel Date: Mon, 15 Oct 2018 13:23:47 +0200 Subject: [PATCH] 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 --- .../server/restapi/change/TestSubmitRule.java | 35 +++++++++++++++---- .../api/change/SubmitTypeRuleIT.java | 10 +----- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java index 9b845b4b19..c7eb7812be 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.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 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 records = submitRuleEvaluatorFactory.create(opts).evaluate(cd); + List 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 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 982613dda0..507205d972 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_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 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 log(String commitish, int n) throws Exception { try (Repository repo = repoManager.openRepository(project); Git git = new Git(repo)) {