Merge "Surface the submit/override expressions even if change is NOT_APPLICABLE"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 8f766ea..0ce262f 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -8242,6 +8242,13 @@
`branch:refs/heads/foo and label:verified=+1`.
|`fulfilled`||
True if the submit requirement is fulfilled for the change.
+|`status`||
+A string containing the status of evaluating the expression which can be one
+of the following: +
+ * `PASS` - expression was evaluated and result is true. +
+ * `FAIL` - expression was evaluated and result is false. +
+ * `ERROR` - an error occurred while evaluating the expression. +
+ * `NOT_EVALUATED` - expression was not evaluated.
|`passing_atoms`|optional|
A list of passing atoms as strings. For the above expression,
`passing_atoms` can contain ["branch:refs/heads/foo"] if the branch predicate is
@@ -8306,16 +8313,17 @@
submit requirement did not define an applicability expression.
Note that fields `expression`, `passing_atoms` and `failing_atoms` are always
omitted for the `applicability_expression_result`.
-|`submittability_expression_result`|optional|
+|`submittability_expression_result`||
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
containing the result of evaluating the submittability expression. +
-If the submit requirement does not apply, the expression is not evaluated and
-the field is not set.
+If the submit requirement does not apply, the `status` field of the result
+will be set to `NOT_EVALUATED`.
|`override_expression_result`|optional|
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
containing the result of evaluating the override expression. +
-Not set if the submit requirement did not define an override expression or
-if it does not apply.
+Not set if the submit requirement did not define an override expression. If the
+submit requirement does not apply, the `status` field of the result will be set
+to `NOT_EVALUATED`.
|===========================
[[submitted-together-info]]
diff --git a/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java b/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
index fcf32eb..c24227d 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementExpressionResult.java
@@ -95,6 +95,11 @@
ImmutableList.of());
}
+ public static SubmitRequirementExpressionResult notEvaluated(SubmitRequirementExpression expr) {
+ return SubmitRequirementExpressionResult.create(
+ expr, Status.NOT_EVALUATED, ImmutableList.of(), ImmutableList.of());
+ }
+
public static TypeAdapter<SubmitRequirementExpressionResult> typeAdapter(Gson gson) {
return new AutoValue_SubmitRequirementExpressionResult.GsonTypeAdapter(gson);
}
@@ -124,7 +129,10 @@
FAIL,
/** Submit requirement expression contains invalid syntax and is not parsable. */
- ERROR
+ ERROR,
+
+ /** Submit requirement expression was not evaluated. */
+ NOT_EVALUATED
}
/**
diff --git a/java/com/google/gerrit/extensions/common/SubmitRequirementExpressionInfo.java b/java/com/google/gerrit/extensions/common/SubmitRequirementExpressionInfo.java
index e9549c9..038b6f8 100644
--- a/java/com/google/gerrit/extensions/common/SubmitRequirementExpressionInfo.java
+++ b/java/com/google/gerrit/extensions/common/SubmitRequirementExpressionInfo.java
@@ -16,7 +16,10 @@
import java.util.List;
-/** Result of evaluating a single submit requirement expression. */
+/**
+ * Result of evaluating a single submit requirement expression. This API entity is populated from
+ * {@link com.google.gerrit.entities.SubmitRequirementExpressionResult}.
+ */
public class SubmitRequirementExpressionInfo {
/** Submit requirement expression as a String. */
@@ -25,6 +28,9 @@
/** A boolean indicating if the expression is fulfilled on a change. */
public boolean fulfilled;
+ /** A status indicating if the expression is fulfilled, non-fulfilled or not evaluated. */
+ public Status status;
+
/**
* A list of all atoms that are passing, for example query "branch:refs/heads/foo and project:bar"
* has two atoms: ["branch:refs/heads/foo", "project:bar"].
@@ -42,4 +48,22 @@
* during its evaluation.
*/
public String errorMessage;
+
+ /**
+ * Values in this enum should match with values in {@link
+ * com.google.gerrit.entities.SubmitRequirementExpressionResult.Status}.
+ */
+ public enum Status {
+ /** Expression was evaluated and the result was true. */
+ PASS,
+
+ /** Expression was evaluated and the result was false. */
+ FAIL,
+
+ /** An error occurred while evaluating the expression. */
+ ERROR,
+
+ /** Expression was not evaluated. */
+ NOT_EVALUATED
+ }
}
diff --git a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
index 96c863e..fcd9e90 100644
--- a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
+++ b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
@@ -65,7 +65,10 @@
boolean hide) {
SubmitRequirementExpressionInfo info = new SubmitRequirementExpressionInfo();
info.expression = hide ? null : expression.expressionString();
- info.fulfilled = result.status().equals(SubmitRequirementExpressionResult.Status.PASS);
+ info.fulfilled =
+ result.status().equals(SubmitRequirementExpressionResult.Status.PASS)
+ || result.status().equals(SubmitRequirementExpressionResult.Status.NOT_EVALUATED);
+ info.status = SubmitRequirementExpressionInfo.Status.valueOf(result.status().name());
info.passingAtoms = hide ? null : result.passingAtoms();
info.failingAtoms = hide ? null : result.failingAtoms();
info.errorMessage = result.errorMessage().isPresent() ? result.errorMessage().get() : null;
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index c57fe27..3628d43 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -105,8 +105,14 @@
sr.applicabilityExpression().isPresent()
? Optional.of(evaluateExpression(sr.applicabilityExpression().get(), cd))
: Optional.empty();
- Optional<SubmitRequirementExpressionResult> submittabilityResult = Optional.empty();
- Optional<SubmitRequirementExpressionResult> overrideResult = Optional.empty();
+ Optional<SubmitRequirementExpressionResult> submittabilityResult =
+ Optional.of(
+ SubmitRequirementExpressionResult.notEvaluated(sr.submittabilityExpression()));
+ Optional<SubmitRequirementExpressionResult> overrideResult =
+ sr.overrideExpression().isPresent()
+ ? Optional.of(
+ SubmitRequirementExpressionResult.notEvaluated(sr.overrideExpression().get()))
+ : Optional.empty();
if (!sr.applicabilityExpression().isPresent()
|| SubmitRequirementResult.assertPass(applicabilityResult)) {
submittabilityResult = Optional.of(evaluateExpression(sr.submittabilityExpression(), cd));
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index 61e55ff..c8e44e7 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -1895,6 +1895,7 @@
/* expression= */ null,
/* passingAtoms= */ null,
/* failingAtoms= */ null,
+ /* status= */ SubmitRequirementExpressionInfo.Status.PASS,
/* fulfilled= */ true);
assertThat(requirement.submittabilityExpressionResult).isNotNull();
}
@@ -1928,9 +1929,16 @@
/* expression= */ null,
/* passingAtoms= */ null,
/* failingAtoms= */ null,
+ /* status= */ SubmitRequirementExpressionInfo.Status.FAIL,
/* fulfilled= */ false);
- assertThat(requirement.submittabilityExpressionResult).isNull();
- assertThat(requirement.overrideExpressionResult).isNull();
+ assertThat(requirement.submittabilityExpressionResult.status)
+ .isEqualTo(SubmitRequirementExpressionInfo.Status.NOT_EVALUATED);
+ assertThat(requirement.submittabilityExpressionResult.expression)
+ .isEqualTo(SubmitRequirementExpression.maxCodeReview().expressionString());
+ assertThat(requirement.overrideExpressionResult.status)
+ .isEqualTo(SubmitRequirementExpressionInfo.Status.NOT_EVALUATED);
+ assertThat(requirement.overrideExpressionResult.expression)
+ .isEqualTo("project:" + project.get());
}
@Test
@@ -1963,12 +1971,14 @@
/* passingAtoms= */ ImmutableList.of(
SubmitRequirementExpression.maxCodeReview().expressionString()),
/* failingAtoms= */ ImmutableList.of(),
+ /* status= */ SubmitRequirementExpressionInfo.Status.PASS,
/* fulfilled= */ true);
assertSubmitRequirementExpression(
requirement.overrideExpressionResult,
/* expression= */ "project:non-existent",
/* passingAtoms= */ ImmutableList.of(),
/* failingAtoms= */ ImmutableList.of("project:non-existent"),
+ /* status= */ SubmitRequirementExpressionInfo.Status.FAIL,
/* fulfilled= */ false);
}
@@ -2003,12 +2013,14 @@
/* passingAtoms= */ ImmutableList.of(),
/* failingAtoms= */ ImmutableList.of(
SubmitRequirementExpression.maxCodeReview().expressionString()),
+ /* status= */ SubmitRequirementExpressionInfo.Status.FAIL,
/* fulfilled= */ false);
assertSubmitRequirementExpression(
requirement.overrideExpressionResult,
/* expression= */ "project:" + project.get(),
/* passingAtoms= */ ImmutableList.of("project:" + project.get()),
/* failingAtoms= */ ImmutableList.of(),
+ /* status= */ SubmitRequirementExpressionInfo.Status.PASS,
/* fulfilled= */ true);
}
@@ -2810,6 +2822,7 @@
@Nullable String expression,
@Nullable List<String> passingAtoms,
@Nullable List<String> failingAtoms,
+ SubmitRequirementExpressionInfo.Status status,
boolean fulfilled) {
assertThat(result.expression).isEqualTo(expression);
if (passingAtoms == null) {
@@ -2822,6 +2835,7 @@
} else {
assertThat(result.failingAtoms).containsExactlyElementsIn(failingAtoms);
}
+ assertThat(result.status).isEqualTo(status);
assertThat(result.fulfilled).isEqualTo(fulfilled);
}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
index 731e0df..8541fc0 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -283,7 +283,10 @@
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.NOT_APPLICABLE);
assertThat(result.applicabilityExpressionResult().get().status()).isEqualTo(Status.FAIL);
- assertThat(result.submittabilityExpressionResult().isPresent()).isFalse();
+ assertThat(result.submittabilityExpressionResult().get().status())
+ .isEqualTo(Status.NOT_EVALUATED);
+ assertThat(result.submittabilityExpressionResult().get().expression().expressionString())
+ .isEqualTo("message:\"Fix bug\"");
assertThat(result.overrideExpressionResult().isPresent()).isFalse();
}
