From 18ef450a7ce48de20541fc689b9d9f17f8e05045 Mon Sep 17 00:00:00 2001 From: Stanley Cheung Date: Thu, 16 Jul 2020 17:25:28 -0700 Subject: [PATCH] Fix ruby protoc plugin when message is in another package --- src/compiler/ruby_generator.cc | 14 +++++------ src/compiler/ruby_generator_string-inl.h | 9 ++++---- .../testing/package_options_import2.proto | 23 +++++++++++++++++++ .../testing/package_options_ruby_style.proto | 2 ++ .../spec/pb/codegen/package_option_spec.rb | 6 ++++- 5 files changed, 40 insertions(+), 14 deletions(-) create mode 100644 src/ruby/spec/pb/codegen/grpc/testing/package_options_import2.proto diff --git a/src/compiler/ruby_generator.cc b/src/compiler/ruby_generator.cc index 2a71aae32bb..3604df53635 100644 --- a/src/compiler/ruby_generator.cc +++ b/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, - Printer* out) { - grpc::string input_type = RubyTypeOf(method->input_type(), package); +void PrintMethod(const MethodDescriptor* method, Printer* out) { + grpc::string input_type = RubyTypeOf(method->input_type()); 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, - Printer* out) { +void PrintService(const ServiceDescriptor* service, 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(); diff --git a/src/compiler/ruby_generator_string-inl.h b/src/compiler/ruby_generator_string-inl.h index 36f659d9973..945f0343e7d 100644 --- a/src/compiler/ruby_generator_string-inl.h +++ b/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, - const grpc::string& package) { +inline grpc::string RubyTypeOf(const grpc::protobuf::Descriptor* descriptor) { 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); diff --git a/src/ruby/spec/pb/codegen/grpc/testing/package_options_import2.proto b/src/ruby/spec/pb/codegen/grpc/testing/package_options_import2.proto new file mode 100644 index 00000000000..c95f5d2e5f6 --- /dev/null +++ b/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 { } +} diff --git a/src/ruby/spec/pb/codegen/grpc/testing/package_options_ruby_style.proto b/src/ruby/spec/pb/codegen/grpc/testing/package_options_ruby_style.proto index 40400b67d2f..98ed1be3bd7 100644 --- a/src/ruby/spec/pb/codegen/grpc/testing/package_options_ruby_style.proto +++ b/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) { } } diff --git a/src/ruby/spec/pb/codegen/package_option_spec.rb b/src/ruby/spec/pb/codegen/package_option_spec.rb index cc6649c5afa..a4668ace520 100644 --- a/src/ruby/spec/pb/codegen/package_option_spec.rb +++ b/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)