Support generating message types as Kotlin classes instead of Java class

[jeffreychuuu]

I'm trying to integrate Grpc with my Kotlin spring boot project which using this package and find that I cannot use the message types since they are generated with Java instead of Kotlin.

So is there any plan to support generation of message type with Kotlin class?

[lowasser]

Not at this time.

I'm very surprised to hear you can't use Java generated code with your Kotlin spring boot project. How is your build set up?

[jeffreychuuu]

My projetc is located at https://github.com/jeffreychu-org/kotlin_example_app in grpc-issue branch.

It may tested simply be

cd /rdbms_service docker-compose up -d to start the PostgreSQL and redis simple start the spring boot app and it will trigger the other project grpc_lib to generate the grpc stub for use This issue struggle me a few days. Many thanks. Cheers.

[jamesward]

Can you provide more details about what isn't work? Would it help if you added in support for the Kotlin protoc stuff? For an example of that, see: https://github.com/grpc/grpc-kotlin/tree/kotlin-protos/examples

[jeffreychuuu]

My latest commit in the master brach in https://github.com/jeffreychu-org/kotlin_example_app fix the issue and can use the grpc_kotlin package successfully

Thanks all:)

[jamesward]

Cool. Glad you figured it out.

[JavierSegoviaCordoba]

@jamesward can this be reopened?

I think Kotlin classes codegen support is necessary and probably supporting some types with sealed classes/interfaces is interesting too.

Is it planned to drop the DSL support in order to avoid mutability and null issues? Personally, I don't think it is a good idea to build a class with a DSL that uses var under the hood because you can forget to initialize some variables, so using data classes or a custom Kotlin class should be great.

class DogService : DogGrpcKt.DogCoroutineImplBase() {
    override suspend fun bark(request: BarkRequest): BarkReply {
        return barkReply {
            // This compiles but it is incorrect.
            // The DSL is not forcing me to set a message.
            // message = "Woof"
        }
    }
}

Something like this should be better

class DogService : DogGrpcKt.DogCoroutineImplBase() {
    override suspend fun bark(request: BarkRequest): BarkReply {
        // I can't forget to set the `message`
        return BarkReply(message = "Woof")
    }
}

[jamesward]

@JavierSegoviaCordoba It would be cool if Kotlin protos created immutable data classes without the Java underpinnings but that'd be a request for the protobuf project, not this one. There may be good reasons for not doing that and I don't know enough about protos to fully understand the reasons for/against this. If you file a feature request, can you cross link it to this issue?

[lowasser]

The behavior you describe wanting is not compatible with the requirements of protocol buffers. Fundamentally, adding a field (at any position in declaration or field number order) must always be a compatible change, and required fields are strongly advised against (and fully dropped in proto3).

Factory methods and constructors are incompatible with these requirements: inserting a new field in the middle can change the meaning of factory(a, b, c) or MyProto(a, b, c). Since proto fields should generally be optional, using vars correctly reflects that they need not be set. Since proto3 also largely drops the distinction between unset and set-to-default, the current nullability model (~nothing is null) is the only plausible default, but where possible, orNull accessors may be additionally provided as extensions. (We have done some work in this area, which we're evaluating before release.)

Given these considerations, even if we had designed something pure-Kotlin, without caring for backward compatibility at all, the design we have today is essentially identical to what we would have built. Since we do have an enormous (almost certainly the majority) segment of the user base who do want drop-in compatibility with their existing Java codebases, the design reasons for the existing approach are pretty overwhelmingly compelling to us.

We may pursue some work in this area for future for non-JVM use cases, but it would have a precisely compatible API.

We have certainly also discussed the use of e.g. sealed classes for oneofs, which we could add compatibly with extensions, but we postponed work in that area to consider the performance implications. Compare kotlin.Result, which isn't implemented with a sealed class for performance reasons; we might consider doing something with a visitor-style API along its lines, but it's not obvious that would be any more convenient to users than the existing API.

[JavierSegoviaCordoba]

@lowasser from my point of view a proto is a contract, updating that contract and the server being able to run without changes breaking that contract is not a good state.

If the proto has a new not null value and no default value, the server, due to the builder pattern, allows you to respond to a broken contract.

Consumers with null safety will get an unset field which should not be null.

Not sure what is happening in other languages but wire from Square is generating classes for the client that would break with this implementation as they will get a Kotlin null pointer exception or something so that the server codegen is not forcing to send a not null value which is in the contract.

Is there a way to check in compile time that the contract is being broken?

EDIT: I have rechecked and wire is setting default values with proto 3, so the only problem is forgetting to send some values. I investigated what you mentioned about dropping required with proto 3 and I guess I am on the other team 😆. The only way to ensure "not breaking" the contract is by creating tests for all, I would prefer breaking in compile time and trying to ensure the compatibility in some way.

[JavierSegoviaCordoba]

@jamesward can you point me to that project please?

[JavierSegoviaCordoba]

@lowasser I did some tests and I think there is no issue if the codegen forces it to be "all required" in the server. It will be backward compatible.

Having this proto:

syntax = "proto3";

service Dog {
  rpc Bark (BarkRequest) returns (BarkReply) {}
}

message BarkRequest {
}

message BarkReply {
  string message = 1;
}

On the server side, it can generate, very simplified, this returns type:

data class BarkReply(val message: String)

If the schema is updated and a new field is added:

syntax = "proto3";

service Dog {
  rpc Bark (BarkRequest) returns (BarkReply) {}
}

message BarkRequest {
}

message BarkReply {
  string message = 1;
  string newField = 2;
}

The codegen in the server will generate this.

data class BarkReply(val message: String, val newField: String)

Then the code doesn't compile and you need to update the service including it.

But the client application (at least with Wire), using the old proto without newFiled is working perfectly for me with the server being deployed with this newField.

So you get type safety in compile time at the same time you get backward compatibility.

[lowasser]

from my point of view a proto is a contract, updating that contract and the server being able to run without changes breaking that contract is not a good state.

Adding fields does not break the contract provided by protos, and is absolutely necessary for code that can evolve over time. The compatible modifications are defined in the proto spec here. Here is the thing that is required to work, but will not (with the described API):

Before code:

message BarkRequest {
  optional string message = 1;
  optional string dog_name = 3;
}

val request = BarkRequest("message", "Fido")
println(request.dogName) // "Fido"

After code:

message BarkRequest{
  optional string message = 1;
  optional string new_field = 2;
  optional string dog_name = 3;
}

val request = BarkRequest("message", "Fido") // this code must not *have* to change
println(request.dogName) // probably prints "" now

Note that this breaks even though the new field is optional -- even though that should be a compatible change. (You might suggest putting required fields first, but then changing a field from required to optional is also an incompatible change, etc etc.)

As recommended in the proto language guide, you should generally be writing runtime validation methods for your protos, not depending on required fields or having compile-time enforcement. We used to think up-front validation and required fields were a good idea, but then we took an arrow to the knee had many avoidable breakages; see e.g. here and here. Java, C++, and all the other officially supported languages have the same behavior here (and the same limitations).

[jamesward]

TIL that the field numbers don't have to be incremented by 1 (thus allowing re-ordering like illustrated here). Thanks @lowasser for the details on this.