Skip to content

KRPC-143 Repeated types in gRPC #328

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -179,12 +179,15 @@ class ModelToKotlinGenerator(
scope("return $platformType.newBuilder().apply", ".build()") {
declaration.actualFields.forEach { field ->
val uppercaseName = field.name.replaceFirstChar { ch -> ch.uppercase() }
val setFieldCall = "set$uppercaseName"
val setFieldCall = when (field.type) {
is FieldType.List -> "addAll$uppercaseName"
else -> "set$uppercaseName"
}

if (field.nullable) {
code("this@toPlatform.${field.name}?.let { $setFieldCall(it${field.toPlatformCast()}) }")
code("this@toPlatform.${field.name}?.let { $setFieldCall(it${field.type.toPlatformCast()}) }")
} else {
code("$setFieldCall(this@toPlatform.${field.name}${field.toPlatformCast()})")
code("$setFieldCall(this@toPlatform.${field.name}${field.type.toPlatformCast()})")
}
}
}
Expand All @@ -197,7 +200,12 @@ class ModelToKotlinGenerator(
) {
scope("return ${declaration.name.simpleName}") {
declaration.actualFields.forEach { field ->
val getter = "this@toKotlin.${field.name}${field.toKotlinCast()}"
val javaName = when (field.type) {
is FieldType.List -> "${field.name}List"
else -> field.name
}

val getter = "this@toKotlin.$javaName${field.type.toKotlinCast()}"
if (field.nullable) {
ifBranch(
prefix = "${field.name} = ",
Expand All @@ -217,33 +225,53 @@ class ModelToKotlinGenerator(
}
}

private fun FieldDeclaration.toPlatformCast(): String {
return when (type) {
private fun FieldType.toPlatformCast(): String {
return when (this) {
FieldType.IntegralType.FIXED32 -> ".toInt()"
FieldType.IntegralType.FIXED64 -> ".toLong()"
FieldType.IntegralType.UINT32 -> ".toInt()"
FieldType.IntegralType.UINT64 -> ".toLong()"
FieldType.IntegralType.BYTES -> ".let { bytes -> com.google.protobuf.ByteString.copyFrom(bytes) }"
is FieldType.Reference -> ".toPlatform()".also {
val fq by type.value
val fq by value
importRootDeclarationIfNeeded(fq, "toPlatform", true)
}
is FieldType.List -> {
when (value) {
is FieldType.Reference -> ".map { it.toPlatform() }".also {
val fq by value.value
importRootDeclarationIfNeeded(fq, "toPlatform", true)
}
is FieldType.IntegralType -> ""
Copy link
Preview

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Double-check that numeric list elements do not require any platform-specific casting. If new numeric types are added in the future, consider handling them explicitly.

Suggested change
is FieldType.IntegralType -> ""
is FieldType.IntegralType -> ".map { it }"

Copilot uses AI. Check for mistakes.

else -> error("Unsupported type: $value")
}
}

else -> ""
}
}

private fun FieldDeclaration.toKotlinCast(): String {
return when (type) {
private fun FieldType.toKotlinCast(): String {
return when (this) {
FieldType.IntegralType.FIXED32 -> ".toUInt()"
FieldType.IntegralType.FIXED64 -> ".toULong()"
FieldType.IntegralType.UINT32 -> ".toUInt()"
FieldType.IntegralType.UINT64 -> ".toULong()"
FieldType.IntegralType.BYTES -> ".toByteArray()"
is FieldType.Reference -> ".toKotlin()".also {
val fq by type.value
val fq by value
importRootDeclarationIfNeeded(fq, "toKotlin", true)
}
is FieldType.List -> {
when (value) {
is FieldType.Reference -> ".map { it.toKotlin() }".also {
val fq by value.value
importRootDeclarationIfNeeded(fq, "toKotlin", true)
}
is FieldType.IntegralType -> ".toList()"
else -> error("Unsupported type: $value")
}
}

else -> ""
}
Expand All @@ -265,17 +293,22 @@ class ModelToKotlinGenerator(
type.fqName.simpleName
}

// KRPC-143 Repeated Types
// is FieldType.List -> {
// "List<${type.valueName.simpleName}>"
// }
//
is FieldType.List -> {
val fqValue = when (val value = type.value) {
is FieldType.Reference -> value.value.value
is FieldType.IntegralType -> value.fqName
else -> error("Unsupported type: $value")
}

"List<${fqValue.safeFullName()}>"
}

// KRPC-145 Map Types
// is FieldType.Map -> {
// "Map<${type.keyName.simpleName}, ${type.valueName.simpleName}>"
// }
else -> {
error("Unsupported type: $type")
error("Unsupported type: $this")
}
}.withNullability(nullable)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,8 @@ class ProtoToModelInterpreter(
name = fqName,
actualFields = fields,
oneOfDeclarations = oneofDeclList.asSequence().mapIndexedNotNull { i, desc -> desc.toModel(i, resolver) },
enumDeclarations = enumTypeList.asSequence().map { it.toModel(resolver, outerClass, parent ?: packageName) },
enumDeclarations = enumTypeList.asSequence()
.map { it.toModel(resolver, outerClass, parent ?: packageName) },
nestedDeclarations = nestedTypeList.asSequence().map { it.toModel(resolver, outerClass, fqName) },
deprecated = options.deprecated,
doc = null,
Expand Down Expand Up @@ -178,25 +179,27 @@ class ProtoToModelInterpreter(
.fullProtoNameToKotlin(firstLetterUpper = true)
// TODO KRPC-146 Nested Types: parent full type resolution
// KRPC-144 Import types
.let { wrapWithLabel(lazy { resolver.resolve(it) }) }
.asReference { resolver.resolve(it) }
.let { wrapWithLabel(it) }
}

hasTypeName() -> {
typeName
.fullProtoNameToKotlin(firstLetterUpper = true)
// TODO KRPC-146 Nested Types: parent full type resolution
// KRPC-144 Import types: we assume local types now
.let { wrapWithLabel(lazy { resolver.resolve(it) }) }
.asReference { resolver.resolve(it) }
.let { wrapWithLabel(it) }
}

else -> {
primitiveType()
wrapWithLabel(primitiveType())
}
}
}

@Suppress("detekt.CyclomaticComplexMethod")
private fun DescriptorProtos.FieldDescriptorProto.primitiveType(): FieldType {
private fun DescriptorProtos.FieldDescriptorProto.primitiveType(): IntegralType {
return when (type) {
Type.TYPE_STRING -> IntegralType.STRING
Type.TYPE_BYTES -> IntegralType.BYTES
Expand All @@ -219,15 +222,18 @@ class ProtoToModelInterpreter(
}
}

private fun DescriptorProtos.FieldDescriptorProto.wrapWithLabel(fqName: Lazy<FqName>): FieldType {
private fun String.asReference(resolver: (String) -> FqName) =
FieldType.Reference(lazy { resolver(this) })

private fun DescriptorProtos.FieldDescriptorProto.wrapWithLabel(fieldType: FieldType): FieldType {
return when (label) {
DescriptorProtos.FieldDescriptorProto.Label.LABEL_REPEATED -> {
FieldType.List(fqName)
FieldType.List(fieldType)
}
// LABEL_OPTIONAL is not actually optional in proto3.
// Actual optional is oneOf with one option and same name
else -> {
FieldType.Reference(fqName)
fieldType
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ data class FieldDeclaration(
sealed interface FieldType {
val defaultValue: String

data class List(val valueName: Lazy<FqName>) : FieldType {
data class List(val value: FieldType) : FieldType {
override val defaultValue: String = "emptyList()"
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,7 @@ import ReferenceTestService
import References
import invoke
import Other
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.sync.Mutex
import kotlinx.coroutines.sync.withLock
import kotlinx.coroutines.test.runTest
import kotlinx.rpc.RpcServer
import kotlinx.rpc.grpc.GrpcClient
import kotlinx.rpc.grpc.GrpcServer
import kotlinx.rpc.registerService
import kotlinx.rpc.withService
import kotlin.test.Test
Expand All @@ -38,6 +32,10 @@ class ReferenceTestServiceImpl : ReferenceTestService {
override suspend fun Optional(message: OptionalTypes): OptionalTypes {
return message
}

override suspend fun Repeated(message: Repeated): Repeated {
return message
}
}

class TestReferenceService : GrpcServerTest() {
Expand Down Expand Up @@ -93,4 +91,26 @@ class TestReferenceService : GrpcServerTest() {
assertEquals(null, resultNullable.age)
assertEquals(null, resultNullable.reference)
}

@Test
fun testRepeated() = runGrpcTest { grpcClient ->
val service = grpcClient.withService<ReferenceTestService>()
val result = service.Repeated(Repeated {
listString = listOf("test", "hello")
listReference = listOf(kotlinx.rpc.protobuf.test.References {
other = kotlinx.rpc.protobuf.test.Other {
field = 42
}
})
})

assertEquals(listOf("test", "hello"), result.listString)
assertEquals(1, result.listReference.size)
assertEquals(42, result.listReference[0].other.field)

val resultEmpty = service.Repeated(Repeated {})

assertEquals(emptyList(), resultEmpty.listString)
assertEquals(emptyList(), resultEmpty.listReference)
}
}
3 changes: 3 additions & 0 deletions protobuf-plugin/src/test/proto/reference_service.proto
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import "reference.proto";
import "reference_package.proto";
import "enum.proto";
import "optional.proto";
import "repeated.proto";

service ReferenceTestService {
rpc Get(References) returns (kotlinx.rpc.protobuf.test.References);

rpc Enum(kotlinx.rpc.protobuf.test.UsingEnum) returns (kotlinx.rpc.protobuf.test.UsingEnum);

rpc Optional(kotlinx.rpc.protobuf.test.OptionalTypes) returns (kotlinx.rpc.protobuf.test.OptionalTypes);

rpc Repeated(kotlinx.rpc.protobuf.test.Repeated) returns (kotlinx.rpc.protobuf.test.Repeated);
}
10 changes: 10 additions & 0 deletions protobuf-plugin/src/test/proto/repeated.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
syntax = "proto3";

package kotlinx.rpc.protobuf.test;

import 'reference_package.proto';

message Repeated {
repeated string listString = 1;
repeated References listReference = 2;
}