Merge branch 'stable-3.4' into stable-3.5

* stable-3.4:
  Fix label operator to work with external groups

Change-Id: Id22027de7c461ee996435dd64c4338aa98346581
Release-Notes: skip
diff --git a/java/com/google/gerrit/server/group/testing/TestGroupBackend.java b/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
index 601ac59..12d8c93 100644
--- a/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
+++ b/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
@@ -118,6 +118,13 @@
 
   @Override
   public Collection<GroupReference> suggest(String name, ProjectState project) {
+    AccountGroup.UUID uuid = AccountGroup.uuid(name);
+    if (handles(uuid)) {
+      GroupDescription.Basic g = get(uuid);
+      if (g != null) {
+        return ImmutableList.of(GroupReference.forGroup(g));
+      }
+    }
     return ImmutableList.of();
   }
 
diff --git a/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java b/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
index 57a2091..be5f803 100644
--- a/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
+++ b/java/com/google/gerrit/server/index/change/IndexedChangeQuery.java
@@ -34,6 +34,7 @@
 import com.google.gerrit.index.query.ResultSet;
 import com.google.gerrit.server.query.change.ChangeData;
 import com.google.gerrit.server.query.change.ChangeDataSource;
+import com.google.gerrit.server.query.change.ChangeIndexPostFilterPredicate;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.Iterator;
@@ -112,13 +113,30 @@
     };
   }
 
+  public boolean postIndexMatch(Predicate<ChangeData> pred, ChangeData cd) {
+    if (pred instanceof ChangeIndexPostFilterPredicate) {
+      checkState(
+          pred.isMatchable(),
+          "match invoked, but child predicate %s doesn't implement %s",
+          pred,
+          Matchable.class.getName());
+      return pred.asMatchable().match(cd);
+    }
+    for (int i = 0; i < pred.getChildCount(); i++) {
+      if (!postIndexMatch(pred.getChild(i), cd)) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   @Override
   public boolean match(ChangeData cd) {
-    if (source != null && fromSource.get(cd) == source) {
+    Predicate<ChangeData> pred = getChild(0);
+    if (source != null && fromSource.get(cd) == source && postIndexMatch(pred, cd)) {
       return true;
     }
 
-    Predicate<ChangeData> pred = getChild(0);
     checkState(
         pred.isMatchable(),
         "match invoked, but child predicate %s doesn't implement %s",
diff --git a/java/com/google/gerrit/server/query/change/ChangeIndexPostFilterPredicate.java b/java/com/google/gerrit/server/query/change/ChangeIndexPostFilterPredicate.java
new file mode 100644
index 0000000..d86d366
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/ChangeIndexPostFilterPredicate.java
@@ -0,0 +1,31 @@
+// Copyright (C) 2022 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.index.FieldDef;
+
+/**
+ * Predicate that is mapped to a field in the change index, with additional filtering done in the
+ * {@code match} method.
+ */
+public abstract class ChangeIndexPostFilterPredicate extends ChangeIndexPredicate {
+  protected ChangeIndexPostFilterPredicate(FieldDef<ChangeData, ?> def, String value) {
+    super(def, value);
+  }
+
+  protected ChangeIndexPostFilterPredicate(FieldDef<ChangeData, ?> def, String name, String value) {
+    super(def, name, value);
+  }
+}
diff --git a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
index 12efecb..4a0b649 100644
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
@@ -30,7 +30,7 @@
 import com.google.gerrit.server.project.ProjectState;
 import java.util.Optional;
 
-public class EqualsLabelPredicate extends ChangeIndexPredicate {
+public class EqualsLabelPredicate extends ChangeIndexPostFilterPredicate {
   protected final ProjectCache projectCache;
   protected final PermissionBackend permissionBackend;
   protected final IdentifiedUser.GenericFactory userFactory;
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index be32138..a7a1795 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -34,6 +34,7 @@
         "//java/com/google/gerrit/server/audit",
         "//java/com/google/gerrit/server/cache/h2",
         "//java/com/google/gerrit/server/cache/mem",
+        "//java/com/google/gerrit/server/group/testing",
         "//java/com/google/gerrit/server/logging",
         "//java/com/google/gerrit/server/restapi",
         "//java/com/google/gerrit/server/schema",
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index 86ceb60..b00cadb 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -25,6 +25,7 @@
 import com.google.gerrit.auth.AuthModule;
 import com.google.gerrit.extensions.client.AuthType;
 import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.registration.DynamicSet;
 import com.google.gerrit.extensions.systemstatus.ServerInformation;
 import com.google.gerrit.gpg.GpgModule;
 import com.google.gerrit.httpd.auth.restapi.OAuthRestModule;
@@ -39,6 +40,7 @@
 import com.google.gerrit.server.GerritPersonIdentProvider;
 import com.google.gerrit.server.LibModuleType;
 import com.google.gerrit.server.PluginUser;
+import com.google.gerrit.server.account.GroupBackend;
 import com.google.gerrit.server.api.GerritApiModule;
 import com.google.gerrit.server.api.PluginApiModule;
 import com.google.gerrit.server.audit.AuditModule;
@@ -77,6 +79,7 @@
 import com.google.gerrit.server.git.PerThreadRequestScope;
 import com.google.gerrit.server.git.SearchingChangeCacheImpl.SearchingChangeCacheImplModule;
 import com.google.gerrit.server.git.WorkQueue;
+import com.google.gerrit.server.group.testing.TestGroupBackend;
 import com.google.gerrit.server.index.account.AccountSchemaDefinitions;
 import com.google.gerrit.server.index.account.AllAccountsIndexer;
 import com.google.gerrit.server.index.change.AllChangesIndexer;
@@ -267,6 +270,8 @@
     install(new ConfigExperimentFeaturesModule());
 
     bind(ProjectOperations.class).to(ProjectOperationsImpl.class);
+    bind(TestGroupBackend.class).in(SINGLETON);
+    DynamicSet.bind(binder(), GroupBackend.class).to(TestGroupBackend.class);
   }
 
   /** Copy of SchemaModule with a slightly different server ID provider. */
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 9acce8f..e9155ce 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -104,6 +104,7 @@
 import com.google.gerrit.server.account.Accounts;
 import com.google.gerrit.server.account.AccountsUpdate;
 import com.google.gerrit.server.account.AuthRequest;
+import com.google.gerrit.server.account.ListGroupMembership;
 import com.google.gerrit.server.account.VersionedAccountQueries;
 import com.google.gerrit.server.account.externalids.ExternalIdFactory;
 import com.google.gerrit.server.change.ChangeInserter;
@@ -112,6 +113,7 @@
 import com.google.gerrit.server.change.PatchSetInserter;
 import com.google.gerrit.server.config.AllUsersName;
 import com.google.gerrit.server.git.meta.MetaDataUpdate;
+import com.google.gerrit.server.group.testing.TestGroupBackend;
 import com.google.gerrit.server.index.change.ChangeField;
 import com.google.gerrit.server.index.change.ChangeIndexCollection;
 import com.google.gerrit.server.index.change.ChangeIndexer;
@@ -189,6 +191,7 @@
   @Inject protected SchemaCreator schemaCreator;
   @Inject protected Sequences seq;
   @Inject protected ThreadLocalRequestContext requestContext;
+  @Inject protected TestGroupBackend testGroupBackend;
   @Inject protected ProjectCache projectCache;
   @Inject protected MetaDataUpdate.Server metaDataUpdateFactory;
   @Inject protected IdentifiedUser.GenericFactory identifiedUserFactory;
@@ -1279,6 +1282,44 @@
   }
 
   @Test
+  public void byLabelExternalGroup() throws Exception {
+    Account.Id user1 = createAccount("user1");
+    Account.Id user2 = createAccount("user2");
+    TestRepository<InMemoryRepositoryManager.Repo> repo = createProject("repo");
+
+    // create group and add users
+    AccountGroup.UUID external_group1 = AccountGroup.uuid("testbackend:group1");
+    AccountGroup.UUID external_group2 = AccountGroup.uuid("testbackend:group2");
+    testGroupBackend.create(external_group1);
+    testGroupBackend.create(external_group2);
+    testGroupBackend.setMembershipsOf(
+        user1, new ListGroupMembership(ImmutableList.of(external_group1)));
+    testGroupBackend.setMembershipsOf(
+        user2, new ListGroupMembership(ImmutableList.of(external_group2)));
+
+    Change change1 = insert(repo, newChange(repo), user1);
+    Change change2 = insert(repo, newChange(repo), user1);
+
+    // post a review with user1 and other_user
+    requestContext.setContext(newRequestContext(user1));
+    gApi.changes()
+        .id(change1.getId().get())
+        .current()
+        .review(new ReviewInput().label("Code-Review", 1));
+    requestContext.setContext(newRequestContext(userId));
+    gApi.changes()
+        .id(change2.getId().get())
+        .current()
+        .review(new ReviewInput().label("Code-Review", 1));
+
+    assertQuery("label:Code-Review=+1," + external_group1.get(), change1);
+    assertQuery("label:Code-Review=+1,group=" + external_group1.get(), change1);
+    assertQuery("label:Code-Review=+1,user=user1", change1);
+    assertQuery("label:Code-Review=+1,user=user2");
+    assertQuery("label:Code-Review=+1,group=" + external_group2.get());
+  }
+
+  @Test
   public void limit() throws Exception {
     TestRepository<Repo> repo = createProject("repo");
     Change last = null;
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index 08456d1..ec1f612 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -29,6 +29,7 @@
         "//java/com/google/gerrit/index/testing",
         "//java/com/google/gerrit/lifecycle",
         "//java/com/google/gerrit/server",
+        "//java/com/google/gerrit/server/group/testing",
         "//java/com/google/gerrit/server/project/testing:project-test-util",
         "//java/com/google/gerrit/server/schema",
         "//java/com/google/gerrit/server/util/time",