Merge branch 'stable-3.5' into stable-3.6
* stable-3.5:
Set version to 3.5.6-SNAPSHOT
Set version to 3.5.5
GroupBackend: Provide a default isOrContainsExternalGroup
Fix LabelPredicate group matching for included external groups
ChangeQueryBuilder: De-dup parseGroup()
ChangeQueryBuilder: containsExternalSubGroups fixup
Fix ownerin/uploaderin for internal groups that include external groups
Fix rendering String input values in plugin project options
Update JGit to a1901305b
Fix negated label: queries with external groups
Fix documentation for 'secure-store-lib'
Bump JGit to 801a56b48
Bump sshd version to 2.9.2
Bump SSHD version to 2.9.1
Documentation: fixup ExternalId case insensitivity
Change-Id: I7412b04ea7a59f6fdaa9a661aee76460aee66919
Release-Notes: skip
diff --git a/Documentation/externalid-case-insensitivity.txt b/Documentation/externalid-case-insensitivity.txt
index b4e8140..57f492c 100644
--- a/Documentation/externalid-case-insensitivity.txt
+++ b/Documentation/externalid-case-insensitivity.txt
@@ -1,29 +1,30 @@
:linkattrs:
= Gerrit Code Review - ExternalId case insensitivity
-Gerrit usernames are case insensitive by default: e.g. johndoe and JohnDoe
-represents the same account. However, for installations older than v3.5.x,
-the usernames were case sensitive, e.g. johndoe and JohnDoe can both exist
+Gerrit usernames are case insensitive by default: e.g. `johndoe` and `JohnDoe`
+represent the same account. However, for installations older than v3.5.x,
+the usernames were case sensitive, e.g. `johndoe` and `JohnDoe` can both exist
as separate accounts. This could lead to issues when migrating an account
-from LDAP to an internal account, if ldap.localUsernameToLowerCase was set.
-Such usernames can also be rather confusing for users, if they try to identify
-authors of comments or changes.
+from LDAP to an internal account, if
+xref:config-gerrit.txt#ldap.localUsernameToLowerCase[ldap.localUsernameToLowerCase]
+was set. Such usernames can also be rather confusing for users, if they try to
+identify authors of comments or changes.
When Gerrit handles case insensitive usernames (external IDs using the
-`gerrit:` or `username:` scheme, their external IDs SHA-1 is always computed
+`gerrit:` or `username:` scheme), their external IDs SHA-1 is always computed
using the lowercase external ID, hence there cannot be any account differing
only in the capitalization of their usernames.
Gerrit installations older than v3.5.x that are switching to the case-insensitive
-username need to migrating all their existing accounts SHA-1s.
+username need to migrate all their existing accounts SHA-1s.
[[migration]]
== Migration
Migrating external ID notes can take several minutes for large sites(for example
-migration ~45000 accounts can take up to five minutes), so administrators choose
-whether to do the migration offline or online, depending on their available
-resources and tolerance for downtime.
+migration ++~++45000 accounts can take up to five minutes), so administrators
+choose whether to do the migration offline or online, depending on their
+available resources and tolerance for downtime.
NOTE: Migration is required only on Gerrit primary instances.
@@ -31,8 +32,10 @@
=== Offline
To run the offline migration execute following steps:
+
* Stop all Gerrit primary instances
* Set the `auth.userNameCaseInsensitive` to false
+
----
[auth]
userNameCaseInsensitive = false
@@ -46,7 +49,7 @@
[--batch]
--
-See: link:pgm-ChangeExternalIdCaseSensitivity.html
+See: link:pgm-ChangeExternalIdCaseSensitivity.html[]
* During the migration `auth.userNameCaseInsensitive` will be set to true
on a node which is executing the migration. When the migration is finished,
@@ -69,13 +72,14 @@
$ ssh -p <port> <host> gerrit migrate-externalids-to-insensitive
----
-See: link:cmd-migrate-externalids-to-insensitive.html
+See: link:cmd-migrate-externalids-to-insensitive.html[]
[online-ha-migration]
== Online migration for high-availability setup
To start the online migration with a setup containing multiple primary
instances execute following steps:
+
* On all Gerrit primary instances set `auth.userNameCaseInsensitive` and
`auth.userNameCaseInsensitiveMigrationMode` and perform a rolling restart
----
@@ -88,7 +92,7 @@
$ ssh -p <port> <host> gerrit migrate-externalids-to-insensitive
----
-See: link:cmd-migrate-externalids-to-insensitive.html
+See: link:cmd-migrate-externalids-to-insensitive.html[]
* When the migration is finished, on all other primary nodes set
`auth.userNameCaseInsensitiveMigrationMode` to false and perform a
@@ -105,6 +109,7 @@
from the case sensitive external ID.
To rollback external ID notes migration execute following steps:
+
* Stop all Gerrit primary instances
* Set the `auth.userNameCaseInsensitive` to true
----
@@ -120,7 +125,7 @@
[--batch]
--
-See: link:pgm-ChangeExternalIdCaseSensitivity.html
+See: link:pgm-ChangeExternalIdCaseSensitivity.html[]
* During the migration `auth.userNameCaseInsensitive` will be set to false
on a node which is executing the migration. When the migration is finished,
diff --git a/Documentation/pgm-SwitchSecureStore.txt b/Documentation/pgm-SwitchSecureStore.txt
index 47de1be..818ce2b 100644
--- a/Documentation/pgm-SwitchSecureStore.txt
+++ b/Documentation/pgm-SwitchSecureStore.txt
@@ -7,7 +7,7 @@
[verse]
--
_java_ -jar gerrit.war _SwitchSecureStore_
- [--new-secure-store-lib]
+ [--new-secure-store-lib=<PATH_TO_JAR>]
--
== DESCRIPTION
diff --git a/Documentation/pgm-init.txt b/Documentation/pgm-init.txt
index 4a758c3..3c9e3fc 100644
--- a/Documentation/pgm-init.txt
+++ b/Documentation/pgm-init.txt
@@ -16,7 +16,7 @@
[--list-plugins]
[--install-plugin=<PLUGIN_NAME>]
[--install-all-plugins]
- [--secure-store-lib]
+ [--secure-store-lib=<PATH_TO_JAR>]
[--dev]
[--skip-all-downloads]
[--skip-download=<LIBRARY_NAME>]
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index 4298663..633081e 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -63,6 +63,7 @@
"//java/com/google/gerrit/pgm/util",
"//java/com/google/gerrit/truth",
"//java/com/google/gerrit/acceptance/config",
+ "//java/com/google/gerrit/acceptance/testsuite/group",
"//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/server/fixes/testing",
"//java/com/google/gerrit/server/data",
diff --git a/java/com/google/gerrit/acceptance/testsuite/group/BUILD b/java/com/google/gerrit/acceptance/testsuite/group/BUILD
new file mode 100644
index 0000000..d4f1175
--- /dev/null
+++ b/java/com/google/gerrit/acceptance/testsuite/group/BUILD
@@ -0,0 +1,25 @@
+load("@rules_java//java:defs.bzl", "java_library")
+
+package(default_testonly = 1)
+
+java_library(
+ name = "group",
+ srcs = glob(["*.java"]),
+ visibility = ["//visibility:public"],
+ deps = [
+ "//java/com/google/gerrit/acceptance:function",
+ "//java/com/google/gerrit/common:annotations",
+ "//java/com/google/gerrit/common:server",
+ "//java/com/google/gerrit/entities",
+ "//java/com/google/gerrit/exceptions",
+ "//java/com/google/gerrit/extensions:api",
+ "//java/com/google/gerrit/server",
+ "//lib:guava",
+ "//lib:jgit",
+ "//lib:jgit-junit",
+ "//lib/auto:auto-value",
+ "//lib/auto:auto-value-annotations",
+ "//lib/commons:lang3",
+ "//lib/guice",
+ ],
+)
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/group/testing/TestGroupBackend.java b/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
index 12d8c93..2d9c798 100644
--- a/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
+++ b/java/com/google/gerrit/server/group/testing/TestGroupBackend.java
@@ -102,6 +102,11 @@
memberships.put(user, membership);
}
+ /** Remove the memberships of the given user. No-op if the user does not have any memberships. */
+ public void removeMembershipsOf(Account.Id user) {
+ memberships.remove(user);
+ }
+
@Override
public boolean handles(AccountGroup.UUID uuid) {
if (uuid != null) {
diff --git a/java/com/google/gerrit/server/index/change/ChangeField.java b/java/com/google/gerrit/server/index/change/ChangeField.java
index ee272b7..c302a36 100644
--- a/java/com/google/gerrit/server/index/change/ChangeField.java
+++ b/java/com/google/gerrit/server/index/change/ChangeField.java
@@ -856,6 +856,10 @@
+ (count != null ? ",count=" + count : "");
}
+ public static String formatLabel(String label, String value, @Nullable Integer count) {
+ return formatLabel(label, value, /* accountId= */ null, count);
+ }
+
public static String formatLabel(
String label, String value, @Nullable Account.Id accountId, @Nullable Integer count) {
return label.toLowerCase()
diff --git a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
index c494024..d6b26b9f 100644
--- a/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
+++ b/java/com/google/gerrit/server/query/change/ChangeQueryBuilder.java
@@ -35,7 +35,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;
@@ -1295,14 +1294,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)) {
+ if (args.groupBackend.isOrContainsExternalGroup(groupId)) {
return new OwnerinPredicate(args.userFactory, groupId);
}
@@ -1318,14 +1312,9 @@
public Predicate<ChangeData> uploaderin(String group) throws QueryParseException, IOException {
checkFieldAvailable(ChangeField.UPLOADER, "uploaderin");
- 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)) {
+ if (args.groupBackend.isOrContainsExternalGroup(groupId)) {
return new UploaderinPredicate(args.userFactory, groupId);
}
@@ -1372,10 +1361,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());
}
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 6aacfc9..0000000
--- a/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
+++ /dev/null
@@ -1,187 +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.common.Nullable;
-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 com.google.gerrit.server.query.change.ChangeData.StorageConstraint;
-import java.util.Optional;
-
-public class EqualsLabelPredicate extends ChangeIndexPostFilterPredicate {
- protected final ProjectCache projectCache;
- protected final PermissionBackend permissionBackend;
- protected final IdentifiedUser.GenericFactory userFactory;
- /** label name to be matched. */
- protected final String label;
-
- /** Expected vote value for the label. */
- protected final int expVal;
-
- /**
- * Number of times the value {@link #expVal} for label {@link #label} should occur. If null, match
- * with any count greater or equal to 1.
- */
- @Nullable protected final Integer count;
-
- /** Account ID that has voted on the label. */
- protected final Account.Id account;
-
- protected final AccountGroup.UUID group;
-
- public EqualsLabelPredicate(
- LabelPredicate.Args args,
- String label,
- int expVal,
- Account.Id account,
- @Nullable Integer count) {
- super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account, count));
- this.permissionBackend = args.permissionBackend;
- this.projectCache = args.projectCache;
- this.userFactory = args.userFactory;
- this.count = count;
- 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;
- }
-
- if (Integer.valueOf(0).equals(count)) {
- // We don't match against count=0 so that the computation is identical to the stored values
- // in the index. We do that since computing count=0 requires looping on all {label_type,
- // vote_value} for the change and storing a {count=0} format for it in the change index which
- // is computationally expensive.
- 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;
- int matchingVotes = 0;
- StorageConstraint currentStorageConstraint = object.getStorageConstraint();
- 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())) {
- matchingVotes += 1;
- }
- }
- }
- object.setStorageConstraint(currentStorageConstraint);
- if (!hasVote && expVal == 0) {
- return true;
- }
-
- return count == null ? matchingVotes >= 1 : matchingVotes == count;
- }
-
- 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 0000000..f572063
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/EqualsLabelPredicates.java
@@ -0,0 +1,234 @@
+// 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.common.Nullable;
+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 com.google.gerrit.server.query.change.ChangeData.StorageConstraint;
+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, @Nullable Integer count) {
+ super(ChangeQueryBuilder.FIELD_LABEL, ChangeField.formatLabel(label, expVal, count));
+ matcher = new Matcher(args, label, expVal, count);
+ }
+
+ @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, @Nullable Integer count) {
+ this(args, label, expVal, null, count);
+ }
+
+ public IndexEqualsLabelPredicate(
+ LabelPredicate.Args args,
+ String label,
+ int expVal,
+ Account.Id account,
+ @Nullable Integer count) {
+ super(ChangeField.LABEL, ChangeField.formatLabel(label, expVal, account, count));
+ this.matcher = new Matcher(args, label, expVal, account, count);
+ }
+
+ @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;
+ /** label name to be matched. */
+ protected final String label;
+ /** Expected vote value for the label. */
+ protected final int expVal;
+
+ /**
+ * Number of times the value {@link #expVal} for label {@link #label} should occur. If null,
+ * match with any count greater or equal to 1.
+ */
+ @Nullable protected final Integer count;
+
+ /** Account ID that has voted on the label. */
+ protected final Account.Id account;
+
+ protected final AccountGroup.UUID group;
+
+ public Matcher(LabelPredicate.Args args, String label, int expVal, @Nullable Integer count) {
+ this(args, label, expVal, null, count);
+ }
+
+ public Matcher(
+ LabelPredicate.Args args,
+ String label,
+ int expVal,
+ Account.Id account,
+ @Nullable Integer count) {
+ 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;
+ this.count = count;
+ }
+
+ public boolean match(ChangeData cd) {
+ Change c = cd.change();
+ if (c == null) {
+ // The change has disappeared.
+ return false;
+ }
+
+ if (Integer.valueOf(0).equals(count)) {
+ // We don't match against count=0 so that the computation is identical to the stored values
+ // in the index. We do that since computing count=0 requires looping on all {label_type,
+ // vote_value} for the change and storing a {count=0} format for it in the change index
+ // which is computationally expensive.
+ 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;
+ int matchingVotes = 0;
+ StorageConstraint currentStorageConstraint = cd.getStorageConstraint();
+ cd.setStorageConstraint(ChangeData.StorageConstraint.INDEX_PRIMARY_NOTEDB_SECONDARY);
+ for (PatchSetApproval psa : cd.currentApprovals()) {
+ if (labelType.matches(psa)) {
+ hasVote = true;
+ if (match(cd, psa)) {
+ matchingVotes += 1;
+ }
+ }
+ }
+ cd.setStorageConstraint(currentStorageConstraint);
+ if (!hasVote && expVal == 0) {
+ return true;
+ }
+
+ return count == null ? matchingVotes >= 1 : matchingVotes == count;
+ }
+
+ 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 2a5a47d..2afaada 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;
@@ -44,6 +45,7 @@
protected final AccountGroup.UUID group;
protected final Integer count;
protected final PredicateArgs.Operator countOp;
+ protected final GroupBackend groupBackend;
protected Args(
ProjectCache projectCache,
@@ -53,7 +55,8 @@
Set<Account.Id> accounts,
AccountGroup.UUID group,
@Nullable Integer count,
- @Nullable PredicateArgs.Operator countOp) {
+ @Nullable PredicateArgs.Operator countOp,
+ GroupBackend groupBackend) {
this.projectCache = projectCache;
this.permissionBackend = permissionBackend;
this.userFactory = userFactory;
@@ -62,6 +65,7 @@
this.group = group;
this.count = count;
this.countOp = countOp;
+ this.groupBackend = groupBackend;
}
}
@@ -96,7 +100,8 @@
accounts,
group,
count,
- countOp)));
+ countOp,
+ a.groupBackend)));
this.value = value;
}
@@ -180,24 +185,36 @@
protected static Predicate<ChangeData> equalsLabelPredicate(
Args args, String label, int expVal, @Nullable Integer count) {
+ 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.
+ return new EqualsLabelPredicates.PostFilterEqualsLabelPredicate(args, label, expVal, count);
+ }
if (args.accounts == null || args.accounts.isEmpty()) {
- return new EqualsLabelPredicate(args, label, expVal, null, count);
+ return new EqualsLabelPredicates.IndexEqualsLabelPredicate(args, label, expVal, count);
}
List<Predicate<ChangeData>> r = new ArrayList<>();
for (Account.Id a : args.accounts) {
- r.add(new EqualsLabelPredicate(args, label, expVal, a, count));
+ r.add(new EqualsLabelPredicates.IndexEqualsLabelPredicate(args, label, expVal, a, count));
}
return or(r);
}
protected static Predicate<ChangeData> magicLabelPredicate(
Args args, MagicLabelVote mlv, @Nullable Integer count) {
+ 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.
+ return new MagicLabelPredicates.PostFilterMagicLabelPredicate(args, mlv, count);
+ }
if (args.accounts == null || args.accounts.isEmpty()) {
- return new MagicLabelPredicate(args, mlv, /* account= */ null, count);
+ return new MagicLabelPredicates.IndexMagicLabelPredicate(args, mlv, count);
}
List<Predicate<ChangeData>> r = new ArrayList<>();
for (Account.Id a : args.accounts) {
- r.add(new MagicLabelPredicate(args, mlv, a, count));
+ r.add(new MagicLabelPredicates.IndexMagicLabelPredicate(args, mlv, a, count));
}
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 5a81ca1..0000000
--- a/java/com/google/gerrit/server/query/change/MagicLabelPredicate.java
+++ /dev/null
@@ -1,114 +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.common.Nullable;
-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;
- @Nullable private final Integer count;
-
- public MagicLabelPredicate(
- LabelPredicate.Args args,
- MagicLabelVote magicLabelVote,
- Account.Id account,
- @Nullable Integer count) {
- super(
- ChangeField.LABEL,
- ChangeField.formatLabel(
- magicLabelVote.label(), magicLabelVote.value().name(), account, count));
- this.account = account;
- this.args = args;
- this.magicLabelVote = magicLabelVote;
- this.count = count;
- }
-
- @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, count);
- }
-
- 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 0000000..c9c8c45
--- /dev/null
+++ b/java/com/google/gerrit/server/query/change/MagicLabelPredicates.java
@@ -0,0 +1,175 @@
+// 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.common.Nullable;
+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, @Nullable Integer count) {
+ super(args, magicLabelVote, count);
+ }
+
+ @Override
+ protected Predicate<ChangeData> numericPredicate(String label, short value) {
+ return new EqualsLabelPredicates.PostFilterEqualsLabelPredicate(args, label, value, count);
+ }
+ }
+
+ private final PostFilterMatcher matcher;
+
+ public PostFilterMagicLabelPredicate(
+ LabelPredicate.Args args, MagicLabelVote magicLabelVote, @Nullable Integer count) {
+ super(
+ ChangeQueryBuilder.FIELD_LABEL,
+ ChangeField.formatLabel(magicLabelVote.label(), magicLabelVote.value().name(), count));
+ this.matcher = new PostFilterMatcher(args, magicLabelVote, count);
+ }
+
+ @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,
+ @Nullable Integer count) {
+ super(args, magicLabelVote, account, count);
+ }
+
+ @Override
+ protected Predicate<ChangeData> numericPredicate(String label, short value) {
+ return new EqualsLabelPredicates.IndexEqualsLabelPredicate(
+ args, label, value, account, count);
+ }
+ }
+
+ private final Matcher matcher;
+
+ public IndexMagicLabelPredicate(
+ LabelPredicate.Args args, MagicLabelVote magicLabelVote, @Nullable Integer count) {
+ this(args, magicLabelVote, null, count);
+ }
+
+ public IndexMagicLabelPredicate(
+ LabelPredicate.Args args,
+ MagicLabelVote magicLabelVote,
+ Account.Id account,
+ @Nullable Integer count) {
+ super(
+ ChangeField.LABEL,
+ ChangeField.formatLabel(
+ magicLabelVote.label(), magicLabelVote.value().name(), account, count));
+ this.matcher = new IndexMatcher(args, magicLabelVote, account, count);
+ }
+
+ @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;
+ @Nullable protected final Integer count;
+
+ public Matcher(
+ LabelPredicate.Args args, MagicLabelVote magicLabelVote, @Nullable Integer count) {
+ this(args, magicLabelVote, null, count);
+ }
+
+ public Matcher(
+ LabelPredicate.Args args,
+ MagicLabelVote magicLabelVote,
+ Account.Id account,
+ @Nullable Integer count) {
+ this.account = account;
+ this.args = args;
+ this.magicLabelVote = magicLabelVote;
+ this.count = count;
+ }
+
+ 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/java/com/google/gerrit/sshd/ChannelIdTrackingUnknownChannelReferenceHandler.java b/java/com/google/gerrit/sshd/ChannelIdTrackingUnknownChannelReferenceHandler.java
index f8ab90e..b9ca79c 100644
--- a/java/com/google/gerrit/sshd/ChannelIdTrackingUnknownChannelReferenceHandler.java
+++ b/java/com/google/gerrit/sshd/ChannelIdTrackingUnknownChannelReferenceHandler.java
@@ -45,7 +45,7 @@
public class ChannelIdTrackingUnknownChannelReferenceHandler
extends DefaultUnknownChannelReferenceHandler implements ChannelListener {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- public static final AttributeKey<Integer> LAST_CHANNEL_ID_KEY = new AttributeKey<>();
+ public static final AttributeKey<Long> LAST_CHANNEL_ID_KEY = new AttributeKey<>();
public static final ChannelIdTrackingUnknownChannelReferenceHandler TRACKER =
new ChannelIdTrackingUnknownChannelReferenceHandler();
@@ -56,9 +56,9 @@
@Override
public void channelInitialized(Channel channel) {
- int channelId = channel.getId();
+ long channelId = channel.getChannelId();
Session session = channel.getSession();
- Integer lastTracked = session.setAttribute(LAST_CHANNEL_ID_KEY, channelId);
+ Long lastTracked = session.setAttribute(LAST_CHANNEL_ID_KEY, channelId);
logger.atFine().log(
"channelInitialized(%s) updated last tracked channel ID %s => %s",
channel, lastTracked, channelId);
@@ -66,9 +66,9 @@
@Override
public Channel handleUnknownChannelCommand(
- ConnectionService service, byte cmd, int channelId, Buffer buffer) throws IOException {
+ ConnectionService service, byte cmd, long channelId, Buffer buffer) throws IOException {
Session session = service.getSession();
- Integer lastTracked = session.getAttribute(LAST_CHANNEL_ID_KEY);
+ Long lastTracked = session.getAttribute(LAST_CHANNEL_ID_KEY);
if ((lastTracked != null) && (channelId <= lastTracked.intValue())) {
// Use TRACE level in order to avoid messages flooding
logger.atFinest().log(
diff --git a/java/com/google/gerrit/testing/BUILD b/java/com/google/gerrit/testing/BUILD
index 861fa00..e5234fe 100644
--- a/java/com/google/gerrit/testing/BUILD
+++ b/java/com/google/gerrit/testing/BUILD
@@ -15,6 +15,7 @@
runtime_deps = ["//java/com/google/gerrit/index/testing"],
deps = [
"//java/com/google/gerrit/acceptance/config",
+ "//java/com/google/gerrit/acceptance/testsuite/group",
"//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/auth",
"//java/com/google/gerrit/common:annotations",
diff --git a/java/com/google/gerrit/testing/InMemoryModule.java b/java/com/google/gerrit/testing/InMemoryModule.java
index b00cadb..49edd4c 100644
--- a/java/com/google/gerrit/testing/InMemoryModule.java
+++ b/java/com/google/gerrit/testing/InMemoryModule.java
@@ -20,6 +20,8 @@
import com.google.common.base.Strings;
import com.google.common.util.concurrent.ListeningExecutorService;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperationsImpl;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperationsImpl;
import com.google.gerrit.auth.AuthModule;
@@ -270,6 +272,7 @@
install(new ConfigExperimentFeaturesModule());
bind(ProjectOperations.class).to(ProjectOperationsImpl.class);
+ bind(GroupOperations.class).to(GroupOperationsImpl.class);
bind(TestGroupBackend.class).in(SINGLETON);
DynamicSet.bind(binder(), GroupBackend.class).to(TestGroupBackend.class);
}
diff --git a/javatests/com/google/gerrit/acceptance/BUILD b/javatests/com/google/gerrit/acceptance/BUILD
index 75c90f2..fe451c4 100644
--- a/javatests/com/google/gerrit/acceptance/BUILD
+++ b/javatests/com/google/gerrit/acceptance/BUILD
@@ -5,6 +5,7 @@
srcs = glob(["**/*.java"]),
deps = [
"//java/com/google/gerrit/acceptance:lib",
+ "//java/com/google/gerrit/acceptance/testsuite/group",
"//java/com/google/gerrit/server/util/time",
"//java/com/google/gerrit/testing:gerrit-test-util",
"//java/com/google/gerrit/truth",
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index 28d9ac7..081fe02 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -50,6 +50,7 @@
import com.google.gerrit.acceptance.ExtensionRegistry.Registration;
import com.google.gerrit.acceptance.FakeSubmitRule;
import com.google.gerrit.acceptance.config.GerritConfig;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.RawInputUtil;
@@ -57,6 +58,7 @@
import com.google.gerrit.entities.AccountGroup;
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.LabelId;
import com.google.gerrit.entities.LabelType;
@@ -204,6 +206,7 @@
@Inject protected AuthRequest.Factory authRequestFactory;
@Inject protected ExternalIdFactory externalIdFactory;
@Inject protected ProjectOperations projectOperations;
+ @Inject protected GroupOperations groupOperations;
@Inject private ProjectConfig.Factory projectConfigFactory;
@@ -752,6 +755,38 @@
assertQuery("ownerin:\"Registered Users\"", change3, change2, change1);
assertQuery("ownerin:\"Registered Users\" project:repo", change3, change2, change1);
assertQuery("ownerin:\"Registered Users\" status:merged", change3);
+
+ GroupDescription.Basic externalGroup = testGroupBackend.create("External Group");
+ try {
+ testGroupBackend.setMembershipsOf(
+ user2, new ListGroupMembership(ImmutableList.of(externalGroup.getGroupUUID())));
+
+ assertQuery("ownerin:\"" + "testbackend:" + externalGroup.getName() + "\"", change3, change2);
+
+ String nameOfGroupThatContainsExternalGroupAsSubgroup = "test-group-1";
+ AccountGroup.UUID uuidOfGroupThatContainsExternalGroupAsSubgroup =
+ groupOperations
+ .newGroup()
+ .name(nameOfGroupThatContainsExternalGroupAsSubgroup)
+ .addSubgroup(externalGroup.getGroupUUID())
+ .create();
+ assertQuery(
+ "ownerin:\"" + nameOfGroupThatContainsExternalGroupAsSubgroup + "\"", change3, change2);
+
+ String nameOfGroupThatContainsExternalGroupAsSubSubgroup = "test-group-2";
+ groupOperations
+ .newGroup()
+ .name(nameOfGroupThatContainsExternalGroupAsSubSubgroup)
+ .addSubgroup(uuidOfGroupThatContainsExternalGroupAsSubgroup)
+ .create();
+ assertQuery(
+ "ownerin:\"" + nameOfGroupThatContainsExternalGroupAsSubSubgroup + "\"",
+ change3,
+ change2);
+ } finally {
+ testGroupBackend.removeMembershipsOf(user2);
+ testGroupBackend.remove(externalGroup.getGroupUUID());
+ }
}
@Test
@@ -766,6 +801,37 @@
CurrentUser user2CurrentUser = userFactory.create(user2);
newPatchSet(repo, change1, user2CurrentUser);
assertQuery("uploaderin:Administrators");
+ assertQuery("uploaderin:\"Registered Users\"", change1);
+
+ GroupDescription.Basic externalGroup = testGroupBackend.create("External Group");
+ try {
+ testGroupBackend.setMembershipsOf(
+ user2, new ListGroupMembership(ImmutableList.of(externalGroup.getGroupUUID())));
+
+ assertQuery("uploaderin:\"" + "testbackend:" + externalGroup.getName() + "\"", change1);
+
+ String nameOfGroupThatContainsExternalGroupAsSubgroup = "test-group-1";
+ AccountGroup.UUID uuidOfGroupThatContainsExternalGroupAsSubgroup =
+ groupOperations
+ .newGroup()
+ .name(nameOfGroupThatContainsExternalGroupAsSubgroup)
+ .addSubgroup(externalGroup.getGroupUUID())
+ .create();
+ assertQuery("uploaderin:\"" + nameOfGroupThatContainsExternalGroupAsSubgroup + "\"", change1);
+
+ String nameOfGroupThatContainsExternalGroupAsSubSubgroup = "test-group-2";
+ groupOperations
+ .newGroup()
+ .name(nameOfGroupThatContainsExternalGroupAsSubSubgroup)
+ .addSubgroup(uuidOfGroupThatContainsExternalGroupAsSubgroup)
+ .create();
+ assertQuery(
+ "uploaderin:\"" + nameOfGroupThatContainsExternalGroupAsSubSubgroup + "\"", change1);
+
+ } finally {
+ testGroupBackend.removeMembershipsOf(user2);
+ testGroupBackend.remove(externalGroup.getGroupUUID());
+ }
}
@Test
@@ -1398,12 +1464,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);
@@ -1422,9 +1501,25 @@
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());
+
+ // 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);
}
@Test
diff --git a/javatests/com/google/gerrit/server/query/change/BUILD b/javatests/com/google/gerrit/server/query/change/BUILD
index 32a646e..57a3c4b 100644
--- a/javatests/com/google/gerrit/server/query/change/BUILD
+++ b/javatests/com/google/gerrit/server/query/change/BUILD
@@ -19,6 +19,7 @@
deps = [
"//java/com/google/gerrit/acceptance:lib",
"//java/com/google/gerrit/acceptance/config",
+ "//java/com/google/gerrit/acceptance/testsuite/group",
"//java/com/google/gerrit/acceptance/testsuite/project",
"//java/com/google/gerrit/common:annotations",
"//java/com/google/gerrit/common:server",
diff --git a/modules/jgit b/modules/jgit
index 82b5aaf..a190130 160000
--- a/modules/jgit
+++ b/modules/jgit
@@ -1 +1 @@
-Subproject commit 82b5aaf7e3e3f881056dd2d4486e02537b0493da
+Subproject commit a1901305b26ed5e0116f138bc02837713d2cf5c3
diff --git a/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.ts b/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.ts
index d5e5027..e23db15 100644
--- a/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.ts
+++ b/polygerrit-ui/app/elements/admin/gr-repo-plugin-config/gr-repo-plugin-config.ts
@@ -184,6 +184,7 @@
) {
return html`
<iron-input
+ .bindValue=${option.info.value ?? ''}
@input=${this._handleStringChange}
data-option-key=${option._key}
>
diff --git a/tools/nongoogle.bzl b/tools/nongoogle.bzl
index d8f7020..e36a383 100644
--- a/tools/nongoogle.bzl
+++ b/tools/nongoogle.bzl
@@ -67,18 +67,18 @@
sha1 = "cb2f351bf4463751201f43bb99865235d5ba07ca",
)
- SSHD_VERS = "2.8.0"
+ SSHD_VERS = "2.9.2"
maven_jar(
name = "sshd-osgi",
artifact = "org.apache.sshd:sshd-osgi:" + SSHD_VERS,
- sha1 = "b2a59b73c045f40d5722b9160d4f909a646d86c9",
+ sha1 = "bac0415734519b2fe433fea196017acf7ed32660",
)
maven_jar(
name = "sshd-sftp",
artifact = "org.apache.sshd:sshd-sftp:" + SSHD_VERS,
- sha1 = "d3cd9bc8d335b3ed1a86d2965deb4d202de27442",
+ sha1 = "7f9089c87b3b44f19998252fd3b68637e3322920",
)
maven_jar(
@@ -89,14 +89,14 @@
maven_jar(
name = "mina-core",
- artifact = "org.apache.mina:mina-core:2.0.21",
- sha1 = "e1a317689ecd438f54e863747e832f741ef8e092",
+ artifact = "org.apache.mina:mina-core:2.0.23",
+ sha1 = "391228b25d3a24434b205444cd262780a9ea61e7",
)
maven_jar(
name = "sshd-mina",
artifact = "org.apache.sshd:sshd-mina:" + SSHD_VERS,
- sha1 = "02f78100cce376198be798a37c84aaf945e8a0f7",
+ sha1 = "765dced3a2b4069bb0c550e18bda057bad8de26f",
)
maven_jar(
