Implement evaluating submit requirements using change data
We evaluate the three expressions: applicability_expression,
blocking_expression and override_expression. We return the result of the
three expressions as well as a combined status: {SATISFIED, UNSATISFIED,
OVERRIDDEN, NOT_APPLICABLE}.
Change-Id: I02ea63980ea189b87c40f12b9a8e5b8dfd743e83
diff --git a/java/com/google/gerrit/entities/SubmitRequirementExpression.java b/java/com/google/gerrit/entities/SubmitRequirementExpression.java
index 0d51efd..c978347 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementExpression.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementExpression.java
@@ -19,12 +19,7 @@
import com.google.gerrit.common.Nullable;
import java.util.Optional;
-/**
- * Describe a applicability, blocking or override expression of a {@link SubmitRequirement}.
- *
- * <p>TODO: Store the tree representation of the parsed expression internally and throw an exception
- * upon creation if the expression syntax is invalid.
- */
+/** Describe a applicability, blocking or override expression of a {@link SubmitRequirement}. */
@AutoValue
public abstract class SubmitRequirementExpression {
diff --git a/java/com/google/gerrit/entities/SubmitRequirementResult.java b/java/com/google/gerrit/entities/SubmitRequirementResult.java
new file mode 100644
index 0000000..b6075ff
--- /dev/null
+++ b/java/com/google/gerrit/entities/SubmitRequirementResult.java
@@ -0,0 +1,129 @@
+// 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.auto.value.extension.memoized.Memoized;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult.Status;
+import java.util.Optional;
+
+/** Result of evaluating a {@link SubmitRequirement} on a given Change. */
+@AutoValue
+public abstract class SubmitRequirementResult {
+ /** Result of evaluating a {@link SubmitRequirement#applicabilityExpression()} on a change. */
+ public abstract Optional<SubmitRequirementExpressionResult> applicabilityExpressionResult();
+
+ /** Result of evaluating a {@link SubmitRequirement#blockingExpression()} ()} on a change. */
+ public abstract SubmitRequirementExpressionResult blockingExpressionResult();
+
+ /** Result of evaluating a {@link SubmitRequirement#overrideExpression()} ()} on a change. */
+ public abstract Optional<SubmitRequirementExpressionResult> overrideExpressionResult();
+
+ @Memoized
+ public Status status() {
+ if (assertError(blockingExpressionResult())
+ || assertError(applicabilityExpressionResult())
+ || assertError(overrideExpressionResult())) {
+ return Status.ERROR;
+ } else if (assertFail(applicabilityExpressionResult())) {
+ return Status.NOT_APPLICABLE;
+ } else if (assertPass(overrideExpressionResult())) {
+ return Status.OVERRIDDEN;
+ } else if (assertPass(blockingExpressionResult())) {
+ return Status.SATISFIED;
+ } else {
+ return Status.UNSATISFIED;
+ }
+ }
+
+ public static Builder builder() {
+ return new AutoValue_SubmitRequirementResult.Builder();
+ }
+
+ public enum Status {
+ /** Submit requirement is fulfilled. */
+ SATISFIED,
+
+ /**
+ * Submit requirement is not satisfied. Happens when {@link
+ * SubmitRequirement#blockingExpression()} evaluates to false.
+ */
+ UNSATISFIED,
+
+ /**
+ * Submit requirement is overridden. Happens when {@link SubmitRequirement#overrideExpression()}
+ * evaluates to true.
+ */
+ OVERRIDDEN,
+
+ /**
+ * Submit requirement is not applicable for a given change. Happens when {@link
+ * SubmitRequirement#applicabilityExpression()} evaluates to false.
+ */
+ NOT_APPLICABLE,
+
+ /**
+ * Any of the applicability, blocking or override expressions contain invalid syntax and are not
+ * parsable.
+ */
+ ERROR
+ }
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+
+ public abstract Builder applicabilityExpressionResult(
+ Optional<SubmitRequirementExpressionResult> value);
+
+ public abstract Builder blockingExpressionResult(SubmitRequirementExpressionResult value);
+
+ public abstract Builder overrideExpressionResult(
+ Optional<SubmitRequirementExpressionResult> value);
+
+ public abstract SubmitRequirementResult build();
+ }
+
+ private boolean assertPass(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.PASS);
+ }
+
+ private boolean assertPass(SubmitRequirementExpressionResult expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.PASS);
+ }
+
+ private boolean assertFail(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.FAIL);
+ }
+
+ private boolean assertError(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.ERROR);
+ }
+
+ private boolean assertError(SubmitRequirementExpressionResult expressionResult) {
+ return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.ERROR);
+ }
+
+ private boolean assertStatus(
+ SubmitRequirementExpressionResult expressionResult,
+ SubmitRequirementExpressionResult.Status status) {
+ return expressionResult.status() == status;
+ }
+
+ private boolean assertStatus(
+ Optional<SubmitRequirementExpressionResult> expressionResult,
+ SubmitRequirementExpressionResult.Status status) {
+ return expressionResult.isPresent() && assertStatus(expressionResult.get(), status);
+ }
+}
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
index 7fc1247..9e6b151 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluator.java
@@ -14,9 +14,11 @@
package com.google.gerrit.server.project;
+import com.google.gerrit.entities.SubmitRequirement;
import com.google.gerrit.entities.SubmitRequirementExpression;
import com.google.gerrit.entities.SubmitRequirementExpressionResult;
import com.google.gerrit.entities.SubmitRequirementExpressionResult.PredicateResult;
+import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.index.query.Predicate;
import com.google.gerrit.index.query.QueryParseException;
import com.google.gerrit.server.query.change.ChangeData;
@@ -24,6 +26,7 @@
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
+import java.util.Optional;
/** Evaluates submit requirements for different change data. */
@Singleton
@@ -47,6 +50,28 @@
changeQueryBuilderProvider.get().parse(expression.expressionString());
}
+ /** Evaluate a {@link SubmitRequirement} on a given {@link ChangeData}. */
+ public SubmitRequirementResult evaluate(SubmitRequirement sr, ChangeData cd) {
+ SubmitRequirementExpressionResult blockingResult =
+ evaluateExpression(sr.blockingExpression(), cd);
+
+ Optional<SubmitRequirementExpressionResult> applicabilityResult =
+ sr.applicabilityExpression().isPresent()
+ ? Optional.of(evaluateExpression(sr.applicabilityExpression().get(), cd))
+ : Optional.empty();
+
+ Optional<SubmitRequirementExpressionResult> overrideResult =
+ sr.overrideExpression().isPresent()
+ ? Optional.of(evaluateExpression(sr.overrideExpression().get(), cd))
+ : Optional.empty();
+
+ return SubmitRequirementResult.builder()
+ .blockingExpressionResult(blockingResult)
+ .applicabilityExpressionResult(applicabilityResult)
+ .overrideExpressionResult(overrideResult)
+ .build();
+ }
+
/** Evaluate a {@link SubmitRequirementExpression} using change data. */
public SubmitRequirementExpressionResult evaluateExpression(
SubmitRequirementExpression expression, ChangeData changeData) {
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
index 19d0c97..2028ae8 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -15,17 +15,30 @@
package com.google.gerrit.acceptance.server.project;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.acceptance.testsuite.project.TestProjectUpdate.allowLabel;
+import static com.google.gerrit.server.group.SystemGroupBackend.REGISTERED_USERS;
+import static com.google.gerrit.server.project.testing.TestLabels.value;
+import com.google.common.collect.MoreCollectors;
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.NoHttpd;
import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.entities.LabelFunction;
+import com.google.gerrit.entities.SubmitRequirement;
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.entities.SubmitRequirementResult;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.server.project.SubmitRequirementsEvaluator;
import com.google.gerrit.server.query.change.ChangeData;
+import com.google.gerrit.server.query.change.InternalChangeQuery;
import com.google.inject.Inject;
+import com.google.inject.Provider;
import java.util.Optional;
import org.junit.Before;
import org.junit.Test;
@@ -33,14 +46,18 @@
@NoHttpd
public class SubmitRequirementsEvaluatorIT extends AbstractDaemonTest {
@Inject SubmitRequirementsEvaluator evaluator;
+ @Inject private ProjectOperations projectOperations;
+ @Inject private Provider<InternalChangeQuery> changeQueryProvider;
private ChangeData changeData;
+ private String changeId;
@Before
public void setUp() throws Exception {
PushOneCommit.Result pushResult =
createChange(testRepo, "refs/heads/master", "Fix a bug", "file.txt", "content", "topic");
changeData = pushResult.getChange();
+ changeId = pushResult.getChangeId();
}
@Test
@@ -105,4 +122,140 @@
.status(false)
.build());
}
+
+ @Test
+ public void submitRequirementIsNotApplicable_whenApplicabilityExpressionIsFalse()
+ throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:non-existent-project",
+ /* blockingExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.NOT_APPLICABLE);
+ }
+
+ @Test
+ public void submitRequirementIsSatisfied_whenBlockingExpressionIsTrue() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* blockingExpr= */ "message:\"Fix a bug\"",
+ /* overrideExpr= */ "");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
+ }
+
+ @Test
+ public void submitRequirementIsUnsatisfied_whenBlockingExpressionIsFalse() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* blockingExpr= */ "label:\"code-review=+2\"",
+ /* overrideExpr= */ "");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.UNSATISFIED);
+ }
+
+ @Test
+ public void submitRequirementIsOverridden_whenOverrideExpressionIsTrue() throws Exception {
+ addLabel("build-cop-override");
+ voteLabel(changeId, "build-cop-override", 1);
+
+ // Reload change data after applying the vote
+ changeData =
+ changeQueryProvider.get().byLegacyChangeId(changeData.getId()).stream()
+ .collect(MoreCollectors.onlyElement());
+
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* blockingExpr= */ "label:\"code-review=+2\"",
+ /* overrideExpr= */ "label:\"build-cop-override=+1\"");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.OVERRIDDEN);
+ }
+
+ @Test
+ public void submitRequirementIsError_whenApplicabilityExpressionHasInvalidSyntax()
+ throws Exception {
+ addLabel("build-cop-override");
+
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "invalid_field:invalid_value",
+ /* blockingExpr= */ "label:\"code-review=+2\"",
+ /* overrideExpr= */ "label:\"build-cop-override=+1\"");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.ERROR);
+ assertThat(result.applicabilityExpressionResult().get().errorMessage().get())
+ .isEqualTo("Unsupported operator invalid_field:invalid_value");
+ }
+
+ @Test
+ public void submitRequirementIsError_whenBlockingExpressionHasInvalidSyntax() throws Exception {
+ addLabel("build-cop-override");
+
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* blockingExpr= */ "invalid_field:invalid_value",
+ /* overrideExpr= */ "label:\"build-cop-override=+1\"");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.ERROR);
+ assertThat(result.blockingExpressionResult().errorMessage().get())
+ .isEqualTo("Unsupported operator invalid_field:invalid_value");
+ }
+
+ @Test
+ public void submitRequirementIsError_whenOverrideExpressionHasInvalidSyntax() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* blockingExpr= */ "label:\"code-review=+2\"",
+ /* overrideExpr= */ "invalid_field:invalid_value");
+
+ SubmitRequirementResult result = evaluator.evaluate(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.ERROR);
+ assertThat(result.overrideExpressionResult().get().errorMessage().get())
+ .isEqualTo("Unsupported operator invalid_field:invalid_value");
+ }
+
+ private void voteLabel(String changeId, String labelName, int score) throws RestApiException {
+ gApi.changes().id(changeId).current().review(new ReviewInput().label(labelName, score));
+ }
+
+ private void addLabel(String labelName) throws Exception {
+ configLabel(
+ project,
+ labelName,
+ LabelFunction.NO_OP,
+ value(1, "ok"),
+ value(0, "No score"),
+ value(-1, "Needs work"));
+
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel(labelName).ref("refs/heads/master").group(REGISTERED_USERS).range(-1, +1))
+ .update();
+ }
+
+ private SubmitRequirement createSubmitRequirement(
+ @Nullable String applicabilityExpr, String blockingExpr, @Nullable String overrideExpr) {
+ return SubmitRequirement.builder()
+ .setName("sr-name")
+ .setDescription(Optional.of("sr-description"))
+ .setApplicabilityExpression(SubmitRequirementExpression.of(applicabilityExpr))
+ .setBlockingExpression(SubmitRequirementExpression.create(blockingExpr))
+ .setOverrideExpression(SubmitRequirementExpression.of(overrideExpr))
+ .setAllowOverrideInChildProjects(false)
+ .build();
+ }
}
