Merge pull request #23519 from stanley-cheung/ruby-protoc-plugin-fix-backport

Backport #23501: Fix ruby protoc plugin when message is in another package

src/compiler/ruby_generator.cc

@@ -38,13 +38,12 @@ namespace grpc_ruby_generator {
 namespace {
 
 // Prints out the method using the ruby gRPC DSL.
-void PrintMethod(const MethodDescriptor* method, const grpc::string& package,
+void PrintMethod(const MethodDescriptor* method, Printer* out) {
-                 Printer* out) {
+  grpc::string input_type = RubyTypeOf(method->input_type());
-  grpc::string input_type = RubyTypeOf(method->input_type(), package);
   if (method->client_streaming()) {
     input_type = "stream(" + input_type + ")";
   }
-  grpc::string output_type = RubyTypeOf(method->output_type(), package);
+  grpc::string output_type = RubyTypeOf(method->output_type());
   if (method->server_streaming()) {
     output_type = "stream(" + output_type + ")";
   }
@@ -62,8 +61,7 @@ void PrintMethod(const MethodDescriptor* method, const grpc::string& package,
 }
 
 // Prints out the service using the ruby gRPC DSL.
-void PrintService(const ServiceDescriptor* service, const grpc::string& package,
+void PrintService(const ServiceDescriptor* service, Printer* out) {
-                  Printer* out) {
   if (service->method_count() == 0) {
     return;
   }
@@ -91,7 +89,7 @@ void PrintService(const ServiceDescriptor* service, const grpc::string& package,
   out->Print(pkg_vars, "self.service_name = '$service_full_name$'\n");
   out->Print("\n");
   for (int i = 0; i < service->method_count(); ++i) {
-    PrintMethod(service->method(i), package, out);
+    PrintMethod(service->method(i), out);
   }
   out->Outdent();
 
@@ -201,7 +199,7 @@ grpc::string GetServices(const FileDescriptor* file) {
     }
     for (int i = 0; i < file->service_count(); ++i) {
       auto service = file->service(i);
-      PrintService(service, file->package(), &out);
+      PrintService(service, &out);
     }
     for (size_t i = 0; i < modules.size(); ++i) {
       out.Outdent();

src/compiler/ruby_generator_string-inl.h

@@ -116,13 +116,12 @@ inline grpc::string RubyPackage(const grpc::protobuf::FileDescriptor* file) {
 }
 
 // RubyTypeOf updates a proto type to the required ruby equivalent.
-inline grpc::string RubyTypeOf(const grpc::protobuf::Descriptor* descriptor,
+inline grpc::string RubyTypeOf(const grpc::protobuf::Descriptor* descriptor) {
-                               const grpc::string& package) {
   std::string proto_type = descriptor->full_name();
-  ReplacePrefix(&proto_type, package,
-                "");                    // remove the leading package if present
-  ReplacePrefix(&proto_type, ".", "");  // remove the leading . (no package)
   if (descriptor->file()->options().has_ruby_package()) {
+    // remove the leading package if present
+    ReplacePrefix(&proto_type, descriptor->file()->package(), "");
+    ReplacePrefix(&proto_type, ".", "");  // remove the leading . (no package)
     proto_type = RubyPackage(descriptor->file()) + "." + proto_type;
   }
   grpc::string res(proto_type);

src/ruby/spec/pb/codegen/grpc/testing/package_options_import2.proto

@@ -0,0 +1,23 @@
+// Copyright 2020 gRPC authors.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+syntax = "proto3";
+
+package grpc.foo;
+
+option ruby_package = "B::Other";
+
+message Foo {
+  message Bar { }
+}

src/ruby/spec/pb/codegen/grpc/testing/package_options_ruby_style.proto

@@ -17,6 +17,7 @@ syntax = "proto3";
 package grpc.testing;
 
 import "grpc/testing/package_options_import.proto";
+import "grpc/testing/package_options_import2.proto";
 
 // For sanity checking package definitions
 option ruby_package = "RPC::Test::New::Package::Options";
@@ -34,6 +35,7 @@ message Bar {
 service AnotherTestService {
   rpc GetTest(AnotherTestRequest) returns (AnotherTestResponse) { }
   rpc OtherTest(Thing) returns (Thing) { }
+  rpc PackageTest(grpc.testing.Thing) returns (grpc.foo.Foo.Bar) { }
   rpc FooTest(Foo) returns (Foo) { }
   rpc NestedMessageTest(Foo) returns (Bar.Baz) { }
 }

src/ruby/spec/pb/codegen/package_option_spec.rb

@@ -27,7 +27,9 @@ describe 'Code Generation Options' do
   end
 
   it 'should generate and respect Ruby style package options' do
-    with_protos(%w[grpc/testing/package_options_ruby_style.proto grpc/testing/package_options_import.proto]) do
+    with_protos(['grpc/testing/package_options_ruby_style.proto',
+                 'grpc/testing/package_options_import.proto',
+                 'grpc/testing/package_options_import2.proto']) do
       expect { RPC::Test::New::Package::Options::AnotherTestService::Service }.to raise_error(NameError)
       expect(require('grpc/testing/package_options_ruby_style_services_pb')).to be_truthy
       expect { RPC::Test::New::Package::Options::AnotherTestService::Service }.to_not raise_error
@@ -38,6 +40,8 @@ describe 'Code Generation Options' do
       expect(services[:GetTest].output).to eq(RPC::Test::New::Package::Options::AnotherTestResponse)
       expect(services[:OtherTest].input).to eq(A::Other::Thing)
       expect(services[:OtherTest].output).to eq(A::Other::Thing)
+      expect(services[:PackageTest].input).to eq(A::Other::Thing)
+      expect(services[:PackageTest].output).to eq(B::Other::Foo::Bar)
       expect(services[:FooTest].input).to eq(RPC::Test::New::Package::Options::Foo)
       expect(services[:FooTest].output).to eq(RPC::Test::New::Package::Options::Foo)
       expect(services[:NestedMessageTest].input).to eq(RPC::Test::New::Package::Options::Foo)