Updates Channel#create_call to the new API

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 2a48f46ce2..76a65fe6fe 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 01328d4a5b..d15f5cca0f 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 2617564571..781c735e75 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 af73294abe..31e38d71b8 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 96e07cacb4..1306336941 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