Merge changes from topic "tag-cache-proto"

* changes:
  Serialize TagSetHolder with protobuf
  TagCache: Eliminate extra lock on cache load
  Extract StringSerializer to its own non-test class

java/com/google/gerrit/server/cache/StringSerializer.java

@@ -0,0 +1,70 @@
+// 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.cache;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+
+import com.google.common.annotations.VisibleForTesting;
+import java.nio.ByteBuffer;
+import java.nio.CharBuffer;
+import java.nio.charset.CharacterCodingException;
+import java.nio.charset.Charset;
+import java.nio.charset.CodingErrorAction;
+
+public enum StringSerializer implements CacheSerializer<String> {
+  INSTANCE;
+
+  @Override
+  public byte[] serialize(String object) {
+    return serialize(UTF_8, object);
+  }
+
+  @VisibleForTesting
+  static byte[] serialize(Charset charset, String s) {
+    if (s.isEmpty()) {
+      return new byte[0];
+    }
+    try {
+      ByteBuffer buf =
+          charset
+              .newEncoder()
+              .onMalformedInput(CodingErrorAction.REPORT)
+              .onUnmappableCharacter(CodingErrorAction.REPORT)
+              .encode(CharBuffer.wrap(s));
+      byte[] result = new byte[buf.remaining()];
+      buf.get(result);
+      return result;
+    } catch (CharacterCodingException e) {
+      throw new IllegalStateException("Failed to serialize string", e);
+    }
+  }
+
+  @Override
+  public String deserialize(byte[] in) {
+    if (in.length == 0) {
+      return "";
+    }
+    try {
+      return UTF_8
+          .newDecoder()
+          .onMalformedInput(CodingErrorAction.REPORT)
+          .onUnmappableCharacter(CodingErrorAction.REPORT)
+          .decode(ByteBuffer.wrap(in))
+          .toString();
+    } catch (CharacterCodingException e) {
+      throw new IllegalStateException("Failed to deserialize string", e);
+    }
+  }
+}

java/com/google/gerrit/server/git/TagCache.java

@@ -17,14 +17,12 @@ package com.google.gerrit.server.git;
 import com.google.common.cache.Cache;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.server.cache.CacheModule;
+import com.google.gerrit.server.cache.StringSerializer;
 import com.google.inject.Inject;
 import com.google.inject.Module;
 import com.google.inject.Singleton;
 import com.google.inject.name.Named;
-import java.io.IOException;
+import java.util.concurrent.ExecutionException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
-import java.io.Serializable;
 import org.eclipse.jgit.lib.ObjectId;
 
 @Singleton
@@ -35,17 +33,19 @@ public class TagCache {
     return new CacheModule() {
       @Override
       protected void configure() {
-        persist(CACHE_NAME, String.class, EntryVal.class);
+        persist(CACHE_NAME, String.class, TagSetHolder.class)
+            .version(1)
+            .keySerializer(StringSerializer.INSTANCE)
+            .valueSerializer(TagSetHolder.Serializer.INSTANCE);
         bind(TagCache.class);
       }
     };
   }
 
-  private final Cache<String, EntryVal> cache;
+  private final Cache<String, TagSetHolder> cache;
-  private final Object createLock = new Object();
 
   @Inject
-  TagCache(@Named(CACHE_NAME) Cache<String, EntryVal> cache) {
+  TagCache(@Named(CACHE_NAME) Cache<String, TagSetHolder> cache) {
     this.cache = cache;
   }
 
@@ -68,62 +68,26 @@ public class TagCache {
     // never fail with an exception. Some of these references can be null
     // (e.g. not all projects are cached, or the cache is not current).
     //
-    EntryVal val = cache.getIfPresent(name.get());
+    TagSetHolder holder = cache.getIfPresent(name.get());
-    if (val != null) {
-      TagSetHolder holder = val.holder;
     if (holder != null) {
       TagSet tags = holder.getTagSet();
       if (tags != null) {
         if (tags.updateFastForward(refName, oldValue, newValue)) {
-            cache.put(name.get(), val);
+          cache.put(name.get(), holder);
-          }
         }
       }
     }
   }
 
   public TagSetHolder get(Project.NameKey name) {
-    EntryVal val = cache.getIfPresent(name.get());
+    try {
-    if (val == null) {
+      return cache.get(name.get(), () -> new TagSetHolder(name));
-      synchronized (createLock) {
+    } catch (ExecutionException e) {
-        val = cache.getIfPresent(name.get());
+      throw new IllegalStateException(e);
-        if (val == null) {
-          val = new EntryVal();
-          val.holder = new TagSetHolder(name);
-          cache.put(name.get(), val);
     }
   }
-    }
-    return val.holder;
-  }
 
   void put(Project.NameKey name, TagSetHolder tags) {
-    EntryVal val = new EntryVal();
+    cache.put(name.get(), tags);
-    val.holder = tags;
-    cache.put(name.get(), val);
-  }
-
-  public static class EntryVal implements Serializable {
-    static final long serialVersionUID = 1L;
-
-    transient TagSetHolder holder;
-
-    private void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
-      holder = new TagSetHolder(new Project.NameKey(in.readUTF()));
-      if (in.readBoolean()) {
-        TagSet tags = new TagSet(holder.getProjectName());
-        tags.readObject(in);
-        holder.setTagSet(tags);
-      }
-    }
-
-    private void writeObject(ObjectOutputStream out) throws IOException {
-      TagSet tags = holder.getTagSet();
-      out.writeUTF(holder.getProjectName().get());
-      out.writeBoolean(tags != null);
-      if (tags != null) {
-        tags.writeObject(out);
-      }
-    }
   }
 }

java/com/google/gerrit/server/git/TagSet.java

@@ -14,16 +14,19 @@
 
 package com.google.gerrit.server.git;
 
-import static org.eclipse.jgit.lib.ObjectIdSerializer.readWithoutMarker;
+import com.google.common.annotations.VisibleForTesting;
-import static org.eclipse.jgit.lib.ObjectIdSerializer.writeWithoutMarker;
+import com.google.common.base.MoreObjects;
-
+import com.google.common.collect.Maps;
 import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.server.cache.ProtoCacheSerializers.ObjectIdConverter;
+import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto.TagSetProto;
+import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto.TagSetProto.CachedRefProto;
+import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto.TagSetProto.TagProto;
+import com.google.protobuf.ByteString;
 import java.io.IOException;
-import java.io.ObjectInputStream;
-import java.io.ObjectOutputStream;
 import java.util.BitSet;
 import java.util.HashMap;
 import java.util.Map;
@@ -47,15 +50,35 @@ class TagSet {
   private final ObjectIdOwnerMap<Tag> tags;
 
   TagSet(Project.NameKey projectName) {
+    this(projectName, new HashMap<>(), new ObjectIdOwnerMap<>());
+  }
+
+  TagSet(Project.NameKey projectName, HashMap<String, CachedRef> refs, ObjectIdOwnerMap<Tag> tags) {
     this.projectName = projectName;
-    this.refs = new HashMap<>();
+    this.refs = refs;
-    this.tags = new ObjectIdOwnerMap<>();
+    this.tags = tags;
+  }
+
+  Project.NameKey getProjectName() {
+    return projectName;
   }
 
   Tag lookupTag(AnyObjectId id) {
     return tags.get(id);
   }
 
+  // Test methods have obtuse names in addition to annotations, since they expose mutable state
+  // which would be easy to corrupt.
+  @VisibleForTesting
+  Map<String, CachedRef> getRefsForTesting() {
+    return refs;
+  }
+
+  @VisibleForTesting
+  ObjectIdOwnerMap<Tag> getTagsForTesting() {
+    return tags;
+  }
+
   boolean updateFastForward(String refName, ObjectId oldValue, ObjectId newValue) {
     CachedRef ref = refs.get(refName);
     if (ref != null) {
@@ -191,36 +214,46 @@ class TagSet {
     }
   }
 
-  void readObject(ObjectInputStream in) throws IOException, ClassNotFoundException {
+  static TagSet fromProto(TagSetProto proto) {
-    int refCnt = in.readInt();
+    ObjectIdConverter idConverter = ObjectIdConverter.create();
-    for (int i = 0; i < refCnt; i++) {
+
-      String name = in.readUTF();
+    HashMap<String, CachedRef> refs = Maps.newHashMapWithExpectedSize(proto.getRefCount());
-      int flag = in.readInt();
+    proto
-      ObjectId id = readWithoutMarker(in);
+        .getRefMap()
-      refs.put(name, new CachedRef(flag, id));
+        .forEach(
+            (n, cr) ->
+                refs.put(n, new CachedRef(cr.getFlag(), idConverter.fromByteString(cr.getId()))));
+    ObjectIdOwnerMap<Tag> tags = new ObjectIdOwnerMap<>();
+    proto
+        .getTagList()
+        .forEach(
+            t ->
+                tags.add(
+                    new Tag(
+                        idConverter.fromByteString(t.getId()),
+                        BitSet.valueOf(t.getFlags().asReadOnlyByteBuffer()))));
+    return new TagSet(new Project.NameKey(proto.getProjectName()), refs, tags);
   }
 
-    int tagCnt = in.readInt();
+  TagSetProto toProto() {
-    for (int i = 0; i < tagCnt; i++) {
+    ObjectIdConverter idConverter = ObjectIdConverter.create();
-      ObjectId id = readWithoutMarker(in);
+    TagSetProto.Builder b = TagSetProto.newBuilder().setProjectName(projectName.get());
-      BitSet flags = (BitSet) in.readObject();
+    refs.forEach(
-      tags.add(new Tag(id, flags));
+        (n, cr) ->
-    }
+            b.putRef(
-  }
+                n,
-
+                CachedRefProto.newBuilder()
-  void writeObject(ObjectOutputStream out) throws IOException {
+                    .setId(idConverter.toByteString(cr.get()))
-    out.writeInt(refs.size());
+                    .setFlag(cr.flag)
-    for (Map.Entry<String, CachedRef> e : refs.entrySet()) {
+                    .build()));
-      out.writeUTF(e.getKey());
+    tags.forEach(
-      out.writeInt(e.getValue().flag);
+        t ->
-      writeWithoutMarker(out, e.getValue().get());
+            b.addTag(
-    }
+                TagProto.newBuilder()
-
+                    .setId(idConverter.toByteString(t))
-    out.writeInt(tags.size());
+                    .setFlags(ByteString.copyFrom(t.refFlags.toByteArray()))
-    for (Tag tag : tags) {
+                    .build()));
-      writeWithoutMarker(out, tag);
+    return b.build();
-      out.writeObject(tag.refFlags);
-    }
   }
 
   private boolean refresh(TagSet old, TagMatcher m) {
@@ -341,8 +374,17 @@ class TagSet {
     return ref.getName().startsWith(Constants.R_TAGS);
   }
 
+  @Override
+  public String toString() {
+    return MoreObjects.toStringHelper(this)
+        .add("projectName", projectName)
+        .add("refs", refs)
+        .add("tags", tags)
+        .toString();
+  }
+
   static final class Tag extends ObjectIdOwnerMap.Entry {
-    private final BitSet refFlags;
+    @VisibleForTesting final BitSet refFlags;
 
     Tag(AnyObjectId id, BitSet flags) {
       super(id);
@@ -352,11 +394,15 @@ class TagSet {
     boolean has(BitSet mask) {
       return refFlags.intersects(mask);
     }
+
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this).addValue(name()).add("refFlags", refFlags).toString();
+    }
   }
 
-  private static final class CachedRef extends AtomicReference<ObjectId> {
+  @VisibleForTesting
-    private static final long serialVersionUID = 1L;
+  static final class CachedRef extends AtomicReference<ObjectId> {
-
     final int flag;
 
     CachedRef(Ref ref, int flag) {
@@ -367,6 +413,15 @@ class TagSet {
       this.flag = flag;
       set(id);
     }
+
+    @Override
+    public String toString() {
+      ObjectId id = get();
+      return MoreObjects.toStringHelper(this)
+          .addValue(id != null ? id.name() : "null")
+          .add("flag", flag)
+          .toString();
+    }
   }
 
   private static final class TagWalk extends RevWalk {

java/com/google/gerrit/server/git/TagSetHolder.java

@@ -16,7 +16,11 @@ package com.google.gerrit.server.git;
 
 import static java.util.stream.Collectors.toList;
 
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.cache.CacheSerializer;
+import com.google.gerrit.server.cache.ProtoCacheSerializers;
+import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto;
 import java.util.Collection;
 import org.eclipse.jgit.lib.Ref;
 import org.eclipse.jgit.lib.Repository;
@@ -24,7 +28,8 @@ import org.eclipse.jgit.lib.Repository;
 public class TagSetHolder {
   private final Object buildLock = new Object();
   private final Project.NameKey projectName;
-  private volatile TagSet tags;
+
+  @Nullable private volatile TagSet tags;
 
   TagSetHolder(Project.NameKey projectName) {
     this.projectName = projectName;
@@ -94,4 +99,30 @@ public class TagSetHolder {
       return cur;
     }
   }
+
+  enum Serializer implements CacheSerializer<TagSetHolder> {
+    INSTANCE;
+
+    @Override
+    public byte[] serialize(TagSetHolder object) {
+      TagSetHolderProto.Builder b =
+          TagSetHolderProto.newBuilder().setProjectName(object.projectName.get());
+      TagSet tags = object.tags;
+      if (tags != null) {
+        b.setTags(tags.toProto());
+      }
+      return ProtoCacheSerializers.toByteArray(b.build());
+    }
+
+    @Override
+    public TagSetHolder deserialize(byte[] in) {
+      TagSetHolderProto proto =
+          ProtoCacheSerializers.parseUnchecked(TagSetHolderProto.parser(), in);
+      TagSetHolder holder = new TagSetHolder(new Project.NameKey(proto.getProjectName()));
+      if (proto.hasTags()) {
+        holder.tags = TagSet.fromProto(proto.getTags());
+      }
+      return holder;
+    }
+  }
 }

javatests/com/google/gerrit/server/cache/StringSerializerTest.java

@@ -0,0 +1,63 @@
+// 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.cache;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+
+import java.nio.charset.CharacterCodingException;
+import java.nio.charset.StandardCharsets;
+import org.junit.Test;
+
+public class StringSerializerTest {
+  @Test
+  public void serialize() {
+    assertThat(StringSerializer.INSTANCE.serialize("")).isEmpty();
+    assertThat(StringSerializer.INSTANCE.serialize("abc")).isEqualTo(new byte[] {'a', 'b', 'c'});
+    assertThat(StringSerializer.INSTANCE.serialize("a\u1234c"))
+        .isEqualTo(new byte[] {'a', (byte) 0xe1, (byte) 0x88, (byte) 0xb4, 'c'});
+  }
+
+  @Test
+  public void serializeInvalidChar() {
+    // Can't use UTF-8 for the test, since it can encode all Unicode code points.
+    try {
+      StringSerializer.serialize(StandardCharsets.US_ASCII, "\u1234");
+      assert_().fail("expected IllegalStateException");
+    } catch (IllegalStateException expected) {
+      assertThat(expected).hasCauseThat().isInstanceOf(CharacterCodingException.class);
+    }
+  }
+
+  @Test
+  public void deserialize() {
+    assertThat(StringSerializer.INSTANCE.deserialize(new byte[0])).isEmpty();
+    assertThat(StringSerializer.INSTANCE.deserialize(new byte[] {'a', 'b', 'c'})).isEqualTo("abc");
+    assertThat(
+            StringSerializer.INSTANCE.deserialize(
+                new byte[] {'a', (byte) 0xe1, (byte) 0x88, (byte) 0xb4, 'c'}))
+        .isEqualTo("a\u1234c");
+  }
+
+  @Test
+  public void deserializeInvalidChar() {
+    try {
+      StringSerializer.INSTANCE.deserialize(new byte[] {(byte) 0xff});
+      assert_().fail("expected IllegalStateException");
+    } catch (IllegalStateException expected) {
+      assertThat(expected).hasCauseThat().isInstanceOf(CharacterCodingException.class);
+    }
+  }
+}

javatests/com/google/gerrit/server/cache/h2/H2CacheTest.java

@@ -15,12 +15,11 @@
 package com.google.gerrit.server.cache.h2;
 
 import static com.google.common.truth.Truth.assertThat;
-import static java.nio.charset.StandardCharsets.UTF_8;
 
 import com.google.common.cache.Cache;
 import com.google.common.cache.CacheBuilder;
 import com.google.common.util.concurrent.MoreExecutors;
-import com.google.gerrit.server.cache.CacheSerializer;
+import com.google.gerrit.server.cache.StringSerializer;
 import com.google.gerrit.server.cache.h2.H2CacheImpl.SqlStore;
 import com.google.gerrit.server.cache.h2.H2CacheImpl.ValueHolder;
 import com.google.inject.TypeLiteral;
@@ -124,23 +123,6 @@ public class H2CacheTest {
     assertThat(oldImpl.getIfPresent("key")).isNull();
   }
 
-  // TODO(dborowitz): Won't be necessary when we use a real StringSerializer in the server code.
-  private enum StringSerializer implements CacheSerializer<String> {
-    INSTANCE;
-
-    @Override
-    public byte[] serialize(String object) {
-      return object.getBytes(UTF_8);
-    }
-
-    @Override
-    public String deserialize(byte[] in) {
-      // TODO(dborowitz): Consider using CharsetDecoder directly in the real implementation, to get
-      // checked exceptions.
-      return new String(in, UTF_8);
-    }
-  }
-
   private static <K, V> Cache<K, ValueHolder<V>> disableMemCache() {
     return CacheBuilder.newBuilder().maximumSize(0).build();
   }

javatests/com/google/gerrit/server/git/TagSetHolderTest.java

@@ -0,0 +1,57 @@
+// 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.git;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
+
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto;
+import org.junit.Test;
+
+public class TagSetHolderTest {
+  @Test
+  public void serializerWithTagSet() throws Exception {
+    TagSetHolder holder = new TagSetHolder(new Project.NameKey("project"));
+    holder.setTagSet(new TagSet(holder.getProjectName()));
+
+    byte[] serialized = TagSetHolder.Serializer.INSTANCE.serialize(holder);
+    assertThat(TagSetHolderProto.parseFrom(serialized))
+        .ignoringRepeatedFieldOrder()
+        .isEqualTo(
+            TagSetHolderProto.newBuilder()
+                .setProjectName("project")
+                .setTags(holder.getTagSet().toProto())
+                .build());
+
+    TagSetHolder deserialized = TagSetHolder.Serializer.INSTANCE.deserialize(serialized);
+    assertThat(deserialized.getProjectName()).isEqualTo(holder.getProjectName());
+    TagSetTest.assertEqual(holder.getTagSet(), deserialized.getTagSet());
+  }
+
+  @Test
+  public void serializerWithoutTagSet() throws Exception {
+    TagSetHolder holder = new TagSetHolder(new Project.NameKey("project"));
+
+    byte[] serialized = TagSetHolder.Serializer.INSTANCE.serialize(holder);
+    assertThat(TagSetHolderProto.parseFrom(serialized))
+        .ignoringRepeatedFieldOrder()
+        .isEqualTo(TagSetHolderProto.newBuilder().setProjectName("project").build());
+
+    TagSetHolder deserialized = TagSetHolder.Serializer.INSTANCE.deserialize(serialized);
+    assertThat(deserialized.getProjectName()).isEqualTo(holder.getProjectName());
+    TagSetTest.assertEqual(holder.getTagSet(), deserialized.getTagSet());
+  }
+}

javatests/com/google/gerrit/server/git/TagSetTest.java

@@ -0,0 +1,147 @@
+// 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.git;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assertWithMessage;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
+import static com.google.gerrit.server.cache.testing.CacheSerializerTestUtil.bytes;
+
+import com.google.common.collect.ImmutableSortedSet;
+import com.google.common.collect.Streams;
+import com.google.gerrit.common.Nullable;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto.TagSetProto;
+import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto.TagSetProto.CachedRefProto;
+import com.google.gerrit.server.cache.proto.Cache.TagSetHolderProto.TagSetProto.TagProto;
+import com.google.gerrit.server.git.TagSet.CachedRef;
+import com.google.gerrit.server.git.TagSet.Tag;
+import java.util.Arrays;
+import java.util.BitSet;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.Map;
+import org.eclipse.jgit.lib.ObjectId;
+import org.eclipse.jgit.lib.ObjectIdOwnerMap;
+import org.junit.Test;
+
+public class TagSetTest {
+  @Test
+  public void roundTripToProto() {
+    HashMap<String, CachedRef> refs = new HashMap<>();
+    refs.put(
+        "refs/heads/master",
+        new CachedRef(1, ObjectId.fromString("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")));
+    refs.put(
+        "refs/heads/branch",
+        new CachedRef(2, ObjectId.fromString("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb")));
+    ObjectIdOwnerMap<Tag> tags = new ObjectIdOwnerMap<>();
+    tags.add(
+        new Tag(
+            ObjectId.fromString("cccccccccccccccccccccccccccccccccccccccc"), newBitSet(1, 3, 5)));
+    tags.add(
+        new Tag(
+            ObjectId.fromString("dddddddddddddddddddddddddddddddddddddddd"), newBitSet(2, 4, 6)));
+    TagSet tagSet = new TagSet(new Project.NameKey("project"), refs, tags);
+
+    TagSetProto proto = tagSet.toProto();
+    assertThat(proto)
+        .ignoringRepeatedFieldOrder()
+        .isEqualTo(
+            TagSetProto.newBuilder()
+                .setProjectName("project")
+                .putRef(
+                    "refs/heads/master",
+                    CachedRefProto.newBuilder()
+                        .setId(
+                            bytes(
+                                0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa,
+                                0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa, 0xaa))
+                        .setFlag(1)
+                        .build())
+                .putRef(
+                    "refs/heads/branch",
+                    CachedRefProto.newBuilder()
+                        .setId(
+                            bytes(
+                                0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb,
+                                0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb, 0xbb))
+                        .setFlag(2)
+                        .build())
+                .addTag(
+                    TagProto.newBuilder()
+                        .setId(
+                            bytes(
+                                0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc,
+                                0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc, 0xcc))
+                        .setFlags(bytes(0x2a))
+                        .build())
+                .addTag(
+                    TagProto.newBuilder()
+                        .setId(
+                            bytes(
+                                0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd,
+                                0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd, 0xdd))
+                        .setFlags(bytes(0x54))
+                        .build())
+                .build());
+
+    assertEqual(tagSet, TagSet.fromProto(proto));
+  }
+
+  // TODO(dborowitz): Find some more common place to put this method, which requires access to
+  // package-private TagSet details.
+  static void assertEqual(@Nullable TagSet a, @Nullable TagSet b) {
+    if (a == null || b == null) {
+      assertWithMessage("only one TagSet is null out of\n%s\n%s", a, b)
+          .that(a == null && b == null)
+          .isTrue();
+      return;
+    }
+    assertThat(a.getProjectName()).isEqualTo(b.getProjectName());
+
+    Map<String, CachedRef> aRefs = a.getRefsForTesting();
+    Map<String, CachedRef> bRefs = b.getRefsForTesting();
+    assertThat(ImmutableSortedSet.copyOf(aRefs.keySet()))
+        .named("ref name set")
+        .isEqualTo(ImmutableSortedSet.copyOf(bRefs.keySet()));
+    for (String name : aRefs.keySet()) {
+      CachedRef aRef = aRefs.get(name);
+      CachedRef bRef = bRefs.get(name);
+      assertThat(aRef.get()).named("value of ref %s", name).isEqualTo(bRef.get());
+      assertThat(aRef.flag).named("flag of ref %s", name).isEqualTo(bRef.flag);
+    }
+
+    ObjectIdOwnerMap<Tag> aTags = a.getTagsForTesting();
+    ObjectIdOwnerMap<Tag> bTags = b.getTagsForTesting();
+    assertThat(getTagIds(aTags)).named("tag ID set").isEqualTo(getTagIds(bTags));
+    for (Tag aTag : aTags) {
+      Tag bTag = bTags.get(aTag);
+      assertThat(aTag.refFlags).named("flags for tag %s", aTag.name()).isEqualTo(bTag.refFlags);
+    }
+  }
+
+  private static ImmutableSortedSet<String> getTagIds(ObjectIdOwnerMap<Tag> bTags) {
+    return Streams.stream(bTags)
+        .map(Tag::name)
+        .collect(ImmutableSortedSet.toImmutableSortedSet(Comparator.naturalOrder()));
+  }
+
+  private BitSet newBitSet(int... bits) {
+    BitSet result = new BitSet();
+    Arrays.stream(bits).forEach(result::set);
+    return result;
+  }
+}

proto/cache.proto

@@ -193,3 +193,29 @@ message ConflictKeyProto {
   string submit_type = 3;
   bool content_merge = 4;
 }
+
+// Serialized form of com.google.gerrit.server.query.git.TagSetHolder.
+// Next ID: 3
+message TagSetHolderProto {
+  string project_name = 1;
+
+  // Next ID: 4
+  message TagSetProto {
+    string project_name = 1;
+
+    // Next ID: 3
+    message CachedRefProto {
+      bytes id = 1;
+      int32 flag = 2;
+    }
+    map<string, CachedRefProto> ref = 2;
+
+    // Next ID: 3
+    message TagProto {
+      bytes id = 1;
+      bytes flags = 2;
+    }
+    repeated TagProto tag = 3;
+  }
+  TagSetProto tags = 2;
+}