Allow querying submit requirements using the label operator
This change is preserving backward compatibility of the label operator
using the status operand with new submit requirements. Previously users
were able to query labels like:
* label:Code-Review=NEED
* or, label:Code-Review=OK
These queries were matching with changes that emitted a submit record
with status=NEED or status=OK respectively. Submit records were emitted
when there were label functions, or custom/prolog submit rules
configured for the project, but these are being deprecated.
With the new submit requirements, submit records are no longer emitted.
This means that any queries with the label formats mentioned above will
no longer match with changes.
In this change, we backfill entries for the label operator with the
status operand from submit requirement results using the following
rules:
* if SR result = UNSATISFIED, we emit two label entries: NEED, REJECT
* if SR result = SATISFIED or OVERRIDDEN, we emit two label entries:
OK, MAY.
For example if a change has a "CR" requirement that is satisfied, a
query with "label:CR=OK" or "label:CR=MAY" will match with the change.
With this change, the format
<label-name>=<submit-record-status>,<approver>
will not work with backfilled submit requirement results. We might
consider implementing this in a future change if there is a need for it.
The implementation is using the submit requirement name as if it was a
label name. This is not very accurate since a SR might rely on some
conditions that's not using labels, but we accept this to simplify the
implementation.
Note that this change also marks the operator
`label:<label-name>=<status>` as deprecated since users should not rely
on it anymore. We'll implement a submit requirment predicate that
should be used for this use case instead.
Google-Bug-Id: b/218663294
Change-Id: Ic8b48101ba95a83a1b8a41bec3e976f1aa97c1e7
Release-Notes: Add support for querying SR Results using the label operator.
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index 7e8a7e0..ac62933 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -798,12 +798,25 @@
Note that a query like `label:Code-Review=+1,count<x` will not match with
changes having zero +1 votes to this label.
-`label:Non-Author-Code-Review=need`::
+`label:Non-Author-Code-Review=need` (deprecated)::
+
Matches changes where the submit rules indicate that a label named
`Non-Author-Code-Review` is needed. (See the
link:prolog-cookbook.html#NonAuthorCodeReview[Prolog Cookbook] for how
this label can be configured.)
++
+This operator is also compatible with
+link:config-submit-requirements.html[submit requirement] results. A submit
+requirement name could be used instead of the label name. The submit record
+statuses are mapped to submit requirement result statuses as follows:
++
+ * {`need`, `reject`} -> {`UNSATISFED`}
+ * {`ok`, `may`} -> {`SATISFIED`, `OVERRIDDEN`}
++
+For example, a query like `label:Code-Review=ok` will also match changes
+having a submit requirement with a result that is either `SATISFIED` or
+`OVERRIDDEN`. Users are encouraged not to rely on this operator since submit
+records are deprecated.
`label:Code-Review=+2,aname`::
`label:Code-Review=ok,aname`::
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index baac95b..c79b993 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -92,6 +92,7 @@
import java.util.Optional;
import java.util.Set;
import java.util.function.Function;
+import java.util.stream.Collectors;
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import org.eclipse.jgit.lib.PersonIdent;
@@ -1209,8 +1210,18 @@
}
public static List<String> formatSubmitRecordValues(ChangeData cd) {
- return formatSubmitRecordValues(
- cd.submitRecords(SUBMIT_RULE_OPTIONS_STRICT), cd.change().getOwner());
+ Set<String> submitRecordValues = new HashSet<>();
+ submitRecordValues.addAll(
+ formatSubmitRecordValues(
+ cd.submitRecords(SUBMIT_RULE_OPTIONS_STRICT), cd.change().getOwner()));
+ // Also backfill results of submit requirements such that users can query submit requirement
+ // results using the label operator, for example a query with "label:CR=NEED" will match with
+ // changes that have a submit-requirement with name="CR" and status=UNSATISFIED.
+ // Reason: We are preserving backward compatibility of the operators `label:$name=$status`
+ // which were previously working with submit records. Now admins can configure submit
+ // requirements and continue querying them with the label operator.
+ submitRecordValues.addAll(formatSubmitRequirementValues(cd.submitRequirements().values()));
+ return submitRecordValues.stream().collect(Collectors.toList());
}
@VisibleForTesting
@@ -1236,6 +1247,46 @@
return result;
}
+ /**
+ * Generate submit requirement result formats that are compatible with the legacy submit record
+ * statuses.
+ */
+ @VisibleForTesting
+ static List<String> formatSubmitRequirementValues(Collection<SubmitRequirementResult> srResults) {
+ List<String> result = new ArrayList<>();
+ for (SubmitRequirementResult srResult : srResults) {
+ switch (srResult.status()) {
+ case SATISFIED:
+ case OVERRIDDEN:
+ result.add(
+ SubmitRecord.Label.Status.OK.name()
+ + ","
+ + srResult.submitRequirement().name().toLowerCase());
+ result.add(
+ SubmitRecord.Label.Status.MAY.name()
+ + ","
+ + srResult.submitRequirement().name().toLowerCase());
+ break;
+ case UNSATISFIED:
+ result.add(
+ SubmitRecord.Label.Status.NEED.name()
+ + ","
+ + srResult.submitRequirement().name().toLowerCase());
+ result.add(
+ SubmitRecord.Label.Status.REJECT.name()
+ + ","
+ + srResult.submitRequirement().name().toLowerCase());
+ break;
+ default:
+ result.add(
+ SubmitRecord.Label.Status.IMPOSSIBLE.name()
+ + ","
+ + srResult.submitRequirement().name().toLowerCase());
+ }
+ }
+ return result;
+ }
+
/** Serialized submit requirements, used for pre-populating results. */
public static final FieldDef<ChangeData, Iterable<byte[]>> STORED_SUBMIT_REQUIREMENTS =
storedOnly("full_submit_requirements")
diff --git a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
index c9a57d0..055cdc5 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/SubmitRequirementIT.java
@@ -2709,6 +2709,41 @@
.isFalse();
}
+ @Test
+ public void queryChangesBySubmitRequirementResultUsingTheLabelPredicate() throws Exception {
+ // Create a non-blocking label and a submit-requirement that necessitates voting on this label.
+ configLabel("LC", LabelFunction.NO_OP);
+ projectOperations
+ .project(project)
+ .forUpdate()
+ .add(allowLabel("LC").ref("refs/heads/master").group(REGISTERED_USERS).range(-1, 1))
+ .update();
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("LC")
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("label:LC=MAX"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ List<ChangeInfo> changeInfos = gApi.changes().query("label:LC=NEED").get();
+ assertThat(changeInfos).hasSize(1);
+ assertThat(changeInfos.get(0).changeId).isEqualTo(changeId);
+ assertThat(gApi.changes().query("label:LC=OK").get()).isEmpty();
+ // case does not matter
+ changeInfos = gApi.changes().query("label:lc=NEED").get();
+ assertThat(changeInfos).hasSize(1);
+ assertThat(changeInfos.get(0).changeId).isEqualTo(changeId);
+
+ voteLabel(r.getChangeId(), "LC", +1);
+ changeInfos = gApi.changes().query("label:LC=OK").get();
+ assertThat(changeInfos.get(0).changeId).isEqualTo(changeId);
+ assertThat(gApi.changes().query("label:LC=NEED").get()).isEmpty();
+ }
+
private void voteLabel(String changeId, String labelName, int score) throws RestApiException {
gApi.changes().id(changeId).current().review(new ReviewInput().label(labelName, score));
}
diff --git a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
index dc1440b..3364540 100644
--- a/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
+++ b/javatests/com/google/gerrit/server/index/change/ChangeFieldTest.java
@@ -27,6 +27,10 @@
import com.google.gerrit.entities.LegacySubmitRequirement;
import com.google.gerrit.entities.Project;
import com.google.gerrit.entities.SubmitRecord;
+import com.google.gerrit.entities.SubmitRequirement;
+import com.google.gerrit.entities.SubmitRequirementExpression;
+import com.google.gerrit.entities.SubmitRequirementExpressionResult;
+import com.google.gerrit.entities.SubmitRequirementResult;
import com.google.gerrit.index.testing.FakeStoredValue;
import com.google.gerrit.server.ReviewerSet;
import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -89,6 +93,18 @@
}
@Test
+ public void formatSubmitRequirementValues() {
+ assertThat(
+ ChangeField.formatSubmitRequirementValues(
+ ImmutableList.of(
+ submitRequirementResult(
+ "CR", "label:CR=+1", SubmitRequirementExpressionResult.Status.PASS),
+ submitRequirementResult(
+ "LC", "label:LC=+1", SubmitRequirementExpressionResult.Status.FAIL))))
+ .containsExactly("MAY,cr", "OK,cr", "NEED,lc", "REJECT,lc");
+ }
+
+ @Test
public void storedSubmitRecords() {
assertStoredRecordRoundTrip(record(SubmitRecord.Status.CLOSED));
@@ -161,6 +177,25 @@
return r;
}
+ private SubmitRequirementResult submitRequirementResult(
+ String srName, String submitExpr, SubmitRequirementExpressionResult.Status submitExprStatus) {
+ return SubmitRequirementResult.builder()
+ .submitRequirement(
+ SubmitRequirement.builder()
+ .setName(srName)
+ .setSubmittabilityExpression(SubmitRequirementExpression.create("NA"))
+ .setAllowOverrideInChildProjects(false)
+ .build())
+ .submittabilityExpressionResult(
+ SubmitRequirementExpressionResult.create(
+ SubmitRequirementExpression.create(submitExpr),
+ submitExprStatus,
+ ImmutableList.of(submitExpr),
+ ImmutableList.of()))
+ .patchSetCommitId(ObjectId.zeroId())
+ .build();
+ }
+
private static SubmitRecord.Label label(
SubmitRecord.Label.Status status, String label, Integer appliedBy) {
SubmitRecord.Label l = new SubmitRecord.Label();
