From 42c6a4ad550442102d06df311ae302b7c7b706ac Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Thu, 20 Oct 2022 19:00:14 +0200 Subject: [PATCH 01/14] Repro unit test --- .../commonTest/kotlin/pbandk/json/JsonTest.kt | 14 + .../kotlin/pbandk/testpb/test_proto2.kt | 76 +++ .../proto/pbandk/testpb/test_proto2.proto | 8 + .../java/pbandk/testpb/java/TestProto2.java | 646 +++++++++++++++++- 4 files changed, 743 insertions(+), 1 deletion(-) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index 78518a53..d61ec0c9 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -5,6 +5,7 @@ import kotlinx.serialization.json.buildJsonObject import pbandk.encodeToByteArray import pbandk.testpb.Bar import pbandk.testpb.TestAllTypesProto3 +import pbandk.testpb.MessageWithEnum import kotlin.test.* class JsonTest { @@ -18,6 +19,19 @@ class JsonTest { assertContentEquals(byteArrayOf(), barBytes, "binary serialization should be empty for null field") } + @Test + fun testMessageWithEnumProto2 + val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) + + // This points to a bug in JSON serialization implementation. + // In proto2, enum string value should be visible in JSON even if the enum value is set to 0th value. + val message0 = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) + assertEquals(message0.encodeToJsonString(jsonConfig), "{}") + + val message1 = MessageWithEnum(value = MessageWithEnum.EnumType.BAR) + assertEquals(message1.encodeToJsonString(jsonConfig), "{\"value\":\"BAR\"}") + } + @Test fun testNullValues() { val json = buildJsonObject { diff --git a/test-types/src/commonMain/kotlin/pbandk/testpb/test_proto2.kt b/test-types/src/commonMain/kotlin/pbandk/testpb/test_proto2.kt index c17744a3..606e83bd 100644 --- a/test-types/src/commonMain/kotlin/pbandk/testpb/test_proto2.kt +++ b/test-types/src/commonMain/kotlin/pbandk/testpb/test_proto2.kt @@ -37,6 +37,58 @@ public data class MessageWithRequiredField( } } +@pbandk.Export +public data class MessageWithEnum( + val value: pbandk.testpb.MessageWithEnum.EnumType? = null, + override val unknownFields: Map = emptyMap() +) : pbandk.Message { + override operator fun plus(other: pbandk.Message?): pbandk.testpb.MessageWithEnum = protoMergeImpl(other) + override val descriptor: pbandk.MessageDescriptor get() = Companion.descriptor + override val protoSize: Int by lazy { super.protoSize } + public companion object : pbandk.Message.Companion { + public val defaultInstance: pbandk.testpb.MessageWithEnum by lazy { pbandk.testpb.MessageWithEnum() } + override fun decodeWith(u: pbandk.MessageDecoder): pbandk.testpb.MessageWithEnum = pbandk.testpb.MessageWithEnum.decodeWithImpl(u) + + override val descriptor: pbandk.MessageDescriptor by lazy { + val fieldsList = ArrayList>(1) + fieldsList.apply { + add( + pbandk.FieldDescriptor( + messageDescriptor = this@Companion::descriptor, + name = "value", + number = 1, + type = pbandk.FieldDescriptor.Type.Enum(enumCompanion = pbandk.testpb.MessageWithEnum.EnumType.Companion, hasPresence = true), + jsonName = "value", + value = pbandk.testpb.MessageWithEnum::value + ) + ) + } + pbandk.MessageDescriptor( + fullName = "testpb.MessageWithEnum", + messageClass = pbandk.testpb.MessageWithEnum::class, + messageCompanion = this, + fields = fieldsList + ) + } + } + + public sealed class EnumType(override val value: Int, override val name: String? = null) : pbandk.Message.Enum { + override fun equals(other: kotlin.Any?): Boolean = other is MessageWithEnum.EnumType && other.value == value + override fun hashCode(): Int = value.hashCode() + override fun toString(): String = "MessageWithEnum.EnumType.${name ?: "UNRECOGNIZED"}(value=$value)" + + public object FOO : EnumType(0, "FOO") + public object BAR : EnumType(1, "BAR") + public class UNRECOGNIZED(value: Int) : EnumType(value) + + public companion object : pbandk.Message.Enum.Companion { + public val values: List by lazy { listOf(FOO, BAR) } + override fun fromValue(value: Int): MessageWithEnum.EnumType = values.firstOrNull { it.value == value } ?: UNRECOGNIZED(value) + override fun fromName(name: String): MessageWithEnum.EnumType = values.firstOrNull { it.name == name } ?: throw IllegalArgumentException("No EnumType with name: $name") + } + } +} + private fun MessageWithRequiredField.protoMergeImpl(plus: pbandk.Message?): MessageWithRequiredField = (plus as? MessageWithRequiredField)?.let { it.copy( unknownFields = unknownFields + plus.unknownFields @@ -58,3 +110,27 @@ private fun MessageWithRequiredField.Companion.decodeWithImpl(u: pbandk.MessageD } return MessageWithRequiredField(foo!!, unknownFields) } + +@pbandk.Export +@pbandk.JsName("orDefaultForMessageWithEnum") +public fun MessageWithEnum?.orDefault(): pbandk.testpb.MessageWithEnum = this ?: MessageWithEnum.defaultInstance + +private fun MessageWithEnum.protoMergeImpl(plus: pbandk.Message?): MessageWithEnum = (plus as? MessageWithEnum)?.let { + it.copy( + value = plus.value ?: value, + unknownFields = unknownFields + plus.unknownFields + ) +} ?: this + +@Suppress("UNCHECKED_CAST") +private fun MessageWithEnum.Companion.decodeWithImpl(u: pbandk.MessageDecoder): MessageWithEnum { + var value: pbandk.testpb.MessageWithEnum.EnumType? = null + + val unknownFields = u.readMessage(this) { _fieldNumber, _fieldValue -> + when (_fieldNumber) { + 1 -> value = _fieldValue as pbandk.testpb.MessageWithEnum.EnumType + } + } + + return MessageWithEnum(value, unknownFields) +} diff --git a/test-types/src/commonMain/proto/pbandk/testpb/test_proto2.proto b/test-types/src/commonMain/proto/pbandk/testpb/test_proto2.proto index b0e3270f..b7f0f323 100644 --- a/test-types/src/commonMain/proto/pbandk/testpb/test_proto2.proto +++ b/test-types/src/commonMain/proto/pbandk/testpb/test_proto2.proto @@ -7,3 +7,11 @@ option java_package = "pbandk.testpb.java"; message MessageWithRequiredField { required bool foo = 1; } + +message MessageWithEnum { + enum EnumType { + FOO = 0; + BAR = 1; + } + optional EnumType value = 1; +} diff --git a/test-types/src/jvmMain/java/pbandk/testpb/java/TestProto2.java b/test-types/src/jvmMain/java/pbandk/testpb/java/TestProto2.java index fe63e56a..a3355415 100644 --- a/test-types/src/jvmMain/java/pbandk/testpb/java/TestProto2.java +++ b/test-types/src/jvmMain/java/pbandk/testpb/java/TestProto2.java @@ -545,11 +545,646 @@ public pbandk.testpb.java.TestProto2.MessageWithRequiredField getDefaultInstance } + public interface MessageWithEnumOrBuilder extends + // @@protoc_insertion_point(interface_extends:testpb.MessageWithEnum) + com.google.protobuf.MessageOrBuilder { + + /** + * optional .testpb.MessageWithEnum.EnumType value = 1; + * @return Whether the value field is set. + */ + boolean hasValue(); + /** + * optional .testpb.MessageWithEnum.EnumType value = 1; + * @return The value. + */ + pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType getValue(); + } + /** + * Protobuf type {@code testpb.MessageWithEnum} + */ + public static final class MessageWithEnum extends + com.google.protobuf.GeneratedMessageV3 implements + // @@protoc_insertion_point(message_implements:testpb.MessageWithEnum) + MessageWithEnumOrBuilder { + private static final long serialVersionUID = 0L; + // Use MessageWithEnum.newBuilder() to construct. + private MessageWithEnum(com.google.protobuf.GeneratedMessageV3.Builder builder) { + super(builder); + } + private MessageWithEnum() { + value_ = 0; + } + + @java.lang.Override + @SuppressWarnings({"unused"}) + protected java.lang.Object newInstance( + UnusedPrivateParameter unused) { + return new MessageWithEnum(); + } + + @java.lang.Override + public final com.google.protobuf.UnknownFieldSet + getUnknownFields() { + return this.unknownFields; + } + private MessageWithEnum( + com.google.protobuf.CodedInputStream input, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws com.google.protobuf.InvalidProtocolBufferException { + this(); + if (extensionRegistry == null) { + throw new java.lang.NullPointerException(); + } + int mutable_bitField0_ = 0; + com.google.protobuf.UnknownFieldSet.Builder unknownFields = + com.google.protobuf.UnknownFieldSet.newBuilder(); + try { + boolean done = false; + while (!done) { + int tag = input.readTag(); + switch (tag) { + case 0: + done = true; + break; + case 8: { + int rawValue = input.readEnum(); + @SuppressWarnings("deprecation") + pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType value = pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType.valueOf(rawValue); + if (value == null) { + unknownFields.mergeVarintField(1, rawValue); + } else { + bitField0_ |= 0x00000001; + value_ = rawValue; + } + break; + } + default: { + if (!parseUnknownField( + input, unknownFields, extensionRegistry, tag)) { + done = true; + } + break; + } + } + } + } catch (com.google.protobuf.InvalidProtocolBufferException e) { + throw e.setUnfinishedMessage(this); + } catch (java.io.IOException e) { + throw new com.google.protobuf.InvalidProtocolBufferException( + e).setUnfinishedMessage(this); + } finally { + this.unknownFields = unknownFields.build(); + makeExtensionsImmutable(); + } + } + public static final com.google.protobuf.Descriptors.Descriptor + getDescriptor() { + return pbandk.testpb.java.TestProto2.internal_static_testpb_MessageWithEnum_descriptor; + } + + @java.lang.Override + protected com.google.protobuf.GeneratedMessageV3.FieldAccessorTable + internalGetFieldAccessorTable() { + return pbandk.testpb.java.TestProto2.internal_static_testpb_MessageWithEnum_fieldAccessorTable + .ensureFieldAccessorsInitialized( + pbandk.testpb.java.TestProto2.MessageWithEnum.class, pbandk.testpb.java.TestProto2.MessageWithEnum.Builder.class); + } + + /** + * Protobuf enum {@code testpb.MessageWithEnum.EnumType} + */ + public enum EnumType + implements com.google.protobuf.ProtocolMessageEnum { + /** + * FOO = 0; + */ + FOO(0), + /** + * BAR = 1; + */ + BAR(1), + ; + + /** + * FOO = 0; + */ + public static final int FOO_VALUE = 0; + /** + * BAR = 1; + */ + public static final int BAR_VALUE = 1; + + + public final int getNumber() { + return value; + } + + /** + * @param value The numeric wire value of the corresponding enum entry. + * @return The enum associated with the given numeric wire value. + * @deprecated Use {@link #forNumber(int)} instead. + */ + @java.lang.Deprecated + public static EnumType valueOf(int value) { + return forNumber(value); + } + + /** + * @param value The numeric wire value of the corresponding enum entry. + * @return The enum associated with the given numeric wire value. + */ + public static EnumType forNumber(int value) { + switch (value) { + case 0: return FOO; + case 1: return BAR; + default: return null; + } + } + + public static com.google.protobuf.Internal.EnumLiteMap + internalGetValueMap() { + return internalValueMap; + } + private static final com.google.protobuf.Internal.EnumLiteMap< + EnumType> internalValueMap = + new com.google.protobuf.Internal.EnumLiteMap() { + public EnumType findValueByNumber(int number) { + return EnumType.forNumber(number); + } + }; + + public final com.google.protobuf.Descriptors.EnumValueDescriptor + getValueDescriptor() { + return getDescriptor().getValues().get(ordinal()); + } + public final com.google.protobuf.Descriptors.EnumDescriptor + getDescriptorForType() { + return getDescriptor(); + } + public static final com.google.protobuf.Descriptors.EnumDescriptor + getDescriptor() { + return pbandk.testpb.java.TestProto2.MessageWithEnum.getDescriptor().getEnumTypes().get(0); + } + + private static final EnumType[] VALUES = values(); + + public static EnumType valueOf( + com.google.protobuf.Descriptors.EnumValueDescriptor desc) { + if (desc.getType() != getDescriptor()) { + throw new java.lang.IllegalArgumentException( + "EnumValueDescriptor is not for this type."); + } + return VALUES[desc.getIndex()]; + } + + private final int value; + + private EnumType(int value) { + this.value = value; + } + + // @@protoc_insertion_point(enum_scope:testpb.MessageWithEnum.EnumType) + } + + private int bitField0_; + public static final int VALUE_FIELD_NUMBER = 1; + private int value_; + /** + * optional .testpb.MessageWithEnum.EnumType value = 1; + * @return Whether the value field is set. + */ + @java.lang.Override public boolean hasValue() { + return ((bitField0_ & 0x00000001) != 0); + } + /** + * optional .testpb.MessageWithEnum.EnumType value = 1; + * @return The value. + */ + @java.lang.Override public pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType getValue() { + @SuppressWarnings("deprecation") + pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType result = pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType.valueOf(value_); + return result == null ? pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType.FOO : result; + } + + private byte memoizedIsInitialized = -1; + @java.lang.Override + public final boolean isInitialized() { + byte isInitialized = memoizedIsInitialized; + if (isInitialized == 1) return true; + if (isInitialized == 0) return false; + + memoizedIsInitialized = 1; + return true; + } + + @java.lang.Override + public void writeTo(com.google.protobuf.CodedOutputStream output) + throws java.io.IOException { + if (((bitField0_ & 0x00000001) != 0)) { + output.writeEnum(1, value_); + } + unknownFields.writeTo(output); + } + + @java.lang.Override + public int getSerializedSize() { + int size = memoizedSize; + if (size != -1) return size; + + size = 0; + if (((bitField0_ & 0x00000001) != 0)) { + size += com.google.protobuf.CodedOutputStream + .computeEnumSize(1, value_); + } + size += unknownFields.getSerializedSize(); + memoizedSize = size; + return size; + } + + @java.lang.Override + public boolean equals(final java.lang.Object obj) { + if (obj == this) { + return true; + } + if (!(obj instanceof pbandk.testpb.java.TestProto2.MessageWithEnum)) { + return super.equals(obj); + } + pbandk.testpb.java.TestProto2.MessageWithEnum other = (pbandk.testpb.java.TestProto2.MessageWithEnum) obj; + + if (hasValue() != other.hasValue()) return false; + if (hasValue()) { + if (value_ != other.value_) return false; + } + if (!unknownFields.equals(other.unknownFields)) return false; + return true; + } + + @java.lang.Override + public int hashCode() { + if (memoizedHashCode != 0) { + return memoizedHashCode; + } + int hash = 41; + hash = (19 * hash) + getDescriptor().hashCode(); + if (hasValue()) { + hash = (37 * hash) + VALUE_FIELD_NUMBER; + hash = (53 * hash) + value_; + } + hash = (29 * hash) + unknownFields.hashCode(); + memoizedHashCode = hash; + return hash; + } + + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom( + java.nio.ByteBuffer data) + throws com.google.protobuf.InvalidProtocolBufferException { + return PARSER.parseFrom(data); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom( + java.nio.ByteBuffer data, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws com.google.protobuf.InvalidProtocolBufferException { + return PARSER.parseFrom(data, extensionRegistry); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom( + com.google.protobuf.ByteString data) + throws com.google.protobuf.InvalidProtocolBufferException { + return PARSER.parseFrom(data); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom( + com.google.protobuf.ByteString data, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws com.google.protobuf.InvalidProtocolBufferException { + return PARSER.parseFrom(data, extensionRegistry); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom(byte[] data) + throws com.google.protobuf.InvalidProtocolBufferException { + return PARSER.parseFrom(data); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom( + byte[] data, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws com.google.protobuf.InvalidProtocolBufferException { + return PARSER.parseFrom(data, extensionRegistry); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom(java.io.InputStream input) + throws java.io.IOException { + return com.google.protobuf.GeneratedMessageV3 + .parseWithIOException(PARSER, input); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom( + java.io.InputStream input, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws java.io.IOException { + return com.google.protobuf.GeneratedMessageV3 + .parseWithIOException(PARSER, input, extensionRegistry); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseDelimitedFrom(java.io.InputStream input) + throws java.io.IOException { + return com.google.protobuf.GeneratedMessageV3 + .parseDelimitedWithIOException(PARSER, input); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseDelimitedFrom( + java.io.InputStream input, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws java.io.IOException { + return com.google.protobuf.GeneratedMessageV3 + .parseDelimitedWithIOException(PARSER, input, extensionRegistry); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom( + com.google.protobuf.CodedInputStream input) + throws java.io.IOException { + return com.google.protobuf.GeneratedMessageV3 + .parseWithIOException(PARSER, input); + } + public static pbandk.testpb.java.TestProto2.MessageWithEnum parseFrom( + com.google.protobuf.CodedInputStream input, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws java.io.IOException { + return com.google.protobuf.GeneratedMessageV3 + .parseWithIOException(PARSER, input, extensionRegistry); + } + + @java.lang.Override + public Builder newBuilderForType() { return newBuilder(); } + public static Builder newBuilder() { + return DEFAULT_INSTANCE.toBuilder(); + } + public static Builder newBuilder(pbandk.testpb.java.TestProto2.MessageWithEnum prototype) { + return DEFAULT_INSTANCE.toBuilder().mergeFrom(prototype); + } + @java.lang.Override + public Builder toBuilder() { + return this == DEFAULT_INSTANCE + ? new Builder() : new Builder().mergeFrom(this); + } + + @java.lang.Override + protected Builder newBuilderForType( + com.google.protobuf.GeneratedMessageV3.BuilderParent parent) { + Builder builder = new Builder(parent); + return builder; + } + /** + * Protobuf type {@code testpb.MessageWithEnum} + */ + public static final class Builder extends + com.google.protobuf.GeneratedMessageV3.Builder implements + // @@protoc_insertion_point(builder_implements:testpb.MessageWithEnum) + pbandk.testpb.java.TestProto2.MessageWithEnumOrBuilder { + public static final com.google.protobuf.Descriptors.Descriptor + getDescriptor() { + return pbandk.testpb.java.TestProto2.internal_static_testpb_MessageWithEnum_descriptor; + } + + @java.lang.Override + protected com.google.protobuf.GeneratedMessageV3.FieldAccessorTable + internalGetFieldAccessorTable() { + return pbandk.testpb.java.TestProto2.internal_static_testpb_MessageWithEnum_fieldAccessorTable + .ensureFieldAccessorsInitialized( + pbandk.testpb.java.TestProto2.MessageWithEnum.class, pbandk.testpb.java.TestProto2.MessageWithEnum.Builder.class); + } + + // Construct using pbandk.testpb.java.TestProto2.MessageWithEnum.newBuilder() + private Builder() { + maybeForceBuilderInitialization(); + } + + private Builder( + com.google.protobuf.GeneratedMessageV3.BuilderParent parent) { + super(parent); + maybeForceBuilderInitialization(); + } + private void maybeForceBuilderInitialization() { + if (com.google.protobuf.GeneratedMessageV3 + .alwaysUseFieldBuilders) { + } + } + @java.lang.Override + public Builder clear() { + super.clear(); + value_ = 0; + bitField0_ = (bitField0_ & ~0x00000001); + return this; + } + + @java.lang.Override + public com.google.protobuf.Descriptors.Descriptor + getDescriptorForType() { + return pbandk.testpb.java.TestProto2.internal_static_testpb_MessageWithEnum_descriptor; + } + + @java.lang.Override + public pbandk.testpb.java.TestProto2.MessageWithEnum getDefaultInstanceForType() { + return pbandk.testpb.java.TestProto2.MessageWithEnum.getDefaultInstance(); + } + + @java.lang.Override + public pbandk.testpb.java.TestProto2.MessageWithEnum build() { + pbandk.testpb.java.TestProto2.MessageWithEnum result = buildPartial(); + if (!result.isInitialized()) { + throw newUninitializedMessageException(result); + } + return result; + } + + @java.lang.Override + public pbandk.testpb.java.TestProto2.MessageWithEnum buildPartial() { + pbandk.testpb.java.TestProto2.MessageWithEnum result = new pbandk.testpb.java.TestProto2.MessageWithEnum(this); + int from_bitField0_ = bitField0_; + int to_bitField0_ = 0; + if (((from_bitField0_ & 0x00000001) != 0)) { + to_bitField0_ |= 0x00000001; + } + result.value_ = value_; + result.bitField0_ = to_bitField0_; + onBuilt(); + return result; + } + + @java.lang.Override + public Builder clone() { + return super.clone(); + } + @java.lang.Override + public Builder setField( + com.google.protobuf.Descriptors.FieldDescriptor field, + java.lang.Object value) { + return super.setField(field, value); + } + @java.lang.Override + public Builder clearField( + com.google.protobuf.Descriptors.FieldDescriptor field) { + return super.clearField(field); + } + @java.lang.Override + public Builder clearOneof( + com.google.protobuf.Descriptors.OneofDescriptor oneof) { + return super.clearOneof(oneof); + } + @java.lang.Override + public Builder setRepeatedField( + com.google.protobuf.Descriptors.FieldDescriptor field, + int index, java.lang.Object value) { + return super.setRepeatedField(field, index, value); + } + @java.lang.Override + public Builder addRepeatedField( + com.google.protobuf.Descriptors.FieldDescriptor field, + java.lang.Object value) { + return super.addRepeatedField(field, value); + } + @java.lang.Override + public Builder mergeFrom(com.google.protobuf.Message other) { + if (other instanceof pbandk.testpb.java.TestProto2.MessageWithEnum) { + return mergeFrom((pbandk.testpb.java.TestProto2.MessageWithEnum)other); + } else { + super.mergeFrom(other); + return this; + } + } + + public Builder mergeFrom(pbandk.testpb.java.TestProto2.MessageWithEnum other) { + if (other == pbandk.testpb.java.TestProto2.MessageWithEnum.getDefaultInstance()) return this; + if (other.hasValue()) { + setValue(other.getValue()); + } + this.mergeUnknownFields(other.unknownFields); + onChanged(); + return this; + } + + @java.lang.Override + public final boolean isInitialized() { + return true; + } + + @java.lang.Override + public Builder mergeFrom( + com.google.protobuf.CodedInputStream input, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws java.io.IOException { + pbandk.testpb.java.TestProto2.MessageWithEnum parsedMessage = null; + try { + parsedMessage = PARSER.parsePartialFrom(input, extensionRegistry); + } catch (com.google.protobuf.InvalidProtocolBufferException e) { + parsedMessage = (pbandk.testpb.java.TestProto2.MessageWithEnum) e.getUnfinishedMessage(); + throw e.unwrapIOException(); + } finally { + if (parsedMessage != null) { + mergeFrom(parsedMessage); + } + } + return this; + } + private int bitField0_; + + private int value_ = 0; + /** + * optional .testpb.MessageWithEnum.EnumType value = 1; + * @return Whether the value field is set. + */ + @java.lang.Override public boolean hasValue() { + return ((bitField0_ & 0x00000001) != 0); + } + /** + * optional .testpb.MessageWithEnum.EnumType value = 1; + * @return The value. + */ + @java.lang.Override + public pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType getValue() { + @SuppressWarnings("deprecation") + pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType result = pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType.valueOf(value_); + return result == null ? pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType.FOO : result; + } + /** + * optional .testpb.MessageWithEnum.EnumType value = 1; + * @param value The value to set. + * @return This builder for chaining. + */ + public Builder setValue(pbandk.testpb.java.TestProto2.MessageWithEnum.EnumType value) { + if (value == null) { + throw new NullPointerException(); + } + bitField0_ |= 0x00000001; + value_ = value.getNumber(); + onChanged(); + return this; + } + /** + * optional .testpb.MessageWithEnum.EnumType value = 1; + * @return This builder for chaining. + */ + public Builder clearValue() { + bitField0_ = (bitField0_ & ~0x00000001); + value_ = 0; + onChanged(); + return this; + } + @java.lang.Override + public final Builder setUnknownFields( + final com.google.protobuf.UnknownFieldSet unknownFields) { + return super.setUnknownFields(unknownFields); + } + + @java.lang.Override + public final Builder mergeUnknownFields( + final com.google.protobuf.UnknownFieldSet unknownFields) { + return super.mergeUnknownFields(unknownFields); + } + + + // @@protoc_insertion_point(builder_scope:testpb.MessageWithEnum) + } + + // @@protoc_insertion_point(class_scope:testpb.MessageWithEnum) + private static final pbandk.testpb.java.TestProto2.MessageWithEnum DEFAULT_INSTANCE; + static { + DEFAULT_INSTANCE = new pbandk.testpb.java.TestProto2.MessageWithEnum(); + } + + public static pbandk.testpb.java.TestProto2.MessageWithEnum getDefaultInstance() { + return DEFAULT_INSTANCE; + } + + @java.lang.Deprecated public static final com.google.protobuf.Parser + PARSER = new com.google.protobuf.AbstractParser() { + @java.lang.Override + public MessageWithEnum parsePartialFrom( + com.google.protobuf.CodedInputStream input, + com.google.protobuf.ExtensionRegistryLite extensionRegistry) + throws com.google.protobuf.InvalidProtocolBufferException { + return new MessageWithEnum(input, extensionRegistry); + } + }; + + public static com.google.protobuf.Parser parser() { + return PARSER; + } + + @java.lang.Override + public com.google.protobuf.Parser getParserForType() { + return PARSER; + } + + @java.lang.Override + public pbandk.testpb.java.TestProto2.MessageWithEnum getDefaultInstanceForType() { + return DEFAULT_INSTANCE; + } + + } + private static final com.google.protobuf.Descriptors.Descriptor internal_static_testpb_MessageWithRequiredField_descriptor; private static final com.google.protobuf.GeneratedMessageV3.FieldAccessorTable internal_static_testpb_MessageWithRequiredField_fieldAccessorTable; + private static final com.google.protobuf.Descriptors.Descriptor + internal_static_testpb_MessageWithEnum_descriptor; + private static final + com.google.protobuf.GeneratedMessageV3.FieldAccessorTable + internal_static_testpb_MessageWithEnum_fieldAccessorTable; public static com.google.protobuf.Descriptors.FileDescriptor getDescriptor() { @@ -561,7 +1196,10 @@ public pbandk.testpb.java.TestProto2.MessageWithRequiredField getDefaultInstance java.lang.String[] descriptorData = { "\n\037pbandk/testpb/test_proto2.proto\022\006testp" + "b\"\'\n\030MessageWithRequiredField\022\013\n\003foo\030\001 \002" + - "(\010B\024\n\022pbandk.testpb.java" + "(\010\"`\n\017MessageWithEnum\022/\n\005value\030\001 \001(\0162 .t" + + "estpb.MessageWithEnum.EnumType\"\034\n\010EnumTy" + + "pe\022\007\n\003FOO\020\000\022\007\n\003BAR\020\001B\024\n\022pbandk.testpb.ja" + + "va" }; descriptor = com.google.protobuf.Descriptors.FileDescriptor .internalBuildGeneratedFileFrom(descriptorData, @@ -573,6 +1211,12 @@ public pbandk.testpb.java.TestProto2.MessageWithRequiredField getDefaultInstance com.google.protobuf.GeneratedMessageV3.FieldAccessorTable( internal_static_testpb_MessageWithRequiredField_descriptor, new java.lang.String[] { "Foo", }); + internal_static_testpb_MessageWithEnum_descriptor = + getDescriptor().getMessageTypes().get(1); + internal_static_testpb_MessageWithEnum_fieldAccessorTable = new + com.google.protobuf.GeneratedMessageV3.FieldAccessorTable( + internal_static_testpb_MessageWithEnum_descriptor, + new java.lang.String[] { "Value", }); } // @@protoc_insertion_point(outer_class_scope) From 15467d984a9c94db3dd2a44930cc0fc7a459d42a Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Thu, 20 Oct 2022 20:35:18 +0200 Subject: [PATCH 02/14] add another case --- .../commonTest/kotlin/pbandk/json/JsonTest.kt | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index d61ec0c9..04e18057 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -20,16 +20,20 @@ class JsonTest { } @Test - fun testMessageWithEnumProto2 + fun testMessageWithEnumProto2() { val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) - // This points to a bug in JSON serialization implementation. - // In proto2, enum string value should be visible in JSON even if the enum value is set to 0th value. - val message0 = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) - assertEquals(message0.encodeToJsonString(jsonConfig), "{}") + // This points to a bug: although the 'value' is not set in 'message', we serialize it with a null. + val message = MessageWithEnum() + assertEquals(message.encodeToJsonString(jsonConfig), "{\"value\":null}") - val message1 = MessageWithEnum(value = MessageWithEnum.EnumType.BAR) - assertEquals(message1.encodeToJsonString(jsonConfig), "{\"value\":\"BAR\"}") + // This points to a bug: enum string value should be visible in JSON even if the enum value is set to 0th value. + val messageWith0 = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) + assertEquals(messageWith0.encodeToJsonString(jsonConfig), "{}") + + // This works as expected. + val messageWith1 = MessageWithEnum(value = MessageWithEnum.EnumType.BAR) + assertEquals(messageWith1.encodeToJsonString(jsonConfig), "{\"value\":\"BAR\"}") } @Test From cbd188b4b765d25fca8347eeef0c57361c13ee82 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Thu, 20 Oct 2022 20:48:37 +0200 Subject: [PATCH 03/14] add a test for proto3 as well --- .../commonTest/kotlin/pbandk/json/JsonTest.kt | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index 04e18057..69c86ac1 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -36,6 +36,24 @@ class JsonTest { assertEquals(messageWith1.encodeToJsonString(jsonConfig), "{\"value\":\"BAR\"}") } + @Test + fun testMessageWithEnumProto3() { + val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) + + // This works as expected. + val message = TestAllTypesProto3() + assertEquals(message.encodeToJsonString(jsonConfig), "{}") + + // This points to a bug: enum string value should be visible in JSON even if the enum value is set to 0th value. + // For proto3, this is not as big of a problem since we can't differentiate between 0th value and missing value. + val messageWith0 = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.FOO) + assertEquals(messageWith0.encodeToJsonString(jsonConfig), "{}") + + // This works as expected. + val messageWith1 = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.BAR) + assertEquals(messageWith1.encodeToJsonString(jsonConfig), "{\"optionalNestedEnum\":\"BAR\"}") + } + @Test fun testNullValues() { val json = buildJsonObject { From 1a97cff2c382c1b5ab2a6d181693c028515b8897 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Fri, 21 Oct 2022 10:03:57 +0200 Subject: [PATCH 04/14] self review --- runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index 69c86ac1..a707c2f5 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -23,11 +23,13 @@ class JsonTest { fun testMessageWithEnumProto2() { val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) - // This points to a bug: although the 'value' is not set in 'message', we serialize it with a null. + // This behavior is unexpected and will be fixed in a follow-up. + // See https://github.com/streem/pbandk/issues/235 for more details. val message = MessageWithEnum() assertEquals(message.encodeToJsonString(jsonConfig), "{\"value\":null}") - // This points to a bug: enum string value should be visible in JSON even if the enum value is set to 0th value. + // This behavior is unexpected and will be fixed in a follow-up. + // See https://github.com/streem/pbandk/issues/235 for more details. val messageWith0 = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) assertEquals(messageWith0.encodeToJsonString(jsonConfig), "{}") @@ -40,16 +42,13 @@ class JsonTest { fun testMessageWithEnumProto3() { val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) - // This works as expected. val message = TestAllTypesProto3() assertEquals(message.encodeToJsonString(jsonConfig), "{}") - // This points to a bug: enum string value should be visible in JSON even if the enum value is set to 0th value. - // For proto3, this is not as big of a problem since we can't differentiate between 0th value and missing value. + // See https://github.com/streem/pbandk/issues/235#issuecomment-1286122161 for more details. val messageWith0 = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.FOO) assertEquals(messageWith0.encodeToJsonString(jsonConfig), "{}") - // This works as expected. val messageWith1 = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.BAR) assertEquals(messageWith1.encodeToJsonString(jsonConfig), "{\"optionalNestedEnum\":\"BAR\"}") } From 6fa3c580376ca4ff4f149a3c40800d3128793389 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Fri, 21 Oct 2022 10:45:21 +0200 Subject: [PATCH 05/14] change order of params for assertEquals --- .../src/commonTest/kotlin/pbandk/json/JsonTest.kt | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index a707c2f5..d43b3039 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -26,16 +26,16 @@ class JsonTest { // This behavior is unexpected and will be fixed in a follow-up. // See https://github.com/streem/pbandk/issues/235 for more details. val message = MessageWithEnum() - assertEquals(message.encodeToJsonString(jsonConfig), "{\"value\":null}") + assertEquals("{\"value\":null}", message.encodeToJsonString(jsonConfig)) // This behavior is unexpected and will be fixed in a follow-up. // See https://github.com/streem/pbandk/issues/235 for more details. val messageWith0 = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) - assertEquals(messageWith0.encodeToJsonString(jsonConfig), "{}") + assertEquals("{}", messageWith0.encodeToJsonString(jsonConfig)) // This works as expected. val messageWith1 = MessageWithEnum(value = MessageWithEnum.EnumType.BAR) - assertEquals(messageWith1.encodeToJsonString(jsonConfig), "{\"value\":\"BAR\"}") + assertEquals("{\"value\":\"BAR\"}", messageWith1.encodeToJsonString(jsonConfig)) } @Test @@ -43,14 +43,14 @@ class JsonTest { val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) val message = TestAllTypesProto3() - assertEquals(message.encodeToJsonString(jsonConfig), "{}") + assertEquals("{}", message.encodeToJsonString(jsonConfig)) // See https://github.com/streem/pbandk/issues/235#issuecomment-1286122161 for more details. val messageWith0 = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.FOO) - assertEquals(messageWith0.encodeToJsonString(jsonConfig), "{}") + assertEquals("{}", messageWith0.encodeToJsonString(jsonConfig)) val messageWith1 = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.BAR) - assertEquals(messageWith1.encodeToJsonString(jsonConfig), "{\"optionalNestedEnum\":\"BAR\"}") + assertEquals("{\"optionalNestedEnum\":\"BAR\"}", messageWith1.encodeToJsonString(jsonConfig)) } @Test From a2105189d79fcd8fb0614213d038ad13b63b0905 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Fri, 21 Oct 2022 10:43:37 +0200 Subject: [PATCH 06/14] Change expected values in unit tests and fix --- .../kotlin/pbandk/internal/json/JsonMessageEncoder.kt | 9 ++++++++- runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt | 9 ++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt b/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt index a90d3b5f..194b2b93 100644 --- a/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt +++ b/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt @@ -39,7 +39,14 @@ internal class JsonMessageEncoder(private val jsonConfig: JsonConfig) : MessageE val value = (fd.value as KProperty1).get(message) if (value == null && fd.oneofMember) continue - if (!fd.oneofMember && !jsonConfig.outputDefaultValues && fd.type.isDefaultValue(value)) continue + + // For optional enums (mostly relevant for proto2) we serialize the value whenever a value is set, + // regardless of whether it's set to a default enum value. + val isOptionalEnum = (fd.type is FieldDescriptor.Type.Enum<*> && fd.type.hasPresence) + if (isOptionalEnum && value == null) continue + + // Don't serialize other default values unless 'outputDefaultValues' is set. + if (!fd.oneofMember && !jsonConfig.outputDefaultValues && !isOptionalEnum && fd.type.isDefaultValue(value)) continue val jsonValue = value ?.takeUnless { diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index d43b3039..6ebbdd68 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -23,17 +23,12 @@ class JsonTest { fun testMessageWithEnumProto2() { val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) - // This behavior is unexpected and will be fixed in a follow-up. - // See https://github.com/streem/pbandk/issues/235 for more details. val message = MessageWithEnum() - assertEquals("{\"value\":null}", message.encodeToJsonString(jsonConfig)) + assertEquals("{}", message.encodeToJsonString(jsonConfig)) - // This behavior is unexpected and will be fixed in a follow-up. - // See https://github.com/streem/pbandk/issues/235 for more details. val messageWith0 = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) - assertEquals("{}", messageWith0.encodeToJsonString(jsonConfig)) + assertEquals("{\"value\":\"FOO\"}", messageWith0.encodeToJsonString(jsonConfig)) - // This works as expected. val messageWith1 = MessageWithEnum(value = MessageWithEnum.EnumType.BAR) assertEquals("{\"value\":\"BAR\"}", messageWith1.encodeToJsonString(jsonConfig)) } From 278a78e67ae1b40baa62d7cb2a2277e0edef1dd2 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Fri, 21 Oct 2022 12:04:53 +0200 Subject: [PATCH 07/14] fix 2 --- .../kotlin/pbandk/internal/json/JsonMessageEncoder.kt | 11 ++--------- .../kotlin/pbandk/json/OutputDefaultValuesTest.kt | 4 ++-- 2 files changed, 4 insertions(+), 11 deletions(-) diff --git a/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt b/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt index 194b2b93..1f490c16 100644 --- a/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt +++ b/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt @@ -38,15 +38,8 @@ internal class JsonMessageEncoder(private val jsonConfig: JsonConfig) : MessageE @Suppress("UNCHECKED_CAST") val value = (fd.value as KProperty1).get(message) - if (value == null && fd.oneofMember) continue - - // For optional enums (mostly relevant for proto2) we serialize the value whenever a value is set, - // regardless of whether it's set to a default enum value. - val isOptionalEnum = (fd.type is FieldDescriptor.Type.Enum<*> && fd.type.hasPresence) - if (isOptionalEnum && value == null) continue - - // Don't serialize other default values unless 'outputDefaultValues' is set. - if (!fd.oneofMember && !jsonConfig.outputDefaultValues && !isOptionalEnum && fd.type.isDefaultValue(value)) continue + if (value == null && (fd.oneofMember || fd.type.hasPresence)) continue + if (!fd.oneofMember && !jsonConfig.outputDefaultValues && !fd.type.hasPresence && fd.type.isDefaultValue(value)) continue val jsonValue = value ?.takeUnless { diff --git a/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt index 3978411e..9ccfa20a 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt @@ -29,12 +29,12 @@ class OutputDefaultValuesTest { assertEquals(JsonPrimitive(""), parsedJson["optionalBytes"]) assertEquals(JsonPrimitive(TestAllTypesProto3.NestedEnum.fromValue(0).name!!), parsedJson["optionalNestedEnum"]) - assertEquals(JsonNull, parsedJson["optionalNestedMessage"]) + assertFalse(parsedJson.containsKey("optionalNestedMessage")) assertEquals(JsonArray(emptyList()), parsedJson["repeatedString"]) assertEquals(JsonObject(emptyMap()), parsedJson["mapBoolBool"]) - assertEquals(JsonNull, parsedJson["optionalStringWrapper"]) + assertFalse(parsedJson.containsKey("optionalStringWrapper")) assertEquals(JsonPrimitive(false), parsedJson["optionalBoolWrapper"]) } From 5e3f046a8bf3c877b1a137bf3d92a29e78ef134a Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Fri, 21 Oct 2022 12:18:47 +0200 Subject: [PATCH 08/14] add test binary json roundtrip test --- runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index d43b3039..b619d12b 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -53,6 +53,14 @@ class JsonTest { assertEquals("{\"optionalNestedEnum\":\"BAR\"}", messageWith1.encodeToJsonString(jsonConfig)) } + @Test + fun binaryJsonRoundTripDoesntLoseInformationForExplicitPresence() { + val originalMessage = TestAllTypesProto3(optionalInt32 = 0) + val roundTripMessage = TestAllTypesProto3.decodeFromJsonString(originalMessage.encodeToJsonString()) + + assertEquals(originalMessage.encodeToByteArray(), roundTripMessage.encodeToByteArray()) + } + @Test fun testNullValues() { val json = buildJsonObject { From c2e705a3d7479a7559c3dd9497ea353f972160d6 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Fri, 21 Oct 2022 12:37:49 +0200 Subject: [PATCH 09/14] remove test --- runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt | 8 -------- 1 file changed, 8 deletions(-) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index b619d12b..d43b3039 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -53,14 +53,6 @@ class JsonTest { assertEquals("{\"optionalNestedEnum\":\"BAR\"}", messageWith1.encodeToJsonString(jsonConfig)) } - @Test - fun binaryJsonRoundTripDoesntLoseInformationForExplicitPresence() { - val originalMessage = TestAllTypesProto3(optionalInt32 = 0) - val roundTripMessage = TestAllTypesProto3.decodeFromJsonString(originalMessage.encodeToJsonString()) - - assertEquals(originalMessage.encodeToByteArray(), roundTripMessage.encodeToByteArray()) - } - @Test fun testNullValues() { val json = buildJsonObject { From 98bc4e0aa7f4147b244552d20379335b8af846ab Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Fri, 21 Oct 2022 12:45:59 +0200 Subject: [PATCH 10/14] simplfiy by introducing a var --- .../kotlin/pbandk/internal/json/JsonMessageEncoder.kt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt b/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt index 1f490c16..90aca84d 100644 --- a/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt +++ b/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt @@ -38,8 +38,9 @@ internal class JsonMessageEncoder(private val jsonConfig: JsonConfig) : MessageE @Suppress("UNCHECKED_CAST") val value = (fd.value as KProperty1).get(message) - if (value == null && (fd.oneofMember || fd.type.hasPresence)) continue - if (!fd.oneofMember && !jsonConfig.outputDefaultValues && !fd.type.hasPresence && fd.type.isDefaultValue(value)) continue + val oneOfOrExplicitPresence = (fd.oneofMember || fd.type.hasPresence) + if (value == null && oneOfOrExplicitPresence) continue + if (!oneOfOrExplicitPresence && !jsonConfig.outputDefaultValues && fd.type.isDefaultValue(value)) continue val jsonValue = value ?.takeUnless { From e749c6a0780e87778c68cef93da9a7cd8080f2b9 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Tue, 25 Oct 2022 11:48:02 +0200 Subject: [PATCH 11/14] address comments --- .../commonTest/kotlin/pbandk/json/JsonTest.kt | 35 ----------------- .../pbandk/json/OutputDefaultValuesTest.kt | 39 +++++++++++++++++++ 2 files changed, 39 insertions(+), 35 deletions(-) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt index d43b3039..78518a53 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/JsonTest.kt @@ -5,7 +5,6 @@ import kotlinx.serialization.json.buildJsonObject import pbandk.encodeToByteArray import pbandk.testpb.Bar import pbandk.testpb.TestAllTypesProto3 -import pbandk.testpb.MessageWithEnum import kotlin.test.* class JsonTest { @@ -19,40 +18,6 @@ class JsonTest { assertContentEquals(byteArrayOf(), barBytes, "binary serialization should be empty for null field") } - @Test - fun testMessageWithEnumProto2() { - val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) - - // This behavior is unexpected and will be fixed in a follow-up. - // See https://github.com/streem/pbandk/issues/235 for more details. - val message = MessageWithEnum() - assertEquals("{\"value\":null}", message.encodeToJsonString(jsonConfig)) - - // This behavior is unexpected and will be fixed in a follow-up. - // See https://github.com/streem/pbandk/issues/235 for more details. - val messageWith0 = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) - assertEquals("{}", messageWith0.encodeToJsonString(jsonConfig)) - - // This works as expected. - val messageWith1 = MessageWithEnum(value = MessageWithEnum.EnumType.BAR) - assertEquals("{\"value\":\"BAR\"}", messageWith1.encodeToJsonString(jsonConfig)) - } - - @Test - fun testMessageWithEnumProto3() { - val jsonConfig = JsonConfig.DEFAULT.copy(compactOutput = true) - - val message = TestAllTypesProto3() - assertEquals("{}", message.encodeToJsonString(jsonConfig)) - - // See https://github.com/streem/pbandk/issues/235#issuecomment-1286122161 for more details. - val messageWith0 = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.FOO) - assertEquals("{}", messageWith0.encodeToJsonString(jsonConfig)) - - val messageWith1 = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.BAR) - assertEquals("{\"optionalNestedEnum\":\"BAR\"}", messageWith1.encodeToJsonString(jsonConfig)) - } - @Test fun testNullValues() { val json = buildJsonObject { diff --git a/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt index 3978411e..184e16e7 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt @@ -1,6 +1,7 @@ package pbandk.json import kotlinx.serialization.json.* +import pbandk.testpb.MessageWithEnum import pbandk.testpb.TestAllTypesProto3 import kotlin.test.Test import kotlin.test.assertEquals @@ -8,6 +9,8 @@ import kotlin.test.assertFalse import kotlin.test.assertTrue class OutputDefaultValuesTest { + private val jsonConfigCompactOutput = JsonConfig.DEFAULT.copy(compactOutput = true) + @Test fun testOutputDefaultValues_false() { val testAllTypesProto3 = TestAllTypesProto3(optionalString = "") @@ -48,4 +51,40 @@ class OutputDefaultValuesTest { assertEquals(JsonNull, parsedJson["optionalString"]) assertEquals(JsonPrimitive(""), parsedJson["optionalStringWrapper"]) } + + @Test + fun testProto2EnumUnsetField() { + val message = MessageWithEnum() + assertEquals("{\"value\":null}", message.encodeToJsonString(jsonConfigCompactOutput)) + } + + @Test + fun testProto2EnumFieldSetToDefaultValue() { + val message = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) + assertEquals("{}", message.encodeToJsonString(jsonConfigCompactOutput)) + } + + @Test + fun testProto2EnumFieldSetToNonDefaultValue() { + val message = MessageWithEnum(value = MessageWithEnum.EnumType.BAR) + assertEquals("{\"value\":\"BAR\"}", message.encodeToJsonString(jsonConfigCompactOutput)) + } + + @Test + fun testProto3EnumUnsetField() { + val message = TestAllTypesProto3() + assertEquals("{}", message.encodeToJsonString(jsonConfigCompactOutput)) + } + + @Test + fun testProto3EnumFieldSetToDefaultValue() { + val message = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.FOO) + assertEquals("{}", message.encodeToJsonString(jsonConfigCompactOutput)) + } + + @Test + fun testProto3EnumFieldSetToNonDefaultValue() { + val message = TestAllTypesProto3(optionalNestedEnum = TestAllTypesProto3.NestedEnum.BAR) + assertEquals("{\"optionalNestedEnum\":\"BAR\"}", message.encodeToJsonString(jsonConfigCompactOutput)) + } } \ No newline at end of file From ceae7e635774f928c9092422d1fba15ed2934ec9 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Tue, 25 Oct 2022 11:52:47 +0200 Subject: [PATCH 12/14] add comment that for proto3 the two tests are equivalent --- .../src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt | 1 + 1 file changed, 1 insertion(+) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt index 184e16e7..df083328 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt @@ -72,6 +72,7 @@ class OutputDefaultValuesTest { @Test fun testProto3EnumUnsetField() { + // Note that this is equivalent to 'testProto3EnumFieldSetToDefaultValue', adding for completeness. val message = TestAllTypesProto3() assertEquals("{}", message.encodeToJsonString(jsonConfigCompactOutput)) } From b8f938edf366121a040219f16a80a93c3a37dcf5 Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Tue, 25 Oct 2022 11:58:25 +0200 Subject: [PATCH 13/14] apply test fixes --- .../kotlin/pbandk/json/OutputDefaultValuesTest.kt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt b/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt index 46dfe77c..3002dcda 100644 --- a/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt +++ b/runtime/src/commonTest/kotlin/pbandk/json/OutputDefaultValuesTest.kt @@ -32,12 +32,12 @@ class OutputDefaultValuesTest { assertEquals(JsonPrimitive(""), parsedJson["optionalBytes"]) assertEquals(JsonPrimitive(TestAllTypesProto3.NestedEnum.fromValue(0).name!!), parsedJson["optionalNestedEnum"]) - assertFalse(parsedJson.containsKey("optionalNestedMessage")) + assertFalse("optionalNestedMessage" in parsedJson) assertEquals(JsonArray(emptyList()), parsedJson["repeatedString"]) assertEquals(JsonObject(emptyMap()), parsedJson["mapBoolBool"]) - assertFalse(parsedJson.containsKey("optionalStringWrapper")) + assertFalse("optionalStringWrapper" in parsedJson) assertEquals(JsonPrimitive(false), parsedJson["optionalBoolWrapper"]) } @@ -55,13 +55,13 @@ class OutputDefaultValuesTest { @Test fun testProto2EnumUnsetField() { val message = MessageWithEnum() - assertEquals("{\"value\":null}", message.encodeToJsonString(jsonConfigCompactOutput)) + assertEquals("{}", message.encodeToJsonString(jsonConfigCompactOutput)) } @Test fun testProto2EnumFieldSetToDefaultValue() { val message = MessageWithEnum(value = MessageWithEnum.EnumType.FOO) - assertEquals("{}", message.encodeToJsonString(jsonConfigCompactOutput)) + assertEquals("{\"value\":\"FOO\"}", message.encodeToJsonString(jsonConfigCompactOutput)) } @Test From 1cc3146a2b941935e80f9352f292d1071c4f47fb Mon Sep 17 00:00:00 2001 From: Anton Grbin Date: Tue, 25 Oct 2022 22:10:48 +0200 Subject: [PATCH 14/14] address comment about oneOf presence --- .../kotlin/pbandk/internal/json/JsonMessageEncoder.kt | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt b/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt index 90aca84d..25f2a361 100644 --- a/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt +++ b/runtime/src/commonMain/kotlin/pbandk/internal/json/JsonMessageEncoder.kt @@ -38,9 +38,8 @@ internal class JsonMessageEncoder(private val jsonConfig: JsonConfig) : MessageE @Suppress("UNCHECKED_CAST") val value = (fd.value as KProperty1).get(message) - val oneOfOrExplicitPresence = (fd.oneofMember || fd.type.hasPresence) - if (value == null && oneOfOrExplicitPresence) continue - if (!oneOfOrExplicitPresence && !jsonConfig.outputDefaultValues && fd.type.isDefaultValue(value)) continue + if (value == null && fd.type.hasPresence) continue + if (!fd.type.hasPresence && !jsonConfig.outputDefaultValues && fd.type.isDefaultValue(value)) continue val jsonValue = value ?.takeUnless {