Merge "SafeProtoConverterTest: test proto defaults"
diff --git a/java/com/google/gerrit/entities/converter/SafeProtoConverter.java b/java/com/google/gerrit/entities/converter/SafeProtoConverter.java
index bab7bb2..f4a66a0 100644
--- a/java/com/google/gerrit/entities/converter/SafeProtoConverter.java
+++ b/java/com/google/gerrit/entities/converter/SafeProtoConverter.java
@@ -2,7 +2,7 @@
import com.google.errorprone.annotations.Immutable;
import com.google.gerrit.common.ConvertibleToProto;
-import com.google.protobuf.MessageLite;
+import com.google.protobuf.Message;
/**
* An extension to {@link ProtoConverter} that enforces the Entity class and the Proto class to stay
@@ -21,7 +21,7 @@
* setters, there is no need to explicitly test your safe converter.
*/
@Immutable
-public interface SafeProtoConverter<P extends MessageLite, C> extends ProtoConverter<P, C> {
+public interface SafeProtoConverter<P extends Message, C> extends ProtoConverter<P, C> {
Class<P> getProtoClass();
diff --git a/javatests/com/google/gerrit/entities/converter/SafeProtoConverterTest.java b/javatests/com/google/gerrit/entities/converter/SafeProtoConverterTest.java
index 42a993d..e892d42 100644
--- a/javatests/com/google/gerrit/entities/converter/SafeProtoConverterTest.java
+++ b/javatests/com/google/gerrit/entities/converter/SafeProtoConverterTest.java
@@ -2,6 +2,7 @@
import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.extensions.proto.ProtoTruth.assertThat;
import com.google.common.collect.ImmutableList;
import com.google.common.primitives.Primitives;
@@ -10,6 +11,8 @@
import com.google.common.testing.ArbitraryInstances;
import com.google.gerrit.common.ConvertibleToProto;
import com.google.gerrit.common.Nullable;
+import com.google.protobuf.Descriptors.FieldDescriptor;
+import com.google.protobuf.Message;
import com.google.protobuf.MessageLite;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
@@ -26,7 +29,6 @@
import java.util.Optional;
import javax.annotation.Nonnull;
import org.apache.commons.lang3.builder.EqualsBuilder;
-import org.junit.Ignore;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -36,7 +38,7 @@
@RunWith(Parameterized.class)
public class SafeProtoConverterTest {
@Parameter(0)
- public SafeProtoConverter<MessageLite, Object> converter;
+ public SafeProtoConverter<Message, Object> converter;
@Parameter(1)
public String converterName;
@@ -49,7 +51,7 @@
.filter(SafeProtoConverter.class::isAssignableFrom)
.filter(clz -> !SafeProtoConverter.class.equals(clz))
.filter(Class::isEnum)
- .map(clz -> (SafeProtoConverter<MessageLite, Object>) clz.getEnumConstants()[0])
+ .map(clz -> (SafeProtoConverter<Message, Object>) clz.getEnumConstants()[0])
.map(clz -> new Object[] {clz, clz.getClass().getSimpleName()})
.collect(toImmutableList());
}
@@ -104,8 +106,16 @@
* update the corresponding Java class accordingly.
*/
@Test
- @Ignore("TODO(b/335372403) - implement")
- public void protoDefaultsKeptOnDoubleConversion() {}
+ public void protoDefaultsKeptOnDoubleConversion() {
+ Message defaultInstance = getProtoDefaultInstance(converter.getProtoClass());
+ Message preFilled = explicitlyFillProtoDefaults(defaultInstance);
+ Message resFromDefault =
+ converter.toProto(converter.fromProto(converter.getProtoClass().cast(preFilled)));
+ Message resFromPrefilled =
+ converter.toProto(converter.fromProto(converter.getProtoClass().cast(preFilled)));
+ assertThat(resFromDefault).isEqualTo(preFilled);
+ assertThat(resFromPrefilled).isEqualTo(preFilled);
+ }
@Nullable
private static Object buildObjectWithFullFieldsOrThrow(Class<?> clz) throws Exception {
@@ -207,4 +217,32 @@
|| String.class.isAssignableFrom(c)
|| Timestamp.class.isAssignableFrom(c);
}
+
+ /**
+ * Returns the default instance for the given MessageLite class, if it has the {@code
+ * getDefaultInstance} static method.
+ *
+ * @param type the protobuf message class
+ * @throws IllegalArgumentException if the given class doesn't have the static {@code
+ * getDefaultInstance} method
+ */
+ public static <T extends MessageLite> T getProtoDefaultInstance(Class<T> type) {
+ try {
+ return type.cast(type.getMethod("getDefaultInstance").invoke(null));
+ } catch (ReflectiveOperationException | ClassCastException e) {
+ throw new IllegalStateException("Cannot get default instance for " + type, e);
+ }
+ }
+
+ private static Message explicitlyFillProtoDefaults(Message defaultInstance) {
+ Message.Builder res = defaultInstance.toBuilder();
+ for (FieldDescriptor f : defaultInstance.getDescriptorForType().getFields()) {
+ if (f.getType().equals(FieldDescriptor.Type.MESSAGE)) {
+ res.setField(f, explicitlyFillProtoDefaults((Message) defaultInstance.getField(f)));
+ } else {
+ res.setField(f, defaultInstance.getField(f));
+ }
+ }
+ return res.build();
+ }
}
diff --git a/proto/entities.proto b/proto/entities.proto
index be04e97..0f577a4 100644
--- a/proto/entities.proto
+++ b/proto/entities.proto
@@ -277,7 +277,7 @@
// Next ID: 2
message ObjectId {
// Hex string representation of the ID.
- optional string name = 1;
+ optional string name = 1 [default="0000000000000000000000000000000000000000"];
}
// Serialized form of a continuation token used for pagination.
@@ -453,11 +453,11 @@
optional Side side = 2 [default = REVISION];
message Range {
// 1-based
- optional int32 start_line = 1;
+ optional int32 start_line = 1 [default = 1];
// 0-based
optional int32 start_char = 2;
// 1-based
- optional int32 end_line = 3;
+ optional int32 end_line = 3 [default = 1];
// 0-based
optional int32 end_char = 4;
}
@@ -466,7 +466,7 @@
// number is identical to the range's end line.
optional Range position_range = 3;
// 1-based
- optional int32 line_number = 4;
+ optional int32 line_number = 4 [default = 1];
}
// If not set, the comment is on the patchset level.
@@ -488,4 +488,4 @@
optional fixed64 written_on_millis = 11;
// Required.
optional string server_id = 12;
-}
\ No newline at end of file
+}
