opendev/gerrit
Code Issues Proposed changes

Merge changes I6875181e,Ia6a88608,I750756ed,I7dd9d6b8,I398eab06, ...

Browse Source 
* changes:
  cache.proto: Remove reference to ProtobufCodec in comments
  Protos: Remove unused method and thus cut dependency on gwtorm
  Remove binary compatibility tests for proto converters
  Remove ProtoGen
  Remove tests ensuring generation of reviewdb.proto
  Add converter for Change protobuf messages
This commit is contained in:
Alice Kober-Sotzek
2018-12-19 10:41:52 +00:00
committed by Gerrit Code Review
parent 828ec98560 14db531b94
commit 6643e6f420
22 changed files with 837 additions and 701 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
java/com/google/gerrit/elasticsearch/AbstractElasticIndex.java
View File

@@ -99,7 +99,7 @@ abstract class AbstractElasticIndex<K, V> implements Index<K, V> {
.collect(toImmutableList());
}
private static <P extends MessageLite, T> T parseProtoFrom(
protected static <P extends MessageLite, T> T parseProtoFrom(
byte[] bytes, ProtoConverter<P, T> converter) {
P message = Protos.parseUnchecked(converter.getParser(), bytes);
return converter.fromProto(message);

5
java/com/google/gerrit/elasticsearch/ElasticChangeIndex.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.elasticsearch;
import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.CHANGE_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;
@@ -43,6 +42,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.ChangeProtoConverter;
import com.google.gerrit.reviewdb.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
import com.google.gerrit.server.ReviewerByEmailSet;
@@ -218,7 +218,8 @@ class ElasticChangeIndex extends AbstractElasticIndex<Change.Id, ChangeData>
}
ChangeData cd =
changeDataFactory.create(CHANGE_CODEC.decode(Base64.decodeBase64(c.getAsString())));
changeDataFactory.create(
parseProtoFrom(Base64.decodeBase64(c.getAsString()), ChangeProtoConverter.INSTANCE));
// Any decoding that is done here must also be done in {@link LuceneChangeIndex}.

4
java/com/google/gerrit/lucene/LuceneChangeIndex.java
View File

@@ -16,7 +16,6 @@ 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.CHANGE_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;
@@ -46,6 +45,7 @@ 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.ChangeProtoConverter;
import com.google.gerrit.reviewdb.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
import com.google.gerrit.reviewdb.converter.ProtoConverter;
@@ -445,7 +445,7 @@ public class LuceneChangeIndex implements ChangeIndex {
IndexableField cb = Iterables.getFirst(doc.get(CHANGE_FIELD), null);
if (cb != null) {
BytesRef proto = cb.binaryValue();
cd = changeDataFactory.create(CHANGE_CODEC.decode(proto.bytes, proto.offset, proto.length));
cd = changeDataFactory.create(parseProtoFrom(proto, ChangeProtoConverter.INSTANCE));
} else {
IndexableField f = Iterables.getFirst(doc.get(idFieldName), null);
Change.Id id = new Change.Id(f.numericValue().intValue());

16
java/com/google/gerrit/proto/BUILD
View File

@@ -1,24 +1,8 @@
java_binary(
name = "ProtoGen",
srcs = ["ProtoGen.java"],
resource_strip_prefix = "resources",
resources = ["//resources/com/google/gerrit/proto"],
visibility = ["//proto:__pkg__"],
deps = [
"//java/com/google/gerrit/reviewdb:server",
"//lib:args4j",
"//lib:guava",
"//lib:gwtorm",
"//lib/jgit/org.eclipse.jgit:jgit",
],
)
java_library(
name = "proto",
srcs = ["Protos.java"],
visibility = ["//visibility:public"],
deps = [
"//lib:gwtorm",
"//lib:protobuf",
],
)

84
java/com/google/gerrit/proto/ProtoGen.java
View File

@@ -1,84 +0,0 @@
// Copyright (C) 2010 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.proto;
import static com.google.common.base.Preconditions.checkState;
import static java.nio.charset.StandardCharsets.UTF_8;
import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gwtorm.schema.java.JavaSchemaModel;
import java.io.BufferedWriter;
import java.io.File;
import java.io.InputStream;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.PrintWriter;
import java.nio.ByteBuffer;
import org.eclipse.jgit.internal.storage.file.LockFile;
import org.eclipse.jgit.util.IO;
import org.kohsuke.args4j.CmdLineException;
import org.kohsuke.args4j.CmdLineParser;
import org.kohsuke.args4j.Option;
import org.kohsuke.args4j.ParserProperties;
public class ProtoGen {
@Option(
name = "--output",
aliases = {"-o"},
required = true,
metaVar = "FILE",
usage = "File to write .proto into")
private File file;
public static void main(String[] argv) throws Exception {
System.exit(new ProtoGen().run(argv));
}
private int run(String[] argv) throws Exception {
CmdLineParser parser = new CmdLineParser(this, ParserProperties.defaults().withAtSyntax(false));
try {
parser.parseArgument(argv);
} catch (CmdLineException e) {
System.err.println(e.getMessage());
System.err.println(getClass().getSimpleName() + " -o output.proto");
parser.printUsage(System.err);
return 1;
}
LockFile lock = new LockFile(file.getAbsoluteFile());
checkState(lock.lock(), "cannot lock %s", file);
try {
JavaSchemaModel jsm = new JavaSchemaModel(ReviewDb.class);
try (OutputStream o = lock.getOutputStream();
PrintWriter out = new PrintWriter(new BufferedWriter(new OutputStreamWriter(o, UTF_8)))) {
String header;
try (InputStream in = getClass().getResourceAsStream("ProtoGenHeader.txt")) {
ByteBuffer buf = IO.readWholeStream(in, 1024);
int ptr = buf.arrayOffset() + buf.position();
int len = buf.remaining();
header = new String(buf.array(), ptr, len, UTF_8);
}
out.write(header);
jsm.generateProto(out);
out.flush();
}
checkState(lock.commit(), "Could not write to %s", file);
} finally {
lock.unlock();
}
return 0;
}
}

24
java/com/google/gerrit/proto/Protos.java
View File

@@ -14,7 +14,6 @@
package com.google.gerrit.proto;
import com.google.gwtorm.protobuf.ProtobufCodec;
import com.google.protobuf.ByteString;
import com.google.protobuf.CodedOutputStream;
import com.google.protobuf.MessageLite;
@@ -68,29 +67,6 @@ public class Protos {
}
}
/**
* Serializes an object to a {@link ByteString} using a protobuf codec.
*
* <p>Guarantees deterministic serialization. No matter whether the use case cares about
* determinism or not, always use this method in preference to {@link
* ProtobufCodec#encodeToByteString(Object)}, which is not guaranteed deterministic.
*
* @param object the object to serialize.
* @param codec codec for serializing.
* @return a {@code ByteString} with the message contents.
*/
public static <T> ByteString toByteString(T object, ProtobufCodec<T> codec) {
try (ByteString.Output bout = ByteString.newOutput()) {
CodedOutputStream cout = CodedOutputStream.newInstance(bout);
cout.useDeterministicSerialization();
codec.encode(object, cout);
cout.flush();
return bout.toByteString();
} catch (IOException e) {
throw new IllegalStateException("exception writing to ByteString", e);
}
}
/**
* Parses a byte array to a protobuf message.
*

47
java/com/google/gerrit/reviewdb/converter/BranchNameKeyProtoConverter.java Normal file
View File

@@ -0,0 +1,47 @@
// 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.Branch;
import com.google.gerrit.reviewdb.client.Project;
import com.google.protobuf.Parser;
public enum BranchNameKeyProtoConverter
implements ProtoConverter<Reviewdb.Branch_NameKey, Branch.NameKey> {
INSTANCE;
private final ProtoConverter<Reviewdb.Project_NameKey, Project.NameKey> projectNameConverter =
ProjectNameKeyProtoConverter.INSTANCE;
@Override
public Reviewdb.Branch_NameKey toProto(Branch.NameKey nameKey) {
return Reviewdb.Branch_NameKey.newBuilder()
.setProjectName(projectNameConverter.toProto(nameKey.getParentKey()))
.setBranchName(nameKey.get())
.build();
}
@Override
public Branch.NameKey fromProto(Reviewdb.Branch_NameKey proto) {
return new Branch.NameKey(
projectNameConverter.fromProto(proto.getProjectName()), proto.getBranchName());
}
@Override
public Parser<Reviewdb.Branch_NameKey> getParser() {
return Reviewdb.Branch_NameKey.parser();
}
}

26
java/com/google/gerrit/reviewdb/server/ReviewDbCodecs.java → java/com/google/gerrit/reviewdb/converter/ChangeKeyProtoConverter.java
View File

@@ -12,15 +12,27 @@
// See the License for the specific language governing permissions and
// limitations under the License.
package com.google.gerrit.reviewdb.server;
package com.google.gerrit.reviewdb.converter;
import com.google.gerrit.proto.reviewdb.Reviewdb;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gwtorm.protobuf.CodecFactory;
import com.google.gwtorm.protobuf.ProtobufCodec;
import com.google.protobuf.Parser;
/** {@link ProtobufCodec} instances for ReviewDb types. */
public class ReviewDbCodecs {
public static final ProtobufCodec<Change> CHANGE_CODEC = CodecFactory.encoder(Change.class);
public enum ChangeKeyProtoConverter implements ProtoConverter<Reviewdb.Change_Key, Change.Key> {
INSTANCE;
private ReviewDbCodecs() {}
@Override
public Reviewdb.Change_Key toProto(Change.Key key) {
return Reviewdb.Change_Key.newBuilder().setId(key.get()).build();
}
@Override
public Change.Key fromProto(Reviewdb.Change_Key proto) {
return new Change.Key(proto.getId());
}
@Override
public Parser<Reviewdb.Change_Key> getParser() {
return Reviewdb.Change_Key.parser();
}
}

126
java/com/google/gerrit/reviewdb/converter/ChangeProtoConverter.java Normal file
View File

@@ -0,0 +1,126 @@
// 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.Branch;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.protobuf.Parser;
import java.sql.Timestamp;
public enum ChangeProtoConverter implements ProtoConverter<Reviewdb.Change, Change> {
INSTANCE;
private final ProtoConverter<Reviewdb.Change_Id, Change.Id> changeIdConverter =
ChangeIdProtoConverter.INSTANCE;
private final ProtoConverter<Reviewdb.Change_Key, Change.Key> changeKeyConverter =
ChangeKeyProtoConverter.INSTANCE;
private final ProtoConverter<Reviewdb.Account_Id, Account.Id> accountIdConverter =
AccountIdProtoConverter.INSTANCE;
private final ProtoConverter<Reviewdb.Branch_NameKey, Branch.NameKey> branchNameConverter =
BranchNameKeyProtoConverter.INSTANCE;
@Override
public Reviewdb.Change toProto(Change change) {
Reviewdb.Change.Builder builder =
Reviewdb.Change.newBuilder()
.setChangeId(changeIdConverter.toProto(change.getId()))
.setRowVersion(change.getRowVersion())
.setChangeKey(changeKeyConverter.toProto(change.getKey()))
.setCreatedOn(change.getCreatedOn().getTime())
.setLastUpdatedOn(change.getLastUpdatedOn().getTime())
.setOwnerAccountId(accountIdConverter.toProto(change.getOwner()))
.setDest(branchNameConverter.toProto(change.getDest()))
.setStatus(change.getStatus().getCode())
.setIsPrivate(change.isPrivate())
.setWorkInProgress(change.isWorkInProgress())
.setReviewStarted(change.hasReviewStarted());
PatchSet.Id currentPatchSetId = change.currentPatchSetId();
// Special behavior necessary to ensure binary compatibility.
builder.setCurrentPatchSetId(currentPatchSetId == null ? 0 : currentPatchSetId.get());
String subject = change.getSubject();
if (subject != null) {
builder.setSubject(subject);
}
String topic = change.getTopic();
if (topic != null) {
builder.setTopic(topic);
}
String originalSubject = change.getOriginalSubjectOrNull();
if (originalSubject != null) {
builder.setOriginalSubject(originalSubject);
}
String submissionId = change.getSubmissionId();
if (submissionId != null) {
builder.setSubmissionId(submissionId);
}
Account.Id assignee = change.getAssignee();
if (assignee != null) {
builder.setAssignee(accountIdConverter.toProto(assignee));
}
Change.Id revertOf = change.getRevertOf();
if (revertOf != null) {
builder.setRevertOf(changeIdConverter.toProto(revertOf));
}
return builder.build();
}
@Override
public Change fromProto(Reviewdb.Change proto) {
Change.Id changeId = changeIdConverter.fromProto(proto.getChangeId());
Change.Key key =
proto.hasChangeKey() ? changeKeyConverter.fromProto(proto.getChangeKey()) : null;
Account.Id owner =
proto.hasOwnerAccountId() ? accountIdConverter.fromProto(proto.getOwnerAccountId()) : null;
Branch.NameKey destination =
proto.hasDest() ? branchNameConverter.fromProto(proto.getDest()) : null;
Change change =
new Change(key, changeId, owner, destination, new Timestamp(proto.getCreatedOn()));
if (proto.hasLastUpdatedOn()) {
change.setLastUpdatedOn(new Timestamp(proto.getLastUpdatedOn()));
}
Change.Status status = Change.Status.forCode((char) proto.getStatus());
if (status != null) {
change.setStatus(status);
}
String subject = proto.hasSubject() ? proto.getSubject() : null;
String originalSubject = proto.hasOriginalSubject() ? proto.getOriginalSubject() : null;
change.setCurrentPatchSet(
new PatchSet.Id(changeId, proto.getCurrentPatchSetId()), subject, originalSubject);
if (proto.hasTopic()) {
change.setTopic(proto.getTopic());
}
if (proto.hasSubmissionId()) {
change.setSubmissionId(proto.getSubmissionId());
}
if (proto.hasAssignee()) {
change.setAssignee(accountIdConverter.fromProto(proto.getAssignee()));
}
change.setPrivate(proto.getIsPrivate());
change.setWorkInProgress(proto.getWorkInProgress());
change.setReviewStarted(proto.getReviewStarted());
if (proto.hasRevertOf()) {
change.setRevertOf(changeIdConverter.fromProto(proto.getRevertOf()));
}
return change;
}
@Override
public Parser<Reviewdb.Change> getParser() {
return Reviewdb.Change.parser();
}
}

39
java/com/google/gerrit/reviewdb/converter/ProjectNameKeyProtoConverter.java Normal file
View File

@@ -0,0 +1,39 @@
// 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.Project;
import com.google.protobuf.Parser;
public enum ProjectNameKeyProtoConverter
implements ProtoConverter<Reviewdb.Project_NameKey, Project.NameKey> {
INSTANCE;
@Override
public Reviewdb.Project_NameKey toProto(Project.NameKey nameKey) {
return Reviewdb.Project_NameKey.newBuilder().setName(nameKey.get()).build();
}
@Override
public Project.NameKey fromProto(Reviewdb.Project_NameKey proto) {
return new Project.NameKey(proto.getName());
}
@Override
public Parser<Reviewdb.Project_NameKey> getParser() {
return Reviewdb.Project_NameKey.parser();
}
}

15
java/com/google/gerrit/server/index/change/ChangeField.java
View File

@@ -23,7 +23,6 @@ import static com.google.gerrit.index.FieldDef.integer;
import static com.google.gerrit.index.FieldDef.prefix;
import static com.google.gerrit.index.FieldDef.storedOnly;
import static com.google.gerrit.index.FieldDef.timestamp;
import static com.google.gerrit.reviewdb.server.ReviewDbCodecs.CHANGE_CODEC;
import static java.nio.charset.StandardCharsets.UTF_8;
import static java.util.stream.Collectors.toList;
import static java.util.stream.Collectors.toSet;
@@ -53,6 +52,7 @@ 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.ChangeProtoConverter;
import com.google.gerrit.reviewdb.converter.PatchSetApprovalProtoConverter;
import com.google.gerrit.reviewdb.converter.PatchSetProtoConverter;
import com.google.gerrit.reviewdb.converter.ProtoConverter;
@@ -468,7 +468,8 @@ public class ChangeField {
/** Serialized change object, used for pre-populating results. */
public static final FieldDef<ChangeData, byte[]> CHANGE =
storedOnly("_change").build(changeGetter(CHANGE_CODEC::encodeToByteArray));
storedOnly("_change")
.build(changeGetter(change -> toProto(ChangeProtoConverter.INSTANCE, change)));
/** Serialized approvals for the current patch set, used for pre-populating results. */
public static final FieldDef<ChangeData, Iterable<byte[]>> APPROVAL =
@@ -854,11 +855,11 @@ public class ChangeField {
}
private static <T> List<byte[]> toProtos(ProtoConverter<?, T> converter, Collection<T> objects) {
return objects
.stream()
.map(converter::toProto)
.map(Protos::toByteArray)
.collect(toImmutableList());
return objects.stream().map(object -> toProto(converter, object)).collect(toImmutableList());
}
private static <T> byte[] toProto(ProtoConverter<?, T> converter, T object) {
return Protos.toByteArray(converter.toProto(object));
}
private static <T> FieldDef.Getter<ChangeData, T> changeGetter(Function<Change, T> func) {