opendev/gerrit
Code Issues Proposed changes

Merge "Perform user.getEffectiveGroups() less eagerly"

Browse Source
This commit is contained in:
Edwin Kempin
2015-02-16 14:05:27 +00:00
committed by Gerrit Code Review
parent 2b1fb34c59 f7569d0cb2
commit a4a16526ba
18 changed files with 115 additions and 28 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
gerrit-server/src/main/java/com/google/gerrit/server/AnonymousUser.java
View File

@@ -14,6 +14,8 @@
package com.google.gerrit.server;
import com.google.common.collect.Iterables;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.CapabilityControl;
@@ -38,6 +40,11 @@ public class AnonymousUser extends CurrentUser {
return new ListGroupMembership(Collections.singleton(SystemGroupBackend.ANONYMOUS_USERS));
}
@Override
public boolean memberOfAny(Iterable<AccountGroup.UUID> ids) {
return Iterables.contains(ids, SystemGroupBackend.ANONYMOUS_USERS);
}
@Override
public Set<Change.Id> getStarredChanges() {
return Collections.emptySet();

10
gerrit-server/src/main/java/com/google/gerrit/server/CurrentUser.java
View File

@@ -14,12 +14,14 @@
package com.google.gerrit.server;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
import com.google.inject.servlet.RequestScoped;
import java.util.Arrays;
import java.util.Collection;
import java.util.Set;
@@ -77,6 +79,14 @@ public abstract class CurrentUser {
*/
public abstract GroupMembership getEffectiveGroups();
public boolean memberOfAny(Iterable<AccountGroup.UUID> ids) {
return getEffectiveGroups().containsAnyOf(ids);
}
public boolean memberOf(AccountGroup.UUID id) {
return memberOfAny(Arrays.asList(id));
}
/** Set of changes starred by this user. */
public abstract Set<Change.Id> getStarredChanges();

6
gerrit-server/src/main/java/com/google/gerrit/server/IdentifiedUser.java
View File

@@ -23,6 +23,7 @@ import com.google.gerrit.reviewdb.client.AccountDiffPreference;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.StarredChange;
import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountCache;
import com.google.gerrit.server.account.AccountState;
@@ -324,6 +325,11 @@ public class IdentifiedUser extends CurrentUser {
return effectiveGroups;
}
@Override
public boolean memberOfAny(Iterable<UUID> ids) {
return groupBackend.memberOfAny(this, ids);
}
@Override
public Set<Change.Id> getStarredChanges() {
if (starredChanges == null) {

6
gerrit-server/src/main/java/com/google/gerrit/server/InternalUser.java
View File

@@ -17,6 +17,7 @@ package com.google.gerrit.server;
import com.google.common.annotations.VisibleForTesting;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
import com.google.inject.Inject;
@@ -51,6 +52,11 @@ public class InternalUser extends CurrentUser {
return GroupMembership.EMPTY;
}
@Override
public boolean memberOfAny(Iterable<UUID> ids) {
return false;
}
@Override
public Set<Change.Id> getStarredChanges() {
return Collections.emptySet();

6
gerrit-server/src/main/java/com/google/gerrit/server/PeerDaemonUser.java
View File

@@ -16,6 +16,7 @@ package com.google.gerrit.server;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
import com.google.gerrit.server.account.CapabilityControl;
import com.google.gerrit.server.account.GroupMembership;
import com.google.inject.Inject;
@@ -49,6 +50,11 @@ public class PeerDaemonUser extends CurrentUser {
return GroupMembership.EMPTY;
}
@Override
public boolean memberOfAny(Iterable<UUID> ids) {
return false;
}
@Override
public Set<Change.Id> getStarredChanges() {
return Collections.emptySet();

26
gerrit-server/src/main/java/com/google/gerrit/server/account/AbstractGroupBackend.java Normal file
View File

@@ -0,0 +1,26 @@
// Copyright (C) 2015 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.account;
import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
import com.google.gerrit.server.IdentifiedUser;
public abstract class AbstractGroupBackend implements GroupBackend {
@Override
public boolean memberOfAny(IdentifiedUser user, Iterable<UUID> ids) {
return membershipsOf(user).containsAnyOf(ids);
}
}

15
gerrit-server/src/main/java/com/google/gerrit/server/account/CapabilityControl.java
View File

@@ -183,10 +183,9 @@ public class CapabilityControl {
// the 'CI Servers' actually use the BATCH queue while everyone else gets
// to use the INTERACTIVE queue without additional grants.
//
GroupMembership groups = user.getEffectiveGroups();
boolean batch = false;
for (PermissionRule r : capabilities.priority) {
if (match(groups, r)) {
if (user.memberOf(r.getGroup().getUUID())) {
switch (r.getAction()) {
case INTERACTIVE:
if (!SystemGroupBackend.isAnonymousOrRegistered(r.getGroup())) {
@@ -265,9 +264,8 @@ public class CapabilityControl {
return rules;
}
GroupMembership groups = user.getEffectiveGroups();
if (rules.size() == 1) {
if (!match(groups, rules.get(0))) {
if (!user.memberOf(rules.get(0).getGroup().getUUID())) {
rules = Collections.emptyList();
}
effective.put(permissionName, rules);
@@ -276,7 +274,7 @@ public class CapabilityControl {
List<PermissionRule> mine = new ArrayList<>(rules.size());
for (PermissionRule rule : rules) {
if (match(groups, rule)) {
if (user.memberOf(rule.getGroup().getUUID())) {
mine.add(rule);
}
}
@@ -304,11 +302,6 @@ public class CapabilityControl {
return rule.getGroup().getUUID();
}
});
return user.getEffectiveGroups().containsAnyOf(ids);
}
private static boolean match(GroupMembership groups,
PermissionRule rule) {
return groups.contains(rule.getGroup().getUUID());
return user.memberOfAny(ids);
}
}

8
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupBackend.java
View File

@@ -50,4 +50,12 @@ public interface GroupBackend {
/** @return the group membership checker for the backend. */
GroupMembership membershipsOf(IdentifiedUser user);
/**
* Check if the user is member of any of the given groups.
*
* @return <code>true</code> if the user is a member of at least one of the
* given groups, <code>false</code> otherwise
*/
boolean memberOfAny(IdentifiedUser user, Iterable<AccountGroup.UUID> ids);
}

4
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java
View File

@@ -133,7 +133,7 @@ public class GroupControl {
*/
return (accountGroup != null && accountGroup.isVisibleToAll())
|| user instanceof InternalUser
|| user.getEffectiveGroups().contains(group.getGroupUUID())
|| user.memberOf(group.getGroupUUID())
|| isOwner()
|| user.getCapabilities().canAdministrateServer();
}
@@ -144,7 +144,7 @@ public class GroupControl {
isOwner = false;
} else if (isOwner == null) {
AccountGroup.UUID ownerUUID = accountGroup.getOwnerGroupUUID();
isOwner = getCurrentUser().getEffectiveGroups().contains(ownerUUID)
isOwner = getCurrentUser().memberOf(ownerUUID)
|| getCurrentUser().getCapabilities().canAdministrateServer();
}
return isOwner;

2
gerrit-server/src/main/java/com/google/gerrit/server/account/IncludingGroupMembership.java
View File

@@ -112,7 +112,7 @@ public class IncludingGroupMembership implements GroupMembership {
}
private boolean search(Set<AccountGroup.UUID> ids) {
return user.getEffectiveGroups().containsAnyOf(ids);
return user.memberOfAny(ids);
}
private ImmutableSet<AccountGroup.UUID> computeKnownGroups() {

2
gerrit-server/src/main/java/com/google/gerrit/server/account/InternalGroupBackend.java
View File

@@ -31,7 +31,7 @@ import java.util.Collection;
/** Implementation of GroupBackend for the internal group system. */
@Singleton
public class InternalGroupBackend implements GroupBackend {
public class InternalGroupBackend extends AbstractGroupBackend {
private static final Function<AccountGroup, GroupReference> ACT_GROUP_TO_GROUP_REF =
new Function<AccountGroup, GroupReference>() {
@Override

23
gerrit-server/src/main/java/com/google/gerrit/server/account/UniversalGroupBackend.java
View File

@@ -19,6 +19,7 @@ import static com.google.gerrit.server.account.GroupBackends.GROUP_REF_NAME_COMP
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Iterables;
import com.google.common.collect.LinkedListMultimap;
import com.google.common.collect.Multimap;
import com.google.common.collect.Sets;
import com.google.gerrit.common.Nullable;
@@ -26,6 +27,7 @@ import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.ProjectControl;
import com.google.inject.Inject;
@@ -43,7 +45,7 @@ import java.util.Set;
* set of GroupBackends.
*/
@Singleton
public class UniversalGroupBackend implements GroupBackend {
public class UniversalGroupBackend extends AbstractGroupBackend {
private static final Logger log =
LoggerFactory.getLogger(UniversalGroupBackend.class);
@@ -95,6 +97,25 @@ public class UniversalGroupBackend implements GroupBackend {
return new UniversalGroupMembership(user);
}
@Override
public boolean memberOfAny(IdentifiedUser user, Iterable<AccountGroup.UUID> ids) {
Multimap<GroupBackend, AccountGroup.UUID> groups = LinkedListMultimap.create();
for (AccountGroup.UUID uuid : ids) {
for (GroupBackend g : backends) {
if (g.handles(uuid)) {
groups.put(g, uuid);
}
}
}
for (Map.Entry<GroupBackend, Collection<UUID>> e : groups.asMap().entrySet()) {
if (e.getKey().memberOfAny(user, e.getValue())) {
return true;
}
}
return false;
}
private class UniversalGroupMembership implements GroupMembership {
private final Map<GroupBackend, GroupMembership> memberships;

4
gerrit-server/src/main/java/com/google/gerrit/server/auth/ldap/LdapGroupBackend.java
View File

@@ -29,7 +29,7 @@ import com.google.gerrit.reviewdb.client.AccountExternalId;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.AbstractGroupBackend;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
import com.google.gerrit.server.auth.ldap.Helper.LdapSchema;
@@ -59,7 +59,7 @@ import javax.security.auth.login.LoginException;
/**
* Implementation of GroupBackend for the LDAP group system.
*/
public class LdapGroupBackend implements GroupBackend {
public class LdapGroupBackend extends AbstractGroupBackend {
private static final Logger log = LoggerFactory.getLogger(LdapGroupBackend.class);
private static final String LDAP_NAME = "ldap/";

4
gerrit-server/src/main/java/com/google/gerrit/server/group/SystemGroupBackend.java
View File

@@ -22,7 +22,7 @@ import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupReference;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.AbstractGroupBackend;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.account.ListGroupMembership;
import com.google.gerrit.server.project.ProjectControl;
@@ -36,7 +36,7 @@ import java.util.Map;
import java.util.SortedMap;
import java.util.TreeMap;
public class SystemGroupBackend implements GroupBackend {
public class SystemGroupBackend extends AbstractGroupBackend {
/** Common UUID assigned to the "Anonymous Users" group. */
public static final AccountGroup.UUID ANONYMOUS_USERS =
new AccountGroup.UUID("global:Anonymous-Users");

10
gerrit-server/src/main/java/com/google/gerrit/server/project/ProjectControl.java
View File

@@ -34,7 +34,6 @@ import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.InternalUser;
import com.google.gerrit.server.account.GroupMembership;
import com.google.gerrit.server.change.IncludedInResolver;
import com.google.gerrit.server.config.CanonicalWebUrl;
import com.google.gerrit.server.config.GitReceivePackGroups;
@@ -299,8 +298,7 @@ public class ProjectControl {
private boolean isDeclaredOwner() {
if (declaredOwner == null) {
GroupMembership effectiveGroups = user.getEffectiveGroups();
declaredOwner = effectiveGroups.containsAnyOf(state.getAllOwners());
declaredOwner = user.memberOfAny(state.getAllOwners());
}
return declaredOwner;
}
@@ -374,11 +372,11 @@ public class ProjectControl {
}
}
if (iUser.getEffectiveGroups().containsAnyOf(okGroupIds)) {
if (iUser.memberOfAny(okGroupIds)) {
return Capable.OK;
}
if (iUser.getEffectiveGroups().containsAnyOf(missingInfoGroupIds)) {
if (iUser.memberOfAny(missingInfoGroupIds)) {
final StringBuilder msg = new StringBuilder();
for (ContributorAgreement ca : contributorAgreements) {
if (ca.isRequireContactInformation()) {
@@ -512,7 +510,7 @@ public class ProjectControl {
} else if (SystemGroupBackend.CHANGE_OWNER.equals(uuid)) {
return isChangeOwner;
} else {
return user.getEffectiveGroups().contains(uuid);
return user.memberOf(uuid);
}
}

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/EqualsLabelPredicate.java
View File

@@ -131,7 +131,7 @@ class EqualsLabelPredicate extends IndexPredicate<ChangeData> {
return false;
}
if (group != null && !reviewer.getEffectiveGroups().contains(group)) {
if (group != null && !reviewer.memberOf(group)) {
return false;
}

2
gerrit-server/src/main/java/com/google/gerrit/server/query/change/OwnerinPredicate.java
View File

@@ -47,7 +47,7 @@ class OwnerinPredicate extends OperatorPredicate<ChangeData> {
}
final IdentifiedUser owner = userFactory.create(dbProvider,
change.getOwner());
return owner.getEffectiveGroups().contains(uuid);
return owner.memberOf(uuid);
}
@Override

6
gerrit-server/src/test/java/com/google/gerrit/server/project/Util.java
View File

@@ -29,6 +29,7 @@ import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.AccountProjectWatch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.reviewdb.client.AccountGroup.UUID;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.rules.PrologEnvironment;
import com.google.gerrit.rules.RulesCache;
@@ -356,6 +357,11 @@ public class Util {
return groups;
}
@Override
public boolean memberOfAny(Iterable<UUID> ids) {
return getEffectiveGroups().containsAnyOf(ids);
}
@Override
public String getUserName() {
return username;