diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 4c71124ec3..7b04e37c92 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -786,6 +786,7 @@ Default is 128 MiB per cache, except: + * `"change_notes"`: disk storage is disabled by default * `"diff_summary"`: default is `1g` (1 GiB of disk space) +* `"external_ids_map"`: disk storage is disabled by default + If 0 or negative, disk storage for the cache is disabled. @@ -862,8 +863,10 @@ of link:config-accounts.html#external-ids[all current external IDs]. The cache may temporarily contain 2 entries, but the second one is promptly expired. + -It is not recommended to change the attributes of this cache away from -the defaults. +It is not recommended to change the in-memory attributes of this cache +away from the defaults. The cache may be persisted by setting +`diskLimit`, which is only recommended if cold start performance is +problematic. cache `"git_tags"`:: + diff --git a/java/com/google/gerrit/server/account/externalids/AllExternalIds.java b/java/com/google/gerrit/server/account/externalids/AllExternalIds.java index 0e86df2dd2..bb1ade753b 100644 --- a/java/com/google/gerrit/server/account/externalids/AllExternalIds.java +++ b/java/com/google/gerrit/server/account/externalids/AllExternalIds.java @@ -15,12 +15,18 @@ package com.google.gerrit.server.account.externalids; import static com.google.common.collect.ImmutableSetMultimap.toImmutableSetMultimap; +import static java.util.stream.Collectors.toList; import com.google.auto.value.AutoValue; import com.google.common.base.Strings; import com.google.common.collect.ImmutableSetMultimap; import com.google.common.collect.SetMultimap; import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.server.cache.proto.Cache.AllExternalIdsProto; +import com.google.gerrit.server.cache.proto.Cache.AllExternalIdsProto.ExternalIdProto; +import com.google.gerrit.server.cache.serialize.CacheSerializer; +import com.google.gerrit.server.cache.serialize.ProtoCacheSerializers; +import com.google.gerrit.server.cache.serialize.ProtoCacheSerializers.ObjectIdConverter; import java.util.Collection; /** Cache value containing all external IDs. */ @@ -48,4 +54,59 @@ public abstract class AllExternalIds { public abstract ImmutableSetMultimap byAccount(); public abstract ImmutableSetMultimap byEmail(); + + enum Serializer implements CacheSerializer { + INSTANCE; + + @Override + public byte[] serialize(AllExternalIds object) { + ObjectIdConverter idConverter = ObjectIdConverter.create(); + AllExternalIdsProto.Builder allBuilder = AllExternalIdsProto.newBuilder(); + object + .byAccount() + .values() + .stream() + .map(extId -> toProto(idConverter, extId)) + .forEach(allBuilder::addExternalId); + return ProtoCacheSerializers.toByteArray(allBuilder.build()); + } + + private static ExternalIdProto toProto(ObjectIdConverter idConverter, ExternalId externalId) { + ExternalIdProto.Builder b = + ExternalIdProto.newBuilder() + .setKey(externalId.key().get()) + .setAccountId(externalId.accountId().get()); + if (externalId.email() != null) { + b.setEmail(externalId.email()); + } + if (externalId.password() != null) { + b.setPassword(externalId.password()); + } + if (externalId.blobId() != null) { + b.setBlobId(idConverter.toByteString(externalId.blobId())); + } + return b.build(); + } + + @Override + public AllExternalIds deserialize(byte[] in) { + ObjectIdConverter idConverter = ObjectIdConverter.create(); + return create( + ProtoCacheSerializers.parseUnchecked(AllExternalIdsProto.parser(), in) + .getExternalIdList() + .stream() + .map(proto -> toExternalId(idConverter, proto)) + .collect(toList())); + } + + private static ExternalId toExternalId(ObjectIdConverter idConverter, ExternalIdProto proto) { + return ExternalId.create( + ExternalId.Key.parse(proto.getKey()), + new Account.Id(proto.getAccountId()), + // ExternalId treats null and empty strings the same, so no need to distinguish here. + proto.getEmail(), + proto.getPassword(), + !proto.getBlobId().isEmpty() ? idConverter.fromByteString(proto.getBlobId()) : null); + } + } } diff --git a/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java b/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java index 228b1e68b7..fc311e7aab 100644 --- a/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java +++ b/java/com/google/gerrit/server/account/externalids/ExternalIdModule.java @@ -16,6 +16,7 @@ package com.google.gerrit.server.account.externalids; import com.google.gerrit.server.account.externalids.ExternalIdCacheImpl.Loader; import com.google.gerrit.server.cache.CacheModule; +import com.google.gerrit.server.cache.serialize.ObjectIdCacheSerializer; import com.google.inject.TypeLiteral; import java.time.Duration; import org.eclipse.jgit.lib.ObjectId; @@ -23,7 +24,7 @@ import org.eclipse.jgit.lib.ObjectId; public class ExternalIdModule extends CacheModule { @Override protected void configure() { - cache(ExternalIdCacheImpl.CACHE_NAME, ObjectId.class, new TypeLiteral() {}) + persist(ExternalIdCacheImpl.CACHE_NAME, ObjectId.class, new TypeLiteral() {}) // The cached data is potentially pretty large and we are always only interested // in the latest value. However, due to a race condition, it is possible for different // threads to observe different values of the meta ref, and hence request different keys @@ -32,7 +33,11 @@ public class ExternalIdModule extends CacheModule { // memory. .maximumWeight(2) .expireFromMemoryAfterAccess(Duration.ofMinutes(1)) - .loader(Loader.class); + .loader(Loader.class) + .diskLimit(-1) + .version(1) + .keySerializer(ObjectIdCacheSerializer.INSTANCE) + .valueSerializer(AllExternalIds.Serializer.INSTANCE); bind(ExternalIdCacheImpl.class); bind(ExternalIdCache.class).to(ExternalIdCacheImpl.class); diff --git a/javatests/com/google/gerrit/server/account/externalids/AllExternalIdsTest.java b/javatests/com/google/gerrit/server/account/externalids/AllExternalIdsTest.java new file mode 100644 index 0000000000..f29ff1fb91 --- /dev/null +++ b/javatests/com/google/gerrit/server/account/externalids/AllExternalIdsTest.java @@ -0,0 +1,143 @@ +// Copyright (C) 2018 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.externalids; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat; +import static com.google.gerrit.server.cache.testing.CacheSerializerTestUtil.byteString; +import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass; + +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSetMultimap; +import com.google.gerrit.reviewdb.client.Account; +import com.google.gerrit.server.account.externalids.AllExternalIds.Serializer; +import com.google.gerrit.server.cache.proto.Cache.AllExternalIdsProto; +import com.google.gerrit.server.cache.proto.Cache.AllExternalIdsProto.ExternalIdProto; +import com.google.inject.TypeLiteral; +import java.util.Arrays; +import org.eclipse.jgit.lib.ObjectId; +import org.junit.Test; + +public class AllExternalIdsTest { + @Test + public void serializeEmptyExternalIds() throws Exception { + assertRoundTrip(allExternalIds(), AllExternalIdsProto.getDefaultInstance()); + } + + @Test + public void serializeMultipleExternalIds() throws Exception { + Account.Id accountId1 = new Account.Id(1001); + Account.Id accountId2 = new Account.Id(1002); + assertRoundTrip( + allExternalIds( + ExternalId.create("scheme1", "id1", accountId1), + ExternalId.create("scheme2", "id2", accountId1), + ExternalId.create("scheme2", "id3", accountId2), + ExternalId.create("scheme3", "id4", accountId2)), + AllExternalIdsProto.newBuilder() + .addExternalId( + ExternalIdProto.newBuilder().setKey("scheme1:id1").setAccountId(1001).build()) + .addExternalId( + ExternalIdProto.newBuilder().setKey("scheme2:id2").setAccountId(1001).build()) + .addExternalId( + ExternalIdProto.newBuilder().setKey("scheme2:id3").setAccountId(1002).build()) + .addExternalId( + ExternalIdProto.newBuilder().setKey("scheme3:id4").setAccountId(1002).build()) + .build()); + } + + @Test + public void serializeExternalIdWithEmail() throws Exception { + assertRoundTrip( + allExternalIds(ExternalId.createEmail(new Account.Id(1001), "foo@example.com")), + AllExternalIdsProto.newBuilder() + .addExternalId( + ExternalIdProto.newBuilder() + .setKey("mailto:foo@example.com") + .setAccountId(1001) + .setEmail("foo@example.com")) + .build()); + } + + @Test + public void serializeExternalIdWithPassword() throws Exception { + assertRoundTrip( + allExternalIds( + ExternalId.create("scheme", "id", new Account.Id(1001), null, "hashed password")), + AllExternalIdsProto.newBuilder() + .addExternalId( + ExternalIdProto.newBuilder() + .setKey("scheme:id") + .setAccountId(1001) + .setPassword("hashed password")) + .build()); + } + + @Test + public void serializeExternalIdWithBlobId() throws Exception { + assertRoundTrip( + allExternalIds( + ExternalId.create( + ExternalId.create("scheme", "id", new Account.Id(1001)), + ObjectId.fromString("deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"))), + AllExternalIdsProto.newBuilder() + .addExternalId( + ExternalIdProto.newBuilder() + .setKey("scheme:id") + .setAccountId(1001) + .setBlobId( + byteString( + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef, + 0xde, 0xad, 0xbe, 0xef, 0xde, 0xad, 0xbe, 0xef))) + .build()); + } + + @Test + public void allExternalIdsMethods() { + assertThatSerializedClass(AllExternalIds.class) + .hasAutoValueMethods( + ImmutableMap.of( + "byAccount", + new TypeLiteral>() {}.getType(), + "byEmail", + new TypeLiteral>() {}.getType())); + } + + @Test + public void externalIdMethods() { + assertThatSerializedClass(ExternalId.class) + .hasAutoValueMethods( + ImmutableMap.of( + "key", ExternalId.Key.class, + "accountId", Account.Id.class, + "email", String.class, + "password", String.class, + "blobId", ObjectId.class)); + } + + private static AllExternalIds allExternalIds(ExternalId... externalIds) { + return AllExternalIds.create(Arrays.asList(externalIds)); + } + + private static void assertRoundTrip( + AllExternalIds allExternalIds, AllExternalIdsProto expectedProto) throws Exception { + AllExternalIdsProto actualProto = + AllExternalIdsProto.parseFrom(Serializer.INSTANCE.serialize(allExternalIds)); + assertThat(actualProto).ignoringRepeatedFieldOrder().isEqualTo(expectedProto); + AllExternalIds actual = + Serializer.INSTANCE.deserialize(Serializer.INSTANCE.serialize(allExternalIds)); + assertThat(actual).isEqualTo(allExternalIds); + } +} diff --git a/proto/cache.proto b/proto/cache.proto index 33f91434d9..c2ac0d92ed 100644 --- a/proto/cache.proto +++ b/proto/cache.proto @@ -219,3 +219,18 @@ message TagSetHolderProto { } TagSetProto tags = 2; } + +// Serialized form of +// com.google.gerrit.server.account.externalids.AllExternalIds. +// Next ID: 2 +message AllExternalIdsProto { + // Next ID: 6 + message ExternalIdProto { + string key = 1; + int32 accountId = 2; + string email = 3; + string password = 4; + bytes blobId = 5; + } + repeated ExternalIdProto external_id = 1; +}