gRPC: Inconsistency in generated method name

[djnalluri]

Describe the bug
The generated method names in quarkus-grpc-protoc-plugin do not match those generated by grpc-java for all valid input. This creates compile errors when the generated Mutiny code tries to delegate to grpc-java's implementation.

Expected behavior
A gRPC method named 'THIS_FAILS' should generate a Java method named 'tHISFAILS'.

Actual behavior
quarkus-grpc-protoc-plugin generates 'tHIS_FAILS' and attempts to delegate to the same name in grpc-java which does not exist.

To Reproduce
Steps to reproduce the behavior:

1. Follow the Getting Started with gRPC guide here: https://quarkus.io/guides/grpc-getting-started
2. Replace helloworld.proto with the following:

syntax = "proto3";

option java_multiple_files = true;
option java_package = "io.quarkus.example";
option java_outer_classname = "HelloWorldProto";

package helloworld;

// The greeting service definition.
service Greeter {
    // Sends a greeting
    rpc SayHello (HelloRequest) returns (HelloReply) {}
    rpc THIS_FAILS (HelloRequest) returns (HelloReply) {}
}

// The request message containing the user's name.
message HelloRequest {
    string name = 1;
}

// The response message containing the greetings
message HelloReply {
    string message = 1;
}

3. Attempt compilation.

Environment:

• Output of uname -a or ver: Linux <hostname> 5.3.0-59-generic #53~18.04.1-Ubuntu SMP Thu Jun 4 14:58:26 UTC 2020 x86_64 x86_64 x86_64 GNU/Linux
• Output of java -version: OpenJDK Runtime Environment GraalVM CE 20.0.0 (build 11.0.6+9-jvmci-20.0-b02)
• Quarkus version or git rev: 1.5.1.Final
• Build tool (ie. output of mvnw --version or gradlew --version):

Apache Maven 3.6.3 (cecedd343002696d0abb50b32b541b8a6ba2883f)
Maven home: /home/<user>/.sdkman/candidates/maven/current
Java version: 11.0.6, vendor: Oracle Corporation, runtime: /home/<user>/.sdkman/candidates/java/20.0.0.r11-grl
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "5.3.0-59-generic", arch: "amd64", family: "unix"

Additional context
grpc-java's algorithm for converting gRPC method name to Java method name can be found here: https://github.com/grpc/grpc-java/blob/40b815058f1209e66086f4426c407875f47fe400/compiler/src/java_plugin/cpp/java_generator.cpp#L115

The location where the method name is being assigned in Quarkus is here:

quarkus/extensions/grpc/protoc/src/main/java/io/quarkus/grpc/protoc/plugin/MutinyGrpcGenerator.java

Line 114 in 64c18ea

methodContext.methodName = lowerCaseFirst(methodProto.getName());

[gsmet]

/cc @michalszynkiewicz @cescoffier

[cescoffier]

The template has been provided by @pmlopes - Paulo, are you aware of this?

[gsmet]

@pmlopes @michalszynkiewicz @cescoffier looks like something we would like for 1.6, no?

[cescoffier]

@gsmet not sure, it's a bit tricky. gRPC convention and java convention are not exactly the same. We have followed the java convention.

[mkouba]

For the record, I've just tried to reproduce the problem in the main branch and got several compilation errors:

[ERROR] /opt/source/protean/quarkus-quickstarts/grpc-plain-text-quickstart/target/generated-sources/grpc/examples/GreeterGrpc.java:[281,28] variable METHODID_SAY_HELLO is already defined in class examples.GreeterGrpc
[ERROR] /opt/source/protean/quarkus-quickstarts/grpc-plain-text-quickstart/target/generated-sources/grpc/examples/GreeterGrpc.java:[304,9] duplicate case label
[ERROR] /opt/source/protean/quarkus-quickstarts/grpc-plain-text-quickstart/target/generated-sources/grpc/examples/MutinyGreeterGrpc.java:[53,74] invalid method reference
  cannot find symbol
    symbol:   method sAY_HELLO()
    location: class examples.GreeterGrpc.GreeterStub

The first two come from the examples.GreeterGrpc which is generated by grpc-java. In other words, even if we change the method name the reproducer app would not compile.

[djnalluri]

The extra compiler errors you're seeing appear to be due to added static final fields since the issue was filed. The two methods in the reproducer are so similarly named that these generated fields overlap. Removing or renaming them to not be so close will leave only the errors described in this issue. I have updated the reproducer to remove the overlap.

[cescoffier]

Unfortunately, there is no much we can do without breaking the assumption made by other Java gRPC libraries (following the same convention)

[djnalluri]

What other gRPC libraries is the project trying to align with? It's been a while since I dove into this code. I'm assuming Vert.x gRPC at a minimum but any others?

[cescoffier]

It seems that both Vert.x and Micronaut uses that convention.
Also, updating now would be a big breaking change for all the code generated with the current convention.

We may be able to fix it, but need to use a flag to enable this new mapping. Wondering if it worth it (it's going to make the code a lot more complicated).

[djnalluri]

I should mention that this may no longer be blocking my use case so I'm fine with closing this issue as part of triage.

[cescoffier]

ah ah ah!

@mkouba fixed this issue already (#27183)

Just adding a few tests to verify!

[cescoffier]

Here are the tests: #29339