summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorNasser Grainawi <nasser.grainawi@linaro.org>2023-01-19 10:24:09 -0700
committerNasser Grainawi <nasser.grainawi@linaro.org>2023-01-24 12:23:33 -0700
commit18abad17ba1ce030bed401e1155fb743484f52e1 (patch)
tree27ba99a274f7068d108993c84a30f02cf8e5828e
parente72f8da257b7433a159d5761fab50bb4a58f7f2e (diff)
Fix negated label: queries with external groups
The ChangeIndexPostFilterPredicate approach doesn't fix the case where the operator is also negated. For example if there was a query like `status:open NOT label:Code-Review=1,group=ldap/testers` then in that scenario we tried to index match Code-Review+1 votes and expected that we would use post-filtering for the group. However, since the operator was negated, we excluded those changes from the results before we reached the post-filter stage, meaning we never include changes where the voter isn't in the given group. This solution is an unoptimized approach that avoids using the index for the label operator whenever an external group is given. A better approach would fix the negation case for any ChangeIndexPostFilterPredicate, but that looks to be much more complicated to implement. Avoiding duplicated code is tricky here since the PostFilter and Index predicates need to extend different classes, but share almost all of the same logic for match(). Using a static inner helper class 'Matcher' as a member of both the PostFilter and Index classes seems to be the simplest way to minimize the duplication. Additionally we can extend that helper class in each of the PostFilter/Index classes within MagicLabelPredicates because they need slightly different behavior for one method. The added tests pass with both the Lucene and FakeIndex implementations, but the FakeIndex also passes without the fix because it uses match() instead of doing some kind of index search operation. Release-Notes: Fixed bug with negated label: operator using an external group Change-Id: I0268476f0e6e3008d30ce49dfbb71ea3b093f7c1
-rw-r--r--java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java158
-rw-r--r--java/com/google/gerrit/server/query/change/EqualsLabelPredicates.java199
-rw-r--r--java/com/google/gerrit/server/query/change/LabelPredicate.java20
-rw-r--r--java/com/google/gerrit/server/query/change/MagicLabelPredicate.java105
-rw-r--r--java/com/google/gerrit/server/query/change/MagicLabelPredicates.java152
-rw-r--r--javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java7
6 files changed, 374 insertions, 267 deletions
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
deleted file mode 100644
index 4a0b649ada..0000000000
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ /dev/null
@@ -1,158 +0,0 @@
-// Copyright (C) 2013 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.Account;
-import com.google.gerrit.entities.AccountGroup;
-import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.LabelType;
-import com.google.gerrit.entities.LabelTypes;
-import com.google.gerrit.entities.PatchSetApproval;
-import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.index.change.ChangeField;
-import com.google.gerrit.server.permissions.ChangePermission;
-import com.google.gerrit.server.permissions.PermissionBackend;
-import com.google.gerrit.server.permissions.PermissionBackendException;
-import com.google.gerrit.server.project.ProjectCache;
-import com.google.gerrit.server.project.ProjectState;
-import java.util.Optional;
-
-public class EqualsLabelPredicate extends ChangeIndexPostFilterPredicate {
- protected final ProjectCache projectCache;
- protected final PermissionBackend permissionBackend;
- protected final IdentifiedUser.GenericFactory userFactory;
- protected final String label;
- protected final int expVal;
- protected final Account.Id account;
- protected final AccountGroup.UUID group;
-
- public EqualsLabelPredicate(
- LabelPredicate.Args args, String label, int expVal, Account.Id account) {
- super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account));
- this.permissionBackend = args.permissionBackend;
- this.projectCache = args.projectCache;
- this.userFactory = args.userFactory;
- this.group = args.group;
- this.label = label;
- this.expVal = expVal;
- this.account = account;
- }
-
- @Override
- public boolean match(ChangeData object) {
- Change c = object.change();
- if (c == null) {
- // The change has disappeared.
- //
- return false;
- }
-
- Optional<ProjectState> project = projectCache.get(c.getDest().project());
- if (!project.isPresent()) {
- // The project has disappeared.
- //
- return false;
- }
-
- LabelType labelType = type(project.get().getLabelTypes(), label);
- if (labelType == null) {
- return false; // Label is not defined by this project.
- }
-
- boolean hasVote = false;
- object.setStorageConstraint(ChangeData.StorageConstraint.INDEX_PRIMARY_NOTEDB_SECONDARY);
- for (PatchSetApproval p : object.currentApprovals()) {
- if (labelType.matches(p)) {
- hasVote = true;
- if (match(object, p.value(), p.accountId())) {
- return true;
- }
- }
- }
-
- if (!hasVote && expVal == 0) {
- return true;
- }
-
- return false;
- }
-
- protected static LabelType type(LabelTypes types, String toFind) {
- if (types.byLabel(toFind).isPresent()) {
- return types.byLabel(toFind).get();
- }
-
- for (LabelType lt : types.getLabelTypes()) {
- if (toFind.equalsIgnoreCase(lt.getName())) {
- return lt;
- }
- }
- return null;
- }
-
- protected boolean match(ChangeData cd, short value, Account.Id approver) {
- if (value != expVal) {
- return false;
- }
-
- if (account != null) {
- // case when account in query is numeric
- if (!account.equals(approver) && !isMagicUser()) {
- return false;
- }
-
- // case when account in query = owner
- if (account.equals(ChangeQueryBuilder.OWNER_ACCOUNT_ID)
- && !cd.change().getOwner().equals(approver)) {
- return false;
- }
-
- // case when account in query = non_uploader
- if (account.equals(ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID)
- && cd.currentPatchSet().uploader().equals(approver)) {
- return false;
- }
- }
-
- IdentifiedUser reviewer = userFactory.create(approver);
- if (group != null && !reviewer.getEffectiveGroups().contains(group)) {
- return false;
- }
-
- // Check the user has 'READ' permission.
- try {
- PermissionBackend.ForChange perm = permissionBackend.absentUser(approver).change(cd);
- if (!projectCache.get(cd.project()).map(ProjectState::statePermitsRead).orElse(false)) {
- return false;
- }
-
- perm.check(ChangePermission.READ);
- return true;
- } catch (PermissionBackendException | AuthException e) {
- return false;
- }
- }
-
- private boolean isMagicUser() {
- return account.equals(ChangeQueryBuilder.OWNER_ACCOUNT_ID)
- || account.equals(ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID);
- }
-
- @Override
- public int getCost() {
- return 1 + (group == null ? 0 : 1);
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicates.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicates.java
new file mode 100644
index 0000000000..7f3978d71c
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicates.java
@@ -0,0 +1,199 @@
+// Copyright (C) 2013 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.Account;
+import com.google.gerrit.entities.AccountGroup;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.LabelTypes;
+import com.google.gerrit.entities.PatchSetApproval;
+import com.google.gerrit.extensions.restapi.AuthException;
+import com.google.gerrit.index.query.PostFilterPredicate;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.index.change.ChangeField;
+import com.google.gerrit.server.permissions.ChangePermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
+import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.project.ProjectCache;
+import com.google.gerrit.server.project.ProjectState;
+import java.util.Optional;
+
+public class EqualsLabelPredicates {
+ public static class PostFilterEqualsLabelPredicate extends PostFilterPredicate<ChangeData> {
+ private final Matcher matcher;
+
+ public PostFilterEqualsLabelPredicate(LabelPredicate.Args args, String label, int expVal) {
+ super(ChangeQueryBuilder.FIELD_LABEL, ChangeField.formatLabel(label, expVal));
+ matcher = new Matcher(args, label, expVal);
+ }
+
+ @Override
+ public boolean match(ChangeData object) {
+ return matcher.match(object);
+ }
+
+ @Override
+ public int getCost() {
+ return 2;
+ }
+ }
+
+ public static class IndexEqualsLabelPredicate extends ChangeIndexPostFilterPredicate {
+ private final Matcher matcher;
+
+ public IndexEqualsLabelPredicate(LabelPredicate.Args args, String label, int expVal) {
+ this(args, label, expVal, null);
+ }
+
+ public IndexEqualsLabelPredicate(
+ LabelPredicate.Args args, String label, int expVal, Account.Id account) {
+ super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account));
+ this.matcher = new Matcher(args, label, expVal, account);
+ }
+
+ @Override
+ public boolean match(ChangeData object) {
+ return matcher.match(object);
+ }
+
+ @Override
+ public int getCost() {
+ return 1 + (matcher.group == null ? 0 : 1);
+ }
+ }
+
+ private static class Matcher {
+ protected final ProjectCache projectCache;
+ protected final PermissionBackend permissionBackend;
+ protected final IdentifiedUser.GenericFactory userFactory;
+ protected final String label;
+ protected final int expVal;
+ protected final Account.Id account;
+ protected final AccountGroup.UUID group;
+
+ public Matcher(LabelPredicate.Args args, String label, int expVal) {
+ this(args, label, expVal, null);
+ }
+
+ public Matcher(LabelPredicate.Args args, String label, int expVal, Account.Id account) {
+ this.permissionBackend = args.permissionBackend;
+ this.projectCache = args.projectCache;
+ this.userFactory = args.userFactory;
+ this.group = args.group;
+ this.label = label;
+ this.expVal = expVal;
+ this.account = account;
+ }
+
+ public boolean match(ChangeData cd) {
+ Change c = cd.change();
+ if (c == null) {
+ // The change has disappeared.
+ return false;
+ }
+
+ Optional<ProjectState> project = projectCache.get(c.getDest().project());
+ if (!project.isPresent()) {
+ // The project has disappeared.
+ return false;
+ }
+
+ LabelType labelType = type(project.get().getLabelTypes(), label);
+ if (labelType == null) {
+ return false; // Label is not defined by this project.
+ }
+
+ boolean hasVote = false;
+ cd.setStorageConstraint(ChangeData.StorageConstraint.INDEX_PRIMARY_NOTEDB_SECONDARY);
+ for (PatchSetApproval psa : cd.currentApprovals()) {
+ if (labelType.matches(psa)) {
+ hasVote = true;
+ if (match(cd, psa)) {
+ return true;
+ }
+ }
+ }
+
+ if (!hasVote && expVal == 0) {
+ return true;
+ }
+
+ return false;
+ }
+
+ private boolean match(ChangeData cd, PatchSetApproval psa) {
+ if (psa.value() != expVal) {
+ return false;
+ }
+ Account.Id approver = psa.accountId();
+
+ if (account != null) {
+ // case when account in query is numeric
+ if (!account.equals(approver) && !isMagicUser()) {
+ return false;
+ }
+
+ // case when account in query = owner
+ if (account.equals(ChangeQueryBuilder.OWNER_ACCOUNT_ID)
+ && !cd.change().getOwner().equals(approver)) {
+ return false;
+ }
+
+ // case when account in query = non_uploader
+ if (account.equals(ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID)
+ && cd.currentPatchSet().uploader().equals(approver)) {
+ return false;
+ }
+ }
+
+ IdentifiedUser reviewer = userFactory.create(approver);
+ if (group != null && !reviewer.getEffectiveGroups().contains(group)) {
+ return false;
+ }
+
+ // Check the user has 'READ' permission.
+ try {
+ PermissionBackend.ForChange perm = permissionBackend.absentUser(approver).change(cd);
+ if (!projectCache.get(cd.project()).map(ProjectState::statePermitsRead).orElse(false)) {
+ return false;
+ }
+
+ perm.check(ChangePermission.READ);
+ return true;
+ } catch (PermissionBackendException | AuthException e) {
+ return false;
+ }
+ }
+
+ private boolean isMagicUser() {
+ return account.equals(ChangeQueryBuilder.OWNER_ACCOUNT_ID)
+ || account.equals(ChangeQueryBuilder.NON_UPLOADER_ACCOUNT_ID);
+ }
+ }
+
+ public static LabelType type(LabelTypes types, String toFind) {
+ if (types.byLabel(toFind).isPresent()) {
+ return types.byLabel(toFind).get();
+ }
+
+ for (LabelType lt : types.getLabelTypes()) {
+ if (toFind.equalsIgnoreCase(lt.getName())) {
+ return lt;
+ }
+ }
+ return null;
+ }
+}
diff --git a/java/com/google/gerrit/server/query/change/LabelPredicate.java b/java/com/google/gerrit/server/query/change/LabelPredicate.java
index 5f017fbf17..cbf5df434f 100644
--- a/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -147,23 +147,35 @@ public class LabelPredicate extends OrPredicate<ChangeData> {
}
protected static Predicate<ChangeData> equalsLabelPredicate(Args args, String label, int expVal) {
+ if (args.group != null && !args.group.isInternalGroup()) {
+ // We can only get members of internal groups and negating an index search that doesn't
+ // include the external group information leads to incorrect query results. Use a
+ // PostFilterPredicate in this case instead.
+ return new EqualsLabelPredicates.PostFilterEqualsLabelPredicate(args, label, expVal);
+ }
if (args.accounts == null || args.accounts.isEmpty()) {
- return new EqualsLabelPredicate(args, label, expVal, null);
+ return new EqualsLabelPredicates.IndexEqualsLabelPredicate(args, label, expVal);
}
List<Predicate<ChangeData>> r = new ArrayList<>();
for (Account.Id a : args.accounts) {
- r.add(new EqualsLabelPredicate(args, label, expVal, a));
+ r.add(new EqualsLabelPredicates.IndexEqualsLabelPredicate(args, label, expVal, a));
}
return or(r);
}
protected static Predicate<ChangeData> magicLabelPredicate(Args args, MagicLabelVote mlv) {
+ if (args.group != null && !args.group.isInternalGroup()) {
+ // We can only get members of internal groups and negating an index search that doesn't
+ // include the external group information leads to incorrect query results. Use a
+ // PostFilterPredicate in this case instead.
+ return new MagicLabelPredicates.PostFilterMagicLabelPredicate(args, mlv);
+ }
if (args.accounts == null || args.accounts.isEmpty()) {
- return new MagicLabelPredicate(args, mlv, /* account= */ null);
+ return new MagicLabelPredicates.IndexMagicLabelPredicate(args, mlv);
}
List<Predicate<ChangeData>> r = new ArrayList<>();
for (Account.Id a : args.accounts) {
- r.add(new MagicLabelPredicate(args, mlv, a));
+ r.add(new MagicLabelPredicates.IndexMagicLabelPredicate(args, mlv, a));
}
return or(r);
}
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java b/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
deleted file mode 100644
index 3917c7902c..0000000000
--- a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
+++ /dev/null
@@ -1,105 +0,0 @@
-// 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.Account;
-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;
- private final Account.Id account;
-
- public MagicLabelPredicate(
- LabelPredicate.Args args, MagicLabelVote magicLabelVote, Account.Id account) {
- super(ChangeField.LABEL, magicLabelVote.formatLabel());
- this.account = account;
- 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);
- }
-
- protected static LabelType type(LabelTypes types, String toFind) {
- if (types.byLabel(toFind).isPresent()) {
- return types.byLabel(toFind).get();
- }
-
- for (LabelType lt : types.getLabelTypes()) {
- if (toFind.equalsIgnoreCase(lt.getName())) {
- return lt;
- }
- }
- return null;
- }
-}
diff --git a/java/com/google/gerrit/server/query/change/MagicLabelPredicates.java b/java/com/google/gerrit/server/query/change/MagicLabelPredicates.java
new file mode 100644
index 0000000000..017abecaa1
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/MagicLabelPredicates.java
@@ -0,0 +1,152 @@
+// 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.gerrit.server.query.change.EqualsLabelPredicates.type;
+
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.entities.Change;
+import com.google.gerrit.entities.LabelType;
+import com.google.gerrit.entities.LabelValue;
+import com.google.gerrit.index.query.PostFilterPredicate;
+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 MagicLabelPredicates {
+ public static class PostFilterMagicLabelPredicate extends PostFilterPredicate<ChangeData> {
+ private static class PostFilterMatcher extends Matcher {
+ public PostFilterMatcher(LabelPredicate.Args args, MagicLabelVote magicLabelVote) {
+ super(args, magicLabelVote);
+ }
+
+ @Override
+ protected Predicate<ChangeData> numericPredicate(String label, short value) {
+ return new EqualsLabelPredicates.PostFilterEqualsLabelPredicate(args, label, value);
+ }
+ }
+
+ private final PostFilterMatcher matcher;
+
+ public PostFilterMagicLabelPredicate(LabelPredicate.Args args, MagicLabelVote magicLabelVote) {
+ super(ChangeQueryBuilder.FIELD_LABEL, magicLabelVote.formatLabel());
+ this.matcher = new PostFilterMatcher(args, magicLabelVote);
+ }
+
+ @Override
+ public boolean match(ChangeData changeData) {
+ return matcher.match(changeData);
+ }
+
+ @Override
+ public int getCost() {
+ return 2;
+ }
+ }
+
+ public static class IndexMagicLabelPredicate extends ChangeIndexPredicate {
+ private static class IndexMatcher extends Matcher {
+ public IndexMatcher(
+ LabelPredicate.Args args, MagicLabelVote magicLabelVote, Account.Id account) {
+ super(args, magicLabelVote, account);
+ }
+
+ @Override
+ protected Predicate<ChangeData> numericPredicate(String label, short value) {
+ return new EqualsLabelPredicates.IndexEqualsLabelPredicate(args, label, value, account);
+ }
+ }
+
+ private final Matcher matcher;
+
+ public IndexMagicLabelPredicate(LabelPredicate.Args args, MagicLabelVote magicLabelVote) {
+ this(args, magicLabelVote, null);
+ }
+
+ public IndexMagicLabelPredicate(
+ LabelPredicate.Args args, MagicLabelVote magicLabelVote, Account.Id account) {
+ super(ChangeField.LABEL, magicLabelVote.formatLabel());
+ this.matcher = new IndexMatcher(args, magicLabelVote, account);
+ }
+
+ @Override
+ public boolean match(ChangeData changeData) {
+ return matcher.match(changeData);
+ }
+ }
+
+ private abstract static class Matcher {
+ protected final LabelPredicate.Args args;
+ protected final MagicLabelVote magicLabelVote;
+ protected final Account.Id account;
+
+ public Matcher(LabelPredicate.Args args, MagicLabelVote magicLabelVote) {
+ this(args, magicLabelVote, null);
+ }
+
+ public Matcher(LabelPredicate.Args args, MagicLabelVote magicLabelVote, Account.Id account) {
+ this.account = account;
+ this.args = args;
+ this.magicLabelVote = magicLabelVote;
+ }
+
+ public boolean match(ChangeData cd) {
+ Change change = cd.change();
+ if (change == null) {
+ return false; // The change has disappeared.
+ }
+
+ Optional<ProjectState> project = args.projectCache.get(change.getDest().project());
+ if (!project.isPresent()) {
+ return false; // The project has disappeared.
+ }
+
+ 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(cd, labelType);
+ case MIN:
+ return matchNumeric(cd, magicLabelVote.label(), labelType.getMin().getValue());
+ case MAX:
+ return matchNumeric(cd, 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 Predicate.or(predicates).asMatchable().match(changeData);
+ }
+
+ private boolean matchNumeric(ChangeData changeData, String label, short value) {
+ return numericPredicate(label, value).asMatchable().match(changeData);
+ }
+
+ protected abstract Predicate<ChangeData> numericPredicate(String label, short value);
+ }
+}
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index c3b220b0e4..cbdc138748 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1317,6 +1317,13 @@ public abstract class AbstractQueryChangesTest extends GerritServerTests {
assertQuery("label:Code-Review=+1,user=user1", change1);
assertQuery("label:Code-Review=+1,user=user2");
assertQuery("label:Code-Review=+1,group=" + external_group2.get());
+
+ // Negated operator tests
+ assertQuery("-label:Code-Review=+1," + external_group1.get(), change2);
+ assertQuery("-label:Code-Review=+1,group=" + external_group1.get(), change2);
+ assertQuery("-label:Code-Review=+1,user=user1", change2);
+ assertQuery("-label:Code-Review=+1,group=" + external_group2.get(), change2, change1);
+ assertQuery("-label:Code-Review=+1,user=user2", change2, change1);
}
@Test