From f68bf38af4fce75a1d1bc2c7ea7a0109a113caa7 Mon Sep 17 00:00:00 2001 From: Alistair Veitch Date: Tue, 28 Jul 2015 14:27:28 -0700 Subject: [PATCH 01/18] add external tag set API --- include/grpc/census.h | 55 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/include/grpc/census.h b/include/grpc/census.h index 379783905aa..eea7ddf7283 100644 --- a/include/grpc/census.h +++ b/include/grpc/census.h @@ -100,6 +100,61 @@ int census_context_deserialize(const char *buffer, census_context **context); * future census calls will result in undefined behavior. */ void census_context_destroy(census_context *context); +/* Max number of characters in tag key */ +#define CENSUS_MAX_TAG_KEY_LENGTH 20 +/* Max number of tag value characters */ +#define CENSUS_MAX_TAG_VALUE_LENGTH 50 + +/* A Census tag set is a collection of key:value string pairs; these form the + basis against which Census metrics will be recorded. Keys are unique within + a tag set. All contexts have an associated tag set. */ +typedef struct census_tag_set census_tag_set; + +/* Returns a pointer to a newly created, empty tag set. If size_hint > 0, + indicates that the tag set is intended to hold approximately that number + of tags. */ +census_tag_set *census_tag_set_create(size_t size_hint); + +/* Add a new tag key/value to an existing tag set; if the tag key already exists + in the tag set, then its value is overwritten with the new one. Can also be + used to delete a tag, by specifying a NULL value. If key is NULL, returns + the number of tags in the tag set. + Return values: + -1: invalid length key or value + positive values: the number of tags in the tag set. */ +int census_tag_set_add(census_tag_set *tags, const char *key, + const char *value); + +/* Destroys a tag set. This function must be called to prevent memory leaks. + Once called, the tag set cannot be used again. */ +void census_tag_set_destroy(census_tag_set *tags); + +/* Get a contexts tag set. */ +census_tag_set *census_context_tag_set(census_context *context); + +/* A read-only representation of a tag for use by census clients. */ +typedef struct { + size_t key_len; /* Number of bytes in tag key. */ + const char *key; /* A pointer to the tag key. May not be null-terminated. */ + size_t value_len; /* Number of bytes in tag value. */ + const char *value; /* Pointer to the tag value. May not be null-terminated. */ +} census_tag_const; + +/* Used to iterate through a tag sets contents. */ +typedef struct census_tag_set_iterator census_tag_set_iterator; + +/* Open a tag set for iteration. The tag set must not be modified while + iteration is ongoing. Returns an iterator for use in following functions. */ +census_tag_set_iterator *census_tag_set_open(census_tag_set *tags); + +/* Get the next tag in the tag set, by writing into the 'tag' argument. Returns + 1 if there is a "next" tag, 0 if there are no more tags. */ +int census_tag_set_next(census_tag_set_iterator *it, census_tag_const *tag); + +/* Close an iterator opened by census_tag_set_open(). The iterator will be + invalidated, and should not be used once close is called. */ +void census_tag_set_close(census_tag_set_iterator *it); + /* A census statistic to be recorded comprises two parts: an ID for the * particular statistic and the value to be recorded against it. */ typedef struct { From a178b86668db21ff5c03951d5d9b0b3ad79e9752 Mon Sep 17 00:00:00 2001 From: Alistair Veitch Date: Tue, 28 Jul 2015 15:11:54 -0700 Subject: [PATCH 02/18] fix comment --- include/grpc/census.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/include/grpc/census.h b/include/grpc/census.h index eea7ddf7283..c5766ed3128 100644 --- a/include/grpc/census.h +++ b/include/grpc/census.h @@ -121,7 +121,7 @@ census_tag_set *census_tag_set_create(size_t size_hint); the number of tags in the tag set. Return values: -1: invalid length key or value - positive values: the number of tags in the tag set. */ + non-negative value: the number of tags in the tag set. */ int census_tag_set_add(census_tag_set *tags, const char *key, const char *value); From 03fad5f11e13a275f0a5b1363c067698536123ef Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Fri, 7 Aug 2015 15:39:10 -0700 Subject: [PATCH 03/18] Better error codes for client_auth_filter. Fixes #2484. --- src/core/security/client_auth_filter.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/core/security/client_auth_filter.c b/src/core/security/client_auth_filter.c index 0e699874bcd..ccd014312e3 100644 --- a/src/core/security/client_auth_filter.c +++ b/src/core/security/client_auth_filter.c @@ -75,11 +75,11 @@ typedef struct { grpc_mdstr *status_key; } channel_data; -static void bubble_up_error(grpc_call_element *elem, const char *error_msg) { +static void bubble_up_error(grpc_call_element *elem, grpc_status_code status, + const char *error_msg) { call_data *calld = elem->call_data; gpr_log(GPR_ERROR, "Client side authentication failure: %s", error_msg); - grpc_transport_stream_op_add_cancellation(&calld->op, - GRPC_STATUS_UNAUTHENTICATED); + grpc_transport_stream_op_add_cancellation(&calld->op, status); grpc_call_next_op(elem, &calld->op); } @@ -94,7 +94,8 @@ static void on_credentials_metadata(void *user_data, grpc_metadata_batch *mdb; size_t i; if (status != GRPC_CREDENTIALS_OK) { - bubble_up_error(elem, "Credentials failed to get metadata."); + bubble_up_error(elem, GRPC_STATUS_UNAUTHENTICATED, + "Credentials failed to get metadata."); return; } GPR_ASSERT(num_md <= MAX_CREDENTIALS_METADATA_COUNT); @@ -154,7 +155,7 @@ static void send_security_metadata(grpc_call_element *elem, if (channel_creds_has_md && call_creds_has_md) { calld->creds = grpc_composite_credentials_create(channel_creds, ctx->creds); if (calld->creds == NULL) { - bubble_up_error(elem, + bubble_up_error(elem, GRPC_STATUS_INVALID_ARGUMENT, "Incompatible credentials set on channel and call."); return; } @@ -182,7 +183,7 @@ static void on_host_checked(void *user_data, grpc_security_status status) { char *error_msg; gpr_asprintf(&error_msg, "Invalid host %s set in :authority metadata.", grpc_mdstr_as_c_string(calld->host)); - bubble_up_error(elem, error_msg); + bubble_up_error(elem, GRPC_STATUS_FAILED_PRECONDITION, error_msg); gpr_free(error_msg); } } @@ -252,7 +253,7 @@ static void auth_start_transport_op(grpc_call_element *elem, gpr_asprintf(&error_msg, "Invalid host %s set in :authority metadata.", call_host); - bubble_up_error(elem, error_msg); + bubble_up_error(elem, GRPC_STATUS_FAILED_PRECONDITION, error_msg); gpr_free(error_msg); } return; /* early exit */ From ca9460bc6d0580bc979abf08bf17ba74473655c4 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Fri, 7 Aug 2015 15:41:15 -0700 Subject: [PATCH 04/18] Fixing test. --- test/core/end2end/tests/bad_hostname.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/end2end/tests/bad_hostname.c b/test/core/end2end/tests/bad_hostname.c index 198ba46cd2b..3007b620248 100644 --- a/test/core/end2end/tests/bad_hostname.c +++ b/test/core/end2end/tests/bad_hostname.c @@ -146,7 +146,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) { cq_expect_completion(cqv, tag(1), 1); cq_verify(cqv); - GPR_ASSERT(status == GRPC_STATUS_UNAUTHENTICATED); + GPR_ASSERT(status == GRPC_STATUS_FAILED_PRECONDITION); gpr_free(details); grpc_metadata_array_destroy(&initial_metadata_recv); From 0c034a01d12753a06a3dbbe202430ff0485ab978 Mon Sep 17 00:00:00 2001 From: yang-g Date: Tue, 11 Aug 2015 11:46:32 -0700 Subject: [PATCH 05/18] client code clean up --- include/grpc++/client_context.h | 4 ---- src/cpp/client/channel.cc | 32 +++++++++++++++++++------------- src/cpp/client/client_context.cc | 9 --------- 3 files changed, 19 insertions(+), 26 deletions(-) diff --git a/include/grpc++/client_context.h b/include/grpc++/client_context.h index 34945f32820..d7fafac9b36 100644 --- a/include/grpc++/client_context.h +++ b/include/grpc++/client_context.h @@ -218,15 +218,11 @@ class ClientContext { void set_call(grpc_call* call, const std::shared_ptr& channel); - grpc_completion_queue* cq() { return cq_; } - void set_cq(grpc_completion_queue* cq) { cq_ = cq; } - grpc::string authority() { return authority_; } bool initial_metadata_received_; std::shared_ptr channel_; grpc_call* call_; - grpc_completion_queue* cq_; gpr_timespec deadline_; grpc::string authority_; std::shared_ptr creds_; diff --git a/src/cpp/client/channel.cc b/src/cpp/client/channel.cc index af7366eb01c..93536ae3670 100644 --- a/src/cpp/client/channel.cc +++ b/src/cpp/client/channel.cc @@ -61,19 +61,25 @@ Channel::~Channel() { grpc_channel_destroy(c_channel_); } Call Channel::CreateCall(const RpcMethod& method, ClientContext* context, CompletionQueue* cq) { - const char* host_str = host_.empty() ? NULL : host_.c_str(); - auto c_call = method.channel_tag() && context->authority().empty() - ? grpc_channel_create_registered_call( - c_channel_, context->propagate_from_call_, - context->propagation_options_.c_bitmask(), cq->cq(), - method.channel_tag(), context->raw_deadline()) - : grpc_channel_create_call( - c_channel_, context->propagate_from_call_, - context->propagation_options_.c_bitmask(), cq->cq(), - method.name(), context->authority().empty() - ? host_str - : context->authority().c_str(), - context->raw_deadline()); + bool registered = method.channel_tag() && context->authority().empty(); + grpc_call* c_call = NULL; + if (registered) { + c_call = grpc_channel_create_registered_call( + c_channel_, context->propagate_from_call_, + context->propagation_options_.c_bitmask(), cq->cq(), + method.channel_tag(), context->raw_deadline()); + } else { + const char* host_str = NULL; + if (!context->authority().empty()) { + host_str = context->authority().c_str(); + } else if (!host_.empty()) { + host_str = host_.c_str(); + } + c_call = grpc_channel_create_call(c_channel_, context->propagate_from_call_, + context->propagation_options_.c_bitmask(), + cq->cq(), method.name(), host_str, + context->raw_deadline()); + } grpc_census_call_set_context(c_call, context->census_context()); GRPC_TIMER_MARK(GRPC_PTAG_CPP_CALL_CREATED, c_call); context->set_call(c_call, shared_from_this()); diff --git a/src/cpp/client/client_context.cc b/src/cpp/client/client_context.cc index 1ed2d389616..dd86e7b108a 100644 --- a/src/cpp/client/client_context.cc +++ b/src/cpp/client/client_context.cc @@ -48,7 +48,6 @@ namespace grpc { ClientContext::ClientContext() : initial_metadata_received_(false), call_(nullptr), - cq_(nullptr), deadline_(gpr_inf_future(GPR_CLOCK_REALTIME)), propagate_from_call_(nullptr) {} @@ -56,14 +55,6 @@ ClientContext::~ClientContext() { if (call_) { grpc_call_destroy(call_); } - if (cq_) { - // Drain cq_. - grpc_completion_queue_shutdown(cq_); - while (grpc_completion_queue_next(cq_, gpr_inf_future(GPR_CLOCK_REALTIME)) - .type != GRPC_QUEUE_SHUTDOWN) - ; - grpc_completion_queue_destroy(cq_); - } } std::unique_ptr ClientContext::FromServerContext( From 8f52853e13a7a6252d1afd152d0ab51572f22401 Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 11 Aug 2015 15:49:44 -0700 Subject: [PATCH 06/18] Fixing error code as discussed during the review. --- src/core/security/client_auth_filter.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/security/client_auth_filter.c b/src/core/security/client_auth_filter.c index ccd014312e3..410852da523 100644 --- a/src/core/security/client_auth_filter.c +++ b/src/core/security/client_auth_filter.c @@ -183,7 +183,7 @@ static void on_host_checked(void *user_data, grpc_security_status status) { char *error_msg; gpr_asprintf(&error_msg, "Invalid host %s set in :authority metadata.", grpc_mdstr_as_c_string(calld->host)); - bubble_up_error(elem, GRPC_STATUS_FAILED_PRECONDITION, error_msg); + bubble_up_error(elem, GRPC_STATUS_INVALID_ARGUMENT, error_msg); gpr_free(error_msg); } } @@ -253,7 +253,7 @@ static void auth_start_transport_op(grpc_call_element *elem, gpr_asprintf(&error_msg, "Invalid host %s set in :authority metadata.", call_host); - bubble_up_error(elem, GRPC_STATUS_FAILED_PRECONDITION, error_msg); + bubble_up_error(elem, GRPC_STATUS_INVALID_ARGUMENT, error_msg); gpr_free(error_msg); } return; /* early exit */ From e41d3ade7b30f84cc1bc94cdbd7f57521111991a Mon Sep 17 00:00:00 2001 From: Julien Boeuf Date: Tue, 11 Aug 2015 15:58:01 -0700 Subject: [PATCH 07/18] Fixing test. --- test/core/end2end/tests/bad_hostname.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/core/end2end/tests/bad_hostname.c b/test/core/end2end/tests/bad_hostname.c index 3007b620248..501db89b7b2 100644 --- a/test/core/end2end/tests/bad_hostname.c +++ b/test/core/end2end/tests/bad_hostname.c @@ -146,7 +146,7 @@ static void simple_request_body(grpc_end2end_test_fixture f) { cq_expect_completion(cqv, tag(1), 1); cq_verify(cqv); - GPR_ASSERT(status == GRPC_STATUS_FAILED_PRECONDITION); + GPR_ASSERT(status == GRPC_STATUS_INVALID_ARGUMENT); gpr_free(details); grpc_metadata_array_destroy(&initial_metadata_recv); From 37ce034dc162ede929999b0239d295e2d8bfd263 Mon Sep 17 00:00:00 2001 From: yang-g Date: Wed, 12 Aug 2015 11:46:26 -0700 Subject: [PATCH 08/18] add const --- src/cpp/client/channel.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/cpp/client/channel.cc b/src/cpp/client/channel.cc index 93536ae3670..1c2eecf7863 100644 --- a/src/cpp/client/channel.cc +++ b/src/cpp/client/channel.cc @@ -61,9 +61,9 @@ Channel::~Channel() { grpc_channel_destroy(c_channel_); } Call Channel::CreateCall(const RpcMethod& method, ClientContext* context, CompletionQueue* cq) { - bool registered = method.channel_tag() && context->authority().empty(); + const bool kRegistered = method.channel_tag() && context->authority().empty(); grpc_call* c_call = NULL; - if (registered) { + if (kRegistered) { c_call = grpc_channel_create_registered_call( c_channel_, context->propagate_from_call_, context->propagation_options_.c_bitmask(), cq->cq(), From 70cb30092141a3d3897a5bbd468a626124131dbd Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Wed, 12 Aug 2015 15:57:10 -0700 Subject: [PATCH 09/18] generate auth URI for JWT access token properly --- src/csharp/Grpc.Core.Tests/ClientBaseTest.cs | 62 +++++++++++++++++++ .../Grpc.Core.Tests/Grpc.Core.Tests.csproj | 1 + src/csharp/Grpc.Core/ClientBase.cs | 25 ++++++-- 3 files changed, 83 insertions(+), 5 deletions(-) create mode 100644 src/csharp/Grpc.Core.Tests/ClientBaseTest.cs diff --git a/src/csharp/Grpc.Core.Tests/ClientBaseTest.cs b/src/csharp/Grpc.Core.Tests/ClientBaseTest.cs new file mode 100644 index 00000000000..2dc10ebe971 --- /dev/null +++ b/src/csharp/Grpc.Core.Tests/ClientBaseTest.cs @@ -0,0 +1,62 @@ +#region Copyright notice and license + +// Copyright 2015, Google Inc. +// All rights reserved. +// +// Redistribution and use in source and binary forms, with or without +// modification, are permitted provided that the following conditions are +// met: +// +// * Redistributions of source code must retain the above copyright +// notice, this list of conditions and the following disclaimer. +// * Redistributions in binary form must reproduce the above +// copyright notice, this list of conditions and the following disclaimer +// in the documentation and/or other materials provided with the +// distribution. +// * Neither the name of Google Inc. nor the names of its +// contributors may be used to endorse or promote products derived from +// this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS +// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT +// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR +// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT +// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT +// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, +// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY +// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT +// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +#endregion + +using System; +using Grpc.Core; +using Grpc.Core.Internal; +using Grpc.Core.Utils; +using NUnit.Framework; + +namespace Grpc.Core.Tests +{ + public class ClientBaseTest + { + [Test] + public void GetAuthUriBase_Valid() + { + Assert.AreEqual("https://some.googleapi.com/", ClientBase.GetAuthUriBase("some.googleapi.com")); + Assert.AreEqual("https://some.googleapi.com/", ClientBase.GetAuthUriBase("dns:///some.googleapi.com/")); + Assert.AreEqual("https://some.googleapi.com/", ClientBase.GetAuthUriBase("dns:///some.googleapi.com:443/")); + Assert.AreEqual("https://some.googleapi.com/", ClientBase.GetAuthUriBase("some.googleapi.com:443/")); + } + + [Test] + public void GetAuthUriBase_Invalid() + { + Assert.IsNull(ClientBase.GetAuthUriBase("some.googleapi.com:")); + Assert.IsNull(ClientBase.GetAuthUriBase("https://some.googleapi.com/")); + Assert.IsNull(ClientBase.GetAuthUriBase("dns://some.googleapi.com:443")); // just two slashes + Assert.IsNull(ClientBase.GetAuthUriBase("")); + } + } +} diff --git a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj index 97ee0454bb0..d6a8f52570b 100644 --- a/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj +++ b/src/csharp/Grpc.Core.Tests/Grpc.Core.Tests.csproj @@ -63,6 +63,7 @@ Version.cs + diff --git a/src/csharp/Grpc.Core/ClientBase.cs b/src/csharp/Grpc.Core/ClientBase.cs index f46184406c5..f240d777b9c 100644 --- a/src/csharp/Grpc.Core/ClientBase.cs +++ b/src/csharp/Grpc.Core/ClientBase.cs @@ -33,9 +33,10 @@ using System; using System.Collections.Generic; +using System.Text.RegularExpressions; using Grpc.Core.Internal; -using System.Text.RegularExpressions; +using Grpc.Core.Utils; namespace Grpc.Core { @@ -46,15 +47,16 @@ namespace Grpc.Core /// public abstract class ClientBase { - static readonly Regex TrailingPortPattern = new Regex(":[0-9]+/?$"); + // Regex for removal of the optional DNS scheme, trailing port, and trailing backslash + static readonly Regex ChannelTargetPattern = new Regex(@"^(dns:\/{3})?([^:\/]+)(:\d+)?\/?$"); + readonly Channel channel; readonly string authUriBase; public ClientBase(Channel channel) { this.channel = channel; - // TODO(jtattermush): we shouldn't need to hand-curate the channel.Target contents. - this.authUriBase = "https://" + TrailingPortPattern.Replace(channel.Target, "") + "/"; + this.authUriBase = GetAuthUriBase(channel.Target); } /// @@ -104,10 +106,23 @@ namespace Grpc.Core { options = options.WithHeaders(new Metadata()); } - var authUri = authUriBase + method.ServiceName; + var authUri = authUriBase != null ? authUriBase + method.ServiceName : null; interceptor(authUri, options.Headers); } return new CallInvocationDetails(channel, method, Host, options); } + + /// + /// Creates Auth URI base from channel's target (the one passed at channel creation). + /// Fully-qualified service name is to be appended to this. + /// + internal static string GetAuthUriBase(string target) + { + var match = ChannelTargetPattern.Match(target); + if (!match.Success) { + return null; + } + return "https://" + match.Groups[2].Value + "/"; + } } } From 046094ded4164acb9206da011770b58224133da6 Mon Sep 17 00:00:00 2001 From: Tim Emiola Date: Wed, 12 Aug 2015 16:55:57 -0700 Subject: [PATCH 10/18] Merged with HEAD --- src/ruby/ext/grpc/rb_channel.c | 81 ++++++++++++++++++++++++++++++++++ 1 file changed, 81 insertions(+) diff --git a/src/ruby/ext/grpc/rb_channel.c b/src/ruby/ext/grpc/rb_channel.c index c973a1db6c9..2129ba34858 100644 --- a/src/ruby/ext/grpc/rb_channel.c +++ b/src/ruby/ext/grpc/rb_channel.c @@ -165,6 +165,65 @@ static VALUE grpc_rb_channel_init(int argc, VALUE *argv, VALUE self) { return self; } +/* + call-seq: + insecure_channel = Channel:new("myhost:8080", {'arg1': 'value1'}) + creds = ... + secure_channel = Channel:new("myhost:443", {'arg1': 'value1'}, creds) + + Creates channel instances. */ +static VALUE grpc_rb_channel_get_connectivity_state(int argc, VALUE *argv, + VALUE self) { + VALUE try_to_connect = Qfalse; + grpc_rb_channel *wrapper = NULL; + grpc_channel *ch = NULL; + + /* "01" == 0 mandatory args, 1 (try_to_connect) is optional */ + rb_scan_args(argc, argv, "01", try_to_connect); + + TypedData_Get_Struct(self, grpc_rb_channel, &grpc_channel_data_type, wrapper); + ch = wrapper->wrapped; + if (ch == NULL) { + rb_raise(rb_eRuntimeError, "closed!"); + return Qnil; + } + return NUM2LONG( + grpc_channel_check_connectivity_state(ch, (int)try_to_connect)); +} + +/* Watch for a change in connectivity state. + + Once the channel connectivity state is different from the last observed + state, tag will be enqueued on cq with success=1 + + If deadline expires BEFORE the state is changed, tag will be enqueued on + the completion queue with success=0 */ +static VALUE grpc_rb_channel_watch_connectivity_state(VALUE self, + VALUE last_state, + VALUE cqueue, + VALUE deadline, + VALUE tag) { + grpc_rb_channel *wrapper = NULL; + grpc_channel *ch = NULL; + grpc_completion_queue *cq = NULL; + + cq = grpc_rb_get_wrapped_completion_queue(cqueue); + TypedData_Get_Struct(self, grpc_rb_channel, &grpc_channel_data_type, wrapper); + ch = wrapper->wrapped; + if (ch == NULL) { + rb_raise(rb_eRuntimeError, "closed!"); + return Qnil; + } + grpc_channel_watch_connectivity_state( + ch, + NUM2LONG(last_state), + grpc_rb_time_timeval(deadline, /* absolute time */ 0), + cq, + ROBJECT(tag)); + + return Qnil; +} + /* Clones Channel instances. Gives Channel a consistent implementation of Ruby's object copy/dup @@ -295,6 +354,22 @@ static void Init_grpc_propagate_masks() { UINT2NUM(GRPC_PROPAGATE_DEFAULTS)); } +static void Init_grpc_connectivity_states() { + /* Constants representing call propagation masks in grpc.h */ + VALUE grpc_rb_mConnectivityStates = rb_define_module_under( + grpc_rb_mGrpcCore, "ConnectivityStates"); + rb_define_const(grpc_rb_mConnectivityStates, "IDLE", + LONG2NUM(GRPC_CHANNEL_IDLE)); + rb_define_const(grpc_rb_mConnectivityStates, "CONNECTING", + LONG2NUM(GRPC_CHANNEL_CONNECTING)); + rb_define_const(grpc_rb_mConnectivityStates, "READY", + LONG2NUM(GRPC_CHANNEL_READY)); + rb_define_const(grpc_rb_mConnectivityStates, "TRANSIENT_FAILURE", + LONG2NUM(GRPC_CHANNEL_TRANSIENT_FAILURE)); + rb_define_const(grpc_rb_mConnectivityStates, "FATAL_FAILURE", + LONG2NUM(GRPC_CHANNEL_FATAL_FAILURE)); +} + void Init_grpc_channel() { grpc_rb_cChannelArgs = rb_define_class("TmpChannelArgs", rb_cObject); grpc_rb_cChannel = @@ -309,6 +384,11 @@ void Init_grpc_channel() { grpc_rb_channel_init_copy, 1); /* Add ruby analogues of the Channel methods. */ + rb_define_method(grpc_rb_cChannel, "connectivity_state", + grpc_rb_channel_get_connectivity_state, + -1); + rb_define_method(grpc_rb_cChannel, "watch_connectivity_state", + grpc_rb_channel_watch_connectivity_state, 4); rb_define_method(grpc_rb_cChannel, "create_call", grpc_rb_channel_create_call, 6); rb_define_method(grpc_rb_cChannel, "target", grpc_rb_channel_get_target, 0); @@ -327,6 +407,7 @@ void Init_grpc_channel() { rb_define_const(grpc_rb_cChannel, "MAX_MESSAGE_LENGTH", ID2SYM(rb_intern(GRPC_ARG_MAX_MESSAGE_LENGTH))); Init_grpc_propagate_masks(); + Init_grpc_connectivity_states(); } /* Gets the wrapped channel from the ruby wrapper */ From 6512d26b1b9bb70724437cb103f1df4b8ef5020c Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 12 Aug 2015 17:33:23 -0700 Subject: [PATCH 11/18] Rename requestMetadata -> requestHeaders --- src/objective-c/GRPCClient/GRPCCall+OAuth2.m | 6 +++--- src/objective-c/GRPCClient/GRPCCall.h | 4 ++-- src/objective-c/GRPCClient/GRPCCall.m | 14 +++++++------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall+OAuth2.m b/src/objective-c/GRPCClient/GRPCCall+OAuth2.m index ed39d4b0f7a..8e64b8b6ae5 100644 --- a/src/objective-c/GRPCClient/GRPCCall+OAuth2.m +++ b/src/objective-c/GRPCClient/GRPCCall+OAuth2.m @@ -40,7 +40,7 @@ static NSString * const kChallengeHeader = @"www-authenticate"; @implementation GRPCCall (OAuth2) - (NSString *)oauth2AccessToken { - NSString *headerValue = self.requestMetadata[kAuthorizationHeader]; + NSString *headerValue = self.requestHeaders[kAuthorizationHeader]; if ([headerValue hasPrefix:kBearerPrefix]) { return [headerValue substringFromIndex:kBearerPrefix.length]; } else { @@ -50,9 +50,9 @@ static NSString * const kChallengeHeader = @"www-authenticate"; - (void)setOauth2AccessToken:(NSString *)token { if (token) { - self.requestMetadata[kAuthorizationHeader] = [kBearerPrefix stringByAppendingString:token]; + self.requestHeaders[kAuthorizationHeader] = [kBearerPrefix stringByAppendingString:token]; } else { - [self.requestMetadata removeObjectForKey:kAuthorizationHeader]; + [self.requestHeaders removeObjectForKey:kAuthorizationHeader]; } } diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 4a8b7fff486..97324cba153 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -68,8 +68,8 @@ extern id const kGRPCStatusMetadataKey; // // For convenience, the property is initialized to an empty NSMutableDictionary, and the setter // accepts (and copies) both mutable and immutable dictionaries. -- (NSMutableDictionary *)requestMetadata; // nonatomic -- (void)setRequestMetadata:(NSDictionary *)requestMetadata; // nonatomic, copy +- (NSMutableDictionary *)requestHeaders; // nonatomic +- (void)setRequestHeaders:(NSDictionary *)requestHeaders; // nonatomic, copy // This dictionary is populated with the HTTP headers received from the server. When the RPC ends, // the HTTP trailers received are added to the dictionary too. It has the same structure as the diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 0f4c811ce41..1065ae735f5 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -89,7 +89,7 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; // the response arrives. GRPCCall *_retainSelf; - NSMutableDictionary *_requestMetadata; + NSMutableDictionary *_requestHeaders; NSMutableDictionary *_responseMetadata; } @@ -121,7 +121,7 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; _requestWriter = requestWriter; - _requestMetadata = [NSMutableDictionary dictionary]; + _requestHeaders = [NSMutableDictionary dictionary]; _responseMetadata = [NSMutableDictionary dictionary]; } return self; @@ -129,12 +129,12 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; #pragma mark Metadata -- (NSMutableDictionary *)requestMetadata { - return _requestMetadata; +- (NSMutableDictionary *)requestHeaders { + return _requestHeaders; } -- (void)setRequestMetadata:(NSDictionary *)requestMetadata { - _requestMetadata = [NSMutableDictionary dictionaryWithDictionary:requestMetadata]; +- (void)setRequestHeaders:(NSDictionary *)requestHeaders { + _requestHeaders = [NSMutableDictionary dictionaryWithDictionary:requestHeaders]; } - (NSDictionary *)responseMetadata { @@ -356,7 +356,7 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; _retainSelf = self; _responseWriteable = [[GRXConcurrentWriteable alloc] initWithWriteable:writeable]; - [self sendHeaders:_requestMetadata]; + [self sendHeaders:_requestHeaders]; [self invokeCall]; } From 4e45a6f82245770059562acbac822be8e484cc9d Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 12 Aug 2015 17:36:14 -0700 Subject: [PATCH 12/18] Rename responseMetadata -> allResponseMetadata --- src/objective-c/GRPCClient/GRPCCall+OAuth2.m | 2 +- src/objective-c/GRPCClient/GRPCCall.h | 2 +- src/objective-c/GRPCClient/GRPCCall.m | 14 +++++++------- src/objective-c/tests/GRPCClientTests.m | 4 ++-- 4 files changed, 11 insertions(+), 11 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall+OAuth2.m b/src/objective-c/GRPCClient/GRPCCall+OAuth2.m index 8e64b8b6ae5..7f4fbacbe3e 100644 --- a/src/objective-c/GRPCClient/GRPCCall+OAuth2.m +++ b/src/objective-c/GRPCClient/GRPCCall+OAuth2.m @@ -57,7 +57,7 @@ static NSString * const kChallengeHeader = @"www-authenticate"; } - (NSString *)oauth2ChallengeHeader { - return self.responseMetadata[kChallengeHeader]; + return self.allResponseMetadata[kChallengeHeader]; } @end diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 97324cba153..b82febb5877 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -78,7 +78,7 @@ extern id const kGRPCStatusMetadataKey; // The first time this object calls |writeValue| on the writeable passed to |startWithWriteable|, // the |responseMetadata| dictionary already contains the response headers. When it calls // |writesFinishedWithError|, the dictionary contains both the response headers and trailers. -@property(atomic, readonly) NSDictionary *responseMetadata; +@property(atomic, readonly) NSDictionary *allResponseMetadata; // The request writer has to write NSData objects into the provided Writeable. The server will // receive each of those separately and in order. diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 1065ae735f5..9714b67a4c2 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -90,7 +90,7 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; GRPCCall *_retainSelf; NSMutableDictionary *_requestHeaders; - NSMutableDictionary *_responseMetadata; + NSMutableDictionary *_allResponseMetadata; } @synthesize state = _state; @@ -122,7 +122,7 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; _requestWriter = requestWriter; _requestHeaders = [NSMutableDictionary dictionary]; - _responseMetadata = [NSMutableDictionary dictionary]; + _allResponseMetadata = [NSMutableDictionary dictionary]; } return self; } @@ -137,8 +137,8 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; _requestHeaders = [NSMutableDictionary dictionaryWithDictionary:requestHeaders]; } -- (NSDictionary *)responseMetadata { - return _responseMetadata; +- (NSDictionary *)allResponseMetadata { + return _allResponseMetadata; } #pragma mark Finish @@ -322,18 +322,18 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; // Response headers received. GRPCCall *strongSelf = weakSelf; if (strongSelf) { - [strongSelf->_responseMetadata addEntriesFromDictionary:headers]; + [strongSelf->_allResponseMetadata addEntriesFromDictionary:headers]; [strongSelf startNextRead]; } } completionHandler:^(NSError *error, NSDictionary *trailers) { GRPCCall *strongSelf = weakSelf; if (strongSelf) { - [strongSelf->_responseMetadata addEntriesFromDictionary:trailers]; + [strongSelf->_allResponseMetadata addEntriesFromDictionary:trailers]; if (error) { NSMutableDictionary *userInfo = [NSMutableDictionary dictionaryWithDictionary:error.userInfo]; - userInfo[kGRPCStatusMetadataKey] = strongSelf->_responseMetadata; + userInfo[kGRPCStatusMetadataKey] = strongSelf->_allResponseMetadata; error = [NSError errorWithDomain:error.domain code:error.code userInfo:userInfo]; } [strongSelf finishWithError:error]; diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index f23102988bd..38a720bb2de 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -168,11 +168,11 @@ static ProtoMethod *kUnaryCallMethod; } completionHandler:^(NSError *errorOrNil) { XCTAssertNotNil(errorOrNil, @"Finished without error!"); XCTAssertEqual(errorOrNil.code, 16, @"Finished with unexpected error: %@", errorOrNil); - XCTAssertEqualObjects(call.responseMetadata, errorOrNil.userInfo[kGRPCStatusMetadataKey], + XCTAssertEqualObjects(call.allResponseMetadata, errorOrNil.userInfo[kGRPCStatusMetadataKey], @"Metadata in the NSError object and call object differ."); NSString *challengeHeader = call.oauth2ChallengeHeader; XCTAssertGreaterThan(challengeHeader.length, 0, - @"No challenge in response headers %@", call.responseMetadata); + @"No challenge in response headers %@", call.allResponseMetadata); [expectation fulfill]; }]; From 2a246540249834b84faf56a154ff29cac3c8ce22 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 12 Aug 2015 17:58:38 -0700 Subject: [PATCH 13/18] More requestMetadata -> requestHeaders renaming --- src/objective-c/GRPCClient/GRPCCall.h | 8 ++++---- src/objective-c/GRPCClient/GRPCCall.m | 5 ++--- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index b82febb5877..bd4340f7241 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -57,12 +57,12 @@ extern id const kGRPCStatusMetadataKey; // These HTTP headers will be passed to the server as part of this call. Each HTTP header is a // name-value pair with string names and either string or binary values. // -// The passed dictionary has to use NSString keys, corresponding to the header names. The -// value associated to each can be a NSString object or a NSData object. E.g.: +// The passed dictionary has to use NSString keys, corresponding to the header names. The value +// associated to each can be a NSString object or a NSData object. E.g.: // -// call.requestMetadata = @{@"Authorization": @"Bearer ..."}; +// call.requestHeaders = @{@"authorization": @"Bearer ..."}; // -// call.requestMetadata[@"SomeBinaryHeader"] = someData; +// call.requestHeaders[@"my-header-bin"] = someData; // // After the call is started, modifying this won't have any effect. // diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 9714b67a4c2..0e5204d0099 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -232,11 +232,10 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; #pragma mark Send headers -// TODO(jcanizales): Rename to commitHeaders. -- (void)sendHeaders:(NSDictionary *)metadata { +- (void)sendHeaders:(NSDictionary *)headers { // TODO(jcanizales): Add error handlers for async failures [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] - initWithMetadata:metadata ?: @{} handler:nil]]]; + initWithMetadata:headers ?: @{} handler:nil]]]; } #pragma mark GRXWriteable implementation From 25884b305c4b3400cd8deb6e0708c12f0f898477 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 12 Aug 2015 18:00:47 -0700 Subject: [PATCH 14/18] nit: Comment formatting --- src/objective-c/GRPCClient/GRPCCall.h | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index bd4340f7241..83a3ce51274 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -81,19 +81,18 @@ extern id const kGRPCStatusMetadataKey; @property(atomic, readonly) NSDictionary *allResponseMetadata; // The request writer has to write NSData objects into the provided Writeable. The server will -// receive each of those separately and in order. -// A gRPC call might not complete until the request writer finishes. On the other hand, the -// request finishing doesn't necessarily make the call to finish, as the server might continue -// sending messages to the response side of the call indefinitely (depending on the semantics of -// the specific remote method called). +// receive each of those separately and in order as distinct messages. +// A gRPC call might not complete until the request writer finishes. On the other hand, the request +// finishing doesn't necessarily make the call to finish, as the server might continue sending +// messages to the response side of the call indefinitely (depending on the semantics of the +// specific remote method called). // To finish a call right away, invoke cancel. - (instancetype)initWithHost:(NSString *)host path:(NSString *)path requestsWriter:(GRXWriter *)requestsWriter NS_DESIGNATED_INITIALIZER; -// Finishes the request side of this call, notifies the server that the RPC -// should be cancelled, and finishes the response side of the call with an error -// of code CANCELED. +// Finishes the request side of this call, notifies the server that the RPC should be cancelled, and +// finishes the response side of the call with an error of code CANCELED. - (void)cancel; // TODO(jcanizales): Let specify a deadline. As a category of GRXWriter? From 0b34c89bc6f72b3cfd46c1b63840849e1df2511d Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 12 Aug 2015 20:19:20 -0700 Subject: [PATCH 15/18] Turn allResponseMetadata into *Headers and *Trailers --- src/objective-c/GRPCClient/GRPCCall+OAuth2.m | 2 +- src/objective-c/GRPCClient/GRPCCall.h | 21 ++++++++----- src/objective-c/GRPCClient/GRPCCall.m | 33 +++++++++++--------- 3 files changed, 33 insertions(+), 23 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall+OAuth2.m b/src/objective-c/GRPCClient/GRPCCall+OAuth2.m index 7f4fbacbe3e..83b0de18e32 100644 --- a/src/objective-c/GRPCClient/GRPCCall+OAuth2.m +++ b/src/objective-c/GRPCClient/GRPCCall+OAuth2.m @@ -57,7 +57,7 @@ static NSString * const kChallengeHeader = @"www-authenticate"; } - (NSString *)oauth2ChallengeHeader { - return self.allResponseMetadata[kChallengeHeader]; + return self.responseHeaders[kChallengeHeader]; } @end diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 83a3ce51274..ffa133cee48 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -71,14 +71,21 @@ extern id const kGRPCStatusMetadataKey; - (NSMutableDictionary *)requestHeaders; // nonatomic - (void)setRequestHeaders:(NSDictionary *)requestHeaders; // nonatomic, copy -// This dictionary is populated with the HTTP headers received from the server. When the RPC ends, -// the HTTP trailers received are added to the dictionary too. It has the same structure as the -// request metadata dictionary. +// This dictionary is populated with the HTTP headers received from the server. This happens before +// any response message is received from the server. It has the same structure as the request +// headers dictionary: Keys are NSString header names; names ending with the suffix "-bin" have a +// NSData value; the others have a NSString value. // -// The first time this object calls |writeValue| on the writeable passed to |startWithWriteable|, -// the |responseMetadata| dictionary already contains the response headers. When it calls -// |writesFinishedWithError|, the dictionary contains both the response headers and trailers. -@property(atomic, readonly) NSDictionary *allResponseMetadata; +// The value of this property is nil until all response headers are received, and will change before +// any of -writeValue: or -writesFinishedWithError: are sent to the writeable. +@property(atomic, readonly) NSDictionary *responseHeaders; + +// Same as responseHeaders, but populated with the HTTP trailers received from the server before the +// call finishes. +// +// The value of this property is nil until all response trailers are received, and will change +// before -writesFinishedWithError: is sent to the writeable. +@property(atomic, readonly) NSDictionary *responseTrailers; // The request writer has to write NSData objects into the provided Writeable. The server will // receive each of those separately and in order as distinct messages. diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 0e5204d0099..326def7f393 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -45,6 +45,9 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; @interface GRPCCall () +// Make them read-write. +@property(atomic, strong) NSDictionary *responseHeaders; +@property(atomic, strong) NSDictionary *responseTrailers; @end // The following methods of a C gRPC call object aren't reentrant, and thus @@ -90,7 +93,6 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; GRPCCall *_retainSelf; NSMutableDictionary *_requestHeaders; - NSMutableDictionary *_allResponseMetadata; } @synthesize state = _state; @@ -122,7 +124,6 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; _requestWriter = requestWriter; _requestHeaders = [NSMutableDictionary dictionary]; - _allResponseMetadata = [NSMutableDictionary dictionary]; } return self; } @@ -137,10 +138,6 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; _requestHeaders = [NSMutableDictionary dictionaryWithDictionary:requestHeaders]; } -- (NSDictionary *)allResponseMetadata { - return _allResponseMetadata; -} - #pragma mark Finish - (void)finishWithError:(NSError *)errorOrNil { @@ -304,35 +301,41 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; // Both handlers will eventually be called, from the network queue. Writes can start immediately // after this. -// The first one (metadataHandler), when the response headers are received. +// The first one (headersHandler), when the response headers are received. // The second one (completionHandler), whenever the RPC finishes for any reason. -- (void)invokeCallWithMetadataHandler:(void(^)(NSDictionary *))metadataHandler +- (void)invokeCallWithHeadersHandler:(void(^)(NSDictionary *))headersHandler completionHandler:(void(^)(NSError *, NSDictionary *))completionHandler { // TODO(jcanizales): Add error handlers for async failures [_wrappedCall startBatchWithOperations:@[[[GRPCOpRecvMetadata alloc] - initWithHandler:metadataHandler]]]; + initWithHandler:headersHandler]]]; [_wrappedCall startBatchWithOperations:@[[[GRPCOpRecvStatus alloc] initWithHandler:completionHandler]]]; } - (void)invokeCall { __weak GRPCCall *weakSelf = self; - [self invokeCallWithMetadataHandler:^(NSDictionary *headers) { + [self invokeCallWithHeadersHandler:^(NSDictionary *headers) { // Response headers received. GRPCCall *strongSelf = weakSelf; if (strongSelf) { - [strongSelf->_allResponseMetadata addEntriesFromDictionary:headers]; + strongSelf.responseHeaders = headers; [strongSelf startNextRead]; } } completionHandler:^(NSError *error, NSDictionary *trailers) { GRPCCall *strongSelf = weakSelf; if (strongSelf) { - [strongSelf->_allResponseMetadata addEntriesFromDictionary:trailers]; + strongSelf.responseTrailers = trailers; if (error) { - NSMutableDictionary *userInfo = - [NSMutableDictionary dictionaryWithDictionary:error.userInfo]; - userInfo[kGRPCStatusMetadataKey] = strongSelf->_allResponseMetadata; + NSMutableDictionary *userInfo = [NSMutableDictionary dictionary]; + if (error.userInfo) { + [userInfo addEntriesFromDictionary:error.userInfo]; + } + userInfo[kGRPCStatusMetadataKey] = strongSelf.responseTrailers; + // TODO(jcanizales): The C gRPC library doesn't guarantee that the headers block will be + // called before this one, so an error might end up with trailers but no headers. We + // shouldn't call finishWithError until ater both blocks are called. It is when this is done + // that we can provide a merged view of response headers and trailers in a thread-safe way. error = [NSError errorWithDomain:error.domain code:error.code userInfo:userInfo]; } [strongSelf finishWithError:error]; From 1ab2a71d077c50faceb2c945bee600452f86d3de Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 12 Aug 2015 20:32:11 -0700 Subject: [PATCH 16/18] Put both headers and trailers in the error.userInfo --- src/objective-c/GRPCClient/GRPCCall.h | 6 ++++-- src/objective-c/GRPCClient/GRPCCall.m | 13 +++++++++---- src/objective-c/tests/GRPCClientTests.m | 8 +++++--- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index ffa133cee48..16d57e81154 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -48,8 +48,10 @@ #import #import -// Key used in |NSError|'s |userInfo| dictionary to store the response metadata sent by the server. -extern id const kGRPCStatusMetadataKey; +// Keys used in |NSError|'s |userInfo| dictionary to store the response headers and trailers sent by +// the server. +extern id const kGRPCHeadersKey; +extern id const kGRPCTrailersKey; // Represents a single gRPC remote call. @interface GRPCCall : GRXWriter diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 326def7f393..ff5d1c5aaff 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -42,7 +42,8 @@ #import "private/NSDictionary+GRPC.h" #import "private/NSError+GRPC.h" -NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; +NSString * const kGRPCHeadersKey = @"io.grpc.HeadersKey"; +NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey"; @interface GRPCCall () // Make them read-write. @@ -331,11 +332,15 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; if (error.userInfo) { [userInfo addEntriesFromDictionary:error.userInfo]; } - userInfo[kGRPCStatusMetadataKey] = strongSelf.responseTrailers; + userInfo[kGRPCTrailersKey] = strongSelf.responseTrailers; // TODO(jcanizales): The C gRPC library doesn't guarantee that the headers block will be // called before this one, so an error might end up with trailers but no headers. We - // shouldn't call finishWithError until ater both blocks are called. It is when this is done - // that we can provide a merged view of response headers and trailers in a thread-safe way. + // shouldn't call finishWithError until ater both blocks are called. It is also when this is + // done that we can provide a merged view of response headers and trailers in a thread-safe + // way. + if (strongSelf.responseHeaders) { + userInfo[kGRPCHeadersKey] = strongSelf.responseHeaders; + } error = [NSError errorWithDomain:error.domain code:error.code userInfo:userInfo]; } [strongSelf finishWithError:error]; diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 38a720bb2de..06581e7599a 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -168,11 +168,13 @@ static ProtoMethod *kUnaryCallMethod; } completionHandler:^(NSError *errorOrNil) { XCTAssertNotNil(errorOrNil, @"Finished without error!"); XCTAssertEqual(errorOrNil.code, 16, @"Finished with unexpected error: %@", errorOrNil); - XCTAssertEqualObjects(call.allResponseMetadata, errorOrNil.userInfo[kGRPCStatusMetadataKey], - @"Metadata in the NSError object and call object differ."); + XCTAssertEqualObjects(call.responseHeaders, errorOrNil.userInfo[kGRPCHeadersKey], + @"Headers in the NSError object and call object differ."); + XCTAssertEqualObjects(call.responseTrailers, errorOrNil.userInfo[kGRPCTrailersKey], + @"Trailers in the NSError object and call object differ."); NSString *challengeHeader = call.oauth2ChallengeHeader; XCTAssertGreaterThan(challengeHeader.length, 0, - @"No challenge in response headers %@", call.allResponseMetadata); + @"No challenge in response headers %@", call.responseHeaders); [expectation fulfill]; }]; From 5263758ebf021d15a93c48b621bdcd3410beab5b Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 12 Aug 2015 17:59:44 -0700 Subject: [PATCH 17/18] Document not to modify headers after start --- src/objective-c/GRPCClient/GRPCCall.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 16d57e81154..4eda499b1a8 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -66,7 +66,7 @@ extern id const kGRPCTrailersKey; // // call.requestHeaders[@"my-header-bin"] = someData; // -// After the call is started, modifying this won't have any effect. +// After the call is started, trying to modify this property is an error. // // For convenience, the property is initialized to an empty NSMutableDictionary, and the setter // accepts (and copies) both mutable and immutable dictionaries. From 25c21d8b2b04848667c6325d2bfe6ddc7674176e Mon Sep 17 00:00:00 2001 From: nmittler Date: Thu, 13 Aug 2015 09:53:27 -0700 Subject: [PATCH 18/18] Fixing type-o --- doc/connectivity-semantics-and-api.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/connectivity-semantics-and-api.md b/doc/connectivity-semantics-and-api.md index 930dff265fe..5427900394b 100644 --- a/doc/connectivity-semantics-and-api.md +++ b/doc/connectivity-semantics-and-api.md @@ -38,7 +38,7 @@ because the server is not yet available), the channel may spend increasingly large amounts of time in this state. IDLE: This is the state where the channel is not even trying to create a -connection because of a lack of new or pending RPCs. New channels MAY be created +connection because of a lack of new or pending RPCs. New RPCs MAY be created in this state. Any attempt to start an RPC on the channel will push the channel out of this state to connecting. When there has been no RPC activity on a channel for a specified IDLE_TIMEOUT, i.e., no new or pending (active) RPCs for this