Implement evaluating submit requirement expressions
We reuse the change query parser to parse submit requirement expressions
into change predicates and evaluate them. In this change, we only
evaluate a single expression, example:
(project:test AND branch:refs/heads/foo) OR message:"Fix bug"
The above expressions can return an overall status = true (meaning that
the submit requirement is passing), and a list of passing and failing
predicates:
passing_predicates=["project:test", "message:fix bug"]
failing_predicates=["branch:refs/heads/foo"]
In a follow-up change, I'll add evaluation for a whole submit
requirement using the applicability, blocking and override expressions.
Change-Id: Ie1706b2083d929f3510201d095d239304543d99d
diff --git a/java/com/google/gerrit/entities/SubmitRequirementExpression.java b/java/com/google/gerrit/entities/SubmitRequirementExpression.java
index 7b31304..0d51efd 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementExpression.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementExpression.java
@@ -45,5 +45,5 @@
}
/** Returns the underlying String representing this {@link SubmitRequirementExpression}. */
- public abstract String expression();
+ public abstract String expressionString();
}
diff --git a/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java b/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
new file mode 100644
index 0000000..94c0e91
--- /dev/null
+++ b/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
@@ -0,0 +1,146 @@
+// Copyright (C) 2021 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.entities;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.collect.ImmutableList;
+import java.util.Optional;
+
+/** Result of evaluating a submit requirement expression on a given Change. */
+@AutoValue
+public abstract class SubmitRequirementExpressionResult {
+
+ /**
+ * Entity detailing the result of evaluating a Submit requirement expression. Contains an empty
+ * {@link Optional} if {@link #status()} is equal to {@link Status#ERROR}.
+ */
+ public abstract Optional<PredicateResult> predicateResult();
+
+ public abstract Optional<String> errorMessage();
+
+ public Status status() {
+ if (predicateResult().isPresent()) {
+ return predicateResult().get().status() ? Status.PASS : Status.FAIL;
+ }
+ return Status.ERROR;
+ }
+
+ public static SubmitRequirementExpressionResult create(PredicateResult predicateResult) {
+ return new AutoValue_SubmitRequirementExpressionResult(
+ Optional.of(predicateResult), Optional.empty());
+ }
+
+ public static SubmitRequirementExpressionResult error(String errorMessage) {
+ return new AutoValue_SubmitRequirementExpressionResult(
+ Optional.empty(), Optional.of(errorMessage));
+ }
+
+ /**
+ * Returns a list of leaf predicate results whose {@link PredicateResult#status()} is true. If
+ * {@link #status()} is equal to {@link Status#ERROR}, an empty list is returned.
+ */
+ public ImmutableList<PredicateResult> getPassingAtoms() {
+ if (predicateResult().isPresent()) {
+ return predicateResult().get().getAtoms(/* status= */ true);
+ }
+ return ImmutableList.of();
+ }
+
+ /**
+ * Returns a list of leaf predicate results whose {@link PredicateResult#status()} is false. If
+ * {@link #status()} is equal to {@link Status#ERROR}, an empty list is returned.
+ */
+ public ImmutableList<PredicateResult> getFailingAtoms() {
+ if (predicateResult().isPresent()) {
+ return predicateResult().get().getAtoms(/* status= */ false);
+ }
+ return ImmutableList.of();
+ }
+
+ public enum Status {
+ /** Submit requirement expression is fulfilled for a given change. */
+ PASS,
+
+ /** Submit requirement expression is failing for a given change. */
+ FAIL,
+
+ /** Submit requirement expression contains invalid syntax and is not parsable. */
+ ERROR
+ }
+
+ /**
+ * Entity detailing the result of evaluating a predicate.
+ *
+ * <p>Example - branch:refs/heads/foo and has:unresolved
+ *
+ * <p>The above predicate is an "And" predicate having two child predicates:
+ *
+ * <ul>
+ * <li>branch:refs/heads/foo
+ * <li>has:unresolved
+ * </ul>
+ *
+ * <p>Each child predicate as well as the parent contains the result of its evaluation.
+ */
+ @AutoValue
+ public abstract static class PredicateResult {
+ abstract ImmutableList<PredicateResult> childPredicateResults();
+
+ abstract String predicateString();
+
+ /** true if the predicate is passing for a given change. */
+ abstract boolean status();
+
+ /**
+ * Returns the list of leaf {@link PredicateResult} whose {@link #status()} is equal to the
+ * {@code status} parameter.
+ */
+ ImmutableList<PredicateResult> getAtoms(boolean status) {
+ ImmutableList.Builder<PredicateResult> atomsList = ImmutableList.builder();
+ getAtomsRecursively(atomsList, status);
+ return atomsList.build();
+ }
+
+ private void getAtomsRecursively(ImmutableList.Builder<PredicateResult> list, boolean status) {
+ if (childPredicateResults().isEmpty() && status() == status) {
+ list.add(this);
+ return;
+ }
+ childPredicateResults().forEach(c -> c.getAtomsRecursively(list, status));
+ }
+
+ public static Builder builder() {
+ return new AutoValue_SubmitRequirementExpressionResult_PredicateResult.Builder();
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract Builder childPredicateResults(ImmutableList<PredicateResult> value);
+
+ protected abstract ImmutableList.Builder<PredicateResult> childPredicateResultsBuilder();
+
+ public abstract Builder predicateString(String value);
+
+ public abstract Builder status(boolean value);
+
+ public Builder addChildPredicateResult(PredicateResult result) {
+ childPredicateResultsBuilder().add(result);
+ return this;
+ }
+
+ public abstract PredicateResult build();
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializer.java b/java/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializer.java
index ad015d1..a76abdb 100644
--- a/java/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializer.java
+++ b/java/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementSerializer.java
@@ -40,10 +40,10 @@
.setName(submitRequirement.name())
.setDescription(submitRequirement.description().orElse(""))
.setApplicabilityExpression(
- submitRequirement.applicabilityExpression().orElse(emptyExpression).expression())
- .setBlockingExpression(submitRequirement.blockingExpression().expression())
+ submitRequirement.applicabilityExpression().orElse(emptyExpression).expressionString())
+ .setBlockingExpression(submitRequirement.blockingExpression().expressionString())
.setOverrideExpression(
- submitRequirement.overrideExpression().orElse(emptyExpression).expression())
+ submitRequirement.overrideExpression().orElse(emptyExpression).expressionString())
.setAllowOverrideInChildProjects(submitRequirement.allowOverrideInChildProjects())
.build();
}
diff --git a/java/com/google/gerrit/server/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index 5ac5ac7..dac1ac6 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -1694,19 +1694,19 @@
SUBMIT_REQUIREMENT,
name,
KEY_SR_APPLICABILITY_EXPRESSION,
- sr.applicabilityExpression().get().expression());
+ sr.applicabilityExpression().get().expressionString());
}
rc.setString(
SUBMIT_REQUIREMENT,
name,
KEY_SR_BLOCKING_EXPRESSION,
- sr.blockingExpression().expression());
+ sr.blockingExpression().expressionString());
if (sr.overrideExpression().isPresent()) {
rc.setString(
SUBMIT_REQUIREMENT,
name,
KEY_SR_OVERRIDE_EXPRESSION,
- sr.overrideExpression().get().expression());
+ sr.overrideExpression().get().expressionString());
}
rc.setBoolean(
SUBMIT_REQUIREMENT,
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
new file mode 100644
index 0000000..7fc1247
--- /dev/null
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
@@ -0,0 +1,76 @@
+// Copyright (C) 2021 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.server.project;
+
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.PredicateResult;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.ChangeQueryBuilder;
+import com.google.inject.Inject;
+import com.google.inject.Provider;
+import com.google.inject.Singleton;
+
+/** Evaluates submit requirements for different change data. */
+@Singleton
+public class SubmitRequirementsEvaluator {
+ private final Provider<ChangeQueryBuilder> changeQueryBuilderProvider;
+
+ @Inject
+ private SubmitRequirementsEvaluator(Provider<ChangeQueryBuilder> changeQueryBuilderProvider) {
+ this.changeQueryBuilderProvider = changeQueryBuilderProvider;
+ }
+
+ /**
+ * Validate a {@link SubmitRequirementExpression}. Callers who wish to validate submit
+ * requirements upon creation or update should use this method.
+ *
+ * @param expression entity containing the expression string.
+ * @throws QueryParseException the expression string contains invalid syntax and can't be parsed.
+ */
+ public void validateExpression(SubmitRequirementExpression expression)
+ throws QueryParseException {
+ changeQueryBuilderProvider.get().parse(expression.expressionString());
+ }
+
+ /** Evaluate a {@link SubmitRequirementExpression} using change data. */
+ public SubmitRequirementExpressionResult evaluateExpression(
+ SubmitRequirementExpression expression, ChangeData changeData) {
+ try {
+ Predicate<ChangeData> predicate =
+ changeQueryBuilderProvider.get().parse(expression.expressionString());
+ PredicateResult predicateResult = evaluatePredicateTree(predicate, changeData);
+ return SubmitRequirementExpressionResult.create(predicateResult);
+ } catch (QueryParseException e) {
+ return SubmitRequirementExpressionResult.error(e.getMessage());
+ }
+ }
+
+ /** Evaluate the predicate recursively using change data. */
+ private PredicateResult evaluatePredicateTree(
+ Predicate<ChangeData> predicate, ChangeData changeData) {
+ PredicateResult.Builder predicateResult =
+ PredicateResult.builder()
+ .predicateString(predicate.toString())
+ .status(predicate.asMatchable().match(changeData));
+ predicate
+ .getChildren()
+ .forEach(
+ c -> predicateResult.addChildPredicateResult(evaluatePredicateTree(c, changeData)));
+ return predicateResult.build();
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
new file mode 100644
index 0000000..19d0c97
--- /dev/null
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -0,0 +1,108 @@
+// Copyright (C) 2021 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.acceptance.server.project;
+
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.PredicateResult;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.Status;
+import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
+import com.google.gerrit.server.query.change.ChangeData;
+import com.google.inject.Inject;
+import java.util.Optional;
+import org.junit.Before;
+import org.junit.Test;
+
+@NoHttpd
+public class SubmitRequirementsEvaluatorIT extends AbstractDaemonTest {
+ @Inject SubmitRequirementsEvaluator evaluator;
+
+ private ChangeData changeData;
+
+ @Before
+ public void setUp() throws Exception {
+ PushOneCommit.Result pushResult =
+ createChange(testRepo, "refs/heads/master", "Fix a bug", "file.txt", "content", "topic");
+ changeData = pushResult.getChange();
+ }
+
+ @Test
+ public void invalidExpression() throws Exception {
+ SubmitRequirementExpression expression =
+ SubmitRequirementExpression.create("invalid_field:invalid_value");
+ SubmitRequirementExpressionResult result = evaluator.evaluateExpression(expression, changeData);
+
+ assertThat(result.status()).isEqualTo(Status.ERROR);
+ assertThat(result.errorMessage().get())
+ .isEqualTo("Unsupported operator invalid_field:invalid_value");
+ }
+
+ @Test
+ public void expressionWithPassingPredicate() throws Exception {
+ SubmitRequirementExpression expression =
+ SubmitRequirementExpression.create("branch:refs/heads/master");
+ SubmitRequirementExpressionResult result = evaluator.evaluateExpression(expression, changeData);
+
+ assertThat(result.status()).isEqualTo(Status.PASS);
+ assertThat(result.errorMessage()).isEqualTo(Optional.empty());
+ }
+
+ @Test
+ public void expressionWithFailingPredicate() throws Exception {
+ SubmitRequirementExpression expression =
+ SubmitRequirementExpression.create("branch:refs/heads/foo");
+ SubmitRequirementExpressionResult result = evaluator.evaluateExpression(expression, changeData);
+
+ assertThat(result.status()).isEqualTo(Status.FAIL);
+ assertThat(result.errorMessage()).isEqualTo(Optional.empty());
+ }
+
+ @Test
+ public void compositeExpression() throws Exception {
+ SubmitRequirementExpression expression =
+ SubmitRequirementExpression.create(
+ String.format(
+ "(project:%s AND branch:refs/heads/foo) OR message:\"Fix a bug\"", project.get()));
+
+ SubmitRequirementExpressionResult result = evaluator.evaluateExpression(expression, changeData);
+
+ assertThat(result.status()).isEqualTo(Status.PASS);
+
+ assertThat(result.getPassingAtoms())
+ .containsExactly(
+ PredicateResult.builder()
+ .predicateString(String.format("project:%s", project.get()))
+ .status(true)
+ .build(),
+ PredicateResult.builder()
+ .predicateString("message:\"Fix a bug\"")
+ .status(true)
+ .build());
+
+ assertThat(result.getFailingAtoms())
+ .containsExactly(
+ PredicateResult.builder()
+ // TODO(ghareeb): querying "branch:" creates a RefPredicate. Fix names so that they
+ // match
+ .predicateString(String.format("ref:refs/heads/foo"))
+ .status(false)
+ .build());
+ }
+}
