From 693ea69b5d79e97edb4d699c0bc90c91f0c7ff61 Mon Sep 17 00:00:00 2001 From: Joseph Ivie Date: Mon, 4 Nov 2024 12:11:40 -0700 Subject: [PATCH 1/2] ProtoName is now supported, allowing the overriding of the field name for ProtoBuf only. --- .../serialization/protobuf/ProtoTypes.kt | 10 ++++ .../schema/ProtoBufSchemaGenerator.kt | 36 ++++++++++----- .../protobuf/schema/SchemaValidationsTest.kt | 46 +++++++++++++++++++ 3 files changed, 81 insertions(+), 11 deletions(-) diff --git a/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/ProtoTypes.kt b/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/ProtoTypes.kt index e673f16f28..db34aa563d 100644 --- a/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/ProtoTypes.kt +++ b/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/ProtoTypes.kt @@ -7,6 +7,16 @@ package kotlinx.serialization.protobuf import kotlinx.serialization.* import kotlinx.serialization.descriptors.* +/** + * Specifies protobuf field name assigned to a Kotlin property or class. + * + * Used to get around invalid naming conventions. + */ +@SerialInfo +@Target(AnnotationTarget.PROPERTY, AnnotationTarget.CLASS) +@ExperimentalSerializationApi +public annotation class ProtoName(public val name: String) + /** * Specifies protobuf field number (a unique number for a field in the protobuf message) * assigned to a Kotlin property. 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 e4826bbbbc..2e6fe37f92 100644 --- a/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt +++ b/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt @@ -182,7 +182,7 @@ public object ProtoBufSchemaGenerator { getAnnotations: (Int) -> List = { parentType.descriptor.getElementAnnotations(it) }, getChildType: (Int) -> TypeDefinition = { parentType.descriptor.getElementDescriptor(it).let(::TypeDefinition) }, getChildNumber: (Int) -> Int = { parentType.descriptor.getElementAnnotations(it).filterIsInstance().singleOrNull()?.number ?: (it + 1) }, - getChildName: (Int) -> String = { parentType.descriptor.getElementName(it) }, + getChildName: (Int) -> String = { parentType.descriptor.getElementNameProtobuf(it) }, inOneOfStruct: Boolean = false, ) { val messageDescriptor = parentType.descriptor @@ -216,7 +216,7 @@ public object ProtoBufSchemaGenerator { getAnnotations = { desc.annotations }, getChildType = { desc.elementDescriptors.single().let(::TypeDefinition) }, getChildNumber = { desc.getElementAnnotations(0).filterIsInstance().singleOrNull()?.number ?: (it + 1) }, - getChildName = { desc.getElementName(0) }, + getChildName = { desc.getElementNameProtobuf(0) }, inOneOfStruct = true, ) } @@ -392,13 +392,15 @@ public object ProtoBufSchemaGenerator { val usedNumbers: MutableSet = mutableSetOf() val duplicatedNumbers: MutableSet = mutableSetOf() enumDescriptor.elementDescriptors.forEachIndexed { index, element -> - val elementName = element.protobufEnumElementName + val annotations = enumDescriptor.getElementAnnotations(index) + + val elementName = annotations.filterIsInstance().singleOrNull()?.name + ?: element.serialName.substringAfterLast('.', element.serialName) elementName.checkIsValidIdentifier { "The enum element name '$elementName' is invalid in the " + - "protobuf schema. Serial name of the enum class '${enumDescriptor.serialName}'" + "protobuf schema. Serial name of the enum class '${enumDescriptor.serialName}'" } - val annotations = enumDescriptor.getElementAnnotations(index) val number = annotations.filterIsInstance().singleOrNull()?.number ?: index if (!usedNumbers.add(number)) { duplicatedNumbers.add(number) @@ -416,6 +418,13 @@ public object ProtoBufSchemaGenerator { appendLine('}') } + private fun SerialDescriptor.getElementNameProtobuf(index: Int): String { + getElementAnnotations(index).forEach { + if(it is ProtoName) return it.name + } + return getElementName(index) + } + private val SerialDescriptor.isOpenPolymorphic: Boolean get() = kind == PolymorphicKind.OPEN @@ -453,8 +462,16 @@ public object ProtoBufSchemaGenerator { get() = kind == PrimitiveKind.INT || kind == PrimitiveKind.LONG || kind == PrimitiveKind.BOOLEAN || kind == PrimitiveKind.STRING + @Suppress("RecursivePropertyAccessor") private val SerialDescriptor.messageOrEnumName: String - get() = (serialName.substringAfterLast('.', serialName)).removeSuffix("?") + get() { + // Try to get the original descriptor to retrieve the ProtoName + if (this is NotNullSerialDescriptor) return this.original.messageOrEnumName + if (nonNullOriginal != this) return nonNullOriginal.messageOrEnumName + + annotations.forEach { if(it is ProtoName) return it.name } + return serialName.substringAfterLast('.', serialName).removeSuffix("?") + } private fun SerialDescriptor.isChildOneOfMessage(index: Int): Boolean = this.getElementDescriptor(index).isSealedPolymorphic && this.getElementAnnotations(index).any { it is ProtoOneOf } @@ -467,9 +484,6 @@ public object ProtoBufSchemaGenerator { } } - private val SerialDescriptor.protobufEnumElementName: String - get() = serialName.substringAfterLast('.', serialName) - private fun SerialDescriptor.scalarTypeName(annotations: List = emptyList()): String { val integerType = annotations.filterIsInstance().firstOrNull()?.type ?: ProtoIntegerType.DEFAULT @@ -548,7 +562,7 @@ public object ProtoBufSchemaGenerator { ): TypeDefinition { val messageDescriptor = messageType.descriptor val fieldDescriptor = messageDescriptor.getElementDescriptor(index) - val fieldName = messageDescriptor.getElementName(index) + val fieldName = messageDescriptor.getElementNameProtobuf(index) val messageName = messageDescriptor.messageOrEnumName val wrapperName = "${messageName}_${fieldName}" @@ -573,7 +587,7 @@ public object ProtoBufSchemaGenerator { description: String ): TypeDefinition { val messageDescriptor = messageType.descriptor - val fieldName = messageDescriptor.getElementName(index) + val fieldName = messageDescriptor.getElementNameProtobuf(index) val messageName = messageDescriptor.messageOrEnumName val wrapperName = "${messageName}_${fieldName}" 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 ffe9d94346..e61289d5e4 100644 --- a/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt +++ b/formats/protobuf/commonTest/src/kotlinx/serialization/protobuf/schema/SchemaValidationsTest.kt @@ -17,9 +17,17 @@ class SchemaValidationsTest { @SerialName("invalid serial name") data class InvalidClassName(val i: Int) + @Serializable + @SerialName("invalid serial name") + @ProtoName("ValidSerialName") + data class InvalidClassNameFixed(val i: Int) + @Serializable data class InvalidClassFieldName(@SerialName("invalid serial name") val i: Int) + @Serializable + data class InvalidClassFieldNameFixed(@ProtoName("okProtoName") @SerialName("invalid serial name") val i: Int) + @Serializable data class FieldNumberDuplicates(@ProtoNumber(42) val i: Int, @ProtoNumber(42) val j: Int) @@ -30,6 +38,11 @@ class SchemaValidationsTest { @SerialName("invalid serial name") enum class InvalidEnumName { SINGLETON } + @Serializable + @SerialName("invalid serial name") + @ProtoName("ValidEnumNameFixed") + enum class InvalidEnumNameFixed { SINGLETON } + @Serializable enum class InvalidEnumElementName { FIRST, @@ -38,6 +51,15 @@ class SchemaValidationsTest { SECOND } + @Serializable + enum class InvalidEnumElementNameFixed { + FIRST, + + @SerialName("invalid serial name") + @ProtoName("validSerialName") + SECOND + } + @Serializable enum class EnumWithExplicitProtoNumberDuplicate { @ProtoNumber(2) @@ -71,18 +93,36 @@ class SchemaValidationsTest { assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) } } + @Test + fun testInvalidEnumElementSerialNameFixed() { + val descriptors = listOf(InvalidEnumElementNameFixed.serializer().descriptor) + println(ProtoBufSchemaGenerator.generateSchemaText(descriptors)) + } + @Test fun testInvalidClassSerialName() { val descriptors = listOf(InvalidClassName.serializer().descriptor) assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) } } + @Test + fun testInvalidClassSerialNameFixed() { + val descriptors = listOf(InvalidClassNameFixed.serializer().descriptor) + println(ProtoBufSchemaGenerator.generateSchemaText(descriptors)) + } + @Test fun testInvalidClassFieldSerialName() { val descriptors = listOf(InvalidClassFieldName.serializer().descriptor) assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) } } + @Test + fun testInvalidClassFieldSerialNameFixed() { + val descriptors = listOf(InvalidClassFieldNameFixed.serializer().descriptor) + println(ProtoBufSchemaGenerator.generateSchemaText(descriptors)) + } + @Test fun testDuplicateSerialNames() { val descriptors = listOf(InvalidClassFieldName.serializer().descriptor) @@ -95,6 +135,12 @@ class SchemaValidationsTest { assertFailsWith(IllegalArgumentException::class) { ProtoBufSchemaGenerator.generateSchemaText(descriptors) } } + @Test + fun testInvalidEnumSerialNameFixed() { + val descriptors = listOf(InvalidEnumNameFixed.serializer().descriptor) + println(ProtoBufSchemaGenerator.generateSchemaText(descriptors)) + } + @Test fun testDuplicationSerialName() { val descriptors = listOf(ValidClass.serializer().descriptor, DuplicateClass.serializer().descriptor) From 6774d8bf805728e09ef5e6331dad50b05957b468 Mon Sep 17 00:00:00 2001 From: Joseph Ivie Date: Mon, 4 Nov 2024 12:20:08 -0700 Subject: [PATCH 2/2] Added reference to ProtoName to the documentation of ProtoBufSchemaGenerator --- .../serialization/protobuf/schema/ProtoBufSchemaGenerator.kt | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 2e6fe37f92..36ae4c2d46 100644 --- a/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt +++ b/formats/protobuf/commonMain/src/kotlinx/serialization/protobuf/schema/ProtoBufSchemaGenerator.kt @@ -16,7 +16,8 @@ import kotlinx.serialization.protobuf.internal.* * An arbitrary Kotlin class represent much wider object domain than the ProtoBuf specification, thus schema generator * has the following list of restrictions: * - * * Serial name of the class and all its fields should be a valid Proto [identifier](https://developers.google.com/protocol-buffers/docs/reference/proto2-spec) + * * Serial name of the class and all its fields should be a valid Proto [identifier](https://developers.google.com/protocol-buffers/docs/reference/proto2-spec). + * If this is a problem for you, you many override serial names for ProtoBuf only by using the [ProtoName] annotation for specific fields or classes. * * Nullable values are allowed only for Kotlin [nullable][SerialDescriptor.isNullable] types, but not [optional][SerialDescriptor.isElementOptional] * in order to properly distinguish "default" and "absent" values. * * The name of the type without the package directive uniquely identifies the proto message type and two or more fields with the same serial name