diff --git a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java index 3665706614..4658eb3a65 100644 --- a/java/com/google/gerrit/extensions/api/changes/RevisionApi.java +++ b/java/com/google/gerrit/extensions/api/changes/RevisionApi.java @@ -23,6 +23,7 @@ import com.google.gerrit.extensions.common.EditInfo; import com.google.gerrit.extensions.common.FileInfo; import com.google.gerrit.extensions.common.MergeableInfo; import com.google.gerrit.extensions.common.RobotCommentInfo; +import com.google.gerrit.extensions.common.TestSubmitRuleInfo; import com.google.gerrit.extensions.common.TestSubmitRuleInput; import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.NotImplementedException; @@ -128,6 +129,8 @@ public interface RevisionApi { SubmitType testSubmitType(TestSubmitRuleInput in) throws RestApiException; + List testSubmitRule(TestSubmitRuleInput in) throws RestApiException; + MergeListRequest getMergeList() throws RestApiException; abstract class MergeListRequest { @@ -357,6 +360,11 @@ public interface RevisionApi { throw new NotImplementedException(); } + @Override + public List testSubmitRule(TestSubmitRuleInput in) throws RestApiException { + throw new NotImplementedException(); + } + @Override public MergeListRequest getMergeList() throws RestApiException { throw new NotImplementedException(); diff --git a/java/com/google/gerrit/extensions/common/TestSubmitRuleInfo.java b/java/com/google/gerrit/extensions/common/TestSubmitRuleInfo.java new file mode 100644 index 0000000000..bd7ebcb091 --- /dev/null +++ b/java/com/google/gerrit/extensions/common/TestSubmitRuleInfo.java @@ -0,0 +1,70 @@ +// Copyright (C) 2018 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package com.google.gerrit.extensions.common; + +import com.google.common.base.MoreObjects; +import java.util.Map; +import java.util.Objects; + +public class TestSubmitRuleInfo { + /** @see com.google.gerrit.common.data.SubmitRecord.Status */ + public String status; + + public String errorMessage; + public Map ok; + public Map reject; + public Map need; + public Map may; + public Map impossible; + + public static class None { + private None() {} + + public static None INSTANCE = new None(); + } + + @Override + public boolean equals(Object o) { + if (o instanceof TestSubmitRuleInfo) { + TestSubmitRuleInfo other = (TestSubmitRuleInfo) o; + return Objects.equals(status, other.status) + && Objects.equals(errorMessage, other.errorMessage) + && Objects.equals(ok, other.ok) + && Objects.equals(reject, other.reject) + && Objects.equals(need, other.need) + && Objects.equals(may, other.may) + && Objects.equals(impossible, other.impossible); + } + return false; + } + + @Override + public int hashCode() { + return Objects.hash(status, errorMessage, ok, reject, need, may, impossible); + } + + @Override + public String toString() { + return MoreObjects.toStringHelper(this) + .add("status", status) + .add("errorMessage", errorMessage) + .add("ok", ok) + .add("reject", reject) + .add("need", need) + .add("may", may) + .add("impossible", impossible) + .toString(); + } +} diff --git a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java index bd602a8273..33f211d936 100644 --- a/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java +++ b/java/com/google/gerrit/server/api/changes/RevisionApiImpl.java @@ -43,6 +43,7 @@ import com.google.gerrit.extensions.common.FileInfo; import com.google.gerrit.extensions.common.Input; import com.google.gerrit.extensions.common.MergeableInfo; import com.google.gerrit.extensions.common.RobotCommentInfo; +import com.google.gerrit.extensions.common.TestSubmitRuleInfo; import com.google.gerrit.extensions.common.TestSubmitRuleInput; import com.google.gerrit.extensions.restapi.BinaryResult; import com.google.gerrit.extensions.restapi.IdString; @@ -76,6 +77,7 @@ import com.google.gerrit.server.restapi.change.Reviewed; import com.google.gerrit.server.restapi.change.RevisionReviewers; import com.google.gerrit.server.restapi.change.RobotComments; import com.google.gerrit.server.restapi.change.Submit; +import com.google.gerrit.server.restapi.change.TestSubmitRule; import com.google.gerrit.server.restapi.change.TestSubmitType; import com.google.inject.Inject; import com.google.inject.Provider; @@ -125,6 +127,7 @@ class RevisionApiImpl implements RevisionApi { private final GetRevisionActions revisionActions; private final TestSubmitType testSubmitType; private final TestSubmitType.Get getSubmitType; + private final Provider testSubmitRule; private final Provider getMergeList; private final PutDescription putDescription; private final GetDescription getDescription; @@ -164,6 +167,7 @@ class RevisionApiImpl implements RevisionApi { GetRevisionActions revisionActions, TestSubmitType testSubmitType, TestSubmitType.Get getSubmitType, + Provider testSubmitRule, Provider getMergeList, PutDescription putDescription, GetDescription getDescription, @@ -201,6 +205,7 @@ class RevisionApiImpl implements RevisionApi { this.revisionActions = revisionActions; this.testSubmitType = testSubmitType; this.getSubmitType = getSubmitType; + this.testSubmitRule = testSubmitRule; this.getMergeList = getMergeList; this.putDescription = putDescription; this.getDescription = getDescription; @@ -558,6 +563,15 @@ class RevisionApiImpl implements RevisionApi { } } + @Override + public List testSubmitRule(TestSubmitRuleInput in) throws RestApiException { + try { + return testSubmitRule.get().apply(revision, in); + } catch (Exception e) { + throw asRestApiException("Cannot test submit rule", e); + } + } + @Override public MergeListRequest getMergeList() throws RestApiException { return new MergeListRequest() { diff --git a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java index cdd7426eb1..c7eb7812be 100644 --- a/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java +++ b/java/com/google/gerrit/server/restapi/change/TestSubmitRule.java @@ -15,27 +15,32 @@ 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; +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; import com.google.inject.Provider; import java.util.LinkedHashMap; import java.util.List; -import java.util.Map; import org.kohsuke.args4j.Option; public class TestSubmitRule implements RestModifyView { @@ -43,7 +48,9 @@ public class TestSubmitRule implements RestModifyView apply(RevisionResource rsrc, TestSubmitRuleInput input) - throws AuthException, OrmException, PermissionBackendException { + public List apply(RevisionResource rsrc, TestSubmitRuleInput input) + throws AuthException, OrmException, PermissionBackendException, BadRequestException { if (input == null) { input = new TestSubmitRuleInput(); } @@ -80,74 +91,76 @@ 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()); + List out = Lists.newArrayListWithCapacity(records.size()); AccountLoader accounts = accountInfoFactory.create(true); for (SubmitRecord r : records) { - out.add(new Record(r, accounts)); + out.add(newSubmitRuleInfo(r, accounts)); } accounts.fill(); return out; } - static class Record { - SubmitRecord.Status status; - String errorMessage; - Map ok; - Map reject; - Map need; - Map may; - Map impossible; + private static TestSubmitRuleInfo newSubmitRuleInfo(SubmitRecord r, AccountLoader accounts) { + TestSubmitRuleInfo info = new TestSubmitRuleInfo(); + info.status = r.status.name(); + info.errorMessage = r.errorMessage; - Record(SubmitRecord r, AccountLoader accounts) { - this.status = r.status; - this.errorMessage = r.errorMessage; - - if (r.labels != null) { - for (SubmitRecord.Label n : r.labels) { - AccountInfo who = n.appliedBy != null ? accounts.get(n.appliedBy) : new AccountInfo(null); - label(n, who); - } - } - } - - private void label(SubmitRecord.Label n, AccountInfo who) { - switch (n.status) { - case OK: - if (ok == null) { - ok = new LinkedHashMap<>(); - } - ok.put(n.label, who); - break; - case REJECT: - if (reject == null) { - reject = new LinkedHashMap<>(); - } - reject.put(n.label, who); - break; - case NEED: - if (need == null) { - need = new LinkedHashMap<>(); - } - need.put(n.label, new None()); - break; - case MAY: - if (may == null) { - may = new LinkedHashMap<>(); - } - may.put(n.label, who); - break; - case IMPOSSIBLE: - if (impossible == null) { - impossible = new LinkedHashMap<>(); - } - impossible.put(n.label, new None()); - break; + if (r.labels != null) { + for (SubmitRecord.Label n : r.labels) { + AccountInfo who = n.appliedBy != null ? accounts.get(n.appliedBy) : new AccountInfo(null); + label(info, n, who); } } + return info; } - static class None {} + private static void label(TestSubmitRuleInfo info, SubmitRecord.Label n, AccountInfo who) { + switch (n.status) { + case OK: + if (info.ok == null) { + info.ok = new LinkedHashMap<>(); + } + info.ok.put(n.label, who); + break; + case REJECT: + if (info.reject == null) { + info.reject = new LinkedHashMap<>(); + } + info.reject.put(n.label, who); + break; + case NEED: + if (info.need == null) { + info.need = new LinkedHashMap<>(); + } + info.need.put(n.label, TestSubmitRuleInfo.None.INSTANCE); + break; + case MAY: + if (info.may == null) { + info.may = new LinkedHashMap<>(); + } + info.may.put(n.label, who); + break; + case IMPOSSIBLE: + if (info.impossible == null) { + info.impossible = new LinkedHashMap<>(); + } + info.impossible.put(n.label, TestSubmitRuleInfo.None.INSTANCE); + break; + } + } } diff --git a/java/com/google/gerrit/server/rules/PrologRule.java b/java/com/google/gerrit/server/rules/PrologRule.java index deddc36d6d..0c54f40b28 100644 --- a/java/com/google/gerrit/server/rules/PrologRule.java +++ b/java/com/google/gerrit/server/rules/PrologRule.java @@ -39,8 +39,8 @@ public class PrologRule implements SubmitRule { @Override public Collection evaluate(ChangeData cd, SubmitRuleOptions opts) { ProjectState projectState = projectCache.get(cd.project()); - // We only want to run the prolog engine if we have at least one rules.pl file to use. - if (projectState == null || !projectState.hasPrologRules()) { + // We only want to run the Prolog engine if we have at least one rules.pl file to use. + if ((projectState == null || !projectState.hasPrologRules()) && opts.rule() == null) { return Collections.emptyList(); } diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java index f73a59cf68..507205d972 100644 --- a/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java +++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitTypeRuleIT.java @@ -29,6 +29,7 @@ import com.google.gerrit.acceptance.PushOneCommit; import com.google.gerrit.extensions.api.changes.ReviewInput; import com.google.gerrit.extensions.api.projects.BranchInput; import com.google.gerrit.extensions.client.SubmitType; +import com.google.gerrit.extensions.common.TestSubmitRuleInfo; import com.google.gerrit.extensions.common.TestSubmitRuleInput; import com.google.gerrit.extensions.restapi.ResourceConflictException; import com.google.gerrit.extensions.restapi.RestApiException; @@ -255,6 +256,36 @@ public class SubmitTypeRuleIT extends AbstractDaemonTest { } } + @Test + public void invalidSubmitRuleWithNoRulesInProject() throws Exception { + String changeId = createChange("master", "change 1").getChangeId(); + + TestSubmitRuleInput in = new TestSubmitRuleInput(); + 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(invalidPrologRuleInfo()); + } + + @Test + public void invalidSubmitRuleWithRulesInProject() throws Exception { + setRulesPl(SUBMIT_TYPE_FROM_SUBJECT); + + String changeId = createChange("master", "change 1").getChangeId(); + + TestSubmitRuleInput in = new TestSubmitRuleInput(); + in.rule = "invalid prolog rule"; + List response = gApi.changes().id(changeId).current().testSubmitRule(in); + assertThat(response).containsExactly(invalidPrologRuleInfo()); + } + + private static TestSubmitRuleInfo invalidPrologRuleInfo() { + TestSubmitRuleInfo info = new TestSubmitRuleInfo(); + info.status = "RULE_ERROR"; + info.errorMessage = "operator expected after expression at: invalid prolog rule end_of_file."; + return info; + } + private List log(String commitish, int n) throws Exception { try (Repository repo = repoManager.openRepository(project); Git git = new Git(repo)) {