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",