Merge pull request #3 from tbetbetbe/grpc-ruby-update-server-api

Grpc ruby update server api

8 files changed, 33 insertions, 78 deletions

diff --git a/src/ruby/bin/interop/interop_server.rb b/src/ruby/bin/interop/interop_server.rb
index b3b7d0c5a3..0819ba9bbc 100755
--- a/src/ruby/bin/interop/interop_server.rb
+++ b/src/ruby/bin/interop/interop_server.rb
@@ -176,12 +176,11 @@ end
 def main
   opts = parse_options
   host = "0.0.0.0:#{opts['port']}"
+  s = GRPC::RpcServer.new
   if opts['secure']
-    s = GRPC::RpcServer.new(creds: test_server_creds)
-    s.add_http2_port(host, true)
+    s.add_http2_port(host, test_server_creds)
     logger.info("... running securely on #{host}")
   else
-    s = GRPC::RpcServer.new
     s.add_http2_port(host)
     logger.info("... running insecurely on #{host}")
   end
diff --git a/src/ruby/bin/math_server.rb b/src/ruby/bin/math_server.rb
index 93277e3932..5cc7613489 100755
--- a/src/ruby/bin/math_server.rb
+++ b/src/ruby/bin/math_server.rb
@@ -173,12 +173,11 @@ def main
     end
   end.parse!
 
+  s = GRPC::RpcServer.new
   if options['secure']
-    s = GRPC::RpcServer.new(creds: test_server_creds)
-    s.add_http2_port(options['host'], true)
+    s.add_http2_port(options['host'], test_server_creds)
     logger.info("... running securely on #{options['host']}")
   else
-    s = GRPC::RpcServer.new
     s.add_http2_port(options['host'])
     logger.info("... running insecurely on #{options['host']}")
   end
diff --git a/src/ruby/bin/noproto_server.rb b/src/ruby/bin/noproto_server.rb
index 435f8f4ebf..9979cb7ebb 100755
--- a/src/ruby/bin/noproto_server.rb
+++ b/src/ruby/bin/noproto_server.rb
@@ -95,12 +95,11 @@ def main
     end
   end.parse!
 
+  s = GRPC::RpcServer.new
   if options['secure']
-    s = GRPC::RpcServer.new(creds: test_server_creds)
-    s.add_http2_port(options['host'], true)
+    s.add_http2_port(options['host'], test_server_creds)
     logger.info("... running securely on #{options['host']}")
   else
-    s = GRPC::RpcServer.new
     s.add_http2_port(options['host'])
     logger.info("... running insecurely on #{options['host']}")
   end
diff --git a/src/ruby/ext/grpc/rb_server.c b/src/ruby/ext/grpc/rb_server.c
index 5954e27d02..c54f02e87a 100644
--- a/src/ruby/ext/grpc/rb_server.c
+++ b/src/ruby/ext/grpc/rb_server.c
@@ -97,35 +97,19 @@ static VALUE grpc_rb_server_alloc(VALUE cls) {
 /*
   call-seq:
     cq = CompletionQueue.new
-    insecure_server = Server.new(cq, {'arg1': 'value1'})
-    server_creds = ...
-    secure_server = Server.new(cq, {'arg1': 'value1'}, server_creds)
+    server = Server.new(cq, {'arg1': 'value1'})
 
   Initializes server instances. */
-static VALUE grpc_rb_server_init(int argc, VALUE *argv, VALUE self) {
-  VALUE cqueue = Qnil;
-  VALUE credentials = Qnil;
-  VALUE channel_args = Qnil;
+static VALUE grpc_rb_server_init(VALUE self, VALUE cqueue, VALUE channel_args) {
   grpc_completion_queue *cq = NULL;
-  grpc_server_credentials *creds = NULL;
   grpc_rb_server *wrapper = NULL;
   grpc_server *srv = NULL;
   grpc_channel_args args;
   MEMZERO(&args, grpc_channel_args, 1);
-
-  /* "21" == 2 mandatory args, 1 (credentials) is optional */
-  rb_scan_args(argc, argv, "21", &cqueue, &channel_args, &credentials);
   cq = grpc_rb_get_wrapped_completion_queue(cqueue);
-
   Data_Get_Struct(self, grpc_rb_server, wrapper);
   grpc_rb_hash_convert_to_channel_args(channel_args, &args);
   srv = grpc_server_create(cq, &args);
-  if (credentials == Qnil) {
-    srv = grpc_server_create(cq, &args);
-  } else {
-    creds = grpc_rb_get_wrapped_server_credentials(credentials);
-    srv = grpc_secure_server_create(creds, cq, &args);
-  }
 
   if (args.args != NULL) {
     xfree(args.args); /* Allocated by grpc_rb_hash_convert_to_channel_args */
@@ -215,33 +199,36 @@ static VALUE grpc_rb_server_destroy(VALUE self) {
 
     // secure port
     server_creds = ...
-    secure_server = Server.new(cq, {'arg1': 'value1'}, creds)
-    secure_server.add_http_port('mydomain:7575', True)
+    secure_server = Server.new(cq, {'arg1': 'value1'})
+    secure_server.add_http_port('mydomain:7575', server_creds)
 
     Adds a http2 port to server */
 static VALUE grpc_rb_server_add_http2_port(int argc, VALUE *argv, VALUE self) {
   VALUE port = Qnil;
-  VALUE is_secure = Qnil;
+  VALUE rb_creds = Qnil;
   grpc_rb_server *s = NULL;
+  grpc_server_credentials *creds = NULL;
   int recvd_port = 0;
 
-  /* "11" == 1 mandatory args, 1 (is_secure) is optional */
-  rb_scan_args(argc, argv, "11", &port, &is_secure);
+  /* "11" == 1 mandatory args, 1 (rb_creds) is optional */
+  rb_scan_args(argc, argv, "11", &port, &rb_creds);
 
   Data_Get_Struct(self, grpc_rb_server, s);
   if (s->wrapped == NULL) {
     rb_raise(rb_eRuntimeError, "closed!");
     return Qnil;
-  } else if (is_secure == Qnil || TYPE(is_secure) != T_TRUE) {
+  } else if (rb_creds == Qnil) {
     recvd_port = grpc_server_add_http2_port(s->wrapped, StringValueCStr(port));
     if (recvd_port == 0) {
       rb_raise(rb_eRuntimeError,
                "could not add port %s to server, not sure why",
                StringValueCStr(port));
     }
-  } else if (TYPE(is_secure) != T_FALSE) {
+  } else {
+    creds = grpc_rb_get_wrapped_server_credentials(rb_creds);
     recvd_port =
-        grpc_server_add_secure_http2_port(s->wrapped, StringValueCStr(port));
+        grpc_server_add_secure_http2_port(s->wrapped, StringValueCStr(port),
+			                  creds);
     if (recvd_port == 0) {
       rb_raise(rb_eRuntimeError,
                "could not add secure port %s to server, not sure why",
@@ -258,7 +245,7 @@ void Init_grpc_server() {
   rb_define_alloc_func(rb_cServer, grpc_rb_server_alloc);
 
   /* Provides a ruby constructor and support for dup/clone. */
-  rb_define_method(rb_cServer, "initialize", grpc_rb_server_init, -1);
+  rb_define_method(rb_cServer, "initialize", grpc_rb_server_init, 2);
   rb_define_method(rb_cServer, "initialize_copy", grpc_rb_server_init_copy, 1);
 
   /* Add the server methods. */
diff --git a/src/ruby/lib/grpc/generic/rpc_server.rb b/src/ruby/lib/grpc/generic/rpc_server.rb
index 6938f71892..35e84023be 100644
--- a/src/ruby/lib/grpc/generic/rpc_server.rb
+++ b/src/ruby/lib/grpc/generic/rpc_server.rb
@@ -81,7 +81,6 @@ module GRPC
                    max_waiting_requests:DEFAULT_MAX_WAITING_REQUESTS,
                    poll_period:INFINITE_FUTURE,
                    completion_queue_override:nil,
-                   creds:nil,
                    server_override:nil,
                    **kw)
       if completion_queue_override.nil?
@@ -95,13 +94,7 @@ module GRPC
       @cq = cq
 
       if server_override.nil?
-        if creds.nil?
-          srv = Core::Server.new(@cq, kw)
-        elsif !creds.is_a? Core::ServerCredentials
-          fail(ArgumentError, 'not a ServerCredentials')
-        else
-          srv = Core::Server.new(@cq, kw, creds)
-        end
+        srv = Core::Server.new(@cq, kw)
       else
         srv = server_override
         fail(ArgumentError, 'not a Server') unless srv.is_a? Core::Server
diff --git a/src/ruby/spec/client_server_spec.rb b/src/ruby/spec/client_server_spec.rb
index 030ff328f2..49a2d3bb4d 100644
--- a/src/ruby/spec/client_server_spec.rb
+++ b/src/ruby/spec/client_server_spec.rb
@@ -95,7 +95,7 @@ shared_context 'setup: tags' do
   end
 
   def new_client_call
-    @ch.create_call('/method', 'localhost', deadline)
+    @ch.create_call('/method', 'foo.test.google.fr', deadline)
   end
 end
 
@@ -346,12 +346,12 @@ end
 describe 'the secure http client/server' do
   before(:example) do
     certs = load_test_certs
-    server_host = 'localhost:0'
+    server_host = '0.0.0.0:0'
     @client_queue = GRPC::Core::CompletionQueue.new
     @server_queue = GRPC::Core::CompletionQueue.new
     server_creds = GRPC::Core::ServerCredentials.new(nil, certs[1], certs[2])
-    @server = GRPC::Core::Server.new(@server_queue, nil, server_creds)
-    server_port = @server.add_http2_port(server_host, true)
+    @server = GRPC::Core::Server.new(@server_queue, nil)
+    server_port = @server.add_http2_port(server_host, server_creds)
     @server.start
     args = { Channel::SSL_TARGET => 'foo.test.google.fr' }
     @ch = Channel.new("0.0.0.0:#{server_port}", args,
@@ -362,11 +362,9 @@ describe 'the secure http client/server' do
     @server.close
   end
 
-  # TODO: uncomment after updating the to the new c api
-  # it_behaves_like 'basic GRPC message delivery is OK' do
-  # end
+  it_behaves_like 'basic GRPC message delivery is OK' do
+  end
 
-  # TODO: uncomment after updating the to the new c api
-  # it_behaves_like 'GRPC metadata delivery works OK' do
-  # end
+  it_behaves_like 'GRPC metadata delivery works OK' do
+  end
 end
diff --git a/src/ruby/spec/generic/rpc_server_spec.rb b/src/ruby/spec/generic/rpc_server_spec.rb
index e8c7060446..d5421d400c 100644
--- a/src/ruby/spec/generic/rpc_server_spec.rb
+++ b/src/ruby/spec/generic/rpc_server_spec.rb
@@ -164,19 +164,6 @@ describe GRPC::RpcServer do
       expect(&blk).to raise_error
     end
 
-    it 'can be created with the creds as valid ServerCedentials' do
-      certs = load_test_certs
-      server_creds = GRPC::Core::ServerCredentials.new(nil, certs[1], certs[2])
-      blk = proc do
-        opts = {
-          a_channel_arg: 'an_arg',
-          creds: server_creds
-        }
-        RpcServer.new(**opts)
-      end
-      expect(&blk).to_not raise_error
-    end
-
     it 'can be created with a server override' do
       opts = { a_channel_arg: 'an_arg', server_override: @server }
       blk = proc do
diff --git a/src/ruby/spec/server_spec.rb b/src/ruby/spec/server_spec.rb
index 5b81f19537..a47e484f97 100644
--- a/src/ruby/spec/server_spec.rb
+++ b/src/ruby/spec/server_spec.rb
@@ -118,10 +118,11 @@ describe Server do
     end
 
     describe 'for secure servers' do
+      let(:cert) { create_test_cert }
       it 'runs without failing' do
         blk = proc do
           s = Server.new(@cq, nil)
-          s.add_http2_port('localhost:0', true)
+          s.add_http2_port('localhost:0', cert)
           s.close
         end
         expect(&blk).to_not raise_error
@@ -130,7 +131,7 @@ describe Server do
       it 'fails if the server is closed' do
         s = Server.new(@cq, nil)
         s.close
-        blk = proc { s.add_http2_port('localhost:0', true) }
+        blk = proc { s.add_http2_port('localhost:0', cert) }
         expect(&blk).to raise_error(RuntimeError)
       end
     end
@@ -138,7 +139,7 @@ describe Server do
 
   shared_examples '#new' do
     it 'takes a completion queue with nil channel args' do
-      expect { Server.new(@cq, nil, create_test_cert) }.to_not raise_error
+      expect { Server.new(@cq, nil) }.to_not raise_error
     end
 
     it 'does not take a hash with bad keys as channel args' do
@@ -195,14 +196,6 @@ describe Server do
     it_behaves_like '#new'
   end
 
-  describe '#new with a secure channel' do
-    def construct_with_args(a)
-      proc { Server.new(@cq, a, create_test_cert) }
-    end
-
-    it_behaves_like '#new'
-  end
-
   def start_a_server
     s = Server.new(@cq, nil)
     s.add_http2_port('0.0.0.0:0')