From 564719d28e9d37a949062c0ffc2361d3612635e7 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Fri, 27 Mar 2015 13:07:34 -0700 Subject: [PATCH] Updates Channel#create_call to the new API --- src/ruby/ext/grpc/rb_channel.c | 35 +++++++++++++++++------ src/ruby/lib/grpc/generic/client_stub.rb | 6 ++-- src/ruby/spec/call_spec.rb | 15 +++++----- src/ruby/spec/channel_spec.rb | 29 +++++++------------ src/ruby/spec/generic/active_call_spec.rb | 2 +- 5 files changed, 48 insertions(+), 39 deletions(-) diff --git a/src/ruby/ext/grpc/rb_channel.c b/src/ruby/ext/grpc/rb_channel.c index 2a48f46ce2e..76a65fe6fe0 100644 --- a/src/ruby/ext/grpc/rb_channel.c +++ b/src/ruby/ext/grpc/rb_channel.c @@ -49,10 +49,16 @@ static ID id_channel; /* id_target is the name of the hidden ivar that preserves a reference to the - * target string used to create the call, preserved so that is does not get + * target string used to create the call, preserved so that it does not get * GCed before the channel */ static ID id_target; +/* id_cqueue is the name of the hidden ivar that preserves a reference to the + * completion queue used to create the call, preserved so that it does not get + * GCed before the channel */ +static ID id_cqueue; + + /* Used during the conversion of a hash to channel args during channel setup */ static VALUE rb_cChannelArgs; @@ -142,6 +148,7 @@ static VALUE grpc_rb_channel_init(int argc, VALUE *argv, VALUE self) { if (ch == NULL) { rb_raise(rb_eRuntimeError, "could not create an rpc channel to target:%s", target_chars); + return Qnil; } rb_ivar_set(self, id_target, target); wrapper->wrapped = ch; @@ -164,6 +171,7 @@ static VALUE grpc_rb_channel_init_copy(VALUE copy, VALUE orig) { if (TYPE(orig) != T_DATA || RDATA(orig)->dfree != (RUBY_DATA_FUNC)grpc_rb_channel_free) { rb_raise(rb_eTypeError, "not a %s", rb_obj_classname(rb_cChannel)); + return Qnil; } Data_Get_Struct(orig, grpc_rb_channel, orig_ch); @@ -177,34 +185,42 @@ static VALUE grpc_rb_channel_init_copy(VALUE copy, VALUE orig) { /* Create a call given a grpc_channel, in order to call method. The request is not sent until grpc_call_invoke is called. */ -static VALUE grpc_rb_channel_create_call(VALUE self, VALUE method, VALUE host, - VALUE deadline) { +static VALUE grpc_rb_channel_create_call(VALUE self, VALUE cqueue, VALUE method, + VALUE host, VALUE deadline) { VALUE res = Qnil; grpc_rb_channel *wrapper = NULL; - grpc_channel *ch = NULL; grpc_call *call = NULL; + grpc_channel *ch = NULL; + grpc_completion_queue *cq = NULL; char *method_chars = StringValueCStr(method); char *host_chars = StringValueCStr(host); + cq = grpc_rb_get_wrapped_completion_queue(cqueue); Data_Get_Struct(self, grpc_rb_channel, wrapper); ch = wrapper->wrapped; if (ch == NULL) { rb_raise(rb_eRuntimeError, "closed!"); + return Qnil; } call = - grpc_channel_create_call_old(ch, method_chars, host_chars, - grpc_rb_time_timeval(deadline, - /* absolute time */ 0)); + grpc_channel_create_call(ch, cq, method_chars, host_chars, + grpc_rb_time_timeval(deadline, + /* absolute time */ 0)); if (call == NULL) { rb_raise(rb_eRuntimeError, "cannot create call with method %s", method_chars); + return Qnil; } res = grpc_rb_wrap_call(call); - /* Make this channel an instance attribute of the call so that is is not GCed + /* Make this channel an instance attribute of the call so that it is not GCed * before the call. */ rb_ivar_set(res, id_channel, self); + + /* Make the completion queue an instance attribute of the call so that it is + * not GCed before the call. */ + rb_ivar_set(res, id_cqueue, cqueue); return res; } @@ -240,11 +256,12 @@ void Init_grpc_channel() { 1); /* Add ruby analogues of the Channel methods. */ - rb_define_method(rb_cChannel, "create_call", grpc_rb_channel_create_call, 3); + rb_define_method(rb_cChannel, "create_call", grpc_rb_channel_create_call, 4); rb_define_method(rb_cChannel, "destroy", grpc_rb_channel_destroy, 0); rb_define_alias(rb_cChannel, "close", "destroy"); id_channel = rb_intern("__channel"); + id_cqueue = rb_intern("__cqueue"); id_target = rb_intern("__target"); rb_define_const(rb_cChannel, "SSL_TARGET", ID2SYM(rb_intern(GRPC_SSL_TARGET_NAME_OVERRIDE_ARG))); diff --git a/src/ruby/lib/grpc/generic/client_stub.rb b/src/ruby/lib/grpc/generic/client_stub.rb index 01328d4a5bc..d15f5cca0fe 100644 --- a/src/ruby/lib/grpc/generic/client_stub.rb +++ b/src/ruby/lib/grpc/generic/client_stub.rb @@ -390,13 +390,13 @@ module GRPC # Creates a new active stub # - # @param ch [GRPC::Channel] the channel used to create the stub. + # @param method [string] the method being called. # @param marshal [Function] f(obj)->string that marshals requests # @param unmarshal [Function] f(string)->obj that unmarshals responses # @param deadline [TimeConst] - def new_active_call(ch, marshal, unmarshal, deadline = nil) + def new_active_call(method, marshal, unmarshal, deadline = nil) absolute_deadline = Core::TimeConsts.from_relative_time(deadline) - call = @ch.create_call(ch, @host, absolute_deadline) + call = @ch.create_call(@queue, method, @host, absolute_deadline) ActiveCall.new(call, @queue, marshal, unmarshal, absolute_deadline, started: false) end diff --git a/src/ruby/spec/call_spec.rb b/src/ruby/spec/call_spec.rb index 26175645719..781c735e750 100644 --- a/src/ruby/spec/call_spec.rb +++ b/src/ruby/spec/call_spec.rb @@ -67,16 +67,17 @@ describe GRPC::Core::RpcErrors do end describe GRPC::Core::Call do + let (:client_queue) { GRPC::Core::CompletionQueue.new } + let (:test_tag) { Object.new } + let (:fake_host) { 'localhost:10101' } + before(:each) do - @tag = Object.new - @client_queue = GRPC::Core::CompletionQueue.new - fake_host = 'localhost:10101' @ch = GRPC::Core::Channel.new(fake_host, nil) end describe '#start_read' do xit 'should fail if called immediately' do - blk = proc { make_test_call.start_read(@tag) } + blk = proc { make_test_call.start_read(test_tag) } expect(&blk).to raise_error GRPC::Core::CallError end end @@ -84,14 +85,14 @@ describe GRPC::Core::Call do describe '#start_write' do xit 'should fail if called immediately' do bytes = GRPC::Core::ByteBuffer.new('test string') - blk = proc { make_test_call.start_write(bytes, @tag) } + blk = proc { make_test_call.start_write(bytes, test_tag) } expect(&blk).to raise_error GRPC::Core::CallError end end describe '#start_write_status' do xit 'should fail if called immediately' do - blk = proc { make_test_call.start_write_status(153, 'x', @tag) } + blk = proc { make_test_call.start_write_status(153, 'x', test_tag) } expect(&blk).to raise_error GRPC::Core::CallError end end @@ -154,7 +155,7 @@ describe GRPC::Core::Call do end def make_test_call - @ch.create_call('dummy_method', 'dummy_host', deadline) + @ch.create_call(client_queue, 'dummy_method', 'dummy_host', deadline) end def deadline diff --git a/src/ruby/spec/channel_spec.rb b/src/ruby/spec/channel_spec.rb index af73294abe4..31e38d71b81 100644 --- a/src/ruby/spec/channel_spec.rb +++ b/src/ruby/spec/channel_spec.rb @@ -36,16 +36,13 @@ def load_test_certs end describe GRPC::Core::Channel do - FAKE_HOST = 'localhost:0' + let(:fake_host) { 'localhost:0' } + let(:cq) { GRPC::Core::CompletionQueue.new } def create_test_cert GRPC::Core::Credentials.new(load_test_certs[0]) end - before(:each) do - @cq = GRPC::Core::CompletionQueue.new - end - shared_examples '#new' do it 'take a host name without channel args' do expect { GRPC::Core::Channel.new('dummy_host', nil) }.not_to raise_error @@ -115,25 +112,23 @@ describe GRPC::Core::Channel do describe '#create_call' do it 'creates a call OK' do - host = FAKE_HOST - ch = GRPC::Core::Channel.new(host, nil) + ch = GRPC::Core::Channel.new(fake_host, nil) deadline = Time.now + 5 blk = proc do - ch.create_call('dummy_method', 'dummy_host', deadline) + ch.create_call(cq, 'dummy_method', 'dummy_host', deadline) end expect(&blk).to_not raise_error end it 'raises an error if called on a closed channel' do - host = FAKE_HOST - ch = GRPC::Core::Channel.new(host, nil) + ch = GRPC::Core::Channel.new(fake_host, nil) ch.close deadline = Time.now + 5 blk = proc do - ch.create_call('dummy_method', 'dummy_host', deadline) + ch.create_call(cq, 'dummy_method', 'dummy_host', deadline) end expect(&blk).to raise_error(RuntimeError) end @@ -141,15 +136,13 @@ describe GRPC::Core::Channel do describe '#destroy' do it 'destroys a channel ok' do - host = FAKE_HOST - ch = GRPC::Core::Channel.new(host, nil) + ch = GRPC::Core::Channel.new(fake_host, nil) blk = proc { ch.destroy } expect(&blk).to_not raise_error end it 'can be called more than once without error' do - host = FAKE_HOST - ch = GRPC::Core::Channel.new(host, nil) + ch = GRPC::Core::Channel.new(fake_host, nil) blk = proc { ch.destroy } blk.call expect(&blk).to_not raise_error @@ -164,15 +157,13 @@ describe GRPC::Core::Channel do describe '#close' do it 'closes a channel ok' do - host = FAKE_HOST - ch = GRPC::Core::Channel.new(host, nil) + ch = GRPC::Core::Channel.new(fake_host, nil) blk = proc { ch.close } expect(&blk).to_not raise_error end it 'can be called more than once without error' do - host = FAKE_HOST - ch = GRPC::Core::Channel.new(host, nil) + ch = GRPC::Core::Channel.new(fake_host, nil) blk = proc { ch.close } blk.call expect(&blk).to_not raise_error diff --git a/src/ruby/spec/generic/active_call_spec.rb b/src/ruby/spec/generic/active_call_spec.rb index 96e07cacb44..1306336941c 100644 --- a/src/ruby/spec/generic/active_call_spec.rb +++ b/src/ruby/spec/generic/active_call_spec.rb @@ -364,7 +364,7 @@ describe GRPC::ActiveCall do end def make_test_call - @ch.create_call('dummy_method', 'dummy_host', deadline) + @ch.create_call(@client_queue, 'dummy_method', 'dummy_host', deadline) end def deadline