diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 80d84be6f1..11980a0d30 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -966,6 +966,19 @@ cache `"groups_bysubgroups"`:: Caches the parent groups of a subgroup. If direct updates are made to the `account_group_includes` table, this cache should be flushed. +cache `"groups_external"`:: ++ +Caches all the external groups available to Gerrit. The cache holds a +single entry which maps to the latest available of all external groups' +UUIDs. This cache uses "groups_external_persisted" to load its value. + +cache `"groups_external_persisted"`:: ++ +Caches all external groups available to Gerrit at some point in history. +The cache key is representation of a specific groups state in NoteDb and +the value is the list of all external groups. +The cache is persisted to enhance performance. + cache `"ldap_groups"`:: + Caches the LDAP groups that a user belongs to, if LDAP has been diff --git a/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java b/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java index 27fde83eb0..073ff84361 100644 --- a/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java +++ b/java/com/google/gerrit/server/account/GroupIncludeCacheImpl.java @@ -17,6 +17,7 @@ package com.google.gerrit.server.account; import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import com.google.common.cache.Cache; import com.google.common.cache.CacheLoader; import com.google.common.cache.LoadingCache; import com.google.common.collect.ImmutableList; @@ -24,7 +25,12 @@ import com.google.common.collect.ImmutableSet; import com.google.common.flogger.FluentLogger; import com.google.gerrit.entities.Account; import com.google.gerrit.entities.AccountGroup; +import com.google.gerrit.proto.Protos; import com.google.gerrit.server.cache.CacheModule; +import com.google.gerrit.server.cache.proto.Cache.AllExternalGroupsProto; +import com.google.gerrit.server.cache.proto.Cache.AllExternalGroupsProto.ExternalGroupProto; +import com.google.gerrit.server.cache.serialize.CacheSerializer; +import com.google.gerrit.server.cache.serialize.StringCacheSerializer; import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.group.db.Groups; import com.google.gerrit.server.logging.Metadata; @@ -49,6 +55,7 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache { private static final String PARENT_GROUPS_NAME = "groups_bysubgroup"; private static final String GROUPS_WITH_MEMBER_NAME = "groups_bymember"; private static final String EXTERNAL_NAME = "groups_external"; + private static final String PERSISTED_EXTERNAL_NAME = "groups_external_persisted"; public static Module module() { return new CacheModule() { @@ -66,8 +73,26 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache { new TypeLiteral>() {}) .loader(ParentGroupsLoader.class); + /** + * Splitting the groups external cache into 2 caches: The first one is in memory, used to + * serve the callers and has a single constant key "EXTERNAL_NAME". The second one is + * persisted, its key represents the groups' state in NoteDb. The in-memory cache is used on + * top of the persisted cache to enhance performance because the cache's value is used on + * every request to Gerrit, potentially many times per request and the key computation can + * become expensive. + */ cache(EXTERNAL_NAME, String.class, new TypeLiteral>() {}) - .loader(AllExternalLoader.class); + .loader(AllExternalInMemoryLoader.class); + + persist( + PERSISTED_EXTERNAL_NAME, + String.class, + new TypeLiteral>() {}) + .diskLimit(-1) + .version(1) + .maximumWeight(0) + .keySerializer(StringCacheSerializer.INSTANCE) + .valueSerializer(ExternalGroupsSerializer.INSTANCE); bind(GroupIncludeCacheImpl.class); bind(GroupIncludeCache.class).to(GroupIncludeCacheImpl.class); @@ -127,6 +152,11 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache { if (!groupId.isInternalGroup()) { logger.atFine().log("Evict external group %s", groupId.get()); + /** + * No need to invalidate the persistent cache, because this eviction will change the state + * of NoteDb causing the persistent cache's loader to use a new key that doesn't exist in + * its cache.n + */ external.invalidate(EXTERNAL_NAME); } } @@ -184,19 +214,54 @@ public class GroupIncludeCacheImpl implements GroupIncludeCache { } } - static class AllExternalLoader extends CacheLoader> { + static class AllExternalInMemoryLoader + extends CacheLoader> { + private final Cache> persisted; + private final GroupsSnapshotReader snapshotReader; private final Groups groups; @Inject - AllExternalLoader(Groups groups) { + AllExternalInMemoryLoader( + @Named(PERSISTED_EXTERNAL_NAME) Cache> persisted, + GroupsSnapshotReader snapshotReader, + Groups groups) { + this.persisted = persisted; + this.snapshotReader = snapshotReader; this.groups = groups; } @Override public ImmutableList load(String key) throws Exception { - try (TraceTimer timer = TraceContext.newTimer("Loading all external groups")) { - return groups.getExternalGroups().collect(toImmutableList()); - } + GroupsSnapshotReader.Snapshot snapshot = snapshotReader.getSnapshot(); + return persisted.get( + snapshot.hash(), + () -> { + try (TraceTimer timer = TraceContext.newTimer("Loading all external groups")) { + return groups.getExternalGroups(snapshot.groupsRefs()).collect(toImmutableList()); + } + }); + } + } + + public enum ExternalGroupsSerializer + implements CacheSerializer> { + INSTANCE; + + @Override + public byte[] serialize(ImmutableList object) { + AllExternalGroupsProto.Builder allBuilder = AllExternalGroupsProto.newBuilder(); + object.stream() + .map(group -> ExternalGroupProto.newBuilder().setGroupUuid(group.get()).build()) + .forEach(allBuilder::addExternalGroup); + return Protos.toByteArray(allBuilder.build()); + } + + @Override + public ImmutableList deserialize(byte[] in) { + return Protos.parseUnchecked(AllExternalGroupsProto.parser(), in).getExternalGroupList() + .stream() + .map(groupProto -> AccountGroup.UUID.parse(groupProto.getGroupUuid())) + .collect(toImmutableList()); } } } diff --git a/java/com/google/gerrit/server/account/GroupsSnapshotReader.java b/java/com/google/gerrit/server/account/GroupsSnapshotReader.java new file mode 100644 index 0000000000..62cdb17603 --- /dev/null +++ b/java/com/google/gerrit/server/account/GroupsSnapshotReader.java @@ -0,0 +1,75 @@ +// Copyright (C) 2020 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.auto.value.AutoValue; +import com.google.common.collect.ImmutableList; +import com.google.common.hash.Hashing; +import com.google.gerrit.entities.RefNames; +import com.google.gerrit.server.config.AllUsersName; +import com.google.gerrit.server.git.GitRepositoryManager; +import com.google.inject.Inject; +import com.google.inject.Singleton; +import java.io.IOException; +import java.nio.ByteBuffer; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.Ref; +import org.eclipse.jgit.lib.Repository; + +/** + * This class is used to compute a compound key that represents the state of the internal groups in + * NoteDb. + */ +@Singleton +public class GroupsSnapshotReader { + private final GitRepositoryManager repoManager; + private final AllUsersName allUsersName; + + @Inject + GroupsSnapshotReader(GitRepositoryManager repoManager, AllUsersName allUsersName) { + this.repoManager = repoManager; + this.allUsersName = allUsersName; + } + + @AutoValue + public abstract static class Snapshot { + /** + * 128-bit hash of all group ref {@link com.google.gerrit.git.ObjectIds}. To be used as cache + * key. + */ + public abstract String hash(); + + /** Snapshot of the state of all relevant NoteDb group refs. */ + public abstract ImmutableList groupsRefs(); + + public static Snapshot create(String hash, ImmutableList groupsRefs) { + return new AutoValue_GroupsSnapshotReader_Snapshot(hash, groupsRefs); + } + } + + /** Retrieves a snapshot key of all internal groups refs from NoteDb. */ + public Snapshot getSnapshot() throws IOException { + try (Repository repo = repoManager.openRepository(allUsersName)) { + ImmutableList groupsRefs = + ImmutableList.copyOf(repo.getRefDatabase().getRefsByPrefix(RefNames.REFS_GROUPS)); + ByteBuffer buf = ByteBuffer.allocate(groupsRefs.size() * Constants.OBJECT_ID_LENGTH); + for (Ref groupRef : groupsRefs) { + groupRef.getObjectId().copyRawTo(buf); + } + String hash = Hashing.murmur3_128().hashBytes(buf.array()).toString(); + return Snapshot.create(hash, groupsRefs); + } + } +} diff --git a/java/com/google/gerrit/server/auth/ldap/FakeLdapGroupBackend.java b/java/com/google/gerrit/server/auth/ldap/FakeLdapGroupBackend.java new file mode 100644 index 0000000000..c2123cb53f --- /dev/null +++ b/java/com/google/gerrit/server/auth/ldap/FakeLdapGroupBackend.java @@ -0,0 +1,87 @@ +// Copyright (C) 2020 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.auth.ldap; + +import static com.google.gerrit.server.auth.ldap.Helper.LDAP_UUID; + +import com.google.gerrit.common.Nullable; +import com.google.gerrit.common.data.GroupDescription; +import com.google.gerrit.common.data.GroupReference; +import com.google.gerrit.entities.AccountGroup; +import com.google.gerrit.server.IdentifiedUser; +import com.google.gerrit.server.account.GroupBackend; +import com.google.gerrit.server.account.GroupMembership; +import com.google.gerrit.server.account.ListGroupMembership; +import com.google.gerrit.server.project.ProjectState; +import java.util.Collection; +import java.util.Collections; + +/** Fake Implementation of an LDAP group backend used for testing */ +public class FakeLdapGroupBackend implements GroupBackend { + + FakeLdapGroupBackend() {} + + @Override + public boolean handles(AccountGroup.UUID uuid) { + /** Returns true if the provided parameter is an LDAP UUID */ + return uuid.get().startsWith(LDAP_UUID); + } + + @Override + public GroupDescription.Basic get(AccountGroup.UUID uuid) { + if (!handles(uuid)) { + return null; + } + + return new GroupDescription.Basic() { + @Override + public AccountGroup.UUID getGroupUUID() { + return uuid; + } + + @Override + public String getName() { + return "fake_group"; + } + + @Override + @Nullable + public String getEmailAddress() { + return null; + } + + @Override + @Nullable + public String getUrl() { + return null; + } + }; + } + + @Override + public Collection suggest(String name, ProjectState project) { + return Collections.emptySet(); + } + + @Override + public GroupMembership membershipsOf(IdentifiedUser user) { + return new ListGroupMembership(Collections.emptyList()); + } + + @Override + public boolean isVisibleToAll(AccountGroup.UUID uuid) { + return true; + } +} diff --git a/java/com/google/gerrit/server/group/db/GroupConfig.java b/java/com/google/gerrit/server/group/db/GroupConfig.java index ab5c9b8563..c12f3a7f82 100644 --- a/java/com/google/gerrit/server/group/db/GroupConfig.java +++ b/java/com/google/gerrit/server/group/db/GroupConfig.java @@ -144,8 +144,25 @@ public class GroupConfig extends VersionedMetaData { public static GroupConfig loadForGroup( Project.NameKey projectName, Repository repository, AccountGroup.UUID groupUuid) throws IOException, ConfigInvalidException { + return loadForGroup(projectName, repository, groupUuid, null); + } + + /** + * @see GroupConfig#loadForGroup(Project.NameKey, Repository, AccountGroup.UUID). This method will + * load the group for a specific revision. + */ + public static GroupConfig loadForGroup( + Project.NameKey projectName, + Repository repository, + AccountGroup.UUID groupUuid, + ObjectId groupRefObjectId) + throws IOException, ConfigInvalidException { GroupConfig groupConfig = new GroupConfig(groupUuid); - groupConfig.load(projectName, repository); + if (groupRefObjectId == null) { + groupConfig.load(projectName, repository); + } else { + groupConfig.load(projectName, repository, groupRefObjectId); + } return groupConfig; } diff --git a/java/com/google/gerrit/server/group/db/Groups.java b/java/com/google/gerrit/server/group/db/Groups.java index 7a1b351f18..163b9c6b0c 100644 --- a/java/com/google/gerrit/server/group/db/Groups.java +++ b/java/com/google/gerrit/server/group/db/Groups.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.group.db; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; +import com.google.common.flogger.FluentLogger; import com.google.gerrit.common.data.GroupReference; import com.google.gerrit.entities.AccountGroup; import com.google.gerrit.entities.AccountGroupByIdAudit; @@ -30,6 +31,8 @@ import java.util.List; import java.util.Optional; import java.util.stream.Stream; import org.eclipse.jgit.errors.ConfigInvalidException; +import org.eclipse.jgit.lib.ObjectId; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.Repository; /** @@ -47,6 +50,8 @@ import org.eclipse.jgit.lib.Repository; */ @Singleton public class Groups { + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private final GitRepositoryManager repoManager; private final AllUsersName allUsersName; private final AuditLogReader auditLogReader; @@ -74,10 +79,28 @@ public class Groups { } } + /** + * Loads an internal group from NoteDb using the group UUID. This method returns the latest state + * of the internal group. + */ private static Optional getGroupFromNoteDb( AllUsersName allUsersName, Repository allUsersRepository, AccountGroup.UUID groupUuid) throws IOException, ConfigInvalidException { - GroupConfig groupConfig = GroupConfig.loadForGroup(allUsersName, allUsersRepository, groupUuid); + return getGroupFromNoteDb(allUsersName, allUsersRepository, groupUuid, null); + } + + /** + * Loads an internal group from NoteDb at the revision provided as {@link ObjectId}. This method + * is used to get a specific state of this group. + */ + private static Optional getGroupFromNoteDb( + AllUsersName allUsersName, + Repository allUsersRepository, + AccountGroup.UUID uuid, + ObjectId groupRefObjectId) + throws IOException, ConfigInvalidException { + GroupConfig groupConfig = + GroupConfig.loadForGroup(allUsersName, allUsersRepository, uuid, groupRefObjectId); Optional loadedGroup = groupConfig.getLoadedGroup(); if (loadedGroup.isPresent()) { // Check consistency with group name notes. @@ -104,24 +127,31 @@ public class Groups { * Returns all known external groups. External groups are 'known' when they are specified as a * subgroup of an internal group. * + * @param internalGroupsRefs contains a list of all groups refs that we should inspect * @return a stream of the UUIDs of the known external groups * @throws IOException if an error occurs while reading from NoteDb * @throws ConfigInvalidException if the data in NoteDb is in an incorrect format */ - public Stream getExternalGroups() throws IOException, ConfigInvalidException { + public Stream getExternalGroups(ImmutableList internalGroupsRefs) + throws IOException, ConfigInvalidException { try (Repository allUsersRepo = repoManager.openRepository(allUsersName)) { - return getExternalGroupsFromNoteDb(allUsersName, allUsersRepo); + return getExternalGroupsFromNoteDb(allUsersName, allUsersRepo, internalGroupsRefs); } } private static Stream getExternalGroupsFromNoteDb( - AllUsersName allUsersName, Repository allUsersRepo) + AllUsersName allUsersName, Repository allUsersRepo, ImmutableList internalGroupsRefs) throws IOException, ConfigInvalidException { - ImmutableList allInternalGroups = GroupNameNotes.loadAllGroups(allUsersRepo); ImmutableSet.Builder allSubgroups = ImmutableSet.builder(); - for (GroupReference internalGroup : allInternalGroups) { + for (Ref internalGroupRef : internalGroupsRefs) { + AccountGroup.UUID uuid = AccountGroup.UUID.fromRef(internalGroupRef.getName()); + if (uuid == null) { + logger.atWarning().log( + "Failed to get the group UUID from ref: %s", internalGroupRef.getName()); + continue; + } Optional group = - getGroupFromNoteDb(allUsersName, allUsersRepo, internalGroup.getUUID()); + getGroupFromNoteDb(allUsersName, allUsersRepo, uuid, internalGroupRef.getObjectId()); group.map(InternalGroup::getSubgroups).ifPresent(allSubgroups::addAll); } return allSubgroups.build().stream().filter(groupUuid -> !groupUuid.isInternalGroup()); diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java index 30eaf2253c..f30926b13f 100644 --- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java +++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java @@ -15,6 +15,7 @@ package com.google.gerrit.acceptance.api.group; import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.collect.MoreCollectors.onlyElement; import static com.google.common.truth.Truth.assertThat; import static com.google.common.truth.Truth8.assertThat; import static com.google.gerrit.acceptance.GitUtil.deleteRef; @@ -72,6 +73,7 @@ import com.google.gerrit.extensions.common.GroupAuditEventInfo.UserMemberAuditEv import com.google.gerrit.extensions.common.GroupInfo; import com.google.gerrit.extensions.common.GroupOptionsInfo; import com.google.gerrit.extensions.events.GroupIndexedListener; +import com.google.gerrit.extensions.registration.DynamicSet; import com.google.gerrit.extensions.restapi.AuthException; import com.google.gerrit.extensions.restapi.BadRequestException; import com.google.gerrit.extensions.restapi.ResourceConflictException; @@ -79,7 +81,10 @@ import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.extensions.restapi.UnprocessableEntityException; import com.google.gerrit.extensions.restapi.Url; import com.google.gerrit.server.ServerInitiated; +import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupIncludeCache; +import com.google.gerrit.server.account.GroupsSnapshotReader; +import com.google.gerrit.server.auth.ldap.FakeLdapGroupBackend; import com.google.gerrit.server.group.InternalGroup; import com.google.gerrit.server.group.PeriodicGroupIndexer; import com.google.gerrit.server.group.SystemGroupBackend; @@ -94,7 +99,9 @@ import com.google.gerrit.server.notedb.Sequences; import com.google.gerrit.server.util.MagicBranch; import com.google.gerrit.server.util.time.TimeUtil; import com.google.gerrit.testing.GerritJUnit.ThrowingRunnable; +import com.google.inject.AbstractModule; import com.google.inject.Inject; +import com.google.inject.Module; import java.io.IOException; import java.lang.annotation.Retention; import java.lang.annotation.Target; @@ -113,6 +120,7 @@ import org.eclipse.jgit.lib.Constants; import org.eclipse.jgit.lib.ObjectId; import org.eclipse.jgit.lib.ObjectInserter; import org.eclipse.jgit.lib.PersonIdent; +import org.eclipse.jgit.lib.Ref; import org.eclipse.jgit.lib.RefUpdate; import org.eclipse.jgit.lib.Repository; import org.eclipse.jgit.revwalk.RevCommit; @@ -138,6 +146,18 @@ public class GroupsIT extends AbstractDaemonTest { @Inject private Sequences seq; @Inject private StalenessChecker stalenessChecker; @Inject private ExtensionRegistry extensionRegistry; + @Inject private GroupsSnapshotReader groupsSnapshotReader; + + @Override + public Module createModule() { + return new AbstractModule() { + @Override + protected void configure() { + /** Binding a {@link FakeLdapGroupBackend} to test adding external groups * */ + DynamicSet.bind(binder(), GroupBackend.class).to(FakeLdapGroupBackend.class); + } + }; + } @After public void consistencyCheck() throws Exception { @@ -186,6 +206,68 @@ public class GroupsIT extends AbstractDaemonTest { assertThat(members).isEmpty(); } + @Test + public void addExternalGroups() throws Exception { + AccountGroup.UUID group1 = groupOperations.newGroup().create(); + AccountGroup.UUID group2 = groupOperations.newGroup().create(); + String g1RefName = RefNames.refsGroups(group1); + String g2RefName = RefNames.refsGroups(group2); + + gApi.groups().id(group1.get()).addGroups("ldap:external_g1"); + gApi.groups().id(group2.get()).addGroups("ldap:external_g2"); + + assertThat(groupIncludeCache.allExternalMembers()) + .containsExactlyElementsIn( + ImmutableList.of( + AccountGroup.UUID.parse("ldap:external_g1"), + AccountGroup.UUID.parse("ldap:external_g2"), + AccountGroup.UUID.parse("global:Registered-Users"))); + + assertThat(groupIncludeCache.parentGroupsOf(AccountGroup.UUID.parse("ldap:external_g1"))) + .containsExactly(group1); + assertThat(groupIncludeCache.parentGroupsOf(AccountGroup.UUID.parse("ldap:external_g2"))) + .containsExactly(group2); + + GroupsSnapshotReader.Snapshot snapshot = groupsSnapshotReader.getSnapshot(); + + gApi.groups().id(group1.get()).removeGroups("ldap:external_g1"); + + GroupsSnapshotReader.Snapshot newSnapshot = groupsSnapshotReader.getSnapshot(); + + /** Make sure groups snapshots are consistent */ + ObjectId g1ObjectId = getObjectIdFromSnapshot(snapshot, g1RefName); + ObjectId g2ObjectId = getObjectIdFromSnapshot(snapshot, g2RefName); + assertThat(snapshot.hash()).isNotEqualTo(newSnapshot.hash()); + assertThat(g1ObjectId).isNotEqualTo(getObjectIdFromSnapshot(newSnapshot, g1RefName)); + assertThat(g2ObjectId).isEqualTo(getObjectIdFromSnapshot(newSnapshot, g2RefName)); + assertThat(snapshot.groupsRefs().stream().map(Ref::getName).collect(toList())) + .containsAtLeastElementsIn(ImmutableList.of(g1RefName, g2RefName)); + assertThat(newSnapshot.groupsRefs().stream().map(Ref::getName).collect(toList())) + .containsAtLeastElementsIn(ImmutableList.of(g1RefName, g2RefName)); + + /** GroupIncludeCache should return ldap:external_g2 only */ + assertThat(groupIncludeCache.allExternalMembers()) + .containsExactlyElementsIn( + ImmutableList.of( + AccountGroup.UUID.parse("ldap:external_g2"), + AccountGroup.UUID.parse("global:Registered-Users"))); + + /** Testing groups.getExternalGroups() with the old Snapshot */ + assertThat(groups.getExternalGroups(snapshot.groupsRefs())) + .containsExactlyElementsIn( + ImmutableList.of( + AccountGroup.UUID.parse("ldap:external_g1"), + AccountGroup.UUID.parse("ldap:external_g2"), + AccountGroup.UUID.parse("global:Registered-Users"))); + } + + private ObjectId getObjectIdFromSnapshot(GroupsSnapshotReader.Snapshot snapshot, String refName) { + return snapshot.groupsRefs().stream() + .filter(r -> r.getName().equals(refName)) + .map(Ref::getObjectId) + .collect(onlyElement()); + } + @Test public void removeMember_nullInMemberInputDoesNotCauseFailure() throws Exception { AccountGroup.UUID group = diff --git a/javatests/com/google/gerrit/server/cache/serialize/BUILD b/javatests/com/google/gerrit/server/cache/serialize/BUILD index ce5f273a38..fa6a71711a 100644 --- a/javatests/com/google/gerrit/server/cache/serialize/BUILD +++ b/javatests/com/google/gerrit/server/cache/serialize/BUILD @@ -5,6 +5,7 @@ junit_tests( srcs = glob(["*.java"]), deps = [ "//java/com/google/gerrit/entities", + "//java/com/google/gerrit/server", "//java/com/google/gerrit/server/cache/serialize", "//java/com/google/gerrit/server/cache/testing", "//java/com/google/gerrit/testing:gerrit-test-util", diff --git a/javatests/com/google/gerrit/server/cache/serialize/ExternalGroupsSerializerTest.java b/javatests/com/google/gerrit/server/cache/serialize/ExternalGroupsSerializerTest.java new file mode 100644 index 0000000000..ba2bfcad47 --- /dev/null +++ b/javatests/com/google/gerrit/server/cache/serialize/ExternalGroupsSerializerTest.java @@ -0,0 +1,53 @@ +// Copyright (C) 2020 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.cache.serialize; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.gerrit.entities.AccountGroup; +import com.google.gerrit.server.account.GroupIncludeCacheImpl.ExternalGroupsSerializer; +import com.google.gerrit.server.cache.proto.Cache.AllExternalGroupsProto; +import com.google.gerrit.server.cache.proto.Cache.AllExternalGroupsProto.ExternalGroupProto; +import com.google.protobuf.InvalidProtocolBufferException; +import org.junit.Test; + +public class ExternalGroupsSerializerTest { + private static final ImmutableList GROUPS = + ImmutableList.of( + AccountGroup.UUID.parse("593f90fcf688109f61b0fd4aa47ddf65abb96012"), + AccountGroup.UUID.parse("bc9f75584ac0362584a64fb3f0095d905415b153")); + + @Test + public void serialize() throws InvalidProtocolBufferException { + byte[] serialized = ExternalGroupsSerializer.INSTANCE.serialize(GROUPS); + + assertThat(AllExternalGroupsProto.parseFrom(serialized)) + .isEqualTo( + AllExternalGroupsProto.newBuilder() + .addAllExternalGroup( + GROUPS.stream() + .map(g -> ExternalGroupProto.newBuilder().setGroupUuid(g.get()).build()) + .collect(toImmutableList())) + .build()); + } + + @Test + public void deserialize() { + byte[] serialized = ExternalGroupsSerializer.INSTANCE.serialize(GROUPS); + assertThat(ExternalGroupsSerializer.INSTANCE.deserialize(serialized)).isEqualTo(GROUPS); + } +} diff --git a/proto/cache.proto b/proto/cache.proto index c80d51bd58..7c0b70266b 100644 --- a/proto/cache.proto +++ b/proto/cache.proto @@ -270,6 +270,15 @@ message AllExternalIdsProto { repeated ExternalIdProto external_id = 1; } +// Serialized form of a list of com.google.gerrit.entities.AccountGroup.UUID +// Next ID: 2 +message AllExternalGroupsProto { + message ExternalGroupProto { + string groupUuid = 1; + } + repeated ExternalGroupProto external_group = 1; +} + // Key for com.google.gerrit.server.git.PureRevertCache. // Next ID: 4 message PureRevertKeyProto {