From e69f088cd973e61da445eddae61c67de6edbca17 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Thu, 7 Jul 2016 15:52:27 -0700 Subject: [PATCH 1/2] Split incoming initial and trailing metadata in Ruby calls --- src/ruby/ext/grpc/rb_call.c | 33 ++++++++++++++++++++++++ src/ruby/lib/grpc/generic/active_call.rb | 22 ++++++++-------- src/ruby/spec/generic/rpc_server_spec.rb | 4 +-- 3 files changed, 46 insertions(+), 13 deletions(-) diff --git a/src/ruby/ext/grpc/rb_call.c b/src/ruby/ext/grpc/rb_call.c index f62397e79f566..212612444368d 100644 --- a/src/ruby/ext/grpc/rb_call.c +++ b/src/ruby/ext/grpc/rb_call.c @@ -71,6 +71,10 @@ static ID id_credentials; * received by the call and subsequently saved on it. */ static ID id_metadata; +/* id_trailing_metadata is the name of the attribute used to access the trailing + * metadata hash received by the call and subsequently saved on it. */ +static ID id_trailing_metadata; + /* id_status is name of the attribute used to access the status object * received by the call and subsequently saved on it. */ static ID id_status; @@ -296,6 +300,30 @@ static VALUE grpc_rb_call_set_metadata(VALUE self, VALUE metadata) { return rb_ivar_set(self, id_metadata, metadata); } +/* + call-seq: + trailing_metadata = call.trailing_metadata + + Gets the trailing metadata object saved on the call */ +static VALUE grpc_rb_call_get_trailing_metadata(VALUE self) { + return rb_ivar_get(self, id_trailing_metadata); +} + +/* + call-seq: + call.trailing_metadata = trailing_metadata + + Saves the trailing metadata hash on the call. */ +static VALUE grpc_rb_call_set_trailing_metadata(VALUE self, VALUE metadata) { + if (!NIL_P(metadata) && TYPE(metadata) != T_HASH) { + rb_raise(rb_eTypeError, "bad metadata: got:<%s> want: ", + rb_obj_classname(metadata)); + return Qnil; + } + + return rb_ivar_set(self, id_trailing_metadata, metadata); +} + /* call-seq: write_flag = call.write_flag @@ -908,6 +936,10 @@ void Init_grpc_call() { rb_define_method(grpc_rb_cCall, "status=", grpc_rb_call_set_status, 1); rb_define_method(grpc_rb_cCall, "metadata", grpc_rb_call_get_metadata, 0); rb_define_method(grpc_rb_cCall, "metadata=", grpc_rb_call_set_metadata, 1); + rb_define_method(grpc_rb_cCall, "trailing_metadata", + grpc_rb_call_get_trailing_metadata, 0); + rb_define_method(grpc_rb_cCall, "trailing_metadata=", + grpc_rb_call_set_trailing_metadata, 1); rb_define_method(grpc_rb_cCall, "write_flag", grpc_rb_call_get_write_flag, 0); rb_define_method(grpc_rb_cCall, "write_flag=", grpc_rb_call_set_write_flag, 1); @@ -916,6 +948,7 @@ void Init_grpc_call() { /* Ids used to support call attributes */ id_metadata = rb_intern("metadata"); + id_trailing_metadata = rb_intern("trailing_metadata"); id_status = rb_intern("status"); id_write_flag = rb_intern("write_flag"); diff --git a/src/ruby/lib/grpc/generic/active_call.rb b/src/ruby/lib/grpc/generic/active_call.rb index a3ac0d48a3bb2..5c142fb161995 100644 --- a/src/ruby/lib/grpc/generic/active_call.rb +++ b/src/ruby/lib/grpc/generic/active_call.rb @@ -43,8 +43,7 @@ def check_status GRPC.logger.debug("Failing with status #{status}") # raise BadStatus, propagating the metadata if present. md = status.metadata - with_sym_keys = Hash[md.each_pair.collect { |x, y| [x.to_sym, y] }] - fail GRPC::BadStatus.new(status.code, status.details, with_sym_keys) + fail GRPC::BadStatus.new(status.code, status.details, md) end status end @@ -61,7 +60,7 @@ class ActiveCall extend Forwardable attr_reader(:deadline) def_delegators :@call, :cancel, :metadata, :write_flag, :write_flag=, - :peer, :peer_cert + :peer, :peer_cert, :trailing_metadata # client_invoke begins a client invocation. # @@ -158,6 +157,9 @@ def writes_done(assert_finished = true) ops[RECV_STATUS_ON_CLIENT] = nil if assert_finished batch_result = @call.run_batch(ops) return unless assert_finished + unless batch_result.status.nil? + @call.trailing_metadata = batch_result.status.metadata + end @call.status = batch_result.status op_is_done batch_result.check_status @@ -169,11 +171,7 @@ def writes_done(assert_finished = true) def finished batch_result = @call.run_batch(RECV_STATUS_ON_CLIENT => nil) unless batch_result.status.nil? - if @call.metadata.nil? - @call.metadata = batch_result.status.metadata - else - @call.metadata.merge!(batch_result.status.metadata) - end + @call.trailing_metadata = batch_result.status.metadata end @call.status = batch_result.status op_is_done @@ -456,17 +454,19 @@ def initialize(wrapped) # SingleReqView limits access to an ActiveCall's methods for use in server # handlers that receive just one request. SingleReqView = view_class(:cancelled, :deadline, :metadata, - :output_metadata, :peer, :peer_cert) + :output_metadata, :peer, :peer_cert, + :trailing_metadata) # MultiReqView limits access to an ActiveCall's methods for use in # server client_streamer handlers. MultiReqView = view_class(:cancelled, :deadline, :each_queued_msg, - :each_remote_read, :metadata, :output_metadata) + :each_remote_read, :metadata, :output_metadata, + :trailing_metadata) # Operation limits access to an ActiveCall's methods for use as # a Operation on the client. Operation = view_class(:cancel, :cancelled, :deadline, :execute, :metadata, :status, :start_call, :wait, :write_flag, - :write_flag=) + :write_flag=, :trailing_metadata) end end diff --git a/src/ruby/spec/generic/rpc_server_spec.rb b/src/ruby/spec/generic/rpc_server_spec.rb index 901c84fc78315..31157cf161ef7 100644 --- a/src/ruby/spec/generic/rpc_server_spec.rb +++ b/src/ruby/spec/generic/rpc_server_spec.rb @@ -95,7 +95,7 @@ class FailingService def initialize(_default_var = 'ignored') @details = 'app error' @code = 101 - @md = { failed_method: 'an_rpc' } + @md = { 'failed_method' => 'an_rpc' } end def an_rpc(_req, _call) @@ -515,7 +515,7 @@ def an_rpc(req, call) op = stub.an_rpc(req, return_op: true, metadata: { k1: 'v1', k2: 'v2' }) expect(op.metadata).to be nil expect(op.execute).to be_a(EchoMsg) - expect(op.metadata).to eq(wanted_trailers) + expect(op.trailing_metadata).to eq(wanted_trailers) @srv.stop t.join end From ae466c8a8d5586b08ef5078febc115775220b8b9 Mon Sep 17 00:00:00 2001 From: murgatroid99 Date: Fri, 8 Jul 2016 09:28:25 -0700 Subject: [PATCH 2/2] Revert changes to SingleReqView and MultiReqView --- src/ruby/lib/grpc/generic/active_call.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/ruby/lib/grpc/generic/active_call.rb b/src/ruby/lib/grpc/generic/active_call.rb index 5c142fb161995..47f249893657b 100644 --- a/src/ruby/lib/grpc/generic/active_call.rb +++ b/src/ruby/lib/grpc/generic/active_call.rb @@ -454,14 +454,12 @@ def initialize(wrapped) # SingleReqView limits access to an ActiveCall's methods for use in server # handlers that receive just one request. SingleReqView = view_class(:cancelled, :deadline, :metadata, - :output_metadata, :peer, :peer_cert, - :trailing_metadata) + :output_metadata, :peer, :peer_cert) # MultiReqView limits access to an ActiveCall's methods for use in # server client_streamer handlers. MultiReqView = view_class(:cancelled, :deadline, :each_queued_msg, - :each_remote_read, :metadata, :output_metadata, - :trailing_metadata) + :each_remote_read, :metadata, :output_metadata) # Operation limits access to an ActiveCall's methods for use as # a Operation on the client.