ProtoKt-generated Code is incompatible with google's gRPC stubs when it comes to builtin types (e.g. Empty)

[RaphaelTarita]

I just upgraded to 0.9.0 and tried to migrate my codebase to use the gRPC stubs that google's protoc-gen-grpc-kotlin generates. However, I came across an issue with the predefined google.protobuf.Empty type:

ProtoKt has (also previously) supplied their own version of Empty, called com.toasttab.protokt.Empty. This has worked fine until now, because gRPC-Kt uses their version of Empty (called com.google.protobuf.Empty) in the generated code. This then causes conflicts between the ServiceGrpcKt (google) and the [name]Grpc (protokt) files.

Since the protokt ecosystem (presumably) depends on their own classes, maybe a solution would be to expose the google.protobuf.Empty source as if it was a custom type, because all custom types get resolved properly.

[andrewparmet]

Interesting find. We don't run into this issue internally since we follow Buf's style guide for services and create and name all request and response types and never use externally-defined messages in our API surfaces. That's what I recommend you do here as well, and I'll clarify this issue in the readme.

com.toasttab.protokt.Empty is generated from the same definition as com.google.protobuf.Empty. We do the same for all protobuf definitions included in protobuf-java (e.g. Timestamp, Duration, etc.), and we're forced to change the package names to avoid conflicting class declarations. (More on that here in #169, which we're still working through.)

Since we rely on the protobuf-java library at runtime and it includes its own copies of those classes, protokt must generate its own copies to avoid conflicting class declarations. Technically we could support the original names if we created our own version of the protobuf runtime that didn't include those classes, but that's a big lift and then we'd have to keep it in sync with any performance improvements or bug fixes in the mainline. If we ever support Kotlin Native we'll have to do that port anyways, but even then we must continue to rename those core classes since any JVM consumer should be free to use protobuf-java as well as protokt.

[RaphaelTarita]

The suggestion to use custom message types insead of google.protobuf.Empty is of course a good one, and will, in short-term, fix my issues. However, I'm now wondering if this problem would then apply to all google.protobuf predefined types (such as the mentioned Timestamp and Duration).

In essence, this means that when using protokt alongside with protobuf-java, one cannot use google's predefined message types, if I understand this correctly.

Would that mean that you'd have to re-define all those message types yourself? Or is there another (general) solution?

[andrewparmet]

protokt regenerates those types as KtMessages under the com.toasttab.protokt package, so you're free to use them. All protokt-generated code will reference the versions under com.toasttab.protokt. We reference predefined types extensively in testing, such as here: https://github.com/open-toast/protokt/blob/main/testing/options/src/main/proto/com/toasttab/protokt/testing/options/wrapper_types.proto. The grpc-kotlin generator has no idea we do that and it simply assumes usage of the same classes that protobuf-java generates.

An alternative solution I forgot to mention is that you can keep using the protobuf-java types in your APIs but not use the grpc-kotlin client and server generator. You can still use the coroutine-based ClientCalls and ServerCalls in the grpc-kotlin runtime, you just won't get the pretty stubs. This is currently how protokt interfaces with grpc-java: https://github.com/open-toast/protokt/blob/main/examples/grpc-java/src/main/kotlin/io/grpc/examples/animals/AnimalsClient.kt#L49 and how it used to interface with grpc-kotlin before #171:

protokt/examples/grpc-kotlin/src/main/kotlin/io/grpc/examples/animals/AnimalsClient.kt

Line 31 in 6319e17

val response = ClientCalls.unaryRpc(channel, DogGrpc.barkMethod, request)

.

[RaphaelTarita]

okay, thanks for the info!