opendev
/
gerrit
Code Issues Proposed changes

Cache instances of type InternalGroup instead of AccountGroup by UUID 

Change-Id: Id4725960eb155945f0344a56e7f36d5f4293750d
This commit is contained in:
Alice Kober-Sotzek 2017-08-25 17:42:23 +02:00
parent e23b34a206
commit 3e6c672b99
20 changed files with 135 additions and 154 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
gerrit-acceptance-framework/src/test/java/com/google/gerrit/acceptance/AbstractDaemonTest.java
View File

@ -92,6 +92,7 @@ import com.google.gerrit.server.config.PluginConfigFactory;
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.gerrit.server.git.MetaDataUpdate;
import com.google.gerrit.server.git.ProjectConfig;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.index.change.ChangeIndex;
import com.google.gerrit.server.index.change.ChangeIndexCollection;
@ -354,7 +355,7 @@ public abstract class AbstractDaemonTest {
// removes all indexed data.
// As a workaround, we simply reindex all available groups here.
for (AccountGroup group : groupCache.all()) {
groupCache.evict(group);
groupCache.evict(group.getGroupUUID(), group.getId(), group.getNameKey());
}
admin = accountCreator.admin();
@ -1187,8 +1188,9 @@ public abstract class AbstractDaemonTest {
String g = createGroup("cla-test-group");
GroupApi groupApi = gApi.groups().id(g);
groupApi.description("CLA test group");
AccountGroup caGroup = groupCache.get(new AccountGroup.UUID(groupApi.detail().id));
GroupReference groupRef = GroupReference.forGroup(caGroup);
InternalGroup caGroup =
groupCache.get(new AccountGroup.UUID(groupApi.detail().id)).orElse(null);
GroupReference groupRef = new GroupReference(caGroup.getGroupUUID(), caGroup.getName());
PermissionRule rule = new PermissionRule(groupRef);
rule.setAction(PermissionRule.Action.ALLOW);
ca = new ContributorAgreement("cla-test");

23
gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
View File

@ -34,6 +34,7 @@ import com.google.gerrit.extensions.restapi.TopLevelResource;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.group.CreateGroup;
import com.google.gerrit.server.group.InternalGroup;
import com.google.inject.Inject;
import java.util.Arrays;
import java.util.List;
@ -44,9 +45,9 @@ import org.junit.Test;
public class SuggestReviewersIT extends AbstractDaemonTest {
@Inject private CreateGroup.Factory createGroupFactory;
private AccountGroup group1;
private AccountGroup group2;
private AccountGroup group3;
private InternalGroup group1;
private InternalGroup group2;
private InternalGroup group3;
private TestAccount user1;
private TestAccount user2;
@ -234,8 +235,8 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
@GerritConfig(name = "addreviewer.maxAllowed", value = "2")
@GerritConfig(name = "addreviewer.maxWithoutConfirmation", value = "1")
public void suggestReviewersGroupSizeConsiderations() throws Exception {
AccountGroup largeGroup = group("large");
AccountGroup mediumGroup = group("medium");
InternalGroup largeGroup = group("large");
InternalGroup mediumGroup = group("medium");
// Both groups have Administrator as a member. Add two users to large
// group to push it past maxAllowed, and one to medium group to push it
@ -424,19 +425,19 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
return gApi.changes().id(changeId).suggestReviewers(query).withLimit(n).get();
}
private AccountGroup group(String name) throws Exception {
private InternalGroup group(String name) throws Exception {
GroupInfo group = createGroupFactory.create(name(name)).apply(TopLevelResource.INSTANCE, null);
return groupCache.get(new AccountGroup.UUID(group.id));
return groupCache.get(new AccountGroup.UUID(group.id)).orElse(null);
}
private TestAccount user(String name, String fullName, String emailName, AccountGroup... groups)
private TestAccount user(String name, String fullName, String emailName, InternalGroup... groups)
throws Exception {
String[] groupNames = Arrays.stream(groups).map(AccountGroup::getName).toArray(String[]::new);
String[] groupNames = Arrays.stream(groups).map(InternalGroup::getName).toArray(String[]::new);
return accountCreator.create(
name(name), name(emailName) + "@example.com", fullName, groupNames);
}
private TestAccount user(String name, String fullName, AccountGroup... groups) throws Exception {
private TestAccount user(String name, String fullName, InternalGroup... groups) throws Exception {
return user(name, fullName, name, groups);
}
@ -461,7 +462,7 @@ public class SuggestReviewersIT extends AbstractDaemonTest {
private void assertReviewers(
List<SuggestedReviewerInfo> actual,
List<TestAccount> expectedUsers,
List<AccountGroup> expectedGroups) {
List<InternalGroup> expectedGroups) {
List<Integer> actualAccountIds =
actual
.stream()

2
gerrit-elasticsearch/src/main/java/com/google/gerrit/elasticsearch/ElasticGroupIndex.java
View File

@ -213,7 +213,7 @@ public class ElasticGroupIndex extends AbstractElasticIndex<AccountGroup.UUID, I
source.getAsJsonObject().get(GroupField.UUID.getName()).getAsString());
// Use the GroupCache rather than depending on any stored fields in the
// document (of which there shouldn't be any).
return groupCache.get().getInternalGroup(uuid);
return groupCache.get().get(uuid);
}
}
}

2
gerrit-lucene/src/main/java/com/google/gerrit/lucene/LuceneGroupIndex.java
View File

@ -199,6 +199,6 @@ public class LuceneGroupIndex extends AbstractLuceneIndex<AccountGroup.UUID, Int
AccountGroup.UUID uuid = new AccountGroup.UUID(doc.getField(UUID.getName()).stringValue());
// Use the GroupCache rather than depending on any stored fields in the
// document (of which there shouldn't be any).
return groupCache.get().getInternalGroup(uuid);
return groupCache.get().get(uuid);
}
}

13
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java
View File

@ -15,7 +15,6 @@
package com.google.gerrit.server.account;
import com.google.common.collect.ImmutableList;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.group.InternalGroup;
import java.io.IOException;
@ -27,13 +26,6 @@ public interface GroupCache {
AccountGroup get(AccountGroup.NameKey name);
/**
* Lookup a group definition by its UUID. The returned definition may be null if the group has
* been deleted and the UUID reference is stale, or was copied from another server.
*/
@Nullable
AccountGroup get(AccountGroup.UUID uuid);
/**
* Looks up an internal group by its UUID.
*
@ -41,7 +33,7 @@ public interface GroupCache {
* @return an {@code Optional} of the internal group, or an empty {@code Optional} if no internal
* group with this UUID exists on this server or an error occurred during lookup
*/
Optional<InternalGroup> getInternalGroup(AccountGroup.UUID groupUuid);
Optional<InternalGroup> get(AccountGroup.UUID groupUuid);
/** @return sorted list of groups. */
ImmutableList<AccountGroup> all();
@ -49,7 +41,8 @@ public interface GroupCache {
/** Notify the cache that a new group was constructed. */
void onCreateGroup(AccountGroup.NameKey newGroupName) throws IOException;
void evict(AccountGroup group) throws IOException;
void evict(AccountGroup.UUID groupUuid, AccountGroup.Id groupId, AccountGroup.NameKey groupName)
throws IOException;
void evictAfterRename(AccountGroup.NameKey oldName, AccountGroup.NameKey newName)
throws IOException;

78
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java
View File

@ -22,7 +22,6 @@ import com.google.common.cache.LoadingCache;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.gerrit.common.TimeUtil;
import com.google.gerrit.common.errors.NoSuchGroupException;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.server.ReviewDb;
@ -63,7 +62,7 @@ public class GroupCacheImpl implements GroupCache {
cache(BYNAME_NAME, String.class, new TypeLiteral<Optional<AccountGroup>>() {})
.loader(ByNameLoader.class);
cache(BYUUID_NAME, String.class, new TypeLiteral<Optional<AccountGroup>>() {})
cache(BYUUID_NAME, String.class, new TypeLiteral<Optional<InternalGroup>>() {})
.loader(ByUUIDLoader.class);
bind(GroupCacheImpl.class);
@ -74,7 +73,7 @@ public class GroupCacheImpl implements GroupCache {
private final LoadingCache<AccountGroup.Id, Optional<AccountGroup>> byId;
private final LoadingCache<String, Optional<AccountGroup>> byName;
private final LoadingCache<String, Optional<AccountGroup>> byUUID;
private final LoadingCache<String, Optional<InternalGroup>> byUUID;
private final SchemaFactory<ReviewDb> schema;
private final Provider<GroupIndexer> indexer;
private final Groups groups;
@ -83,7 +82,7 @@ public class GroupCacheImpl implements GroupCache {
GroupCacheImpl(
@Named(BYID_NAME) LoadingCache<AccountGroup.Id, Optional<AccountGroup>> byId,
@Named(BYNAME_NAME) LoadingCache<String, Optional<AccountGroup>> byName,
@Named(BYUUID_NAME) LoadingCache<String, Optional<AccountGroup>> byUUID,
@Named(BYUUID_NAME) LoadingCache<String, Optional<InternalGroup>> byUUID,
SchemaFactory<ReviewDb> schema,
Provider<GroupIndexer> indexer,
Groups groups) {
@ -107,17 +106,19 @@ public class GroupCacheImpl implements GroupCache {
}
@Override
public void evict(AccountGroup group) throws IOException {
if (group.getId() != null) {
byId.invalidate(group.getId());
public void evict(
AccountGroup.UUID groupUuid, AccountGroup.Id groupId, AccountGroup.NameKey groupName)
throws IOException {
if (groupId != null) {
byId.invalidate(groupId);
}
if (group.getNameKey() != null) {
byName.invalidate(group.getNameKey().get());
if (groupName != null) {
byName.invalidate(groupName.get());
}
if (group.getGroupUUID() != null) {
byUUID.invalidate(group.getGroupUUID().get());
if (groupUuid != null) {
byUUID.invalidate(groupUuid.get());
}
indexer.get().index(group.getGroupUUID());
indexer.get().index(groupUuid);
}
@Override
@ -140,51 +141,23 @@ public class GroupCacheImpl implements GroupCache {
try {
return byName.get(name.get()).orElse(null);
} catch (ExecutionException e) {
log.warn(String.format("Cannot lookup group %s by name", name.get()), e);
log.warn(String.format("Cannot look up group %s by name", name.get()), e);
return null;
}
}
@Override
public AccountGroup get(AccountGroup.UUID uuid) {
if (uuid == null) {
return null;
}
try {
return byUUID.get(uuid.get()).orElse(null);
} catch (ExecutionException e) {
log.warn(String.format("Cannot lookup group %s by uuid", uuid.get()), e);
return null;
}
}
@Override
public Optional<InternalGroup> getInternalGroup(AccountGroup.UUID groupUuid) {
public Optional<InternalGroup> get(AccountGroup.UUID groupUuid) {
if (groupUuid == null) {
return Optional.empty();
}
Optional<AccountGroup> accountGroup = Optional.empty();
try {
accountGroup = byUUID.get(groupUuid.get());
return byUUID.get(groupUuid.get());
} catch (ExecutionException e) {
log.warn(String.format("Cannot lookup group %s by uuid", groupUuid.get()), e);
}
if (!accountGroup.isPresent()) {
log.warn(String.format("Cannot look up group %s by uuid", groupUuid.get()), e);
return Optional.empty();
}
try (ReviewDb db = schema.open()) {
ImmutableSet<Account.Id> members = groups.getMembers(db, groupUuid).collect(toImmutableSet());
ImmutableSet<AccountGroup.UUID> includes =
groups.getIncludes(db, groupUuid).collect(toImmutableSet());
return accountGroup.map(group -> InternalGroup.create(group, members, includes));
} catch (OrmException | NoSuchGroupException e) {
log.warn(
String.format("Cannot lookup members or sub-groups of group %s", groupUuid.get()), e);
}
return Optional.empty();
}
@Override
@ -244,7 +217,7 @@ public class GroupCacheImpl implements GroupCache {
}
}
static class ByUUIDLoader extends CacheLoader<String, Optional<AccountGroup>> {
static class ByUUIDLoader extends CacheLoader<String, Optional<InternalGroup>> {
private final SchemaFactory<ReviewDb> schema;
private final Groups groups;
@ -255,9 +228,20 @@ public class GroupCacheImpl implements GroupCache {
}
@Override
public Optional<AccountGroup> load(String uuid) throws Exception {
public Optional<InternalGroup> load(String uuid) throws Exception {
try (ReviewDb db = schema.open()) {
return groups.getGroup(db, new AccountGroup.UUID(uuid));
AccountGroup.UUID groupUuid = new AccountGroup.UUID(uuid);
Optional<AccountGroup> accountGroup = groups.getGroup(db, groupUuid);
if (!accountGroup.isPresent()) {
return Optional.empty();
}
ImmutableSet<Account.Id> members =
groups.getMembers(db, groupUuid).collect(toImmutableSet());
ImmutableSet<AccountGroup.UUID> includes =
groups.getIncludes(db, groupUuid).collect(toImmutableSet());
return accountGroup.map(group -> InternalGroup.create(group, members, includes));
}
}
}

26
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupComparator.java
View File

@ -1,26 +0,0 @@
// Copyright (C) 2011 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;
import java.util.Comparator;
public class GroupComparator implements Comparator<AccountGroup> {
@Override
public int compare(AccountGroup group1, AccountGroup group2) {
return group1.getName().compareTo(group2.getName());
}
}

16
gerrit-server/src/main/java/com/google/gerrit/server/account/GroupMembers.java
View File

@ -20,6 +20,7 @@ import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.SystemGroupBackend;
import com.google.gerrit.server.project.NoSuchProjectException;
import com.google.gerrit.server.project.ProjectControl;
@ -29,6 +30,7 @@ import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import java.util.Collections;
import java.util.HashSet;
import java.util.Optional;
import java.util.Set;
public class GroupMembers {
@ -69,9 +71,9 @@ public class GroupMembers {
if (SystemGroupBackend.PROJECT_OWNERS.equals(groupUUID)) {
return getProjectOwners(project, seen);
}
AccountGroup group = groupCache.get(groupUUID);
if (group != null) {
return getGroupMembers(group, project, seen);
Optional<InternalGroup> group = groupCache.get(groupUUID);
if (group.isPresent()) {
return getGroupMembers(group.get(), project, seen);
}
return Collections.emptySet();
}
@ -96,7 +98,7 @@ public class GroupMembers {
}
private Set<Account> getGroupMembers(
final AccountGroup group, Project.NameKey project, Set<AccountGroup.UUID> seen)
InternalGroup group, Project.NameKey project, Set<AccountGroup.UUID> seen)
throws NoSuchGroupException, OrmException, NoSuchProjectException, IOException {
seen.add(group.getGroupUUID());
final GroupDetail groupDetail = groupDetailFactory.create(group.getGroupUUID()).call();
@ -107,9 +109,9 @@ public class GroupMembers {
}
for (AccountGroup.UUID groupIncludeUuid : groupDetail.getIncludes()) {
AccountGroup includedGroup = groupCache.get(groupIncludeUuid);
if (includedGroup != null && !seen.contains(includedGroup.getGroupUUID())) {
members.addAll(listAccounts(includedGroup.getGroupUUID(), project, seen));
Optional<InternalGroup> includedGroup = groupCache.get(groupIncludeUuid);
if (includedGroup.isPresent() && !seen.contains(includedGroup.get().getGroupUUID())) {
members.addAll(listAccounts(includedGroup.get().getGroupUUID(), project, seen));
}
}
return members;

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

@ -19,11 +19,13 @@ import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.group.InternalGroup;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
@ -91,8 +93,8 @@ public class IncludingGroupMembership implements GroupMembership {
}
memberOf.put(id, false);
AccountGroup group = groupCache.get(id);
if (group == null) {
Optional<InternalGroup> group = groupCache.get(id);
if (!group.isPresent()) {
continue;
}
if (search(includeCache.subgroupsOf(id))) {

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

@ -17,10 +17,10 @@ package com.google.gerrit.server.account;
import static java.util.stream.Collectors.toList;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
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.group.InternalGroupDescription;
import com.google.gerrit.server.project.ProjectState;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@ -56,11 +56,7 @@ public class InternalGroupBackend implements GroupBackend {
return null;
}
AccountGroup g = groupCache.get(uuid);
if (g == null) {
return null;
}
return GroupDescriptions.forAccountGroup(g);
return groupCache.get(uuid).map(InternalGroupDescription::new).orElse(null);
}
@Override

8
gerrit-server/src/main/java/com/google/gerrit/server/group/GetAuditLog.java
View File

@ -15,7 +15,6 @@
package com.google.gerrit.server.group;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.common.data.GroupDescriptions;
import com.google.gerrit.extensions.common.AccountInfo;
import com.google.gerrit.extensions.common.GroupAuditEventInfo;
import com.google.gerrit.extensions.common.GroupInfo;
@ -38,6 +37,7 @@ import java.util.ArrayList;
import java.util.Collections;
import java.util.Comparator;
import java.util.List;
import java.util.Optional;
@Singleton
public class GetAuditLog implements RestReadView<GroupResource> {
@ -94,10 +94,10 @@ public class GetAuditLog implements RestReadView<GroupResource> {
for (AccountGroupByIdAud auditEvent :
db.get().accountGroupByIdAud().byGroup(group.getId()).toList()) {
AccountGroup.UUID includedGroupUUID = auditEvent.getKey().getIncludeUUID();
AccountGroup includedGroup = groupCache.get(includedGroupUUID);
Optional<InternalGroup> includedGroup = groupCache.get(includedGroupUUID);
GroupInfo member;
if (includedGroup != null) {
member = groupJson.format(GroupDescriptions.forAccountGroup(includedGroup));
if (includedGroup.isPresent()) {
member = groupJson.format(new InternalGroupDescription(includedGroup.get()));
} else {
GroupDescription.Basic groupDescription = groupBackend.get(includedGroupUUID);
member = new GroupInfo();

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

@ -170,7 +170,7 @@ public class GroupsUpdate {
ReviewDb db, AccountGroup.UUID groupUuid, Consumer<AccountGroup> groupConsumer)
throws OrmException, IOException, NoSuchGroupException {
AccountGroup updatedGroup = updateGroupInDb(db, groupUuid, groupConsumer);
groupCache.evict(updatedGroup);
groupCache.evict(updatedGroup.getGroupUUID(), updatedGroup.getId(), updatedGroup.getNameKey());
}
@VisibleForTesting
@ -221,7 +221,7 @@ public class GroupsUpdate {
db.accountGroupNames().deleteKeys(ImmutableList.of(oldName));
groupCache.evict(group);
groupCache.evict(group.getGroupUUID(), group.getId(), group.getNameKey());
groupCache.evictAfterRename(oldName, newName);
@SuppressWarnings("unused")

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

@ -24,6 +24,7 @@ import com.google.gerrit.server.group.Index.Input;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
import java.util.Optional;
@Singleton
public class Index implements RestModifyView<GroupResource, Input> {
@ -49,9 +50,11 @@ public class Index implements RestModifyView<GroupResource, Input> {
String.format("External Group Not Allowed: %s", groupUuid.get()));
}
AccountGroup accountGroup = groupCache.get(groupUuid);
Optional<InternalGroup> group = groupCache.get(groupUuid);
// evicting the group from the cache, reindexes the group
groupCache.evict(accountGroup);
if (group.isPresent()) {
groupCache.evict(group.get().getGroupUUID(), group.get().getId(), group.get().getNameKey());
}
return Response.none();
}
}

45
gerrit-server/src/main/java/com/google/gerrit/server/group/ListGroups.java
View File

@ -14,8 +14,11 @@
package com.google.gerrit.server.group;
import static com.google.common.collect.ImmutableList.toImmutableList;
import com.google.common.base.MoreObjects;
import com.google.common.base.Strings;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.GroupDescription;
@ -35,7 +38,6 @@ import com.google.gerrit.server.account.AccountResource;
import com.google.gerrit.server.account.GetGroups;
import com.google.gerrit.server.account.GroupBackend;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupComparator;
import com.google.gerrit.server.account.GroupControl;
import com.google.gerrit.server.project.ProjectControl;
import com.google.gwtorm.server.OrmException;
@ -43,13 +45,14 @@ import com.google.inject.Inject;
import com.google.inject.Provider;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.Comparator;
import java.util.EnumSet;
import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.SortedMap;
import java.util.TreeMap;
@ -58,6 +61,8 @@ import org.kohsuke.args4j.Option;
/** List groups visible to the calling user. */
public class ListGroups implements RestReadView<TopLevelResource> {
private static final Comparator<GroupDescription.Internal> GROUP_COMPARATOR =
Comparator.comparing(GroupDescription.Basic::getName);
protected final GroupCache groupCache;
@ -266,33 +271,32 @@ public class ListGroups implements RestReadView<TopLevelResource> {
private List<GroupInfo> getAllGroups() throws OrmException {
List<GroupInfo> groupInfos;
List<AccountGroup> groupList;
List<GroupDescription.Internal> groupList;
if (!projects.isEmpty()) {
Map<AccountGroup.UUID, AccountGroup> groups = new HashMap<>();
Map<AccountGroup.UUID, GroupDescription.Internal> groups = new HashMap<>();
for (ProjectControl projectControl : projects) {
final Set<GroupReference> groupsRefs = projectControl.getAllGroups();
for (GroupReference groupRef : groupsRefs) {
final AccountGroup group = groupCache.get(groupRef.getUUID());
if (group != null) {
groups.put(group.getGroupUUID(), group);
}
Optional<InternalGroup> internalGroup = groupCache.get(groupRef.getUUID());
internalGroup.ifPresent(
group -> groups.put(group.getGroupUUID(), new InternalGroupDescription(group)));
}
}
groupList = filterGroups(groups.values());
} else {
groupList = filterGroups(groupCache.all());
groupList = filterGroups(getAllExistingInternalGroups());
}
groupInfos = Lists.newArrayListWithCapacity(groupList.size());
int found = 0;
int foundIndex = 0;
for (AccountGroup group : groupList) {
for (GroupDescription.Internal group : groupList) {
if (foundIndex++ < start) {
continue;
}
if (limit > 0 && ++found > limit) {
break;
}
groupInfos.add(json.addOptions(options).format(GroupDescriptions.forAccountGroup(group)));
groupInfos.add(json.addOptions(options).format(group));
}
return groupInfos;
}
@ -355,7 +359,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
List<GroupInfo> groups = new ArrayList<>();
int found = 0;
int foundIndex = 0;
for (AccountGroup g : filterGroups(groupCache.all())) {
for (GroupDescription.Internal g : filterGroups(getAllExistingInternalGroups())) {
GroupControl ctl = groupControlFactory.controlFor(g);
try {
if (genericGroupControlFactory.controlFor(user, g.getGroupUUID()).isOwner()) {
@ -374,10 +378,19 @@ public class ListGroups implements RestReadView<TopLevelResource> {
return groups;
}
private List<AccountGroup> filterGroups(Collection<AccountGroup> groups) {
List<AccountGroup> filteredGroups = new ArrayList<>(groups.size());
private ImmutableList<GroupDescription.Internal> getAllExistingInternalGroups() {
return groupCache
.all()
.stream()
.map(GroupDescriptions::forAccountGroup)
.collect(toImmutableList());
}
private List<GroupDescription.Internal> filterGroups(
Collection<GroupDescription.Internal> groups) {
List<GroupDescription.Internal> filteredGroups = new ArrayList<>(groups.size());
Pattern pattern = Strings.isNullOrEmpty(matchRegex) ? null : Pattern.compile(matchRegex);
for (AccountGroup group : groups) {
for (GroupDescription.Internal group : groups) {
if (!Strings.isNullOrEmpty(matchSubstring)) {
if (!group
.getName()
@ -402,7 +415,7 @@ public class ListGroups implements RestReadView<TopLevelResource> {
filteredGroups.add(group);
}
}
Collections.sort(filteredGroups, new GroupComparator());
filteredGroups.sort(GROUP_COMPARATOR);
return filteredGroups;
}
}

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

@ -34,6 +34,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import org.kohsuke.args4j.Option;
public class ListMembers implements RestReadView<GroupResource> {
@ -85,15 +86,15 @@ public class ListMembers implements RestReadView<GroupResource> {
seenGroups.add(groupUUID);
final Map<Account.Id, AccountInfo> members = new HashMap<>();
final AccountGroup group = groupCache.get(groupUUID);
if (group == null) {
Optional<InternalGroup> group = groupCache.get(groupUUID);
if (!group.isPresent()) {
// the included group is an external group and can't be resolved
return Collections.emptyMap();
}
final GroupDetail groupDetail;
try {
groupDetail = groupDetailFactory.create(group.getGroupUUID()).call();
groupDetail = groupDetailFactory.create(group.get().getGroupUUID()).call();
} catch (NoSuchGroupException e) {
// the included group is not visible
return Collections.emptyMap();

9
gerrit-server/src/main/java/com/google/gerrit/server/index/group/AllGroupsIndexer.java
View File

@ -94,11 +94,12 @@ public class AllGroupsIndexer extends SiteIndexer<AccountGroup.UUID, InternalGro
executor.submit(
() -> {
try {
AccountGroup oldGroup = groupCache.get(uuid);
if (oldGroup != null) {
groupCache.evict(oldGroup);
Optional<InternalGroup> oldGroup = groupCache.get(uuid);
if (oldGroup.isPresent()) {
InternalGroup group = oldGroup.get();
groupCache.evict(group.getGroupUUID(), group.getId(), group.getNameKey());
}
Optional<InternalGroup> internalGroup = groupCache.getInternalGroup(uuid);
Optional<InternalGroup> internalGroup = groupCache.get(uuid);
if (internalGroup.isPresent()) {
index.replace(internalGroup.get());
} else {

2
gerrit-server/src/main/java/com/google/gerrit/server/index/group/GroupIndexerImpl.java
View File

@ -66,7 +66,7 @@ public class GroupIndexerImpl implements GroupIndexer {
@Override
public void index(AccountGroup.UUID uuid) throws IOException {
for (Index<AccountGroup.UUID, InternalGroup> i : getWriteIndexes()) {
Optional<InternalGroup> internalGroup = groupCache.getInternalGroup(uuid);
Optional<InternalGroup> internalGroup = groupCache.get(uuid);
if (internalGroup.isPresent()) {
i.replace(internalGroup.get());
} else {

7
gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupQueryBuilder.java
View File

@ -29,6 +29,7 @@ import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.group.InternalGroup;
import com.google.inject.Inject;
import java.util.List;
import java.util.Optional;
/** Parses a query string meant to be applied to group objects. */
public class GroupQueryBuilder extends QueryBuilder<InternalGroup> {
@ -90,9 +91,9 @@ public class GroupQueryBuilder extends QueryBuilder<InternalGroup> {
@Operator
public Predicate<InternalGroup> owner(String owner) throws QueryParseException {
AccountGroup group = args.groupCache.get(new AccountGroup.UUID(owner));
if (group != null) {
return GroupPredicates.owner(group.getGroupUUID());
Optional<InternalGroup> group = args.groupCache.get(new AccountGroup.UUID(owner));
if (group.isPresent()) {
return GroupPredicates.owner(group.get().getGroupUUID());
}
GroupReference g = GroupBackends.findBestSuggestion(args.groupBackend, owner);
if (g == null) {

10
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/ListGroupsCommand.java
View File

@ -22,12 +22,14 @@ import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.restapi.Url;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.server.group.ListGroups;
import com.google.gerrit.server.ioutil.ColumnFormatter;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
import com.google.gerrit.util.cli.Options;
import com.google.inject.Inject;
import java.util.Optional;
import org.kohsuke.args4j.Option;
@CommandMetaData(
@ -60,15 +62,15 @@ public class ListGroupsCommand extends SshCommand {
for (GroupInfo info : listGroups.get()) {
formatter.addColumn(MoreObjects.firstNonNull(info.name, "n/a"));
if (verboseOutput) {
AccountGroup o =
Optional<InternalGroup> group =
info.ownerId != null
? groupCache.get(new AccountGroup.UUID(Url.decode(info.ownerId)))
: null;
: Optional.empty();
formatter.addColumn(Url.decode(info.id));
formatter.addColumn(Strings.nullToEmpty(info.description));
formatter.addColumn(o != null ? o.getName() : "n/a");
formatter.addColumn(o != null ? o.getGroupUUID().get() : "");
formatter.addColumn(group.map(InternalGroup::getName).orElse("n/a"));
formatter.addColumn(group.map(g -> g.getGroupUUID().get()).orElse(""));
formatter.addColumn(
Boolean.toString(MoreObjects.firstNonNull(info.options.visibleToAll, Boolean.FALSE)));
}

8
gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetMembersCommand.java
View File

@ -18,6 +18,7 @@ import static java.util.stream.Collectors.joining;
import static java.util.stream.Collectors.toList;
import com.google.common.base.MoreObjects;
import com.google.common.collect.Streams;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
import com.google.gerrit.extensions.restapi.TopLevelResource;
@ -31,6 +32,7 @@ import com.google.gerrit.server.group.DeleteIncludedGroups;
import com.google.gerrit.server.group.DeleteMembers;
import com.google.gerrit.server.group.GroupResource;
import com.google.gerrit.server.group.GroupsCollection;
import com.google.gerrit.server.group.InternalGroup;
import com.google.gerrit.sshd.CommandMetaData;
import com.google.gerrit.sshd.SshCommand;
import com.google.inject.Inject;
@ -149,7 +151,11 @@ public class SetMembersCommand extends SshCommand {
String action, GroupResource group, List<AccountGroup.UUID> groupUuidList)
throws UnsupportedEncodingException, IOException {
String names =
groupUuidList.stream().map(uuid -> groupCache.get(uuid).getName()).collect(joining(", "));
groupUuidList
.stream()
.map(uuid -> groupCache.get(uuid).map(InternalGroup::getName))
.flatMap(Streams::stream)
.collect(joining(", "));
out.write(
String.format("Groups %s group %s: %s\n", action, group.getName(), names).getBytes(ENC));
}