opendev/gerrit
Code Issues Proposed changes

Cache list of all groups in the group cache

Browse Source 
By caching the list of all groups we avoid to query the database each
time the GroupListScreen is displayed.

Change-Id: I3c7445c9e0ba5711638352dbd0f4d4ff0b7198d7
Signed-off-by: Edwin Kempin <edwin.kempin@sap.com>
This commit is contained in:
Edwin Kempin
2011-09-01 14:40:44 +02:00
parent 0d957de5d3
commit 8e7a35be30
8 changed files with 193 additions and 40 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
gerrit-common/src/main/java/com/google/gerrit/common/data/GroupList.java
View File

@@ -22,6 +22,14 @@ public class GroupList {
protected List<GroupDetail> groups;
protected boolean canCreateGroup;
public GroupList() {
}
public GroupList(final List<GroupDetail> groups, final boolean canCreateGroup) {
this.groups = groups;
this.canCreateGroup = canCreateGroup;
}
public List<GroupDetail> getGroups() {
return groups;
}

1
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/AccountModule.java
View File

@@ -35,6 +35,7 @@ public class AccountModule extends RpcServletModule {
factory(GroupDetailHandler.Factory.class);
factory(MyGroupsFactory.Factory.class);
factory(RenameGroup.Factory.class);
factory(VisibleGroups.Factory.class);
}
});
rpc(AccountSecurityImpl.class);

40
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/GroupAdminServiceImpl.java
View File

@@ -53,7 +53,6 @@ import java.util.Set;
class GroupAdminServiceImpl extends BaseServiceImplementation implements
GroupAdminService {
private final Provider<IdentifiedUser> identifiedUser;
private final AccountCache accountCache;
private final AccountResolver accountResolver;
private final Realm accountRealm;
@@ -64,6 +63,7 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
private final CreateGroup.Factory createGroupFactory;
private final RenameGroup.Factory renameGroupFactory;
private final GroupDetailHandler.Factory groupDetailFactory;
private final VisibleGroups.Factory visibleGroupsFactory;
@Inject
GroupAdminServiceImpl(final Provider<ReviewDb> schema,
@@ -75,9 +75,9 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
final GroupControl.Factory groupControlFactory,
final CreateGroup.Factory createGroupFactory,
final RenameGroup.Factory renameGroupFactory,
final GroupDetailHandler.Factory groupDetailFactory) {
final GroupDetailHandler.Factory groupDetailFactory,
final VisibleGroups.Factory visibleGroupsFactory) {
super(schema, currentUser);
this.identifiedUser = currentUser;
this.accountCache = accountCache;
this.groupIncludeCache = groupIncludeCache;
this.accountResolver = accountResolver;
@@ -87,41 +87,11 @@ class GroupAdminServiceImpl extends BaseServiceImplementation implements
this.createGroupFactory = createGroupFactory;
this.renameGroupFactory = renameGroupFactory;
this.groupDetailFactory = groupDetailFactory;
this.visibleGroupsFactory = visibleGroupsFactory;
}
public void visibleGroups(final AsyncCallback<GroupList> callback) {
run(callback, new Action<GroupList>() {
public GroupList run(ReviewDb db) throws OrmException,
NoSuchGroupException {
final IdentifiedUser user = identifiedUser.get();
final List<AccountGroup> list;
if (user.getCapabilities().canAdministrateServer()) {
list = db.accountGroups().all().toList();
} else {
list = new ArrayList<AccountGroup>();
for(final AccountGroup group : db.accountGroups().all().toList()) {
final GroupControl c = groupControlFactory.controlFor(group);
if (c.isVisible()) {
list.add(c.getAccountGroup());
}
}
}
Collections.sort(list, new Comparator<AccountGroup>() {
public int compare(final AccountGroup a, final AccountGroup b) {
return a.getName().compareTo(b.getName());
}
});
List<GroupDetail> l = new ArrayList<GroupDetail>();
for(AccountGroup group : list) {
l.add(groupDetailFactory.create(group.getId()).call());
}
GroupList res = new GroupList();
res.setGroups(l);
res.setCanCreateGroup(user.getCapabilities().canCreateGroup());
return res;
}
});
visibleGroupsFactory.create().to(callback);
}
public void createGroup(final String newName,

75
gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/account/VisibleGroups.java Normal file
View File

@@ -0,0 +1,75 @@
// 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.httpd.rpc.account;
import com.google.gerrit.common.data.GroupDetail;
import com.google.gerrit.common.data.GroupList;
import com.google.gerrit.httpd.rpc.Handler;
import com.google.gerrit.reviewdb.AccountGroup;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.account.GroupCache;
import com.google.gerrit.server.account.GroupControl;
import com.google.inject.Inject;
import com.google.inject.Provider;
import org.apache.commons.collections.CollectionUtils;
import java.util.ArrayList;
import java.util.List;
public class VisibleGroups extends Handler<GroupList> {
interface Factory {
VisibleGroups create();
}
private final Provider<IdentifiedUser> identifiedUser;
private final GroupCache groupCache;
private final GroupControl.Factory groupControlFactory;
private final GroupDetailHandler.Factory groupDetailFactory;
@Inject
VisibleGroups(final Provider<IdentifiedUser> currentUser,
final GroupCache groupCache,
final GroupControl.Factory groupControlFactory,
final GroupDetailHandler.Factory groupDetailFactory) {
this.identifiedUser = currentUser;
this.groupCache = groupCache;
this.groupControlFactory = groupControlFactory;
this.groupDetailFactory = groupDetailFactory;
}
@Override
public GroupList call() throws Exception {
final IdentifiedUser user = identifiedUser.get();
final List<AccountGroup> list = new ArrayList<AccountGroup>();
if (user.getCapabilities().canAdministrateServer()) {
CollectionUtils.addAll(list, groupCache.all().iterator());
} else {
for(final AccountGroup group : groupCache.all()) {
final GroupControl c = groupControlFactory.controlFor(group);
if (c.isVisible()) {
list.add(c.getAccountGroup());
}
}
}
List<GroupDetail> l = new ArrayList<GroupDetail>();
for(AccountGroup group : list) {
l.add(groupDetailFactory.create(group.getId()).call());
}
return new GroupList(l, user.getCapabilities().canCreateGroup());
}
}

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

@@ -28,7 +28,14 @@ public interface GroupCache {
public Collection<AccountGroup> get(AccountGroup.ExternalNameKey externalName);
/** @return sorted iteration of groups. */
public abstract Iterable<AccountGroup> all();
/** Notify the cache that a new group was constructed. */
public void onCreateGroup(AccountGroup.NameKey newGroupName);
public void evict(AccountGroup group);
public void evictAfterRename(AccountGroup.NameKey oldName);
public void evictAfterRename(final AccountGroup.NameKey oldName,
final AccountGroup.NameKey newName);
}

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

@@ -27,8 +27,14 @@ import com.google.inject.Singleton;
import com.google.inject.TypeLiteral;
import com.google.inject.name.Named;
import java.util.ArrayList;
import java.util.Collection;
import java.util.Collections;
import java.util.List;
import java.util.SortedSet;
import java.util.TreeSet;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
/** Tracks group objects in memory for efficient access. */
@Singleton
@@ -37,6 +43,7 @@ public class GroupCacheImpl implements GroupCache {
private static final String BYNAME_NAME = "groups_byname";
private static final String BYUUID_NAME = "groups_byuuid";
private static final String BYEXT_NAME = "groups_byext";
private static final String BYNAME_LIST = "groups_byname_list";
public static Module module() {
return new CacheModule() {
@@ -59,6 +66,10 @@ public class GroupCacheImpl implements GroupCache {
core(byExternalName, BYEXT_NAME) //
.populateWith(ByExternalNameLoader.class);
final TypeLiteral<Cache<ListKey, SortedSet<AccountGroup.NameKey>>> listType =
new TypeLiteral<Cache<ListKey, SortedSet<AccountGroup.NameKey>>>() {};
core(listType, BYNAME_LIST).populateWith(Lister.class);
bind(GroupCacheImpl.class);
bind(GroupCache.class).to(GroupCacheImpl.class);
}
@@ -69,17 +80,22 @@ public class GroupCacheImpl implements GroupCache {
private final Cache<AccountGroup.NameKey, AccountGroup> byName;
private final Cache<AccountGroup.UUID, AccountGroup> byUUID;
private final Cache<AccountGroup.ExternalNameKey, Collection<AccountGroup>> byExternalName;
private final Cache<ListKey,SortedSet<AccountGroup.NameKey>> list;
private final Lock listLock;
@Inject
GroupCacheImpl(
@Named(BYID_NAME) Cache<AccountGroup.Id, AccountGroup> byId,
@Named(BYNAME_NAME) Cache<AccountGroup.NameKey, AccountGroup> byName,
@Named(BYUUID_NAME) Cache<AccountGroup.UUID, AccountGroup> byUUID,
@Named(BYEXT_NAME) Cache<AccountGroup.ExternalNameKey, Collection<AccountGroup>> byExternalName) {
@Named(BYEXT_NAME) Cache<AccountGroup.ExternalNameKey, Collection<AccountGroup>> byExternalName,
@Named(BYNAME_LIST) final Cache<ListKey, SortedSet<AccountGroup.NameKey>> list) {
this.byId = byId;
this.byName = byName;
this.byUUID = byUUID;
this.byExternalName = byExternalName;
this.list = list;
this.listLock = new ReentrantLock(true /* fair */);
}
public AccountGroup get(final AccountGroup.Id groupId) {
@@ -93,8 +109,10 @@ public class GroupCacheImpl implements GroupCache {
byExternalName.remove(group.getExternalNameKey());
}
public void evictAfterRename(final AccountGroup.NameKey oldName) {
public void evictAfterRename(final AccountGroup.NameKey oldName,
final AccountGroup.NameKey newName) {
byName.remove(oldName);
updateGroupList(oldName, newName);
}
public AccountGroup get(final AccountGroup.NameKey name) {
@@ -110,6 +128,41 @@ public class GroupCacheImpl implements GroupCache {
return byExternalName.get(externalName);
}
@Override
public Iterable<AccountGroup> all() {
final List<AccountGroup> groups = new ArrayList<AccountGroup>();
for (final AccountGroup.NameKey groupName : list.get(ListKey.ALL)) {
final AccountGroup group = get(groupName);
if (group != null) {
groups.add(group);
}
}
return Collections.unmodifiableList(groups);
}
@Override
public void onCreateGroup(final AccountGroup.NameKey newGroupName) {
updateGroupList(null, newGroupName);
}
private void updateGroupList(final AccountGroup.NameKey nameToRemove,
final AccountGroup.NameKey nameToAdd) {
listLock.lock();
try {
SortedSet<AccountGroup.NameKey> n = list.get(ListKey.ALL);
n = new TreeSet<AccountGroup.NameKey>(n);
if (nameToRemove != null) {
n.remove(nameToRemove);
}
if (nameToAdd != null) {
n.add(nameToAdd);
}
list.put(ListKey.ALL, Collections.unmodifiableSortedSet(n));
} finally {
listLock.unlock();
}
}
static class ByIdLoader extends EntryCreator<AccountGroup.Id, AccountGroup> {
private final SchemaFactory<ReviewDb> schema;
@@ -216,4 +269,38 @@ public class GroupCacheImpl implements GroupCache {
}
}
}
static class ListKey {
static final ListKey ALL = new ListKey();
private ListKey() {
}
}
static class Lister extends EntryCreator<ListKey, SortedSet<AccountGroup.NameKey>> {
private final SchemaFactory<ReviewDb> schema;
@Inject
Lister(final SchemaFactory<ReviewDb> sf) {
schema = sf;
}
@Override
public SortedSet<AccountGroup.NameKey> createEntry(ListKey key)
throws Exception {
final ReviewDb db = schema.open();
try {
final List<AccountGroupName> groupNames =
db.accountGroupNames().all().toList();
final SortedSet<AccountGroup.NameKey> groups =
new TreeSet<AccountGroup.NameKey>();
for (final AccountGroupName groupName : groupNames) {
groups.add(groupName.getNameKey());
}
return Collections.unmodifiableSortedSet(groups);
} finally {
db.close();
}
}
}
}

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

@@ -49,17 +49,20 @@ public class PerformCreateGroup {
private final GroupIncludeCache groupIncludeCache;
private final IdentifiedUser currentUser;
private final PersonIdent serverIdent;
private final GroupCache groupCache;
@Inject
PerformCreateGroup(final ReviewDb db, final AccountCache accountCache,
final GroupIncludeCache groupIncludeCache,
final IdentifiedUser currentUser,
@GerritPersonIdent final PersonIdent serverIdent) {
@GerritPersonIdent final PersonIdent serverIdent,
final GroupCache groupCache) {
this.db = db;
this.accountCache = accountCache;
this.groupIncludeCache = groupIncludeCache;
this.currentUser = currentUser;
this.serverIdent = serverIdent;
this.groupCache = groupCache;
}
/**
@@ -125,6 +128,8 @@ public class PerformCreateGroup {
addGroups(groupId, initialGroups);
}
groupCache.onCreateGroup(nameKey);
return groupId;
}

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

@@ -107,7 +107,7 @@ public class PerformRenameGroup {
}
groupCache.evict(group);
groupCache.evictAfterRename(old);
groupCache.evictAfterRename(old, key);
renameGroupOpFactory.create( //
currentUser.newCommitterIdent(new Date(), TimeZone.getDefault()), //
group.getGroupUUID(), //