Merge "Submit requirements: always hide applicability expressions on the API"
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 0a203cc..33e7804 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -8263,19 +8263,19 @@
The `SubmitRequirementExpressionInfo` describes the result of evaluating a
single submit requirement expression, for example `label:code-review=+2`.
-[options="header",cols="1,6"]
+[options="header",cols="1,^1,5"]
|===========================
-|Field Name |Description
-|`expression`|
+|Field Name ||Description
+|`expression`|optional|
The submit requirement expression as a string, for example
`branch:refs/heads/foo and label:verified=+1`.
-|`fulfilled`|
+|`fulfilled`||
True if the submit requirement is fulfilled for the change.
-|`passing_atoms`|
+|`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
fulfilled for the change.
-|`failing_atoms`|
+|`failing_atoms`|optional|
A list of failing atoms. This is similar to `passing_atoms` except that it
contains the list of predicates that are not fulfilled for the change.
|===========================
@@ -8329,6 +8329,8 @@
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
containing the result of evaluating the applicability expression. Not set if the
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`||
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
containing the result of evaluating the submittability expression.
diff --git a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
index 8eeec62..4833197 100644
--- a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
+++ b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
@@ -36,28 +36,36 @@
if (req.applicabilityExpression().isPresent()) {
info.applicabilityExpressionResult =
submitRequirementExpressionToInfo(
- req.applicabilityExpression().get(), result.applicabilityExpressionResult().get());
+ req.applicabilityExpression().get(),
+ result.applicabilityExpressionResult().get(),
+ /* hide= */ true); // Always hide applicability expressions on the API
}
if (req.overrideExpression().isPresent()) {
info.overrideExpressionResult =
submitRequirementExpressionToInfo(
- req.overrideExpression().get(), result.overrideExpressionResult().get());
+ req.overrideExpression().get(),
+ result.overrideExpressionResult().get(),
+ /* hide= */ false);
}
info.submittabilityExpressionResult =
submitRequirementExpressionToInfo(
- req.submittabilityExpression(), result.submittabilityExpressionResult());
+ req.submittabilityExpression(),
+ result.submittabilityExpressionResult(),
+ /* hide= */ false);
info.status = SubmitRequirementResultInfo.Status.valueOf(result.status().toString());
info.isLegacy = result.isLegacy();
return info;
}
private static SubmitRequirementExpressionInfo submitRequirementExpressionToInfo(
- SubmitRequirementExpression expression, SubmitRequirementExpressionResult result) {
+ SubmitRequirementExpression expression,
+ SubmitRequirementExpressionResult result,
+ boolean hide) {
SubmitRequirementExpressionInfo info = new SubmitRequirementExpressionInfo();
- info.expression = expression.expressionString();
+ info.expression = hide ? null : expression.expressionString();
info.fulfilled = result.status().equals(SubmitRequirementExpressionResult.Status.PASS);
- info.passingAtoms = result.passingAtoms();
- info.failingAtoms = result.failingAtoms();
+ info.passingAtoms = hide ? null : result.passingAtoms();
+ info.failingAtoms = hide ? null : result.failingAtoms();
return info;
}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 2e1bd54..58c222a 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -89,6 +89,7 @@
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.request.RequestScopeOperations;
import com.google.gerrit.common.FooterConstants;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.RawInputUtil;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.entities.Account;
@@ -153,6 +154,7 @@
import com.google.gerrit.extensions.common.LegacySubmitRequirementInfo;
import com.google.gerrit.extensions.common.RevisionInfo;
import com.google.gerrit.extensions.common.SubmitRecordInfo;
+import com.google.gerrit.extensions.common.SubmitRequirementExpressionInfo;
import com.google.gerrit.extensions.common.SubmitRequirementInput;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo;
import com.google.gerrit.extensions.common.SubmitRequirementResultInfo.Status;
@@ -5059,6 +5061,35 @@
}
@Test
+ @GerritConfig(
+ name = "experiments.enabled",
+ value = ExperimentFeaturesConstants.GERRIT_BACKEND_REQUEST_FEATURE_ENABLE_SUBMIT_REQUIREMENTS)
+ public void submitRequirement_applicabilityExpressionIsAlwaysHidden() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setApplicabilityExpression(SubmitRequirementExpression.of("branch:refs/heads/master"))
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:Code-Review=+2"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 2);
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ SubmitRequirementResultInfo requirement =
+ changeInfo.submitRequirements.stream().collect(MoreCollectors.onlyElement());
+ assertSubmitRequirementExpression(
+ requirement.applicabilityExpressionResult,
+ /* expression= */ null,
+ /* passingAtoms= */ null,
+ /* failingAtoms= */ null,
+ /* fulfilled= */ true);
+ }
+
+ @Test
public void submitRequirements_notServedIfExperimentNotEnabled() throws Exception {
configSubmitRequirement(
project,
@@ -5675,6 +5706,26 @@
.collect(toImmutableList())));
}
+ private void assertSubmitRequirementExpression(
+ SubmitRequirementExpressionInfo result,
+ @Nullable String expression,
+ @Nullable List<String> passingAtoms,
+ @Nullable List<String> failingAtoms,
+ boolean fulfilled) {
+ assertThat(result.expression).isEqualTo(expression);
+ if (passingAtoms == null) {
+ assertThat(result.passingAtoms).isNull();
+ } else {
+ assertThat(result.passingAtoms).containsExactlyElementsIn(passingAtoms);
+ }
+ if (failingAtoms == null) {
+ assertThat(result.failingAtoms).isNull();
+ } else {
+ assertThat(result.failingAtoms).containsExactlyElementsIn(failingAtoms);
+ }
+ assertThat(result.fulfilled).isEqualTo(fulfilled);
+ }
+
private Project.NameKey createProjectForPush(SubmitType submitType) throws Exception {
Project.NameKey project = projectOperations.newProject().submitType(submitType).create();
projectOperations
