diff options
author | Lin Xi <mogud@qq.com> | 2022-06-16 17:09:04 +0800 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-06-16 12:09:04 +0300 |
commit | 74072c5d4fa72aa253962af72fe16a7ac2c755e6 (patch) | |
tree | 490b08f68ee2431b02a1ec3c0a793240042dc2e0 | |
parent | 7e03eedf4ff09ed3007e1c872f3a9fcfba62853f (diff) | |
download | kotlinx.serialization-74072c5d4fa72aa253962af72fe16a7ac2c755e6.tar.gz |
Fix protocol buffer enum schema generation (#1967)
fix protocol buffer schema generated from enum elements with 'ProtoNumber' annotation
5 files changed, 76 insertions, 4 deletions
diff --git a/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt b/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt index e54370fb..8f326939 100644 --- a/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt +++ b/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt @@ -190,7 +190,7 @@ public object ProtoBufSchemaGenerator { val annotations = messageDescriptor.getElementAnnotations(index) - val number = annotations.filterIsInstance<ProtoNumber>().singleOrNull()?.number ?: index + 1 + val number = annotations.filterIsInstance<ProtoNumber>().singleOrNull()?.number ?: (index + 1) if (!usedNumbers.add(number)) { throw IllegalArgumentException("Field number $number is repeated in the class with serial name ${messageDescriptor.serialName}") } @@ -319,19 +319,35 @@ public object ProtoBufSchemaGenerator { } val safeSerialName = removeLineBreaks(enumDescriptor.serialName) if (safeSerialName != enumName) { - append("// serial name '").append(enumName).appendLine('\'') + append("// serial name '").append(safeSerialName).appendLine('\'') } append("enum ").append(enumName).appendLine(" {") - enumDescriptor.elementDescriptors.forEachIndexed { number, element -> + val usedNumbers: MutableSet<Int> = mutableSetOf() + val duplicatedNumbers: MutableSet<Int> = mutableSetOf() + enumDescriptor.elementDescriptors.forEachIndexed { index, element -> val elementName = element.protobufEnumElementName elementName.checkIsValidIdentifier { "The enum element name '$elementName' is invalid in the " + "protobuf schema. Serial name of the enum class '${enumDescriptor.serialName}'" } + + val annotations = enumDescriptor.getElementAnnotations(index) + val number = annotations.filterIsInstance<ProtoNumber>().singleOrNull()?.number ?: index + if (!usedNumbers.add(number)) { + duplicatedNumbers.add(number) + } + append(" ").append(elementName).append(" = ").append(number).appendLine(';') } + if (duplicatedNumbers.isNotEmpty()) { + throw IllegalArgumentException( + "The class with serial name ${enumDescriptor.serialName} has duplicate " + + "elements with numbers $duplicatedNumbers" + ) + } + appendLine('}') } diff --git a/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt b/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt index b7332312..03302502 100644 --- a/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt +++ b/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt @@ -3,7 +3,6 @@ package kotlinx.serialization.protobuf.schema import kotlinx.serialization.* import kotlinx.serialization.protobuf.* import kotlin.test.Test -import kotlin.test.assertContains import kotlin.test.assertFailsWith class SchemaValidationsTest { @@ -39,6 +38,33 @@ class SchemaValidationsTest { SECOND } + @Serializable + enum class EnumWithExplicitProtoNumberDuplicate { + @ProtoNumber(2) + FIRST, + @ProtoNumber(2) + SECOND, + } + + @Serializable + enum class EnumWithImplicitProtoNumberDuplicate { + FIRST, + @ProtoNumber(0) + SECOND, + } + + @Test + fun testExplicitDuplicateEnumElementProtoNumber() { + val descriptors = listOf(EnumWithExplicitProtoNumberDuplicate.serializer().descriptor) + assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) } + } + + @Test + fun testImplicitDuplicateEnumElementProtoNumber() { + val descriptors = listOf(EnumWithImplicitProtoNumberDuplicate.serializer().descriptor) + assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) } + } + @Test fun testInvalidEnumElementSerialName() { val descriptors = listOf(InvalidEnumElementName.serializer().descriptor) diff --git a/formats/protobuf/jvmTest/resources/EnumWithProtoNumber.proto b/formats/protobuf/jvmTest/resources/EnumWithProtoNumber.proto new file mode 100644 index 00000000..21528036 --- /dev/null +++ b/formats/protobuf/jvmTest/resources/EnumWithProtoNumber.proto @@ -0,0 +1,11 @@ +syntax = "proto2"; + +package kotlinx.serialization.protobuf.schema.generator; + +// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.EnumWithProtoNumber' +enum EnumWithProtoNumber { + ZERO = 0; + THREE = 3; + TWO = 2; + FIVE = 5; +} diff --git a/formats/protobuf/jvmTest/resources/common/schema.proto b/formats/protobuf/jvmTest/resources/common/schema.proto index e8a0b4cd..79e2d79e 100644 --- a/formats/protobuf/jvmTest/resources/common/schema.proto +++ b/formats/protobuf/jvmTest/resources/common/schema.proto @@ -131,6 +131,14 @@ message OptionalCollections { map<int32, int32> nullableOptionalMap = 8; } +// serial name 'kotlinx.serialization.protobuf.schema.GenerationTest.EnumWithProtoNumber' +enum EnumWithProtoNumber { + ZERO = 0; + THREE = 3; + TWO = 2; + FIVE = 5; +} + enum OverriddenEnumName { FIRST = 0; OverriddenElementName = 1; diff --git a/formats/protobuf/jvmTest/src/kotlinx/serialization/protobuf/schema/GenerationTest.kt b/formats/protobuf/jvmTest/src/kotlinx/serialization/protobuf/schema/GenerationTest.kt index 7b075a4e..61a2dce5 100644 --- a/formats/protobuf/jvmTest/src/kotlinx/serialization/protobuf/schema/GenerationTest.kt +++ b/formats/protobuf/jvmTest/src/kotlinx/serialization/protobuf/schema/GenerationTest.kt @@ -25,6 +25,7 @@ internal val commonClasses = listOf( GenerationTest.LegacyMapHolder::class, GenerationTest.NullableNestedCollections::class, GenerationTest.OptionalCollections::class, + GenerationTest.EnumWithProtoNumber::class, ) class GenerationTest { @@ -180,6 +181,16 @@ class GenerationTest { val legacyMap: Map<List<Int>?, List<Int>?> ) + @Serializable + enum class EnumWithProtoNumber { + ZERO, + @ProtoNumber(3) + THREE, + TWO, + @ProtoNumber(5) + FIVE, + } + @Test fun testIndividuals() { assertSchemaForClass(OptionsClass::class, mapOf("java_package" to "api.proto", "java_outer_classname" to "Outer")) |