Do not evaluate submittability and override expression if SubmitRequirement is non-applicable
This is a performance optimization for expensive SubmitRequirements:
since for non-applicable SubmitRequirement the
submittabilityExpressionResult and overridesExpressionResult are not
used, it is not needed to evaluate potentially expensive
SubmitRequirementExpression.
This change also makes submittabilityExpressionResult Optional field in
SubmitRequirementResult, changing the storage format in NoteDb.
The previous change made serialization/deserialization both backward
and forward compatible, so that updated binaries could read old format
and the non-updated binaries could read new data.
Google-Bug-Id: b/216444397
Change-Id: Iede127dc7bf6daa75ca80f1eda23229ca42b2a12
diff --git a/Documentation/rest-api-changes.txt b/Documentation/rest-api-changes.txt
index 82a9638..fb81b71 100644
--- a/Documentation/rest-api-changes.txt
+++ b/Documentation/rest-api-changes.txt
@@ -8286,13 +8286,16 @@
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`||
+|`submittability_expression_result`|optional|
A link:#submit-requirement-expression-info[SubmitRequirementExpressionInfo]
-containing the result of evaluating the submittability expression.
+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.
|`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.
+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.
|===========================
[[submitted-together-info]]
diff --git a/java/com/google/gerrit/entities/SubmitRequirement.java b/java/com/google/gerrit/entities/SubmitRequirement.java
index 3f91cc7..673665d 100644
--- a/java/com/google/gerrit/entities/SubmitRequirement.java
+++ b/java/com/google/gerrit/entities/SubmitRequirement.java
@@ -44,7 +44,7 @@
* Expression of the condition that makes the requirement applicable. The expression should be
* evaluated for a specific {@link Change} and if it returns false, the requirement becomes
* irrelevant for the change (i.e. {@link #submittabilityExpression()} and {@link
- * #overrideExpression()} become irrelevant).
+ * #overrideExpression()} are not evaluated).
*
* <p>An empty {@link Optional} indicates that the requirement is applicable for any change.
*/
diff --git a/java/com/google/gerrit/entities/SubmitRequirementResult.java b/java/com/google/gerrit/entities/SubmitRequirementResult.java
index a97560b..18c27a1 100644
--- a/java/com/google/gerrit/entities/SubmitRequirementResult.java
+++ b/java/com/google/gerrit/entities/SubmitRequirementResult.java
@@ -32,12 +32,18 @@
public abstract Optional<SubmitRequirementExpressionResult> applicabilityExpressionResult();
/**
- * Result of evaluating a {@link SubmitRequirement#submittabilityExpression()} ()} on a change.
+ * Result of evaluating a {@link SubmitRequirement#submittabilityExpression()} on a change.
+ *
+ * <p>Empty if submit requirement does not apply.
*/
- @Nullable
- public abstract SubmitRequirementExpressionResult submittabilityExpressionResult();
+ public abstract Optional<SubmitRequirementExpressionResult> submittabilityExpressionResult();
- /** Result of evaluating a {@link SubmitRequirement#overrideExpression()} ()} on a change. */
+ /**
+ * Result of evaluating a {@link SubmitRequirement#overrideExpression()} on a change.
+ *
+ * <p>Empty if submit requirement does not apply, or if the submit requirement did not define an
+ * override expression.
+ */
public abstract Optional<SubmitRequirementExpressionResult> overrideExpressionResult();
/** SHA-1 of the patchset commit ID for which the submit requirement was evaluated. */
@@ -71,11 +77,11 @@
"Applicability expression result has an error: "
+ applicabilityExpressionResult().get().errorMessage().get());
}
- if (submittabilityExpressionResult() != null
- && submittabilityExpressionResult().errorMessage().isPresent()) {
+ if (submittabilityExpressionResult().isPresent()
+ && submittabilityExpressionResult().get().errorMessage().isPresent()) {
return Optional.of(
"Submittability expression result has an error: "
- + submittabilityExpressionResult().errorMessage().get());
+ + submittabilityExpressionResult().get().errorMessage().get());
}
if (overrideExpressionResult().isPresent()
&& overrideExpressionResult().get().errorMessage().isPresent()) {
@@ -168,6 +174,9 @@
Optional<SubmitRequirementExpressionResult> value);
public abstract Builder submittabilityExpressionResult(
+ Optional<SubmitRequirementExpressionResult> value);
+
+ public abstract Builder submittabilityExpressionResult(
@Nullable SubmitRequirementExpressionResult value);
public abstract Builder overrideExpressionResult(
@@ -182,33 +191,25 @@
public abstract SubmitRequirementResult build();
}
- private boolean assertPass(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ public static boolean assertPass(Optional<SubmitRequirementExpressionResult> expressionResult) {
return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.PASS);
}
- private boolean assertPass(@Nullable SubmitRequirementExpressionResult expressionResult) {
- return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.PASS);
- }
-
- private boolean assertFail(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ public static boolean assertFail(Optional<SubmitRequirementExpressionResult> expressionResult) {
return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.FAIL);
}
- private boolean assertError(Optional<SubmitRequirementExpressionResult> expressionResult) {
+ public static boolean assertError(Optional<SubmitRequirementExpressionResult> expressionResult) {
return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.ERROR);
}
- private boolean assertError(@Nullable SubmitRequirementExpressionResult expressionResult) {
- return assertStatus(expressionResult, SubmitRequirementExpressionResult.Status.ERROR);
- }
-
- private boolean assertStatus(
- @Nullable SubmitRequirementExpressionResult expressionResult,
+ private static boolean assertStatus(
+ SubmitRequirementExpressionResult expressionResult,
SubmitRequirementExpressionResult.Status status) {
- return expressionResult != null && expressionResult.status() == status;
+ return expressionResult.status() == status;
}
- private boolean assertStatus(
+ private static boolean assertStatus(
Optional<SubmitRequirementExpressionResult> expressionResult,
SubmitRequirementExpressionResult.Status status) {
return expressionResult.isPresent() && assertStatus(expressionResult.get(), status);
diff --git a/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java b/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
index 7b87be8..4778038 100644
--- a/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
+++ b/java/com/google/gerrit/extensions/common/SubmitRequirementResultInfo.java
@@ -14,6 +14,8 @@
package com.google.gerrit.extensions.common;
+import com.google.gerrit.common.Nullable;
+
/** Result of evaluating a submit requirement on a change. */
public class SubmitRequirementResultInfo {
public enum Status {
@@ -63,11 +65,11 @@
public boolean isLegacy;
/** Result of evaluating the applicability expression. */
- public SubmitRequirementExpressionInfo applicabilityExpressionResult;
+ @Nullable public SubmitRequirementExpressionInfo applicabilityExpressionResult;
/** Result of evaluating the submittability expression. */
- public SubmitRequirementExpressionInfo submittabilityExpressionResult;
+ @Nullable public SubmitRequirementExpressionInfo submittabilityExpressionResult;
/** Result of evaluating the override expression. */
- public SubmitRequirementExpressionInfo overrideExpressionResult;
+ @Nullable public SubmitRequirementExpressionInfo overrideExpressionResult;
}
diff --git a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
index 7793b76..96c863e 100644
--- a/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
+++ b/java/com/google/gerrit/server/change/SubmitRequirementsJson.java
@@ -47,11 +47,11 @@
result.overrideExpressionResult().get(),
/* hide= */ false);
}
- if (result.submittabilityExpressionResult() != null) {
+ if (result.submittabilityExpressionResult().isPresent()) {
info.submittabilityExpressionResult =
submitRequirementExpressionToInfo(
req.submittabilityExpression(),
- result.submittabilityExpressionResult(),
+ result.submittabilityExpressionResult().get(),
/* hide= */ false);
}
info.status = SubmitRequirementResultInfo.Status.valueOf(result.status().toString());
diff --git a/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
index e655d92..a815f57 100644
--- a/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
+++ b/java/com/google/gerrit/server/notedb/SubmitRequirementProtoConverter.java
@@ -58,10 +58,10 @@
SubmitRequirementExpressionResultSerializer.serialize(
r.applicabilityExpressionResult().get()));
}
- if (r.submittabilityExpressionResult() != null) {
+ if (r.submittabilityExpressionResult().isPresent()) {
builder.setSubmittabilityExpressionResult(
SubmitRequirementExpressionResultSerializer.serialize(
- r.submittabilityExpressionResult()));
+ r.submittabilityExpressionResult().get()));
}
if (r.overrideExpressionResult().isPresent()) {
builder.setOverrideExpressionResult(
diff --git a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
index 905719c..9b0f796 100644
--- a/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
+++ b/java/com/google/gerrit/server/project/SubmitRequirementsEvaluatorImpl.java
@@ -100,24 +100,27 @@
// Use a request context to execute predicates as an internal user with expanded visibility.
// This is so that the evaluation does not depend on who is running the current request (e.g.
// a "ownerin" predicate with group that is not visible to the person making this request).
- SubmitRequirementExpressionResult blockingResult =
- evaluateExpression(sr.submittabilityExpression(), 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();
+ Optional<SubmitRequirementExpressionResult> submittabilityResult = Optional.empty();
+ Optional<SubmitRequirementExpressionResult> overrideResult = Optional.empty();
+ if (!sr.applicabilityExpression().isPresent()
+ || SubmitRequirementResult.assertPass(applicabilityResult)) {
+ submittabilityResult = Optional.of(evaluateExpression(sr.submittabilityExpression(), cd));
+ overrideResult =
+ sr.overrideExpression().isPresent()
+ ? Optional.of(evaluateExpression(sr.overrideExpression().get(), cd))
+ : Optional.empty();
+ }
return SubmitRequirementResult.builder()
.legacy(Optional.of(false))
.submitRequirement(sr)
.patchSetCommitId(cd.currentPatchSet().commitId())
- .submittabilityExpressionResult(blockingResult)
+ .submittabilityExpressionResult(submittabilityResult)
.applicabilityExpressionResult(applicabilityResult)
.overrideExpressionResult(overrideResult)
.build();
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index cb1b7b1..ff76546 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -22,6 +22,7 @@
import static com.google.gerrit.server.project.testing.TestLabels.label;
import static com.google.gerrit.server.project.testing.TestLabels.value;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.Iterables;
@@ -1081,9 +1082,9 @@
SubmitRequirementResult result =
notes.getSubmitRequirementsResult().stream().collect(MoreCollectors.onlyElement());
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
- assertThat(result.submittabilityExpressionResult().status())
+ assertThat(result.submittabilityExpressionResult().get().status())
.isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- assertThat(result.submittabilityExpressionResult().expression().expressionString())
+ assertThat(result.submittabilityExpressionResult().get().expression().expressionString())
.isEqualTo("label:Code-Review=MAX");
}
}
@@ -1121,9 +1122,9 @@
SubmitRequirementResult result =
notes.getSubmitRequirementsResult().stream().collect(MoreCollectors.onlyElement());
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
- assertThat(result.submittabilityExpressionResult().status())
+ assertThat(result.submittabilityExpressionResult().get().status())
.isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- assertThat(result.submittabilityExpressionResult().expression().expressionString())
+ assertThat(result.submittabilityExpressionResult().get().expression().expressionString())
.isEqualTo("label:Code-Review=MAX");
}
}
@@ -1815,6 +1816,120 @@
/* passingAtoms= */ null,
/* failingAtoms= */ null,
/* fulfilled= */ true);
+ assertThat(requirement.submittabilityExpressionResult).isNotNull();
+ }
+
+ @Test
+ public void submitRequirement_nonApplicable_submittabilityAndOverrideNotEvaluated()
+ throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setApplicabilityExpression(
+ SubmitRequirementExpression.of("branch:refs/heads/non-existent"))
+ .setSubmittabilityExpression(SubmitRequirementExpression.maxCodeReview())
+ .setOverrideExpression(SubmitRequirementExpression.of("project:" + project.get()))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 2);
+
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements, "Code-Review", Status.NOT_APPLICABLE, /* isLegacy= */ false);
+ SubmitRequirementResultInfo requirement =
+ changeInfo.submitRequirements.stream().collect(MoreCollectors.onlyElement());
+ assertSubmitRequirementExpression(
+ requirement.applicabilityExpressionResult,
+ /* expression= */ null,
+ /* passingAtoms= */ null,
+ /* failingAtoms= */ null,
+ /* fulfilled= */ false);
+ assertThat(requirement.submittabilityExpressionResult).isNull();
+ assertThat(requirement.overrideExpressionResult).isNull();
+ }
+
+ @Test
+ public void submitRequirement_emptyApplicable_submittabilityAndOverrideEvaluated()
+ throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setApplicabilityExpression(Optional.empty())
+ .setSubmittabilityExpression(SubmitRequirementExpression.maxCodeReview())
+ .setOverrideExpression(SubmitRequirementExpression.of("project:non-existent"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 2);
+
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements, "Code-Review", Status.SATISFIED, /* isLegacy= */ false);
+ SubmitRequirementResultInfo requirement =
+ changeInfo.submitRequirements.stream().collect(MoreCollectors.onlyElement());
+ assertThat(requirement.applicabilityExpressionResult).isNull();
+ assertSubmitRequirementExpression(
+ requirement.submittabilityExpressionResult,
+ /* expression= */ SubmitRequirementExpression.maxCodeReview().expressionString(),
+ /* passingAtoms= */ ImmutableList.of(
+ SubmitRequirementExpression.maxCodeReview().expressionString()),
+ /* failingAtoms= */ ImmutableList.of(),
+ /* fulfilled= */ true);
+ assertSubmitRequirementExpression(
+ requirement.overrideExpressionResult,
+ /* expression= */ "project:non-existent",
+ /* passingAtoms= */ ImmutableList.of(),
+ /* failingAtoms= */ ImmutableList.of("project:non-existent"),
+ /* fulfilled= */ false);
+ }
+
+ @Test
+ public void submitRequirement_overriden_submittabilityEvaluated() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("Code-Review")
+ .setApplicabilityExpression(Optional.empty())
+ .setSubmittabilityExpression(SubmitRequirementExpression.maxCodeReview())
+ .setOverrideExpression(SubmitRequirementExpression.of("project:" + project.get()))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ voteLabel(changeId, "Code-Review", 1);
+
+ ChangeInfo changeInfo = gApi.changes().id(changeId).get();
+ assertSubmitRequirementStatus(
+ changeInfo.submitRequirements, "Code-Review", Status.OVERRIDDEN, /* isLegacy= */ false);
+ SubmitRequirementResultInfo requirement =
+ changeInfo.submitRequirements.stream()
+ .filter(sr -> !sr.isLegacy)
+ .collect(MoreCollectors.onlyElement());
+ assertThat(requirement.applicabilityExpressionResult).isNull();
+ assertSubmitRequirementExpression(
+ requirement.submittabilityExpressionResult,
+ /* expression= */ SubmitRequirementExpression.maxCodeReview().expressionString(),
+ /* passingAtoms= */ ImmutableList.of(),
+ /* failingAtoms= */ ImmutableList.of(
+ SubmitRequirementExpression.maxCodeReview().expressionString()),
+ /* fulfilled= */ false);
+ assertSubmitRequirementExpression(
+ requirement.overrideExpressionResult,
+ /* expression= */ "project:" + project.get(),
+ /* passingAtoms= */ ImmutableList.of("project:" + project.get()),
+ /* failingAtoms= */ ImmutableList.of(),
+ /* fulfilled= */ true);
}
@Test
@@ -2132,9 +2247,9 @@
SubmitRequirementResult result =
notes.getSubmitRequirementsResult().stream().collect(MoreCollectors.onlyElement());
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
- assertThat(result.submittabilityExpressionResult().status())
+ assertThat(result.submittabilityExpressionResult().get().status())
.isEqualTo(SubmitRequirementExpressionResult.Status.PASS);
- assertThat(result.submittabilityExpressionResult().expression().expressionString())
+ assertThat(result.submittabilityExpressionResult().get().expression().expressionString())
.isEqualTo("topic:test");
}
}
diff --git a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
index 61e204f..43c7b6b 100644
--- a/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/project/SubmitRequirementsEvaluatorIT.java
@@ -238,6 +238,68 @@
}
@Test
+ public void submittabilityAndOverrideNotEvaluated_whenApplicabilityIsFalse() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:non-existent-project",
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "");
+
+ 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.overrideExpressionResult().isPresent()).isFalse();
+ }
+
+ @Test
+ public void submittabilityAndOverrideEvaluated_whenApplicabilityIsEmpty() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ null,
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "label:\"build-cop-override=-1\"");
+
+ SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
+ assertThat(result.applicabilityExpressionResult().isPresent()).isFalse();
+ assertThat(result.submittabilityExpressionResult().get().status()).isEqualTo(Status.PASS);
+ assertThat(result.overrideExpressionResult().get().status()).isEqualTo(Status.FAIL);
+ }
+
+ @Test
+ public void submittabilityAndOverrideEvaluated_whenApplicabilityIsTrue() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ "project:" + project.get(),
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "label:\"build-cop-override=-1\"");
+
+ SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
+
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.SATISFIED);
+ assertThat(result.applicabilityExpressionResult().get().status()).isEqualTo(Status.PASS);
+ assertThat(result.submittabilityExpressionResult().get().status()).isEqualTo(Status.PASS);
+ assertThat(result.overrideExpressionResult().get().status()).isEqualTo(Status.FAIL);
+ }
+
+ @Test
+ public void submittabilityIsEvaluated_whenOverrideApplies() throws Exception {
+ SubmitRequirement sr =
+ createSubmitRequirement(
+ /* applicabilityExpr= */ null,
+ /* submittabilityExpr= */ "message:\"Fix bug\"",
+ /* overrideExpr= */ "project:" + project.get());
+
+ SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
+ assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.OVERRIDDEN);
+
+ assertThat(result.applicabilityExpressionResult().isPresent()).isFalse();
+ assertThat(result.submittabilityExpressionResult().get().status()).isEqualTo(Status.PASS);
+ assertThat(result.overrideExpressionResult().get().status()).isEqualTo(Status.PASS);
+ }
+
+ @Test
public void submitRequirementIsSatisfied_whenSubmittabilityExpressionIsTrue() throws Exception {
SubmitRequirement sr =
createSubmitRequirement(
@@ -260,7 +322,7 @@
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.UNSATISFIED);
- assertThat(result.submittabilityExpressionResult().failingAtoms())
+ assertThat(result.submittabilityExpressionResult().get().failingAtoms())
.containsExactly("label:\"Code-Review=+2\"");
}
@@ -314,7 +376,7 @@
SubmitRequirementResult result = evaluator.evaluateRequirement(sr, changeData);
assertThat(result.status()).isEqualTo(SubmitRequirementResult.Status.ERROR);
- assertThat(result.submittabilityExpressionResult().errorMessage().get())
+ assertThat(result.submittabilityExpressionResult().get().errorMessage().get())
.isEqualTo("Unsupported operator invalid_field:invalid_value");
}
diff --git a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
index 2418d1c..6abe4d1 100644
--- a/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
+++ b/javatests/com/google/gerrit/server/cache/serialize/entities/SubmitRequirementJsonSerializerTest.java
@@ -118,11 +118,11 @@
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ "\"passingAtoms\":[\"refs/heads/master\"],"
+ "\"failingAtoms\":[]}},"
- + "\"submittabilityExpressionResult\":{"
+ + "\"submittabilityExpressionResult\":{\"value\":{"
+ "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
- + "\"failingAtoms\":[]},"
+ + "\"failingAtoms\":[]}},"
+ "\"overrideExpressionResult\":{\"value\":{"
+ "\"expression\":{\"expressionString\":\"label:Override=+1\"},"
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
@@ -196,44 +196,29 @@
}
@Test
- public void submitRequirementResult_deserialize_nonOptionalSubmittabilityExpressionResultField()
+ public void submitRequirementResult_deserialize_optionalSubmittabilityExpressionResultField()
throws Exception {
assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(srReqResultSerial))
.isEqualTo(srReqResult);
}
@Test
- public void submitRequirementResult_deserialize_optionalSubmittabilityExpressionResultField()
+ public void submitRequirementResult_deserialize_nonOptionalSubmittabilityExpressionResultField()
throws Exception {
- String newFormatSrReqResultSerial =
+ String oldFormatSrReqResultSerial =
srReqResultSerial.replace(
- "\"submittabilityExpressionResult\":{"
- + "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
- + "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
- + "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
- + "\"failingAtoms\":[]},",
"\"submittabilityExpressionResult\":{\"value\":{"
+ "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
- + "\"failingAtoms\":[]}},");
- assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(newFormatSrReqResultSerial))
- .isEqualTo(srReqResult);
- }
-
- @Test
- public void submitRequirementResult_deserialize_emptyOptionalSubmittabilityExpressionResultField()
- throws Exception {
- String newFormatSrReqResultSerial =
- srReqResultSerial.replace(
+ + "\"failingAtoms\":[]}},",
"\"submittabilityExpressionResult\":{"
+ "\"expression\":{\"expressionString\":\"label:\\\"Code-Review=+2\\\"\"},"
+ "\"status\":\"PASS\",\"errorMessage\":{\"value\":null},"
+ "\"passingAtoms\":[\"label:\\\"Code-Review=+2\\\"\"],"
- + "\"failingAtoms\":[]},",
- "\"submittabilityExpressionResult\":{\"value\":null},");
- assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(newFormatSrReqResultSerial))
- .isEqualTo(srReqResult.toBuilder().submittabilityExpressionResult(null).build());
+ + "\"failingAtoms\":[]},");
+ assertThat(SubmitRequirementResult.typeAdapter(gson).fromJson(oldFormatSrReqResultSerial))
+ .isEqualTo(srReqResult);
}
@Test
@@ -243,6 +228,20 @@
}
@Test
+ public void submitRequirementResult_emptySubmittabilityExpressionResultField_roundTrip()
+ throws Exception {
+ SubmitRequirementResult srResult =
+ srReqResult
+ .toBuilder()
+ .submittabilityExpressionResult(Optional.empty())
+ .applicabilityExpressionResult(Optional.empty())
+ .overrideExpressionResult(Optional.empty())
+ .build();
+ TypeAdapter<SubmitRequirementResult> adapter = SubmitRequirementResult.typeAdapter(gson);
+ assertThat(adapter.fromJson(adapter.toJson(srResult))).isEqualTo(srResult);
+ }
+
+ @Test
public void deserializeSubmitRequirementResult_withJGitPatchsetIdFormat() throws Exception {
String srResultSerialJgitFormat =
srReqResultSerial.replace(
diff --git a/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java b/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
index 034e7ef..22207cc 100644
--- a/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
+++ b/javatests/com/google/gerrit/server/project/SubmitRequirementsAdapterTest.java
@@ -371,7 +371,7 @@
assertThat(r.submitRequirement().submittabilityExpression().expressionString())
.isEqualTo(submitExpression);
assertThat(r.status()).isEqualTo(status);
- assertThat(r.submittabilityExpressionResult().status()).isEqualTo(expressionStatus);
+ assertThat(r.submittabilityExpressionResult().get().status()).isEqualTo(expressionStatus);
}
private SubmitRecord createSubmitRecord(
