diff --git a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java index 5fbd4118e5..dda8d14118 100644 --- a/gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java +++ b/gerrit-pgm/src/main/java/com/google/gerrit/pgm/MigrateToNoteDb.java @@ -20,8 +20,11 @@ import static java.util.stream.Collectors.joining; import static java.util.stream.Collectors.toList; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.gerrit.elasticsearch.ElasticIndexModule; import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.lifecycle.LifecycleManager; +import com.google.gerrit.lucene.LuceneIndexModule; import com.google.gerrit.pgm.util.BatchProgramModule; import com.google.gerrit.pgm.util.RuntimeShutdown; import com.google.gerrit.pgm.util.SiteProgram; @@ -30,10 +33,11 @@ import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.change.ChangeResource; import com.google.gerrit.server.extensions.events.GitReferenceUpdated; -import com.google.gerrit.server.index.DummyIndexModule; +import com.google.gerrit.server.index.IndexModule; import com.google.gerrit.server.notedb.rebuild.NoteDbMigrator; import com.google.inject.Inject; import com.google.inject.Injector; +import com.google.inject.Module; import com.google.inject.Provider; import java.util.ArrayList; import java.util.List; @@ -160,12 +164,23 @@ public class MigrateToNoteDb extends SiteProgram { public void configure() { install(dbInjector.getInstance(BatchProgramModule.class)); bind(GitReferenceUpdated.class).toInstance(GitReferenceUpdated.DISABLED); - install(new DummyIndexModule()); + install(getIndexModule()); factory(ChangeResource.Factory.class); } }); } + private Module getIndexModule() { + switch (IndexModule.getIndexType(dbInjector)) { + case LUCENE: + return LuceneIndexModule.singleVersionWithExplicitVersions(ImmutableMap.of(), threads); + case ELASTICSEARCH: + return ElasticIndexModule.singleVersionWithExplicitVersions(ImmutableMap.of(), threads); + default: + throw new IllegalStateException("unsupported index.type"); + } + } + private void stop() { try { LifecycleManager m = sysManager; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java index 4c6f6703d3..44ffb90693 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/AccountCacheImpl.java @@ -20,6 +20,7 @@ import static com.google.gerrit.server.account.externalids.ExternalId.SCHEME_USE import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableSet; +import com.google.common.collect.Streams; import com.google.gerrit.common.Nullable; import com.google.gerrit.common.TimeUtil; import com.google.gerrit.extensions.client.GeneralPreferencesInfo; @@ -33,6 +34,7 @@ import com.google.gerrit.server.account.externalids.ExternalIds; import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.config.AllUsersName; import com.google.gerrit.server.group.Groups; +import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.index.account.AccountIndexer; import com.google.gerrit.server.query.account.InternalAccountQuery; import com.google.gwtorm.server.OrmException; @@ -46,7 +48,6 @@ import com.google.inject.name.Named; import java.io.IOException; import java.util.Collections; import java.util.HashMap; -import java.util.Objects; import java.util.Optional; import java.util.Set; import java.util.concurrent.ExecutionException; @@ -199,8 +200,8 @@ public class AccountCacheImpl implements AccountCache { groups .getGroupsWithMember(db, who) .map(groupCache::get) - .map(AccountGroup::getGroupUUID) - .filter(Objects::nonNull) + .flatMap(Streams::stream) + .map(InternalGroup::getGroupUUID) .collect(toImmutableSet()); try { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java index 122c0bc836..82bcce377e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCache.java @@ -22,7 +22,14 @@ import java.util.Optional; /** Tracks group objects in memory for efficient access. */ public interface GroupCache { - AccountGroup get(AccountGroup.Id groupId); + /** + * Looks up an internal group by its ID. + * + * @param groupId the ID of the internal group + * @return an {@code Optional} of the internal group, or an empty {@code Optional} if no internal + * group with this ID exists on this server or an error occurred during lookup + */ + Optional get(AccountGroup.Id groupId); /** * Looks up an internal group by its name. diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java index c95517b590..988439c8e7 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupCacheImpl.java @@ -21,7 +21,6 @@ import com.google.common.cache.CacheLoader; 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.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.server.ReviewDb; @@ -57,7 +56,7 @@ public class GroupCacheImpl implements GroupCache { return new CacheModule() { @Override protected void configure() { - cache(BYID_NAME, AccountGroup.Id.class, new TypeLiteral>() {}) + cache(BYID_NAME, AccountGroup.Id.class, new TypeLiteral>() {}) .loader(ByIdLoader.class); cache(BYNAME_NAME, String.class, new TypeLiteral>() {}) @@ -72,7 +71,7 @@ public class GroupCacheImpl implements GroupCache { }; } - private final LoadingCache> byId; + private final LoadingCache> byId; private final LoadingCache> byName; private final LoadingCache> byUUID; private final SchemaFactory schema; @@ -81,7 +80,7 @@ public class GroupCacheImpl implements GroupCache { @Inject GroupCacheImpl( - @Named(BYID_NAME) LoadingCache> byId, + @Named(BYID_NAME) LoadingCache> byId, @Named(BYNAME_NAME) LoadingCache> byName, @Named(BYUUID_NAME) LoadingCache> byUUID, SchemaFactory schema, @@ -96,13 +95,12 @@ public class GroupCacheImpl implements GroupCache { } @Override - public AccountGroup get(AccountGroup.Id groupId) { + public Optional get(AccountGroup.Id groupId) { try { - Optional g = byId.get(groupId); - return g.isPresent() ? g.get() : missing(groupId); + return byId.get(groupId); } catch (ExecutionException e) { log.warn("Cannot load group " + groupId, e); - return missing(groupId); + return Optional.empty(); } } @@ -171,26 +169,17 @@ public class GroupCacheImpl implements GroupCache { indexer.get().index(group.getGroupUUID()); } - private static AccountGroup missing(AccountGroup.Id key) { - AccountGroup.NameKey name = new AccountGroup.NameKey("Deleted Group" + key); - return new AccountGroup(name, key, null, TimeUtil.nowTs()); - } - - static class ByIdLoader extends CacheLoader> { - private final SchemaFactory schema; - private final Groups groups; + static class ByIdLoader extends CacheLoader> { + private final Provider groupQueryProvider; @Inject - ByIdLoader(SchemaFactory sf, Groups groups) { - schema = sf; - this.groups = groups; + ByIdLoader(Provider groupQueryProvider) { + this.groupQueryProvider = groupQueryProvider; } @Override - public Optional load(AccountGroup.Id key) throws Exception { - try (ReviewDb db = schema.open()) { - return groups.getGroup(db, key); - } + public Optional load(AccountGroup.Id key) throws Exception { + return groupQueryProvider.get().byId(key); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java index 5af4898a8c..020a04dc88 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupControl.java @@ -21,12 +21,15 @@ import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.group.InternalGroup; +import com.google.gerrit.server.group.InternalGroupDescription; import com.google.gerrit.server.permissions.GlobalPermission; import com.google.gerrit.server.permissions.PermissionBackend; import com.google.gerrit.server.permissions.PermissionBackendException; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; +import java.util.Optional; /** Access control management for a group of accounts managed in Gerrit. */ public class GroupControl { @@ -71,11 +74,11 @@ public class GroupControl { } public GroupControl controlFor(AccountGroup.Id groupId) throws NoSuchGroupException { - final AccountGroup group = groupCache.get(groupId); - if (group == null) { - throw new NoSuchGroupException(groupId); - } - return controlFor(GroupDescriptions.forAccountGroup(group)); + Optional group = groupCache.get(groupId); + return group + .map(InternalGroupDescription::new) + .map(this::controlFor) + .orElseThrow(() -> new NoSuchGroupException(groupId)); } public GroupControl controlFor(AccountGroup.UUID groupId) throws NoSuchGroupException { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java index 10c002c01f..c686928c96 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java @@ -19,11 +19,13 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableList; +import com.google.common.collect.Streams; import com.google.gerrit.common.errors.NoSuchGroupException; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.cache.CacheModule; import com.google.gerrit.server.group.Groups; +import com.google.gerrit.server.group.InternalGroup; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.SchemaFactory; import com.google.inject.Inject; @@ -33,7 +35,6 @@ import com.google.inject.TypeLiteral; import com.google.inject.name.Named; import java.util.Collection; import java.util.Collections; -import java.util.Objects; import java.util.concurrent.ExecutionException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -174,8 +175,8 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache { return groups .getParentGroups(db, key) .map(groupCache::get) - .map(AccountGroup::getGroupUUID) - .filter(Objects::nonNull) + .flatMap(Streams::stream) + .map(InternalGroup::getGroupUUID) .collect(toImmutableList()); } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java index a1aefc5147..e55397ec64 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/CreateGroup.java @@ -56,6 +56,7 @@ import java.util.Collection; import java.util.Collections; import java.util.List; import java.util.Locale; +import java.util.Optional; import org.eclipse.jgit.errors.ConfigInvalidException; import org.eclipse.jgit.lib.Config; import org.eclipse.jgit.lib.PersonIdent; @@ -198,10 +199,8 @@ public class CreateGroup implements RestModifyView new AccountGroup(createGroupArgs.getGroup(), groupId, uuid, TimeUtil.nowTs()); group.setVisibleToAll(createGroupArgs.visibleToAll); if (createGroupArgs.ownerGroupId != null) { - AccountGroup ownerGroup = groupCache.get(createGroupArgs.ownerGroupId); - if (ownerGroup != null) { - group.setOwnerGroupUUID(ownerGroup.getGroupUUID()); - } + Optional ownerGroup = groupCache.get(createGroupArgs.ownerGroupId); + ownerGroup.map(InternalGroup::getGroupUUID).ifPresent(group::setOwnerGroupUUID); } if (createGroupArgs.groupDescription != null) { group.setDescription(createGroupArgs.groupDescription); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/DbGroupMemberAuditListener.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/DbGroupMemberAuditListener.java index ce287d0c0d..5af7ebdf8b 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/DbGroupMemberAuditListener.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/DbGroupMemberAuditListener.java @@ -152,7 +152,7 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { Account.Id accountId = m.getAccountId(); String userName = accountCache.get(accountId).getUserName(); AccountGroup.Id groupId = m.getAccountGroupId(); - String groupName = groupCache.get(groupId).getName(); + String groupName = getGroupName(groupId); descriptions.add( MessageFormat.format( @@ -168,7 +168,7 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { AccountGroup.UUID groupUuid = m.getIncludeUUID(); String groupName = groupBackend.get(groupUuid).getName(); AccountGroup.Id targetGroupId = m.getGroupId(); - String targetGroupName = groupCache.get(targetGroupId).getName(); + String targetGroupName = getGroupName(targetGroupId); descriptions.add( MessageFormat.format( @@ -178,6 +178,10 @@ class DbGroupMemberAuditListener implements GroupMemberAuditListener { logOrmException(header, me, descriptions, e); } + private String getGroupName(AccountGroup.Id groupId) { + return groupCache.get(groupId).map(InternalGroup::getName).orElse("Deleted group " + groupId); + } + private void logOrmException(String header, Account.Id me, Iterable values, OrmException e) { StringBuilder message = new StringBuilder(header); message.append(" "); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/group/Groups.java b/gerrit-server/src/main/java/com/google/gerrit/server/group/Groups.java index a1947ecfca..1e5d634445 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/group/Groups.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/group/Groups.java @@ -59,18 +59,6 @@ public class Groups { return group.orElseThrow(() -> new NoSuchGroupException(groupUuid)); } - /** - * Returns the {@code AccountGroup} for the specified ID if it exists. - * - * @param db the {@code ReviewDb} instance to use for lookups - * @param groupId the ID of the group - * @return the found {@code AccountGroup} if it exists, or else an empty {@code Optional} - * @throws OrmException if the group couldn't be retrieved from ReviewDb - */ - public Optional getGroup(ReviewDb db, AccountGroup.Id groupId) throws OrmException { - return Optional.ofNullable(db.accountGroups().get(groupId)); - } - /** * Returns the {@code AccountGroup} for the specified UUID if it exists. * diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupPredicates.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupPredicates.java index 983d3b3694..d02f6a46fa 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupPredicates.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/group/GroupPredicates.java @@ -24,6 +24,10 @@ import com.google.gerrit.server.index.group.GroupField; import java.util.Locale; public class GroupPredicates { + public static Predicate id(AccountGroup.Id groupId) { + return new GroupPredicate(GroupField.ID, groupId.toString()); + } + public static Predicate uuid(AccountGroup.UUID uuid) { return new GroupPredicate(GroupField.UUID, GroupQueryBuilder.FIELD_UUID, uuid.get()); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/query/group/InternalGroupQuery.java b/gerrit-server/src/main/java/com/google/gerrit/server/query/group/InternalGroupQuery.java index b261e259fe..513fffe78a 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/query/group/InternalGroupQuery.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/query/group/InternalGroupQuery.java @@ -20,6 +20,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; import com.google.gerrit.index.IndexConfig; import com.google.gerrit.index.query.InternalQuery; +import com.google.gerrit.index.query.Predicate; import com.google.gerrit.reviewdb.client.AccountGroup; import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.index.group.GroupIndexCollection; @@ -46,7 +47,16 @@ public class InternalGroupQuery extends InternalQuery { } public Optional byName(AccountGroup.NameKey groupName) throws OrmException { - List groups = query(GroupPredicates.name(groupName.get())); + return getOnlyGroup(GroupPredicates.name(groupName.get()), "group name '" + groupName + "'"); + } + + public Optional byId(AccountGroup.Id groupId) throws OrmException { + return getOnlyGroup(GroupPredicates.id(groupId), "group id '" + groupId + "'"); + } + + private Optional getOnlyGroup( + Predicate predicate, String groupDescription) throws OrmException { + List groups = query(predicate); if (groups.isEmpty()) { return Optional.empty(); } @@ -57,7 +67,7 @@ public class InternalGroupQuery extends InternalQuery { ImmutableList groupUuids = groups.stream().map(InternalGroup::getGroupUUID).collect(toImmutableList()); - log.warn(String.format("Ambiguous group name '%s' for groups %s.", groupName, groupUuids)); + log.warn(String.format("Ambiguous %s for groups %s.", groupDescription, groupUuids)); return Optional.empty(); } }