Submit requirements: Add support for querying labels with MAX/MIN/ANY
This change adds support for creating different submit requirement
expressions using MAX, MIN or ANY for label values, the syntax works
like the following:
submittability_expression = label:code-review=MAX
This would allow us to express the current MaxWithBlock function as
submittability_expression =
label:code-review=MAX AND -label:code-review=MIN
This change adds this support for the change query language as well,
since the change query language is used by submit requirements.
Change-Id: I290f3f699291f324b760291daa3b27a718344803
diff --git a/Documentation/user-search.txt b/Documentation/user-search.txt
index a2dc31f..a9779b1 100644
--- a/Documentation/user-search.txt
+++ b/Documentation/user-search.txt
@@ -702,6 +702,18 @@
to one of the fields in the
link:rest-api-changes.html#submit-record[SubmitRecord] REST API entity.
+`label:Code-Review=MAX`::
++
+Matches changes with label voted with the highest possible score.
+
+`label:Code-Review=MIN`::
++
+Matches changes with label voted with the lowest possible score.
+
+`label:Code-Review=ANY`::
++
+Matches changes with label voted with any score.
+
`label:Non-Author-Code-Review=need`::
+
Matches changes where the submit rules indicate that a label named
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index 9c39c6e..810cd4d 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -43,11 +43,13 @@
import com.google.common.flogger.FluentLogger;
import com.google.common.io.Files;
import com.google.common.primitives.Longs;
+import com.google.gerrit.common.Nullable;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.Address;
import com.google.gerrit.entities.AttentionSetUpdate;
import com.google.gerrit.entities.Change;
import com.google.gerrit.entities.ChangeMessage;
+import com.google.gerrit.entities.LabelType;
import com.google.gerrit.entities.LegacySubmitRequirement;
import com.google.gerrit.entities.PatchSetApproval;
import com.google.gerrit.entities.Project;
@@ -72,6 +74,7 @@
import com.google.gerrit.server.query.change.ChangeData;
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeStatusPredicate;
+import com.google.gerrit.server.query.change.MagicLabelValue;
import com.google.gson.Gson;
import com.google.protobuf.MessageLite;
import java.sql.Timestamp;
@@ -602,16 +605,35 @@
for (PatchSetApproval a : cd.currentApprovals()) {
if (a.value() != 0 && !a.isLegacySubmit()) {
allApprovals.add(formatLabel(a.label(), a.value(), a.accountId()));
+ LabelType labelType = cd.getLabelTypes().byLabel(a.labelId());
+ allApprovals.addAll(getMaxMinAnyLabels(a.label(), a.value(), labelType, a.accountId()));
if (owners && cd.change().getOwner().equals(a.accountId())) {
allApprovals.add(formatLabel(a.label(), a.value(), ChangeQueryBuilder.OWNER_ACCOUNT_ID));
+ allApprovals.addAll(
+ getMaxMinAnyLabels(
+ a.label(), a.value(), labelType, ChangeQueryBuilder.OWNER_ACCOUNT_ID));
}
distinctApprovals.add(formatLabel(a.label(), a.value()));
+ distinctApprovals.addAll(getMaxMinAnyLabels(a.label(), a.value(), labelType, null));
}
}
allApprovals.addAll(distinctApprovals);
return allApprovals;
}
+ private static List<String> getMaxMinAnyLabels(
+ String label, short labelVal, LabelType labelType, @Nullable Account.Id accountId) {
+ List<String> labels = new ArrayList<>();
+ if (labelVal == labelType.getMaxPositive()) {
+ labels.add(formatLabel(label, MagicLabelValue.MAX.name(), accountId));
+ }
+ if (labelVal == labelType.getMaxNegative()) {
+ labels.add(formatLabel(label, MagicLabelValue.MIN.name(), accountId));
+ }
+ labels.add(formatLabel(label, MagicLabelValue.ANY.name(), accountId));
+ return labels;
+ }
+
public static Set<String> getAuthorParts(ChangeData cd) {
return SchemaUtil.getPersonParts(cd.getAuthor());
}
@@ -696,6 +718,17 @@
+ (accountId != null ? "," + formatAccount(accountId) : "");
}
+ public static String formatLabel(String label, String value) {
+ return formatLabel(label, value, null);
+ }
+
+ public static String formatLabel(String label, String value, @Nullable Account.Id accountId) {
+ return label.toLowerCase()
+ + "="
+ + value
+ + (accountId != null ? "," + formatAccount(accountId) : "");
+ }
+
private static String formatAccount(Account.Id accountId) {
if (ChangeQueryBuilder.OWNER_ACCOUNT_ID.equals(accountId)) {
return ChangeQueryBuilder.ARG_ID_OWNER;
diff --git a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
index 6355674..879da4f 100644
--- a/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
+++ b/java/com/google/gerrit/server/index/change/ChangeSchemaDefinitions.java
@@ -150,6 +150,9 @@
*/
static final Schema<ChangeData> V63 = schema(V62, false);
+ /** Added support for MIN/MAX/ANY for {@link ChangeField#LABEL} */
+ static final Schema<ChangeData> V64 = schema(V63, false);
+
/**
* Name of the change index to be used when contacting index backends or loading configurations.
*/
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 94b5442..131de74 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -973,7 +973,7 @@
int eq = name.indexOf('=');
if (args.getSchema().hasField(ChangeField.SUBMIT_RECORD) && eq > 0) {
String statusName = name.substring(eq + 1).toUpperCase();
- if (!isInt(statusName)) {
+ if (!isInt(statusName) && !MagicLabelValue.tryParse(statusName).isPresent()) {
SubmitRecord.Label.Status status =
Enums.getIfPresent(SubmitRecord.Label.Status.class, statusName).orNull();
if (status == null) {
diff --git a/java/com/google/gerrit/server/query/change/LabelPredicate.java b/java/com/google/gerrit/server/query/change/LabelPredicate.java
index 38d1dbe..989b4bb 100644
--- a/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -14,6 +14,7 @@
package com.google.gerrit.server.query.change;
+import com.google.common.collect.ImmutableList;
import com.google.common.collect.Lists;
import com.google.gerrit.entities.Account;
import com.google.gerrit.entities.AccountGroup;
@@ -59,12 +60,12 @@
protected static class Parsed {
protected final String label;
protected final String test;
- protected final int expVal;
+ protected final int numericValue;
- protected Parsed(String label, String test, int expVal) {
+ protected Parsed(String label, String test, int numericValue) {
this.label = label;
this.test = test;
- this.expVal = expVal;
+ this.numericValue = numericValue;
}
}
@@ -83,6 +84,14 @@
protected static List<Predicate<ChangeData>> predicates(Args args) {
String v = args.value;
+
+ try {
+ MagicLabelVote mlv = MagicLabelVote.parseWithEquals(v);
+ return ImmutableList.of(new MagicLabelPredicate(args, mlv));
+ } catch (IllegalArgumentException e) {
+ // Try next format.
+ }
+
Parsed parsed = null;
try {
@@ -108,7 +117,7 @@
} else {
range =
RangeUtil.getRange(
- parsed.label, parsed.test, parsed.expVal, -MAX_LABEL_VALUE, MAX_LABEL_VALUE);
+ parsed.label, parsed.test, parsed.numericValue, -MAX_LABEL_VALUE, MAX_LABEL_VALUE);
}
String prefix = range.prefix;
int min = range.min;
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
new file mode 100644
index 0000000..e3c58e47
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
@@ -0,0 +1,101 @@
+// 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.server.query.change;
+
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.LabelTypes;
+import com.google.gerrit.entities.LabelValue;
+import com.google.gerrit.index.query.Predicate;
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gerrit.server.project.ProjectState;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Optional;
+
+public class MagicLabelPredicate extends ChangeIndexPredicate {
+ protected final LabelPredicate.Args args;
+ private final MagicLabelVote magicLabelVote;
+
+ public MagicLabelPredicate(LabelPredicate.Args args, MagicLabelVote magicLabelVote) {
+ super(ChangeField.LABEL, magicLabelVote.formatLabel());
+ this.args = args;
+ this.magicLabelVote = magicLabelVote;
+ }
+
+ @Override
+ public boolean match(ChangeData changeData) {
+ Change change = changeData.change();
+ if (change == null) {
+ // The change has disappeared.
+ //
+ return false;
+ }
+
+ Optional<ProjectState> project = args.projectCache.get(change.getDest().project());
+ if (!project.isPresent()) {
+ // The project has disappeared.
+ //
+ return false;
+ }
+
+ LabelType labelType = type(project.get().getLabelTypes(), magicLabelVote.label());
+ if (labelType == null) {
+ return false; // Label is not defined by this project.
+ }
+
+ switch (magicLabelVote.value()) {
+ case ANY:
+ return matchAny(changeData, labelType);
+ case MIN:
+ return matchNumeric(changeData, magicLabelVote.label(), labelType.getMin().getValue());
+ case MAX:
+ return matchNumeric(changeData, magicLabelVote.label(), labelType.getMax().getValue());
+ }
+
+ throw new IllegalStateException("Unsupported magic label value: " + magicLabelVote.value());
+ }
+
+ private boolean matchAny(ChangeData changeData, LabelType labelType) {
+ List<Predicate<ChangeData>> predicates = new ArrayList<>();
+ for (LabelValue labelValue : labelType.getValues()) {
+ if (labelValue.getValue() != 0) {
+ predicates.add(numericPredicate(labelType.getName(), labelValue.getValue()));
+ }
+ }
+ return or(predicates).asMatchable().match(changeData);
+ }
+
+ private boolean matchNumeric(ChangeData changeData, String label, short value) {
+ return numericPredicate(label, value).match(changeData);
+ }
+
+ private EqualsLabelPredicate numericPredicate(String label, short value) {
+ return new EqualsLabelPredicate(args, label, value, /* account= */ null);
+ }
+
+ protected static LabelType type(LabelTypes types, String toFind) {
+ if (types.byLabel(toFind) != null) {
+ return types.byLabel(toFind);
+ }
+
+ for (LabelType lt : types.getLabelTypes()) {
+ if (toFind.equalsIgnoreCase(lt.getName())) {
+ return lt;
+ }
+ }
+ return null;
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelValue.java b/java/com/google/gerrit/server/query/change/MagicLabelValue.java
new file mode 100644
index 0000000..c4bcbe3
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/MagicLabelValue.java
@@ -0,0 +1,31 @@
+// 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.server.query.change;
+
+import java.util.Optional;
+
+public enum MagicLabelValue {
+ ANY,
+ MIN,
+ MAX;
+
+ public static Optional<MagicLabelValue> tryParse(String value) {
+ try {
+ return Optional.of(MagicLabelValue.valueOf(value));
+ } catch (IllegalArgumentException e) {
+ return Optional.empty();
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelVote.java b/java/com/google/gerrit/server/query/change/MagicLabelVote.java
new file mode 100644
index 0000000..c29ac72
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/MagicLabelVote.java
@@ -0,0 +1,47 @@
+// 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.server.query.change;
+
+import static com.google.common.base.Preconditions.checkArgument;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.base.Strings;
+import com.google.gerrit.entities.LabelType;
+import java.util.Locale;
+
+/** An entity representing a special label vote that's not numeric, e.g. MAX, MIN, etc... */
+@AutoValue
+public abstract class MagicLabelVote {
+ public static MagicLabelVote parseWithEquals(String text) {
+ checkArgument(!Strings.isNullOrEmpty(text), "Empty label vote");
+ int e = text.lastIndexOf('=');
+ checkArgument(e >= 0, "Label vote missing '=': %s", text);
+ String label = text.substring(0, e);
+ String voteValue = text.substring(e + 1);
+ return create(label, MagicLabelValue.valueOf(voteValue));
+ }
+
+ public static MagicLabelVote create(String label, MagicLabelValue value) {
+ return new AutoValue_MagicLabelVote(LabelType.checkNameInternal(label), value);
+ }
+
+ public abstract String label();
+
+ public abstract MagicLabelValue value();
+
+ public String formatLabel() {
+ return label().toLowerCase(Locale.US) + "=" + value().name();
+ }
+}
diff --git a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
index 42354ca..976e828 100644
--- a/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java
@@ -4020,6 +4020,86 @@
}
@Test
+ public void submitRequirement_withLabelEqualsMax() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label:code-review=MAX"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+
+ voteLabel(changeId, "code-review", 2);
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ }
+
+ @Test
+ public void submitRequirement_withLabelEqualsMinBlockingSubmission() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("-label:code-review=MIN"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ // Requirement is satisfied because there are no votes
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+
+ voteLabel(changeId, "code-review", -1);
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ // Requirement is still satisfied because -1 is not the max negative value
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+
+ voteLabel(changeId, "code-review", -2);
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ // Requirement is now unsatisfied because -2 is the max negative value
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+ }
+
+ @Test
+ public void submitRequirement_withLabelEqualsAny() throws Exception {
+ configSubmitRequirement(
+ project,
+ SubmitRequirement.builder()
+ .setName("code-review")
+ .setSubmittabilityExpression(
+ SubmitRequirementExpression.create("label:code-review=ANY"))
+ .setAllowOverrideInChildProjects(false)
+ .build());
+
+ PushOneCommit.Result r = createChange();
+ String changeId = r.getChangeId();
+
+ ChangeInfo change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.UNSATISFIED);
+
+ voteLabel(changeId, "code-review", 1);
+ change = gApi.changes().id(changeId).get();
+ assertThat(change.submitRequirements).hasSize(1);
+ assertSubmitRequirementStatus(change.submitRequirements, "code-review", Status.SATISFIED);
+ }
+
+ @Test
public void submitRequirementIsSatisfied_whenSubmittabilityExpressionIsFulfilled()
throws Exception {
configSubmitRequirement(
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 3c10fbc..1f29f45 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1007,6 +1007,7 @@
changes.put(-1, reviewMinus1Change);
changes.put(-2, reviewMinus2Change);
+ assertQuery("label:Code-Review=MIN", reviewMinus2Change);
assertQuery("label:Code-Review=-2", reviewMinus2Change);
assertQuery("label:Code-Review-2", reviewMinus2Change);
assertQuery("label:Code-Review=-1", reviewMinus1Change);
@@ -1018,6 +1019,13 @@
assertQuery("label:Code-Review=+2", reviewPlus2Change);
assertQuery("label:Code-Review=2", reviewPlus2Change);
assertQuery("label:Code-Review+2", reviewPlus2Change);
+ assertQuery("label:Code-Review=MAX", reviewPlus2Change);
+ assertQuery(
+ "label:Code-Review=ANY",
+ reviewPlus2Change,
+ reviewPlus1Change,
+ reviewMinus1Change,
+ reviewMinus2Change);
assertQuery("label:Code-Review>-3", codeReviewInRange(changes, -2, 2));
assertQuery("label:Code-Review>=-2", codeReviewInRange(changes, -2, 2));
