Implement service & method descriptor lookup in Ruby

[KJTsanaktsidis]

This PR implements lookup of service descriptor and method descriptor objects in Ruby as described in issue #14891.

It contains three implementations - one for the CRuby extension API, one for JRuby, and one for FFI.

With this patch,

• DescriptorPool#lookup('fully.qualified.service.name') works and returns a Google::Protobuf::ServiceDescriptor object
• You can call #options on that to get the service options
• You can call #methods on that to get the services' methods as Google::Protobuf::MethodDescriptor objects,
• You can call MethodDescriptor#options to get method options
• You can also get the streaming flags & input/output types of the method with #input_type, #output_type, #client_streaming, and #server_streaming.

In order to make the FFI implementation work, I had to mark some more methods in the UPB header as exported - I guess that's something which will have to be done on the UPB side, like this protocolbuffers/upb@01fed1c

CC @dazuma & @haberman from the original issue, and @JasonLunn (since you work on protobuf it seems - small world!)

I apologies for the large volume of copy-pasta'd code from the existing descriptor class implementations into the new ones - I felt this was probably better than designing new abstractions to reduce it off the bat though; this feels like it "fits in" with the existing implementation.

[KJTsanaktsidis]

So the actual utility of this for the original use-case in #14891 (getting RPC options) might be somewhat dubious without support for being able to get message extensions (without resorting to #to_json). I guess that can be tackled in a subsequent PR

[haberman]

You can get extensions now, though the API is slightly awkward:

protobuf/ruby/tests/basic_proto2.rb

Lines 260 to 351 in eb70b34

	def test_extension
	message = TestExtensions.new
	extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.optional_int32_extension'
	assert_instance_of Google::Protobuf::FieldDescriptor, extension
	assert_equal 0, extension.get(message)
	extension.set message, 42
	assert_equal 42, extension.get(message)
	end
	def test_extension_json
	omit "Java Protobuf JsonFormat does not handle Proto2 extensions" if defined? JRUBY_VERSION and :NATIVE == Google::Protobuf::IMPLEMENTATION
	message = TestExtensions.decode_json '{"[basic_test_proto2.optional_int32_extension]": 123}'
	extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.optional_int32_extension'
	assert_instance_of Google::Protobuf::FieldDescriptor, extension
	assert_equal 123, extension.get(message)
	end
	def test_extension_json_separate_pool
	omit "Java Protobuf JsonFormat does not handle Proto2 extensions" if defined? JRUBY_VERSION and :NATIVE == Google::Protobuf::IMPLEMENTATION
	pool = Google::Protobuf::DescriptorPool.new
	# This serialized descriptor is a subset of basic_test_proto2.proto:
	#
	# syntax = "proto2";
	# package basic_test_proto2;
	#
	# message TestExtensions {
	# extensions 1 to max;
	# }
	#
	# extend TestExtensions {
	# # Same extension as basic_test_proto2.proto, but with a different
	# # name.
	# optional int32 different_optional_int32_extension = 1;
	# }
	#
	descriptor_data = "\n\x17\x62\x61sic_test_proto2.proto\x12\x11\x62\x61sic_test_proto2\"\x1a\n\x0eTestExtensions*\x08\x08\x01\x10\x80\x80\x80\x80\x02:M\n\"different_optional_int32_extension\x12!.basic_test_proto2.TestExtensions\x18\x01 \x01(\x05"
	pool.add_serialized_file(descriptor_data)
	message_class = pool.lookup("basic_test_proto2.TestExtensions").msgclass
	extension = pool.lookup 'basic_test_proto2.different_optional_int32_extension'
	message = message_class.decode_json '{"[basic_test_proto2.different_optional_int32_extension]": 123}'
	assert_equal 123, extension.get(message)
	message2 = message_class.decode_json(message_class.encode_json(message))
	assert_equal 123, extension.get(message2)
	end
	def test_nested_extension
	message = TestExtensions.new
	extension = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.TestNestedExtension.test'
	assert_instance_of Google::Protobuf::FieldDescriptor, extension
	assert_equal 'test', extension.get(message)
	extension.set message, 'another test'
	assert_equal 'another test', extension.get(message)
	end
	def test_message_set_extension_json_roundtrip
	omit "Java Protobuf JsonFormat does not handle Proto2 extensions" if defined? JRUBY_VERSION and :NATIVE == Google::Protobuf::IMPLEMENTATION
	message = TestMessageSet.new
	ext1 = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.TestMessageSetExtension1.message_set_extension'
	assert_instance_of Google::Protobuf::FieldDescriptor, ext1
	ext2 = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.TestMessageSetExtension2.message_set_extension'
	assert_instance_of Google::Protobuf::FieldDescriptor, ext2
	ext3 = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.message_set_extension3'
	assert_instance_of Google::Protobuf::FieldDescriptor, ext3
	ext1.set(message, ext1.subtype.msgclass.new(i: 42))
	ext2.set(message, ext2.subtype.msgclass.new(str: 'foo'))
	ext3.set(message, ext3.subtype.msgclass.new(text: 'bar'))
	message_text = message.to_json
	parsed_message = TestMessageSet.decode_json message_text
	assert_equal message, parsed_message
	end
	def test_message_set_extension_roundtrip
	message = TestMessageSet.new
	ext1 = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.TestMessageSetExtension1.message_set_extension'
	assert_instance_of Google::Protobuf::FieldDescriptor, ext1
	ext2 = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.TestMessageSetExtension2.message_set_extension'
	assert_instance_of Google::Protobuf::FieldDescriptor, ext2
	ext3 = Google::Protobuf::DescriptorPool.generated_pool.lookup 'basic_test_proto2.message_set_extension3'
	assert_instance_of Google::Protobuf::FieldDescriptor, ext3
	ext1.set(message, ext1.subtype.msgclass.new(i: 42))
	ext2.set(message, ext2.subtype.msgclass.new(str: 'foo'))
	ext3.set(message, ext3.subtype.msgclass.new(text: 'bar'))
	encoded_message = TestMessageSet.encode message
	decoded_message = TestMessageSet.decode encoded_message
	assert_equal message, decoded_message
	end
	end
	end

[haberman]

This sounds great; I haven't taken a look at the code yet, but from the description this sounds like just the right approach.

that's something which will have to be done on the UPB side

The upb code is now in this repo, and the old repo is defunct.

We should probably make the banner at the top of the upb readme bigger and more emphatic about this point.

[JasonLunn]

The FFI tests are currently failing with errors like:

Caught exception Function 'upb_MethodDef_Service' not found in [/usr/local/rvm/gems/ruby-2.7.0/gems/google-protobuf-4.27.0/ext/google/protobuf_c/x86_64-linux/libprotobuf_c_ffi.so] while loading FFI implementation of google/protobuf. Falling back to native implementation.

Similar changes as you've already made in ruby-upb.h to apply the UPB_API macro may also needed in ruby-upb.c.

[haberman]

Similar changes as you've already made in ruby-upb.h to apply the UPB_API macro may also needed in ruby-upb.c.

I think they should be made in the upb tree instead. Those changes should then automatically propagate to ruby-upb.c.

[KJTsanaktsidis]

I do want to point out that this shadows Object#methods, which is a bit icky. Perhaps we should rename this #rpc_methods or #service_methods or #method_descriptors or some such?

[KJTsanaktsidis]

I think they should be made in the upb tree instead. Those changes should then automatically propagate to ruby-upb.c.

How does this happen? I made the changes in the upb/ tree and regenerated the almagamation with bazel build //upb:gen_ruby_amalgamation; however, that just updates the amalgamation in the bazel-bin/ build directory:

% bazel build //upb:gen_ruby_amalgamation
INFO: Analyzed target //upb:gen_ruby_amalgamation (0 packages loaded, 0 targets configured).
INFO: Found 1 target...
Target //upb:gen_ruby_amalgamation up-to-date:
  bazel-bin/upb/ruby-upb.c
  bazel-bin/upb/ruby-upb.h
INFO: Elapsed time: 0.288s, Critical Path: 0.01s
INFO: 1 process: 1 internal.
INFO: Build completed successfully, 1 total action

For now I regenerated the amalgamation, then copied it into ruby/ext and checked it in to this PR, but if there's some automatic process which is supposed to address that I can drop c93913c

[haberman]

but if there's some automatic process which is supposed to address that I can drop c93913c

Yes, there is an automated process that will regenerate the files; for example see: a3c33a8

[haberman]

It appears that none of our other descriptor types have methods that return an enumerator. For consistency, I'd prefer to start with just each or each_method. If we want to add methods returning enumerators later, let's do it consistently across all the descriptor types.

[KJTsanaktsidis]

If we want to add methods returning enumerators later, let's do it consistently across all the descriptor types.

So some of our other descriptor types (including plain message Descriptor) are enumerables though (they include the Enumerable mixin), which is more or less the same thing. So you can do

descriptor.each { |field| puts "have field #{field}" }

but also things like

descriptor.to_a
# or
descriptor.select { |field| field.type == 'int32' }

I didn't implement ServiceDescriptor#each, however, because I don't think a service is-a collection of RPC methods in the same way that a message type is-a collection of fields. So instead, I implemented #each_method, and thus implemented #methods to return an enumerable in the same way that self is an enumerator for a message descriptor.

If you like I could just implement #each_method as ServiecDescriptor#each instead, if you think service descriptor is-a list of RPC methods. (as a bonus that would solve the #methods method shadowing Object#methods too, since we wouldn't need that since self is the enumerable then.

[haberman]

I see your point, that "service is-a collection of methods" is a bit of a stretch. But methods are the only thing you can list inside a service {} block in a .proto file, other than options, which have their own accessor. The fact that this conveniently resolves the #methods shadowing is another argument in its favor.

[KJTsanaktsidis]

alright 👍 the current version of my PR just implements #each/Enumerable directly on ServiceDescriptor. Thanks for talking through that with me.

[haberman]

Agreed, shadowing an existing method would be better to avoid if possible.

[KJTsanaktsidis]

I'll rename #each_method to #each and include Enumerable then, which is how the other descriptors list their children. Then I don't need this method at all.

[haberman]

We'll want to update these in

protobuf/upb/reflection/def_pool.h

Line 69 in 3ab1276

const upb_ServiceDef* upb_DefPool_FindServiceByName(const upb_DefPool* s,

, which should automatically update this copy.

[KJTsanaktsidis]

I did that as well; i'll drop the commit which updates these vendored copies.

[KJTsanaktsidis]

@haberman OK, I dropped the each_method/to_enum stuff in favour of just implementing #each and including Enumerable like the other descriptor types with children. WDYT?

[KJTsanaktsidis]

@haberman anything i can do to move this along?

[dorner]

This is exactly what I need :) I'm looking at generating Rails routes from proto options mirroring what grpc-gateway does (so basically allowing us to have two different processes, one running grpc, one running REST, both funneling to the same handler). I'm trying to write a protoc plugin in Ruby (I have one working in Go, but it means that whoever uses it would need a cumbersome "download binary and put in your path" workflow - I'd rather do it in pure Ruby). Right now this is impossible because a) I can't get the original method protos since I can't call .methodson the service descriptor, and b) I can't access the options I need even if I could.

[haberman]

Sorry for the delay. I think there is still an unresolved comment from me in the code. In the tests you have a comment saying that there is no API for getting an extension, but that isn't true -- see the other tests that get and set extensions. There's no generated code for it yet, but you can lookup an extension in the descriptor pool and then call extension.get(msg) and extension.set(msg).

[KJTsanaktsidis]

Oh, apologies, I didn't see that. Thanks for the tip about how to use extensions from Ruby. It should be good to go now.