Merge "Don't limit PerThreadCache rather limit PerThreadProjectCache" into stable-3.5
diff --git a/java/com/google/gerrit/server/account/GroupBackend.java b/java/com/google/gerrit/server/account/GroupBackend.java
index 91edaf2..e02c27b 100644
--- a/java/com/google/gerrit/server/account/GroupBackend.java
+++ b/java/com/google/gerrit/server/account/GroupBackend.java
@@ -46,4 +46,28 @@
/** Returns {@code true} if the group with the given UUID is visible to all registered users. */
boolean isVisibleToAll(AccountGroup.UUID uuid);
+
+ default boolean isOrContainsExternalGroup(AccountGroup.UUID groupId) {
+ if (groupId != null) {
+ GroupDescription.Basic groupDescription = get(groupId);
+ if (!(groupDescription instanceof GroupDescription.Internal)
+ || containsExternalSubGroups((GroupDescription.Internal) groupDescription)) {
+ return true;
+ }
+ }
+ return false;
+ }
+
+ private boolean containsExternalSubGroups(GroupDescription.Internal internalGroup) {
+ for (AccountGroup.UUID subGroupUuid : internalGroup.getSubgroups()) {
+ GroupDescription.Basic subGroupDescription = get(subGroupUuid);
+ if (!(subGroupDescription instanceof GroupDescription.Internal)) {
+ return true;
+ }
+ if (containsExternalSubGroups((GroupDescription.Internal) subGroupDescription)) {
+ return true;
+ }
+ }
+ return false;
+ }
}
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index 0ffdb1c..c70a948 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -34,7 +34,6 @@
import com.google.gerrit.entities.Address;
import com.google.gerrit.entities.BranchNameKey;
import com.google.gerrit.entities.Change;
-import com.google.gerrit.entities.GroupDescription;
import com.google.gerrit.entities.GroupReference;
import com.google.gerrit.entities.PatchSet;
import com.google.gerrit.entities.Project;
@@ -1239,15 +1238,9 @@
@Operator
public Predicate<ChangeData> ownerin(String group) throws QueryParseException, IOException {
- GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, group);
- if (g == null) {
- throw error("Group " + group + " not found");
- }
-
+ GroupReference g = parseGroup(group);
AccountGroup.UUID groupId = g.getUUID();
- GroupDescription.Basic groupDescription = args.groupBackend.get(groupId);
- if (!(groupDescription instanceof GroupDescription.Internal)
- || containsExernalSubGroups((GroupDescription.Internal) groupDescription)) {
+ if (args.groupBackend.isOrContainsExternalGroup(groupId)) {
return new OwnerinPredicate(args.userFactory, groupId);
}
@@ -1265,15 +1258,9 @@
throw new QueryParseException("'uploader' operator is not supported by change index version");
}
- GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, group);
- if (g == null) {
- throw error("Group " + group + " not found");
- }
-
+ GroupReference g = parseGroup(group);
AccountGroup.UUID groupId = g.getUUID();
- GroupDescription.Basic groupDescription = args.groupBackend.get(groupId);
- if (!(groupDescription instanceof GroupDescription.Internal)
- || containsExernalSubGroups((GroupDescription.Internal) groupDescription)) {
+ if (args.groupBackend.isOrContainsExternalGroup(groupId)) {
return new UploaderinPredicate(args.userFactory, groupId);
}
@@ -1323,10 +1310,7 @@
@Operator
public Predicate<ChangeData> reviewerin(String group) throws QueryParseException {
- GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, group);
- if (g == null) {
- throw error("Group " + group + " not found");
- }
+ GroupReference g = parseGroup(group);
return new ReviewerinPredicate(args.userFactory, g.getUUID());
}
@@ -1647,22 +1631,6 @@
return Predicate.and(predicates);
}
- private boolean containsExernalSubGroups(GroupDescription.Internal internalGroup)
- throws IOException {
- for (AccountGroup.UUID subGroupUuid : internalGroup.getSubgroups()) {
- GroupDescription.Basic subGroupDescription = args.groupBackend.get(subGroupUuid);
- if (!(subGroupDescription instanceof GroupDescription.Internal)) {
- return true;
- }
- boolean containsExernalSubGroups =
- containsExernalSubGroups((GroupDescription.Internal) subGroupDescription);
- if (containsExernalSubGroups) {
- return true;
- }
- }
- return false;
- }
-
private Set<Account.Id> getMembers(AccountGroup.UUID g) throws IOException {
Set<Account.Id> accounts;
Set<Account.Id> allMembers =
diff --git a/java/com/google/gerrit/server/query/change/LabelPredicate.java b/java/com/google/gerrit/server/query/change/LabelPredicate.java
index cbf5df4..15356f9 100644
--- a/java/com/google/gerrit/server/query/change/LabelPredicate.java
+++ b/java/com/google/gerrit/server/query/change/LabelPredicate.java
@@ -23,6 +23,7 @@
import com.google.gerrit.index.query.RangeUtil;
import com.google.gerrit.index.query.RangeUtil.Range;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.util.LabelVote;
@@ -40,6 +41,7 @@
protected final String value;
protected final Set<Account.Id> accounts;
protected final AccountGroup.UUID group;
+ protected final GroupBackend groupBackend;
protected Args(
ProjectCache projectCache,
@@ -47,13 +49,15 @@
IdentifiedUser.GenericFactory userFactory,
String value,
Set<Account.Id> accounts,
- AccountGroup.UUID group) {
+ AccountGroup.UUID group,
+ GroupBackend groupBackend) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.userFactory = userFactory;
this.value = value;
this.accounts = accounts;
this.group = group;
+ this.groupBackend = groupBackend;
}
}
@@ -78,7 +82,14 @@
AccountGroup.UUID group) {
super(
predicates(
- new Args(a.projectCache, a.permissionBackend, a.userFactory, value, accounts, group)));
+ new Args(
+ a.projectCache,
+ a.permissionBackend,
+ a.userFactory,
+ value,
+ accounts,
+ group,
+ a.groupBackend)));
this.value = value;
}
@@ -147,7 +158,7 @@
}
protected static Predicate<ChangeData> equalsLabelPredicate(Args args, String label, int expVal) {
- if (args.group != null && !args.group.isInternalGroup()) {
+ if (args.groupBackend.isOrContainsExternalGroup(args.group)) {
// 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.
@@ -164,7 +175,7 @@
}
protected static Predicate<ChangeData> magicLabelPredicate(Args args, MagicLabelVote mlv) {
- if (args.group != null && !args.group.isInternalGroup()) {
+ if (args.groupBackend.isOrContainsExternalGroup(args.group)) {
// 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.
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 74929ad..bda17da 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1356,12 +1356,25 @@
// create group and add users
AccountGroup.UUID external_group1 = AccountGroup.uuid("testbackend:group1");
AccountGroup.UUID external_group2 = AccountGroup.uuid("testbackend:group2");
+ String nameOfGroupThatContainsExternalGroupAsSubgroup = "test-group-1";
+ String nameOfGroupThatContainsExternalGroupAsSubSubgroup = "test-group-2";
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)));
+ AccountGroup.UUID uuidOfGroupThatContainsExternalGroupAsSubgroup =
+ groupOperations
+ .newGroup()
+ .name(nameOfGroupThatContainsExternalGroupAsSubgroup)
+ .addSubgroup(external_group1)
+ .create();
+ groupOperations
+ .newGroup()
+ .name(nameOfGroupThatContainsExternalGroupAsSubSubgroup)
+ .addSubgroup(uuidOfGroupThatContainsExternalGroupAsSubgroup)
+ .create();
Change change1 = insert(repo, newChange(repo), user1);
Change change2 = insert(repo, newChange(repo), user1);
@@ -1380,6 +1393,10 @@
assertQuery("label:Code-Review=+1," + external_group1.get(), change1);
assertQuery("label:Code-Review=+1,group=" + external_group1.get(), change1);
+ assertQuery(
+ "label:Code-Review=+1,group=" + nameOfGroupThatContainsExternalGroupAsSubgroup, change1);
+ assertQuery(
+ "label:Code-Review=+1,group=" + nameOfGroupThatContainsExternalGroupAsSubSubgroup, 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());
@@ -1387,6 +1404,11 @@
// 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,group=" + nameOfGroupThatContainsExternalGroupAsSubgroup, change2);
+ assertQuery(
+ "-label:Code-Review=+1,group=" + nameOfGroupThatContainsExternalGroupAsSubSubgroup,
+ 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);
diff --git a/tools/maven/gerrit-acceptance-framework_pom.xml b/tools/maven/gerrit-acceptance-framework_pom.xml
index 3850801..2fe03ec 100644
--- a/tools/maven/gerrit-acceptance-framework_pom.xml
+++ b/tools/maven/gerrit-acceptance-framework_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-acceptance-framework</artifactId>
- <version>3.5.5-SNAPSHOT</version>
+ <version>3.5.6-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Acceptance Test Framework</name>
<description>Framework for Gerrit's acceptance tests</description>
diff --git a/tools/maven/gerrit-extension-api_pom.xml b/tools/maven/gerrit-extension-api_pom.xml
index 6ecc0dd..1892142 100644
--- a/tools/maven/gerrit-extension-api_pom.xml
+++ b/tools/maven/gerrit-extension-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-extension-api</artifactId>
- <version>3.5.5-SNAPSHOT</version>
+ <version>3.5.6-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Extension API</name>
<description>API for Gerrit Extensions</description>
diff --git a/tools/maven/gerrit-plugin-api_pom.xml b/tools/maven/gerrit-plugin-api_pom.xml
index 82ae2f613..2687bc7 100644
--- a/tools/maven/gerrit-plugin-api_pom.xml
+++ b/tools/maven/gerrit-plugin-api_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-plugin-api</artifactId>
- <version>3.5.5-SNAPSHOT</version>
+ <version>3.5.6-SNAPSHOT</version>
<packaging>jar</packaging>
<name>Gerrit Code Review - Plugin API</name>
<description>API for Gerrit Plugins</description>
diff --git a/tools/maven/gerrit-war_pom.xml b/tools/maven/gerrit-war_pom.xml
index 3e87a94..d7f1726 100644
--- a/tools/maven/gerrit-war_pom.xml
+++ b/tools/maven/gerrit-war_pom.xml
@@ -2,7 +2,7 @@
<modelVersion>4.0.0</modelVersion>
<groupId>com.google.gerrit</groupId>
<artifactId>gerrit-war</artifactId>
- <version>3.5.5-SNAPSHOT</version>
+ <version>3.5.6-SNAPSHOT</version>
<packaging>war</packaging>
<name>Gerrit Code Review - WAR</name>
<description>Gerrit WAR</description>
diff --git a/version.bzl b/version.bzl
index a50f19a..45246e8 100644
--- a/version.bzl
+++ b/version.bzl
@@ -2,4 +2,4 @@
# Used by :api_install and :api_deploy targets
# when talking to the destination repository.
#
-GERRIT_VERSION = "3.5.5-SNAPSHOT"
+GERRIT_VERSION = "3.5.6-SNAPSHOT"
