wrong code generated for nested message types

[pradeepk]

I am trying to generate akka-grpc code for a proto file with nested message types:

syntax = "proto3";

option java_package = "ai.grakn.rpc.proto";
option java_outer_classname = "KeyspaceProto";

package keyspace;

service KeyspaceService {
    rpc create (Keyspace.Create.Req) returns (Keyspace.Create.Res);
    rpc retrieve (Keyspace.Retrieve.Req) returns (Keyspace.Retrieve.Res);
    rpc delete (Keyspace.Delete.Req) returns (Keyspace.Delete.Res);
}

message Keyspace {
    message Retrieve {
        message Req {}
        message Res {
            repeated string names = 1;
        }
    }

    message Create {
        message Req {
            string name = 1;
        }
        message Res {}
    }

    message Delete {
        message Req {
            string name = 1;
        }
        message Res {}
    }
}

And what gets generated is this:

// Generated by Akka gRPC. DO NOT EDIT.
package ai.grakn.rpc.proto;

import java.util.concurrent.CompletionStage;

import akka.NotUsed;
import akka.stream.javadsl.Source;

import akka.grpc.ProtobufSerializer;
import akka.grpc.javadsl.GoogleProtobufSerializer;

public interface KeyspaceService {

  CompletionStage<ai.grakn.rpc.proto.KeyspaceProto.Res> create(ai.grakn.rpc.proto.KeyspaceProto.Req in);

  CompletionStage<ai.grakn.rpc.proto.KeyspaceProto.Res> retrieve(ai.grakn.rpc.proto.KeyspaceProto.Req in);

  CompletionStage<ai.grakn.rpc.proto.KeyspaceProto.Res> delete(ai.grakn.rpc.proto.KeyspaceProto.Req in);

  static String name = "keyspace.KeyspaceService";

  public static class Serializers {

      public static ProtobufSerializer<ai.grakn.rpc.proto.KeyspaceProto.Req> ReqSerializer = new GoogleProtobufSerializer<>(ai.grakn.rpc.proto.KeyspaceProto.Req.class);

      public static ProtobufSerializer<ai.grakn.rpc.proto.KeyspaceProto.Res> ResSerializer = new GoogleProtobufSerializer<>(ai.grakn.rpc.proto.KeyspaceProto.Res.class);

  }
}

2 issues:

1. The fully qualified type names should be ai.grakn.rpc.proto.KeyspaceProto.Keyspace.Create.Req etc.
2. There should be 6 serializers defined - not 2

[mcanlas]

Not sure the exact cause but you are probably getting clashes since the name parts Req and Res are used multiple times (despite being nested).

Have you tried giving them different names and seeing what happens?

[pradeepk]

If you flatten, I am sure it will work (I haven't tried it). With the nested types, t.getContainingType needs to be recursively checked to get the nesting right. So for example the following patch worked for me:

diff --git a/codegen/src/main/scala/akka/grpc/gen/javadsl/Method.scala b/codegen/src/main/scala/akka/grpc/gen/javadsl/Method.scala
index 172536a..fd2105a 100644
--- a/codegen/src/main/scala/akka/grpc/gen/javadsl/Method.scala
+++ b/codegen/src/main/scala/akka/grpc/gen/javadsl/Method.scala
@@ -62,12 +62,20 @@ object Method {
   private def methodName(name: String) =
     name.head.toLower +: name.tail

-  def messageType(t: Descriptor) =
-    "_root_." + t.getFile.getOptions.getJavaPackage + "." + protoName(t) + "." + t.getName
-
   /** Java API */
   def getMessageType(t: Descriptor) = {
-    t.getFile.getOptions.getJavaPackage + "." + outerClass(t) + t.getName
+    t.getFile.getOptions.getJavaPackage + "." + outerClass(t) + nestedName(t)
+  }
+
+  def getMessageTypeName(t: Descriptor) = {
+    nestedName(t).replaceFirst("\\.", "_")
+  }
+
+  def nestedName(t: Descriptor): String = {
+    if (t.getContainingType == null)
+      t.getName
+    else
+      nestedName(t.getContainingType) + "." + t.getName
   }

   private def outerClass(t: Descriptor) = {
diff --git a/codegen/src/main/scala/akka/grpc/gen/javadsl/Serializer.scala b/codegen/src/main/scala/akka/grpc/gen/javadsl/Serializer.scala
index e69ec43..9606ae9 100644
--- a/codegen/src/main/scala/akka/grpc/gen/javadsl/Serializer.scala
+++ b/codegen/src/main/scala/akka/grpc/gen/javadsl/Serializer.scala
@@ -10,7 +10,7 @@ final case class Serializer(name: String, init: String, messageType: String)

 object Serializer {
   def apply(messageType: Descriptor): Serializer = Serializer(
-    messageType.getName + "Serializer",
+    Method.getMessageTypeName(messageType) + "Serializer",
     s"new GoogleProtobufSerializer<>(${Method.getMessageType(messageType)}.class)",
     Method.getMessageType(messageType))
 }

[thyandrecardoso]

Hi,

Sorry for resuscitating this issue, but the title matches my problem although it might not be exactly the same thing... I am having an issue where the names of the Serializers get duplicated for a proto definition where I want to define nested messages and the innermost name is repeated.

For example, consider the following:

service Example {
    rpc exampleRpc1 (Namespace1.SomeRequest) returns (Namespace1.SomeReply) {}
    rpc exampleRpc2 (Namespace2.SomeRequest) returns (Namespace2.SomeReply) {}
}

message Namespace1 {
    message SomeRequest {
        string name = 1;
    }
    message SomeReply {
        string message = 1;
    }
}

message Namespace2 {
    message SomeRequest {
        string name = 1;
    }
    message SomeReply {
        string message = 1;
    }
}

Compiling this would result in errors like:

SomeRequestSerializer is already defined as value SomeRequestSerializer

Because the resulting scala service description would be something like:

object Example extends akka.grpc.ServiceDescription {
  val name = "Example"

  val descriptor: com.google.protobuf.Descriptors.FileDescriptor =
    com.example.helloworld.HelloworldProto.javaDescriptor;

  object Serializers {
    import akka.grpc.scaladsl.ScalapbProtobufSerializer

    val SomeRequestSerializer = new ScalapbProtobufSerializer(com.example.helloworld.Namespace1.SomeRequest.messageCompanion)

    val SomeRequestSerializer = new ScalapbProtobufSerializer(com.example.helloworld.Namespace2.SomeRequest.messageCompanion)

    val SomeReplySerializer = new ScalapbProtobufSerializer(com.example.helloworld.Namespace1.SomeReply.messageCompanion)

    val SomeReplySerializer = new ScalapbProtobufSerializer(com.example.helloworld.Namespace2.SomeReply.messageCompanion)

  }
}

[alexmtrmd]

same here. member names are generated without any regard for package name