Add converter for PatchSet protobuf messages

The use of ProtobufCodec requires that value classes need to be
annotated with @Column, which won't be possible as soon as we have
removed gwtorm. Hence, provide a hand-written converter for PatchSet
protobuf messages.

As protobuf PatchSets are currently used in caches and indices and we
don't want to invalidate those, we have to ensure binary compatibility.
Prove that the new converter generates binary compatible results via
dedicated tests. Those tests will be removed after this change.

Change-Id: I23b0b7b8dfafb8971f1c486952f5cfb832dd5cfe

java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.elasticsearch;
 
 import static com.google.common.base.Preconditions.checkArgument;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.gson.FieldNamingPolicy.LOWER_CASE_WITH_UNDERSCORES;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static org.apache.commons.codec.binary.Base64.decodeBase64;
@@ -24,6 +25,7 @@ import com.google.common.collect.FluentIterable;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Lists;
+import com.google.common.collect.Streams;
 import com.google.common.flogger.FluentLogger;
 import com.google.common.io.CharStreams;
 import com.google.gerrit.common.Nullable;
@@ -40,6 +42,8 @@ import com.google.gerrit.index.query.DataSource;
 import com.google.gerrit.index.query.FieldBundle;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.proto.Protos;
+import com.google.gerrit.reviewdb.converter.ProtoConverter;
 import com.google.gerrit.server.config.SitePaths;
 import com.google.gerrit.server.index.IndexUtils;
 import com.google.gson.Gson;
@@ -51,6 +55,7 @@ import com.google.gson.JsonParser;
 import com.google.gwtorm.protobuf.ProtobufCodec;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.ResultSet;
+import com.google.protobuf.MessageLite;
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.InputStreamReader;
@@ -95,6 +100,25 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
         .toList();
   }
 
+  protected static <T> List<T> decodeProtos(
+      JsonObject doc, String fieldName, ProtoConverter<?, T> converter) {
+    JsonArray field = doc.getAsJsonArray(fieldName);
+    if (field == null) {
+      return null;
+    }
+    return Streams.stream(field)
+        .map(JsonElement::toString)
+        .map(Base64::decodeBase64)
+        .map(bytes -> parseProtoFrom(bytes, converter))
+        .collect(toImmutableList());
+  }
+
+  private static <P extends MessageLite, T> T parseProtoFrom(
+      byte[] bytes, ProtoConverter<P, T> converter) {
+    P message = Protos.parseUnchecked(converter.getParser(), bytes);
+    return converter.fromProto(message);
+  }
+
   static String getContent(Response response) throws IOException {
     HttpEntity responseEntity = response.getEntity();
     String content = "";

java/com/google/gerrit/elasticsearch/BUILD

@@ -9,6 +9,7 @@ java_library(
         "//java/com/google/gerrit/index:query_exception",
         "//java/com/google/gerrit/index/project",
         "//java/com/google/gerrit/lifecycle",
+        "//java/com/google/gerrit/proto",
         "//java/com/google/gerrit/reviewdb:server",
         "//java/com/google/gerrit/server",
         "//lib:gson",

java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java

@@ -16,7 +16,6 @@ package com.google.gerrit.elasticsearch;
 
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.CHANGE_CODEC;
-import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
 import static com.google.gerrit.server.index.change.ChangeIndexRewriter.CLOSED_STATUSES;
 import static com.google.gerrit.server.index.change.ChangeIndexRewriter.OPEN_STATUSES;
 import static java.nio.charset.StandardCharsets.UTF_8;
@@ -45,6 +44,7 @@ import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Change.Id;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.ReviewerByEmailSet;
 import com.google.gerrit.server.ReviewerSet;
@@ -230,7 +230,8 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
     // Any decoding that is done here must also be done in {@link LuceneChangeIndex}.
 
     // Patch sets.
-    cd.setPatchSets(decodeProtos(source, ChangeField.PATCH_SET.getName(), PATCH_SET_CODEC));
+    cd.setPatchSets(
+        decodeProtos(source, ChangeField.PATCH_SET.getName(), PatchSetProtoConverter.INSTANCE));
 
     // Approvals.
     if (source.get(ChangeField.APPROVAL.getName()) != null) {

java/com/google/gerrit/lucene/BUILD

@@ -31,11 +31,13 @@ java_library(
         "//java/com/google/gerrit/index:query_exception",
         "//java/com/google/gerrit/index/project",
         "//java/com/google/gerrit/lifecycle",
+        "//java/com/google/gerrit/proto",
         "//java/com/google/gerrit/reviewdb:server",
         "//java/com/google/gerrit/server",
         "//java/com/google/gerrit/server/logging",
         "//lib:guava",
         "//lib:gwtorm",
+        "//lib:protobuf",
         "//lib/flogger:api",
         "//lib/guice",
         "//lib/guice:guice-assistedinject",

java/com/google/gerrit/lucene/LuceneChangeIndex.java

@@ -14,10 +14,10 @@
 
 package com.google.gerrit.lucene;
 
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.gerrit.lucene.AbstractLuceneIndex.sortFieldName;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.CHANGE_CODEC;
-import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
 import static com.google.gerrit.server.git.QueueProvider.QueueType.INTERACTIVE;
 import static com.google.gerrit.server.index.change.ChangeField.LEGACY_ID;
 import static com.google.gerrit.server.index.change.ChangeField.PROJECT;
@@ -42,10 +42,13 @@ import com.google.gerrit.index.Schema;
 import com.google.gerrit.index.query.FieldBundle;
 import com.google.gerrit.index.query.Predicate;
 import com.google.gerrit.index.query.QueryParseException;
+import com.google.gerrit.proto.Protos;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
+import com.google.gerrit.reviewdb.converter.ProtoConverter;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.StarredChangesUtil;
 import com.google.gerrit.server.config.GerritServerConfig;
@@ -65,6 +68,7 @@ import com.google.gwtorm.server.ResultSet;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.google.inject.assistedinject.Assisted;
+import com.google.protobuf.MessageLite;
 import java.io.IOException;
 import java.nio.file.Path;
 import java.util.ArrayList;
@@ -511,7 +515,7 @@ public class LuceneChangeIndex implements ChangeIndex {
   }
 
   private void decodePatchSets(ListMultimap<String, IndexableField> doc, ChangeData cd) {
-    List<PatchSet> patchSets = decodeProtos(doc, PATCH_SET_FIELD, PATCH_SET_CODEC);
+    List<PatchSet> patchSets = decodeProtos(doc, PATCH_SET_FIELD, PatchSetProtoConverter.INSTANCE);
     if (!patchSets.isEmpty()) {
       // Will be an empty list for schemas prior to when this field was stored;
       // this cannot be valid since a change needs at least one patch set.
@@ -666,6 +670,23 @@ public class LuceneChangeIndex implements ChangeIndex {
     return result;
   }
 
+  private static <T> List<T> decodeProtos(
+      ListMultimap<String, IndexableField> doc, String fieldName, ProtoConverter<?, T> converter) {
+    return doc.get(fieldName)
+        .stream()
+        .map(IndexableField::binaryValue)
+        .map(bytesRef -> parseProtoFrom(bytesRef, converter))
+        .collect(toImmutableList());
+  }
+
+  private static <P extends MessageLite, T> T parseProtoFrom(
+      BytesRef bytesRef, ProtoConverter<P, T> converter) {
+    P message =
+        Protos.parseUnchecked(
+            converter.getParser(), bytesRef.bytes, bytesRef.offset, bytesRef.length);
+    return converter.fromProto(message);
+  }
+
   private static List<byte[]> copyAsBytes(Collection<IndexableField> fields) {
     return fields
         .stream()

java/com/google/gerrit/proto/Protos.java

@@ -46,6 +46,28 @@ public class Protos {
     }
   }
 
+  /**
+   * Serializes a proto to a {@code ByteString}.
+   *
+   * <p>Guarantees deterministic serialization. No matter whether the use case cares about
+   * determinism or not, always use this method in preference to {@link MessageLite#toByteString()},
+   * which is not guaranteed deterministic.
+   *
+   * @param message the proto message to serialize
+   * @return a {@code ByteString} with the message contents
+   */
+  public static ByteString toByteString(MessageLite message) {
+    try (ByteString.Output bout = ByteString.newOutput(message.getSerializedSize())) {
+      CodedOutputStream outputStream = CodedOutputStream.newInstance(bout);
+      outputStream.useDeterministicSerialization();
+      message.writeTo(outputStream);
+      outputStream.flush();
+      return bout.toByteString();
+    } catch (IOException e) {
+      throw new IllegalStateException("exception writing to ByteString", e);
+    }
+  }
+
   /**
    * Serializes an object to a {@link ByteString} using a protobuf codec.
    *
@@ -84,5 +106,38 @@ public class Protos {
     }
   }
 
+  /**
+   * Parses a specific segment of a byte array to a protobuf message.
+   *
+   * @param parser parser for the proto type
+   * @param in byte array with the message contents
+   * @param offset offset in the byte array to start reading from
+   * @param length amount of read bytes
+   * @return parsed proto
+   */
+  public static <M extends MessageLite> M parseUnchecked(
+      Parser<M> parser, byte[] in, int offset, int length) {
+    try {
+      return parser.parseFrom(in, offset, length);
+    } catch (IOException e) {
+      throw new IllegalArgumentException("exception parsing byte array to proto", e);
+    }
+  }
+
+  /**
+   * Parses a {@code ByteString} to a protobuf message.
+   *
+   * @param parser parser for the proto type
+   * @param byteString {@code ByteString} with the message contents
+   * @return parsed proto
+   */
+  public static <M extends MessageLite> M parseUnchecked(Parser<M> parser, ByteString byteString) {
+    try {
+      return parser.parseFrom(byteString);
+    } catch (IOException e) {
+      throw new IllegalArgumentException("exception parsing ByteString to proto", e);
+    }
+  }
+
   private Protos() {}
 }

java/com/google/gerrit/reviewdb/BUILD

@@ -10,5 +10,7 @@ java_library(
         "//java/com/google/gerrit/extensions:api",
         "//lib:guava",
         "//lib:gwtorm",
+        "//lib:protobuf",
+        "//proto:reviewdb_java_proto",
     ],
 )

java/com/google/gerrit/reviewdb/client/PatchSet.java

@@ -39,7 +39,7 @@ public final class PatchSet {
     return isChangeRef(name);
   }
 
-  static String joinGroups(List<String> groups) {
+  public static String joinGroups(List<String> groups) {
     if (groups == null) {
       throw new IllegalArgumentException("groups may not be null");
     }

java/com/google/gerrit/reviewdb/converter/AccountIdProtoConverter.java

@@ -0,0 +1,38 @@
+// 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.reviewdb.converter;
+
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.protobuf.Parser;
+
+public enum AccountIdProtoConverter implements ProtoConverter<Reviewdb.Account_Id, Account.Id> {
+  INSTANCE;
+
+  @Override
+  public Reviewdb.Account_Id toProto(Account.Id accountId) {
+    return Reviewdb.Account_Id.newBuilder().setId(accountId.get()).build();
+  }
+
+  @Override
+  public Account.Id fromProto(Reviewdb.Account_Id proto) {
+    return new Account.Id(proto.getId());
+  }
+
+  @Override
+  public Parser<Reviewdb.Account_Id> getParser() {
+    return Reviewdb.Account_Id.parser();
+  }
+}

java/com/google/gerrit/reviewdb/converter/ChangeIdProtoConverter.java

@@ -0,0 +1,38 @@
+// 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.reviewdb.converter;
+
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.protobuf.Parser;
+
+public enum ChangeIdProtoConverter implements ProtoConverter<Reviewdb.Change_Id, Change.Id> {
+  INSTANCE;
+
+  @Override
+  public Reviewdb.Change_Id toProto(Change.Id changeId) {
+    return Reviewdb.Change_Id.newBuilder().setId(changeId.get()).build();
+  }
+
+  @Override
+  public Change.Id fromProto(Reviewdb.Change_Id proto) {
+    return new Change.Id(proto.getId());
+  }
+
+  @Override
+  public Parser<Reviewdb.Change_Id> getParser() {
+    return Reviewdb.Change_Id.parser();
+  }
+}

java/com/google/gerrit/reviewdb/converter/PatchSetIdProtoConverter.java

@@ -0,0 +1,45 @@
+// 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.reviewdb.converter;
+
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.protobuf.Parser;
+
+public enum PatchSetIdProtoConverter implements ProtoConverter<Reviewdb.PatchSet_Id, PatchSet.Id> {
+  INSTANCE;
+
+  private final ProtoConverter<Reviewdb.Change_Id, Change.Id> changeIdConverter =
+      ChangeIdProtoConverter.INSTANCE;
+
+  @Override
+  public Reviewdb.PatchSet_Id toProto(PatchSet.Id patchSetId) {
+    return Reviewdb.PatchSet_Id.newBuilder()
+        .setChangeId(changeIdConverter.toProto(patchSetId.getParentKey()))
+        .setPatchSetId(patchSetId.get())
+        .build();
+  }
+
+  @Override
+  public PatchSet.Id fromProto(Reviewdb.PatchSet_Id proto) {
+    return new PatchSet.Id(changeIdConverter.fromProto(proto.getChangeId()), proto.getPatchSetId());
+  }
+
+  @Override
+  public Parser<Reviewdb.PatchSet_Id> getParser() {
+    return Reviewdb.PatchSet_Id.parser();
+  }
+}

java/com/google/gerrit/reviewdb/converter/PatchSetProtoConverter.java

@@ -0,0 +1,93 @@
+// 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.reviewdb.converter;
+
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.protobuf.Parser;
+import java.sql.Timestamp;
+import java.util.List;
+
+public enum PatchSetProtoConverter implements ProtoConverter<Reviewdb.PatchSet, PatchSet> {
+  INSTANCE;
+
+  private final ProtoConverter<Reviewdb.PatchSet_Id, PatchSet.Id> patchSetIdConverter =
+      PatchSetIdProtoConverter.INSTANCE;
+  private final ProtoConverter<Reviewdb.RevId, RevId> revIdConverter = RevIdProtoConverter.INSTANCE;
+  private final ProtoConverter<Reviewdb.Account_Id, Account.Id> accountIdConverter =
+      AccountIdProtoConverter.INSTANCE;
+
+  @Override
+  public Reviewdb.PatchSet toProto(PatchSet patchSet) {
+    Reviewdb.PatchSet.Builder builder =
+        Reviewdb.PatchSet.newBuilder().setId(patchSetIdConverter.toProto(patchSet.getId()));
+    RevId revision = patchSet.getRevision();
+    if (revision != null) {
+      builder.setRevision(revIdConverter.toProto(revision));
+    }
+    Account.Id uploader = patchSet.getUploader();
+    if (uploader != null) {
+      builder.setUploaderAccountId(accountIdConverter.toProto(uploader));
+    }
+    Timestamp createdOn = patchSet.getCreatedOn();
+    if (createdOn != null) {
+      builder.setCreatedOn(createdOn.getTime());
+    }
+    List<String> groups = patchSet.getGroups();
+    if (!groups.isEmpty()) {
+      builder.setGroups(PatchSet.joinGroups(groups));
+    }
+    String pushCertificate = patchSet.getPushCertificate();
+    if (pushCertificate != null) {
+      builder.setPushCertificate(pushCertificate);
+    }
+    String description = patchSet.getDescription();
+    if (description != null) {
+      builder.setDescription(description);
+    }
+    return builder.build();
+  }
+
+  @Override
+  public PatchSet fromProto(Reviewdb.PatchSet proto) {
+    PatchSet patchSet = new PatchSet(patchSetIdConverter.fromProto(proto.getId()));
+    if (proto.hasRevision()) {
+      patchSet.setRevision(revIdConverter.fromProto(proto.getRevision()));
+    }
+    if (proto.hasUploaderAccountId()) {
+      patchSet.setUploader(accountIdConverter.fromProto(proto.getUploaderAccountId()));
+    }
+    if (proto.hasCreatedOn()) {
+      patchSet.setCreatedOn(new Timestamp(proto.getCreatedOn()));
+    }
+    if (proto.hasGroups()) {
+      patchSet.setGroups(PatchSet.splitGroups(proto.getGroups()));
+    }
+    if (proto.hasPushCertificate()) {
+      patchSet.setPushCertificate(proto.getPushCertificate());
+    }
+    if (proto.hasDescription()) {
+      patchSet.setDescription(proto.getDescription());
+    }
+    return patchSet;
+  }
+
+  @Override
+  public Parser<Reviewdb.PatchSet> getParser() {
+    return Reviewdb.PatchSet.parser();
+  }
+}

java/com/google/gerrit/reviewdb/converter/ProtoConverter.java

@@ -0,0 +1,27 @@
+// 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.reviewdb.converter;
+
+import com.google.protobuf.MessageLite;
+import com.google.protobuf.Parser;
+
+public interface ProtoConverter<P extends MessageLite, C> {
+
+  P toProto(C valueClass);
+
+  C fromProto(P proto);
+
+  Parser<P> getParser();
+}

java/com/google/gerrit/reviewdb/converter/RevIdProtoConverter.java

@@ -0,0 +1,38 @@
+// 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.reviewdb.converter;
+
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.protobuf.Parser;
+
+public enum RevIdProtoConverter implements ProtoConverter<Reviewdb.RevId, RevId> {
+  INSTANCE;
+
+  @Override
+  public Reviewdb.RevId toProto(RevId revId) {
+    return Reviewdb.RevId.newBuilder().setId(revId.get()).build();
+  }
+
+  @Override
+  public RevId fromProto(Reviewdb.RevId proto) {
+    return new RevId(proto.getId());
+  }
+
+  @Override
+  public Parser<Reviewdb.RevId> getParser() {
+    return Reviewdb.RevId.parser();
+  }
+}

java/com/google/gerrit/reviewdb/server/ReviewDbCodecs.java

@@ -16,7 +16,6 @@ package com.google.gerrit.reviewdb.server;
 
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
-import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gwtorm.protobuf.CodecFactory;
 import com.google.gwtorm.protobuf.ProtobufCodec;
@@ -31,8 +30,5 @@ public class ReviewDbCodecs {
   public static final ProtobufCodec<ChangeMessage> MESSAGE_CODEC =
       CodecFactory.encoder(ChangeMessage.class);
 
-  public static final ProtobufCodec<PatchSet> PATCH_SET_CODEC =
-      CodecFactory.encoder(PatchSet.class);
-
   private ReviewDbCodecs() {}
 }

java/com/google/gerrit/server/index/change/ChangeField.java

@@ -15,6 +15,7 @@
 package com.google.gerrit.server.index.change;
 
 import static com.google.common.base.MoreObjects.firstNonNull;
+import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.gerrit.index.FieldDef.exact;
 import static com.google.gerrit.index.FieldDef.fullText;
 import static com.google.gerrit.index.FieldDef.intRange;
@@ -24,7 +25,6 @@ import static com.google.gerrit.index.FieldDef.storedOnly;
 import static com.google.gerrit.index.FieldDef.timestamp;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.CHANGE_CODEC;
-import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
 import static java.nio.charset.StandardCharsets.UTF_8;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
@@ -46,6 +46,7 @@ import com.google.gerrit.index.FieldDef;
 import com.google.gerrit.index.RefState;
 import com.google.gerrit.index.SchemaUtil;
 import com.google.gerrit.mail.Address;
+import com.google.gerrit.proto.Protos;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
@@ -53,6 +54,8 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RefNames;
+import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
+import com.google.gerrit.reviewdb.converter.ProtoConverter;
 import com.google.gerrit.server.OutputFormat;
 import com.google.gerrit.server.ReviewerByEmailSet;
 import com.google.gerrit.server.ReviewerSet;
@@ -596,7 +599,8 @@ public class ChangeField {
 
   /** Serialized patch set object, used for pre-populating results. */
   public static final FieldDef<ChangeData, Iterable<byte[]>> PATCH_SET =
-      storedOnly("_patch_set").buildRepeatable(cd -> toProtos(PATCH_SET_CODEC, cd.patchSets()));
+      storedOnly("_patch_set")
+          .buildRepeatable(cd -> toProtos(PatchSetProtoConverter.INSTANCE, cd.patchSets()));
 
   /** Users who have edits on this change. */
   public static final FieldDef<ChangeData, Iterable<Integer>> EDITBY =
@@ -874,6 +878,14 @@ public class ChangeField {
     return result;
   }
 
+  private static <T> List<byte[]> toProtos(ProtoConverter<?, T> converter, Collection<T> objects) {
+    return objects
+        .stream()
+        .map(converter::toProto)
+        .map(Protos::toByteArray)
+        .collect(toImmutableList());
+  }
+
   private static <T> FieldDef.Getter<ChangeData, T> changeGetter(Function<Change, T> func) {
     return in -> in.change() != null ? func.apply(in.change()) : null;
   }

java/com/google/gerrit/server/notedb/ChangeNotesState.java

@@ -19,10 +19,8 @@ import static com.google.common.base.Preconditions.checkState;
 import static com.google.common.collect.ImmutableList.toImmutableList;
 import static com.google.common.collect.ImmutableListMultimap.toImmutableListMultimap;
 import static com.google.common.collect.ImmutableSet.toImmutableSet;
-import static com.google.gerrit.proto.Protos.toByteString;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.MESSAGE_CODEC;
-import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
 import static java.util.Objects.requireNonNull;
 
 import com.google.auto.value.AutoValue;
@@ -50,6 +48,8 @@ import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.Project;
 import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
+import com.google.gerrit.reviewdb.converter.ProtoConverter;
 import com.google.gerrit.server.OutputFormat;
 import com.google.gerrit.server.ReviewerByEmailSet;
 import com.google.gerrit.server.ReviewerSet;
@@ -64,6 +64,8 @@ import com.google.gerrit.server.cache.serialize.ObjectIdConverter;
 import com.google.gerrit.server.index.change.ChangeField.StoredSubmitRecord;
 import com.google.gerrit.server.notedb.NoteDbChangeState.PrimaryStorage;
 import com.google.gson.Gson;
+import com.google.protobuf.ByteString;
+import com.google.protobuf.MessageLite;
 import java.io.IOException;
 import java.sql.Timestamp;
 import java.util.List;
@@ -455,8 +457,12 @@ public abstract class ChangeNotesState {
 
       object.pastAssignees().forEach(a -> b.addPastAssignee(a.get()));
       object.hashtags().forEach(b::addHashtag);
-      object.patchSets().forEach(e -> b.addPatchSet(toByteString(e.getValue(), PATCH_SET_CODEC)));
-      object.approvals().forEach(e -> b.addApproval(toByteString(e.getValue(), APPROVAL_CODEC)));
+      object
+          .patchSets()
+          .forEach(e -> b.addPatchSet(toByteString(e.getValue(), PatchSetProtoConverter.INSTANCE)));
+      object
+          .approvals()
+          .forEach(e -> b.addApproval(Protos.toByteString(e.getValue(), APPROVAL_CODEC)));
 
       object.reviewers().asTable().cellSet().forEach(c -> b.addReviewer(toReviewerSetEntry(c)));
       object
@@ -480,7 +486,9 @@ public abstract class ChangeNotesState {
       object
           .submitRecords()
           .forEach(r -> b.addSubmitRecord(GSON.toJson(new StoredSubmitRecord(r))));
-      object.changeMessages().forEach(m -> b.addChangeMessage(toByteString(m, MESSAGE_CODEC)));
+      object
+          .changeMessages()
+          .forEach(m -> b.addChangeMessage(Protos.toByteString(m, MESSAGE_CODEC)));
       object.publishedComments().values().forEach(c -> b.addPublishedComment(GSON.toJson(c)));
 
       if (object.readOnlyUntil() != null) {
@@ -490,6 +498,12 @@ public abstract class ChangeNotesState {
       return Protos.toByteArray(b.build());
     }
 
+    @VisibleForTesting
+    static <T> ByteString toByteString(T object, ProtoConverter<?, T> converter) {
+      MessageLite message = converter.toProto(object);
+      return Protos.toByteString(message);
+    }
+
     private static ChangeColumnsProto toChangeColumnsProto(ChangeColumns cols) {
       ChangeColumnsProto.Builder b =
           ChangeColumnsProto.newBuilder()
@@ -574,7 +588,7 @@ public abstract class ChangeNotesState {
                   proto
                       .getPatchSetList()
                       .stream()
-                      .map(PATCH_SET_CODEC::decode)
+                      .map(bytes -> parseProtoFrom(PatchSetProtoConverter.INSTANCE, bytes))
                       .map(ps -> Maps.immutableEntry(ps.getId(), ps))
                       .collect(toImmutableList()))
               .approvals(
@@ -619,6 +633,12 @@ public abstract class ChangeNotesState {
       return b.build();
     }
 
+    private static <P extends MessageLite, T> T parseProtoFrom(
+        ProtoConverter<P, T> converter, ByteString byteString) {
+      P message = Protos.parseUnchecked(converter.getParser(), byteString);
+      return converter.fromProto(message);
+    }
+
     private static ChangeColumns toChangeColumns(Change.Id changeId, ChangeColumnsProto proto) {
       ChangeColumns.Builder b =
           ChangeColumns.builder()

javatests/com/google/gerrit/proto/ProtosTest.java

@@ -21,11 +21,12 @@ import com.google.gerrit.server.cache.proto.Cache.ChangeNotesKeyProto;
 import com.google.gerrit.server.cache.proto.Cache.ChangeNotesStateProto;
 import com.google.gerrit.testing.GerritBaseTests;
 import com.google.protobuf.ByteString;
+import java.util.Arrays;
 import org.junit.Test;
 
 public class ProtosTest extends GerritBaseTests {
   @Test
-  public void parseUncheckedWrongProtoType() {
+  public void parseUncheckedByteArrayWrongProtoType() {
     ChangeNotesKeyProto proto =
         ChangeNotesKeyProto.newBuilder()
             .setProject("project")
@@ -42,7 +43,7 @@ public class ProtosTest extends GerritBaseTests {
   }
 
   @Test
-  public void parseUncheckedInvalidData() {
+  public void parseUncheckedByteArrayInvalidData() {
     byte[] bytes = new byte[] {0x00};
     try {
       Protos.parseUnchecked(ChangeNotesStateProto.parser(), bytes);
@@ -53,7 +54,7 @@ public class ProtosTest extends GerritBaseTests {
   }
 
   @Test
-  public void parseUnchecked() {
+  public void parseUncheckedByteArray() {
     ChangeNotesKeyProto proto =
         ChangeNotesKeyProto.newBuilder()
             .setProject("project")
@@ -63,4 +64,93 @@ public class ProtosTest extends GerritBaseTests {
     byte[] bytes = Protos.toByteArray(proto);
     assertThat(Protos.parseUnchecked(ChangeNotesKeyProto.parser(), bytes)).isEqualTo(proto);
   }
+
+  @Test
+  public void parseUncheckedSegmentOfByteArrayWrongProtoType() {
+    ChangeNotesKeyProto proto =
+        ChangeNotesKeyProto.newBuilder()
+            .setProject("project")
+            .setChangeId(1234)
+            .setId(ByteString.copyFromUtf8("foo"))
+            .build();
+    byte[] bytes = Protos.toByteArray(proto);
+    try {
+      Protos.parseUnchecked(ChangeNotesStateProto.parser(), bytes, 0, bytes.length);
+      assert_().fail("expected IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      // Expected.
+    }
+  }
+
+  @Test
+  public void parseUncheckedSegmentOfByteArrayInvalidData() {
+    byte[] bytes = new byte[] {0x00};
+    try {
+      Protos.parseUnchecked(ChangeNotesStateProto.parser(), bytes, 0, bytes.length);
+      assert_().fail("expected IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      // Expected.
+    }
+  }
+
+  @Test
+  public void parseUncheckedSegmentOfByteArray() {
+    ChangeNotesKeyProto proto =
+        ChangeNotesKeyProto.newBuilder()
+            .setProject("project")
+            .setChangeId(1234)
+            .setId(ByteString.copyFromUtf8("foo"))
+            .build();
+    byte[] protoBytes = Protos.toByteArray(proto);
+    int offset = 3;
+    int length = protoBytes.length;
+    byte[] bytes = new byte[length + 20];
+    Arrays.fill(bytes, (byte) 1);
+    System.arraycopy(protoBytes, 0, bytes, offset, length);
+
+    ChangeNotesKeyProto parsedProto =
+        Protos.parseUnchecked(ChangeNotesKeyProto.parser(), bytes, offset, length);
+
+    assertThat(parsedProto).isEqualTo(proto);
+  }
+
+  @Test
+  public void parseUncheckedByteStringWrongProtoType() {
+    ChangeNotesKeyProto proto =
+        ChangeNotesKeyProto.newBuilder()
+            .setProject("project")
+            .setChangeId(1234)
+            .setId(ByteString.copyFromUtf8("foo"))
+            .build();
+    ByteString byteString = Protos.toByteString(proto);
+    try {
+      Protos.parseUnchecked(ChangeNotesStateProto.parser(), byteString);
+      assert_().fail("expected IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      // Expected.
+    }
+  }
+
+  @Test
+  public void parseUncheckedByteStringInvalidData() {
+    ByteString byteString = ByteString.copyFrom(new byte[] {0x00});
+    try {
+      Protos.parseUnchecked(ChangeNotesStateProto.parser(), byteString);
+      assert_().fail("expected IllegalArgumentException");
+    } catch (IllegalArgumentException e) {
+      // Expected.
+    }
+  }
+
+  @Test
+  public void parseUncheckedByteString() {
+    ChangeNotesKeyProto proto =
+        ChangeNotesKeyProto.newBuilder()
+            .setProject("project")
+            .setChangeId(1234)
+            .setId(ByteString.copyFromUtf8("foo"))
+            .build();
+    ByteString byteString = Protos.toByteString(proto);
+    assertThat(Protos.parseUnchecked(ChangeNotesKeyProto.parser(), byteString)).isEqualTo(proto);
+  }
 }

javatests/com/google/gerrit/reviewdb/converter/AccountIdProtoConverterTest.java

@@ -0,0 +1,69 @@
+// 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.reviewdb.converter;
+
+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.SerializedClassSubject.assertThatSerializedClass;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.protobuf.Parser;
+import org.junit.Test;
+
+public class AccountIdProtoConverterTest {
+  private final AccountIdProtoConverter accountIdProtoConverter = AccountIdProtoConverter.INSTANCE;
+
+  @Test
+  public void allValuesConvertedToProto() {
+    Account.Id accountId = new Account.Id(24);
+
+    Reviewdb.Account_Id proto = accountIdProtoConverter.toProto(accountId);
+
+    Reviewdb.Account_Id expectedProto = Reviewdb.Account_Id.newBuilder().setId(24).build();
+    assertThat(proto).isEqualTo(expectedProto);
+  }
+
+  @Test
+  public void allValuesConvertedToProtoAndBackAgain() {
+    Account.Id accountId = new Account.Id(34832);
+
+    Account.Id convertedAccountId =
+        accountIdProtoConverter.fromProto(accountIdProtoConverter.toProto(accountId));
+
+    assertThat(convertedAccountId).isEqualTo(accountId);
+  }
+
+  @Test
+  public void protoCanBeParsedFromBytes() throws Exception {
+    Reviewdb.Account_Id proto = Reviewdb.Account_Id.newBuilder().setId(24).build();
+    byte[] bytes = proto.toByteArray();
+
+    Parser<Reviewdb.Account_Id> parser = accountIdProtoConverter.getParser();
+    Reviewdb.Account_Id parsedProto = parser.parseFrom(bytes);
+
+    assertThat(parsedProto).isEqualTo(proto);
+  }
+
+  /**
+   * See {@link com.google.gerrit.server.cache.testing.SerializedClassSubject} for background and
+   * what to do if this test fails.
+   */
+  @Test
+  public void fieldsExistAsExpected() {
+    assertThatSerializedClass(Account.Id.class).hasFields(ImmutableMap.of("id", int.class));
+  }
+}

javatests/com/google/gerrit/reviewdb/converter/BUILD

@@ -0,0 +1,33 @@
+load("//tools/bzl:junit.bzl", "junit_tests")
+
+COMPATIBLITY_TEST_SRCS = glob(["*CompatibilityTest.java"])
+
+junit_tests(
+    name = "proto_converter_tests",
+    srcs = glob(
+        ["*.java"],
+        exclude = COMPATIBLITY_TEST_SRCS,
+    ),
+    deps = [
+        "//java/com/google/gerrit/reviewdb:server",
+        "//java/com/google/gerrit/server/cache/testing",
+        "//lib:guava",
+        "//lib:protobuf",
+        "//lib/truth",
+        "//lib/truth:truth-proto-extension",
+        "//proto:reviewdb_java_proto",
+    ],
+)
+
+junit_tests(
+    name = "compatibility_tests",
+    srcs = COMPATIBLITY_TEST_SRCS,
+    deps = [
+        "//java/com/google/gerrit/proto",
+        "//java/com/google/gerrit/reviewdb:server",
+        "//lib:guava",
+        "//lib:gwtorm-client",
+        "//lib:protobuf",
+        "//lib/truth",
+    ],
+)

javatests/com/google/gerrit/reviewdb/converter/ChangeIdProtoConverterTest.java

@@ -0,0 +1,69 @@
+// 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.reviewdb.converter;
+
+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.SerializedClassSubject.assertThatSerializedClass;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.protobuf.Parser;
+import org.junit.Test;
+
+public class ChangeIdProtoConverterTest {
+  private final ChangeIdProtoConverter changeIdProtoConverter = ChangeIdProtoConverter.INSTANCE;
+
+  @Test
+  public void allValuesConvertedToProto() {
+    Change.Id changeId = new Change.Id(94);
+
+    Reviewdb.Change_Id proto = changeIdProtoConverter.toProto(changeId);
+
+    Reviewdb.Change_Id expectedProto = Reviewdb.Change_Id.newBuilder().setId(94).build();
+    assertThat(proto).isEqualTo(expectedProto);
+  }
+
+  @Test
+  public void allValuesConvertedToProtoAndBackAgain() {
+    Change.Id changeId = new Change.Id(2903482);
+
+    Change.Id convertedChangeId =
+        changeIdProtoConverter.fromProto(changeIdProtoConverter.toProto(changeId));
+
+    assertThat(convertedChangeId).isEqualTo(changeId);
+  }
+
+  @Test
+  public void protoCanBeParsedFromBytes() throws Exception {
+    Reviewdb.Change_Id proto = Reviewdb.Change_Id.newBuilder().setId(94).build();
+    byte[] bytes = proto.toByteArray();
+
+    Parser<Reviewdb.Change_Id> parser = changeIdProtoConverter.getParser();
+    Reviewdb.Change_Id parsedProto = parser.parseFrom(bytes);
+
+    assertThat(parsedProto).isEqualTo(proto);
+  }
+
+  /**
+   * See {@link com.google.gerrit.server.cache.testing.SerializedClassSubject} for background and
+   * what to do if this test fails.
+   */
+  @Test
+  public void fieldsExistAsExpected() {
+    assertThatSerializedClass(Change.Id.class).hasFields(ImmutableMap.of("id", int.class));
+  }
+}

javatests/com/google/gerrit/reviewdb/converter/PatchSetConverterCompatibilityTest.java

@@ -0,0 +1,137 @@
+// 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.reviewdb.converter;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+import static com.google.common.collect.Iterables.getOnlyElement;
+import static com.google.common.truth.Truth.assertThat;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Lists;
+import com.google.gerrit.proto.Protos;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gwtorm.protobuf.CodecFactory;
+import com.google.gwtorm.protobuf.ProtobufCodec;
+import com.google.gwtorm.server.OrmException;
+import com.google.protobuf.ByteString;
+import com.google.protobuf.CodedOutputStream;
+import com.google.protobuf.MessageLite;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.sql.Timestamp;
+import java.util.Collection;
+import java.util.List;
+import org.junit.Test;
+
+// TODO(aliceks): Delete after proving binary compatibility.
+public class PatchSetConverterCompatibilityTest {
+
+  private final ProtobufCodec<PatchSet> patchSetCodec = CodecFactory.encoder(PatchSet.class);
+  private final PatchSetProtoConverter patchSetProtoConverter = PatchSetProtoConverter.INSTANCE;
+
+  @Test
+  public void changeIndexFieldWithAllValuesIsBinaryCompatible() throws Exception {
+    PatchSet patchSet = new PatchSet(new PatchSet.Id(new Change.Id(103), 73));
+    patchSet.setRevision(new RevId("aabbccddeeff"));
+    patchSet.setUploader(new Account.Id(452));
+    patchSet.setCreatedOn(new Timestamp(930349320L));
+    patchSet.setGroups(ImmutableList.of("group1, group2"));
+    patchSet.setPushCertificate("my push certificate");
+    patchSet.setDescription("This is a patch set description.");
+    ImmutableList<PatchSet> patchSets = ImmutableList.of(patchSet);
+
+    byte[] resultOfOldConverter = getOnlyElement(convertToProtos_old(patchSetCodec, patchSets));
+    byte[] resultOfNewConverter =
+        getOnlyElement(convertToProtos_new(patchSetProtoConverter, patchSets));
+
+    assertThat(resultOfNewConverter).isEqualTo(resultOfOldConverter);
+  }
+
+  @Test
+  public void changeIndexFieldWithMandatoryValuesIsBinaryCompatible() throws Exception {
+    PatchSet patchSet = new PatchSet(new PatchSet.Id(new Change.Id(103), 73));
+    ImmutableList<PatchSet> patchSets = ImmutableList.of(patchSet);
+
+    byte[] resultOfOldConverter = getOnlyElement(convertToProtos_old(patchSetCodec, patchSets));
+    byte[] resultOfNewConverter =
+        getOnlyElement(convertToProtos_new(patchSetProtoConverter, patchSets));
+
+    assertThat(resultOfNewConverter).isEqualTo(resultOfOldConverter);
+  }
+
+  @Test
+  public void changeNotesFieldWithAllValuesIsBinaryCompatible() {
+    PatchSet patchSet = new PatchSet(new PatchSet.Id(new Change.Id(103), 73));
+    patchSet.setRevision(new RevId("aabbccddeeff"));
+    patchSet.setUploader(new Account.Id(452));
+    patchSet.setCreatedOn(new Timestamp(930349320L));
+    patchSet.setGroups(ImmutableList.of("group1, group2"));
+    patchSet.setPushCertificate("my push certificate");
+    patchSet.setDescription("This is a patch set description.");
+
+    ByteString resultOfOldConverter = Protos.toByteString(patchSet, patchSetCodec);
+    ByteString resultOfNewConverter = toByteString(patchSet, patchSetProtoConverter);
+
+    assertThat(resultOfNewConverter).isEqualTo(resultOfOldConverter);
+  }
+
+  @Test
+  public void changeNotesFieldWithMandatoryValuesIsBinaryCompatible() {
+    PatchSet patchSet = new PatchSet(new PatchSet.Id(new Change.Id(103), 73));
+
+    ByteString resultOfOldConverter = Protos.toByteString(patchSet, patchSetCodec);
+    ByteString resultOfNewConverter = toByteString(patchSet, patchSetProtoConverter);
+
+    assertThat(resultOfNewConverter).isEqualTo(resultOfOldConverter);
+  }
+
+  // Copied from ChangeField.
+  private static <T> List<byte[]> convertToProtos_old(ProtobufCodec<T> codec, Collection<T> objs)
+      throws OrmException {
+    List<byte[]> result = Lists.newArrayListWithCapacity(objs.size());
+    ByteArrayOutputStream out = new ByteArrayOutputStream(256);
+    try {
+      for (T obj : objs) {
+        out.reset();
+        CodedOutputStream cos = CodedOutputStream.newInstance(out);
+        codec.encode(obj, cos);
+        cos.flush();
+        result.add(out.toByteArray());
+      }
+    } catch (IOException e) {
+      throw new OrmException(e);
+    }
+    return result;
+  }
+
+  // Copied from ChangeField.
+  private static <T> List<byte[]> convertToProtos_new(
+      ProtoConverter<?, T> converter, Collection<T> objects) {
+    return objects
+        .stream()
+        .map(converter::toProto)
+        .map(Protos::toByteArray)
+        .collect(toImmutableList());
+  }
+
+  // Copied from ChangeNotesState.Serializer.
+  private static <T> ByteString toByteString(T object, ProtoConverter<?, T> converter) {
+    MessageLite message = converter.toProto(object);
+    return Protos.toByteString(message);
+  }
+}

javatests/com/google/gerrit/reviewdb/converter/PatchSetIdProtoConverterTest.java

@@ -0,0 +1,85 @@
+// 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.reviewdb.converter;
+
+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.SerializedClassSubject.assertThatSerializedClass;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.protobuf.Parser;
+import java.lang.reflect.Type;
+import org.junit.Test;
+
+public class PatchSetIdProtoConverterTest {
+  private final PatchSetIdProtoConverter patchSetIdProtoConverter =
+      PatchSetIdProtoConverter.INSTANCE;
+
+  @Test
+  public void allValuesConvertedToProto() {
+    PatchSet.Id patchSetId = new PatchSet.Id(new Change.Id(103), 73);
+
+    Reviewdb.PatchSet_Id proto = patchSetIdProtoConverter.toProto(patchSetId);
+
+    Reviewdb.PatchSet_Id expectedProto =
+        Reviewdb.PatchSet_Id.newBuilder()
+            .setChangeId(Reviewdb.Change_Id.newBuilder().setId(103))
+            .setPatchSetId(73)
+            .build();
+    assertThat(proto).isEqualTo(expectedProto);
+  }
+
+  @Test
+  public void allValuesConvertedToProtoAndBackAgain() {
+    PatchSet.Id patchSetId = new PatchSet.Id(new Change.Id(20), 13);
+
+    PatchSet.Id convertedPatchSetId =
+        patchSetIdProtoConverter.fromProto(patchSetIdProtoConverter.toProto(patchSetId));
+
+    assertThat(convertedPatchSetId).isEqualTo(patchSetId);
+  }
+
+  @Test
+  public void protoCanBeParsedFromBytes() throws Exception {
+    Reviewdb.PatchSet_Id proto =
+        Reviewdb.PatchSet_Id.newBuilder()
+            .setChangeId(Reviewdb.Change_Id.newBuilder().setId(103))
+            .setPatchSetId(73)
+            .build();
+    byte[] bytes = proto.toByteArray();
+
+    Parser<Reviewdb.PatchSet_Id> parser = patchSetIdProtoConverter.getParser();
+    Reviewdb.PatchSet_Id parsedProto = parser.parseFrom(bytes);
+
+    assertThat(parsedProto).isEqualTo(proto);
+  }
+
+  /**
+   * See {@link com.google.gerrit.server.cache.testing.SerializedClassSubject} for background and
+   * what to do if this test fails.
+   */
+  @Test
+  public void fieldsExistAsExpected() {
+    assertThatSerializedClass(PatchSet.Id.class)
+        .hasFields(
+            ImmutableMap.<String, Type>builder()
+                .put("changeId", Change.Id.class)
+                .put("patchSetId", int.class)
+                .build());
+  }
+}

javatests/com/google/gerrit/reviewdb/converter/PatchSetProtoConverterTest.java

@@ -0,0 +1,139 @@
+// 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.reviewdb.converter;
+
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
+import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableMap;
+import com.google.common.truth.Truth;
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.PatchSet;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.protobuf.Parser;
+import java.lang.reflect.Type;
+import java.sql.Timestamp;
+import org.junit.Test;
+
+public class PatchSetProtoConverterTest {
+  private final PatchSetProtoConverter patchSetProtoConverter = PatchSetProtoConverter.INSTANCE;
+
+  @Test
+  public void allValuesConvertedToProto() {
+    PatchSet patchSet = new PatchSet(new PatchSet.Id(new Change.Id(103), 73));
+    patchSet.setRevision(new RevId("aabbccddeeff"));
+    patchSet.setUploader(new Account.Id(452));
+    patchSet.setCreatedOn(new Timestamp(930349320L));
+    patchSet.setGroups(ImmutableList.of("group1, group2"));
+    patchSet.setPushCertificate("my push certificate");
+    patchSet.setDescription("This is a patch set description.");
+
+    Reviewdb.PatchSet proto = patchSetProtoConverter.toProto(patchSet);
+
+    Reviewdb.PatchSet expectedProto =
+        Reviewdb.PatchSet.newBuilder()
+            .setId(
+                Reviewdb.PatchSet_Id.newBuilder()
+                    .setChangeId(Reviewdb.Change_Id.newBuilder().setId(103))
+                    .setPatchSetId(73))
+            .setRevision(Reviewdb.RevId.newBuilder().setId("aabbccddeeff"))
+            .setUploaderAccountId(Reviewdb.Account_Id.newBuilder().setId(452))
+            .setCreatedOn(930349320L)
+            .setGroups("group1, group2")
+            .setPushCertificate("my push certificate")
+            .setDescription("This is a patch set description.")
+            .build();
+    assertThat(proto).isEqualTo(expectedProto);
+  }
+
+  @Test
+  public void mandatoryValuesConvertedToProto() {
+    PatchSet patchSet = new PatchSet(new PatchSet.Id(new Change.Id(103), 73));
+
+    Reviewdb.PatchSet proto = patchSetProtoConverter.toProto(patchSet);
+
+    Reviewdb.PatchSet expectedProto =
+        Reviewdb.PatchSet.newBuilder()
+            .setId(
+                Reviewdb.PatchSet_Id.newBuilder()
+                    .setChangeId(Reviewdb.Change_Id.newBuilder().setId(103))
+                    .setPatchSetId(73))
+            .build();
+    assertThat(proto).isEqualTo(expectedProto);
+  }
+
+  @Test
+  public void allValuesConvertedToProtoAndBackAgain() {
+    PatchSet patchSet = new PatchSet(new PatchSet.Id(new Change.Id(103), 73));
+    patchSet.setRevision(new RevId("aabbccddeeff"));
+    patchSet.setUploader(new Account.Id(452));
+    patchSet.setCreatedOn(new Timestamp(930349320L));
+    patchSet.setGroups(ImmutableList.of("group1, group2"));
+    patchSet.setPushCertificate("my push certificate");
+    patchSet.setDescription("This is a patch set description.");
+
+    PatchSet convertedPatchSet =
+        patchSetProtoConverter.fromProto(patchSetProtoConverter.toProto(patchSet));
+    Truth.assertThat(convertedPatchSet).isEqualTo(patchSet);
+  }
+
+  @Test
+  public void mandatoryValuesConvertedToProtoAndBackAgain() {
+    PatchSet patchSet = new PatchSet(new PatchSet.Id(new Change.Id(103), 73));
+
+    PatchSet convertedPatchSet =
+        patchSetProtoConverter.fromProto(patchSetProtoConverter.toProto(patchSet));
+    Truth.assertThat(convertedPatchSet).isEqualTo(patchSet);
+  }
+
+  @Test
+  public void protoCanBeParsedFromBytes() throws Exception {
+    Reviewdb.PatchSet proto =
+        Reviewdb.PatchSet.newBuilder()
+            .setId(
+                Reviewdb.PatchSet_Id.newBuilder()
+                    .setChangeId(Reviewdb.Change_Id.newBuilder().setId(103))
+                    .setPatchSetId(73))
+            .build();
+    byte[] bytes = proto.toByteArray();
+
+    Parser<Reviewdb.PatchSet> parser = patchSetProtoConverter.getParser();
+    Reviewdb.PatchSet parsedProto = parser.parseFrom(bytes);
+
+    assertThat(parsedProto).isEqualTo(proto);
+  }
+
+  /**
+   * See {@link com.google.gerrit.server.cache.testing.SerializedClassSubject} for background and
+   * what to do if this test fails.
+   */
+  @Test
+  public void fieldsExistAsExpected() {
+    assertThatSerializedClass(PatchSet.class)
+        .hasFields(
+            ImmutableMap.<String, Type>builder()
+                .put("id", PatchSet.Id.class)
+                .put("revision", RevId.class)
+                .put("uploader", Account.Id.class)
+                .put("createdOn", Timestamp.class)
+                .put("groups", String.class)
+                .put("pushCertificate", String.class)
+                .put("description", String.class)
+                .build());
+  }
+}

javatests/com/google/gerrit/reviewdb/converter/RevIdProtoConverterTest.java

@@ -0,0 +1,68 @@
+// 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.reviewdb.converter;
+
+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.SerializedClassSubject.assertThatSerializedClass;
+
+import com.google.common.collect.ImmutableMap;
+import com.google.gerrit.proto.reviewdb.Reviewdb;
+import com.google.gerrit.reviewdb.client.RevId;
+import com.google.protobuf.Parser;
+import org.junit.Test;
+
+public class RevIdProtoConverterTest {
+  private final RevIdProtoConverter revIdProtoConverter = RevIdProtoConverter.INSTANCE;
+
+  @Test
+  public void allValuesConvertedToProto() {
+    RevId revId = new RevId("9903402f303249e");
+
+    Reviewdb.RevId proto = revIdProtoConverter.toProto(revId);
+
+    Reviewdb.RevId expectedProto = Reviewdb.RevId.newBuilder().setId("9903402f303249e").build();
+    assertThat(proto).isEqualTo(expectedProto);
+  }
+
+  @Test
+  public void allValuesConvertedToProtoAndBackAgain() {
+    RevId revId = new RevId("ff3934a320bb");
+
+    RevId convertedRevId = revIdProtoConverter.fromProto(revIdProtoConverter.toProto(revId));
+
+    assertThat(convertedRevId).isEqualTo(revId);
+  }
+
+  @Test
+  public void protoCanBeParsedFromBytes() throws Exception {
+    Reviewdb.RevId proto = Reviewdb.RevId.newBuilder().setId("9903402f303249e").build();
+    byte[] bytes = proto.toByteArray();
+
+    Parser<Reviewdb.RevId> parser = revIdProtoConverter.getParser();
+    Reviewdb.RevId parsedProto = parser.parseFrom(bytes);
+
+    assertThat(parsedProto).isEqualTo(proto);
+  }
+
+  /**
+   * See {@link com.google.gerrit.server.cache.testing.SerializedClassSubject} for background and
+   * what to do if this test fails.
+   */
+  @Test
+  public void fieldsExistAsExpected() {
+    assertThatSerializedClass(RevId.class).hasFields(ImmutableMap.of("id", String.class));
+  }
+}

javatests/com/google/gerrit/server/notedb/ChangeNotesStateTest.java

@@ -19,8 +19,8 @@ import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
 import static com.google.gerrit.proto.Protos.toByteString;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.APPROVAL_CODEC;
 import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.MESSAGE_CODEC;
-import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.PATCH_SET_CODEC;
 import static com.google.gerrit.server.cache.testing.SerializedClassSubject.assertThatSerializedClass;
+import static com.google.gerrit.server.notedb.ChangeNotesState.Serializer.toByteString;
 
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
@@ -31,6 +31,7 @@ import com.google.common.collect.Iterables;
 import com.google.gerrit.common.data.SubmitRecord;
 import com.google.gerrit.common.data.SubmitRequirement;
 import com.google.gerrit.mail.Address;
+import com.google.gerrit.proto.Protos;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
@@ -39,6 +40,7 @@ import com.google.gerrit.reviewdb.client.LabelId;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.client.PatchSetApproval;
 import com.google.gerrit.reviewdb.client.RevId;
+import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
 import com.google.gerrit.server.ReviewerByEmailSet;
 import com.google.gerrit.server.ReviewerSet;
 import com.google.gerrit.server.ReviewerStatusUpdate;
@@ -340,14 +342,14 @@ public class ChangeNotesStateTest extends GerritBaseTests {
     ps1.setUploader(new Account.Id(2000));
     ps1.setRevision(new RevId("aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"));
     ps1.setCreatedOn(cols.createdOn());
-    ByteString ps1Bytes = toByteString(ps1, PATCH_SET_CODEC);
+    ByteString ps1Bytes = toByteString(ps1, PatchSetProtoConverter.INSTANCE);
     assertThat(ps1Bytes.size()).isEqualTo(66);
 
     PatchSet ps2 = new PatchSet(new PatchSet.Id(ID, 2));
     ps2.setUploader(new Account.Id(3000));
     ps2.setRevision(new RevId("bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb"));
     ps2.setCreatedOn(cols.lastUpdatedOn());
-    ByteString ps2Bytes = toByteString(ps2, PATCH_SET_CODEC);
+    ByteString ps2Bytes = toByteString(ps2, PatchSetProtoConverter.INSTANCE);
     assertThat(ps2Bytes.size()).isEqualTo(66);
     assertThat(ps2Bytes).isNotEqualTo(ps1Bytes);
 
@@ -372,7 +374,7 @@ public class ChangeNotesStateTest extends GerritBaseTests {
                 new PatchSet.Id(ID, 1), new Account.Id(2001), new LabelId("Code-Review")),
             (short) 1,
             new Timestamp(1212L));
-    ByteString a1Bytes = toByteString(a1, APPROVAL_CODEC);
+    ByteString a1Bytes = Protos.toByteString(a1, APPROVAL_CODEC);
     assertThat(a1Bytes.size()).isEqualTo(43);
 
     PatchSetApproval a2 =

proto/BUILD

@@ -11,14 +11,11 @@ java_proto_library(
 
 proto_library(
     name = "reviewdb_proto",
-    srcs = [":reviewdb.proto"],
+    srcs = ["reviewdb.proto"],
 )
 
 java_proto_library(
     name = "reviewdb_java_proto",
-    visibility = [
-        "//javatests/com/google/gerrit/proto:__pkg__",
-        "//tools/eclipse:__pkg__",
-    ],
+    visibility = ["//visibility:public"],
     deps = [":reviewdb_proto"],
 )