From 9855a34e1af51211764fc96485f79ce7692dd1ce Mon Sep 17 00:00:00 2001 From: Eric Anderson Date: Mon, 11 May 2015 09:22:47 -0700 Subject: [PATCH 01/21] Add Connectivity doc This is a faithful representation of the current version of the Google Doc with only formatting changes. --- doc/connectivity-semantics-and-api.md | 122 ++++++++++++++++++++++++++ 1 file changed, 122 insertions(+) create mode 100644 doc/connectivity-semantics-and-api.md diff --git a/doc/connectivity-semantics-and-api.md b/doc/connectivity-semantics-and-api.md new file mode 100644 index 00000000000..842ff8adc18 --- /dev/null +++ b/doc/connectivity-semantics-and-api.md @@ -0,0 +1,122 @@ +gRPC Connectivity Semantics and API +=================================== + +This document describes the connectivity semantics for gRPC channels and the +corresponding impact on RPCs. We then discuss an API. + +States of Connectivity +---------------------- + +gRPC Channels provide the abstraction over which clients can communicate with +servers.The client-side channel object can be constructed using little more +than a DNS name. Channels encapsulate a range of functionality including name +resolution, establishing a TCP connection (with retries and backoff) and TLS +handshakes. Channels can also handle errors on established connections and +reconnect, or in the case of HTTP/2 GO_AWAY, re-resolve the name and reconnect. + +To hide the details of all this activity from the user of the gRPC API (i.e., +application code) while exposing meaningful information about the state of a +channel, we use a state machine with four states, defined below: + +CONNECTING: The channel is trying to establish a connection and is waiting to +make progress on one of the steps involved in name resolution, TCP connection +establishment or TLS handshake. This is the initial state for all channels upon +creation. + +READY: The channel has successfully established a connection all the way +through TLS handshake (or equivalent) and all subsequent attempt to communicate +have succeeded (or are pending without any known failure ). + +TRANSIENT_FAILURE: There has been some transient failure (such as a TCP 3-way +handshake timing out or a socket error). Channels in this state will eventually +switch to the CONNECTING state and try to establish a connection again. Since +retries are done with exponential backoff, channels that fail to connect will +start out spending very little time in this state but as the attempts fail +repeatedly, the channel will spend increasingly large amounts of time in this +state. For many non-fatal failures (e.g., TCP connection attempts timing out +because the server is not yet available), the channel may be stuck in this +state for an indefinitely large amount of time. + +FATAL_FAILURE: There has been a fatal failure and the channel will never +attempt to establish a connection again. (e.g., a server presenting an invalid +TLS certificate) + +Channels that enter this state never leave this state. + +The following table lists the legal transitions from one state to another and +corresponding reasons. Empty cells denote disallowed transitions. + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
From/ToCONNECTINGREADYTRANSIENT_FAILUREFATAL_FAILURE
CONNECTINGIncremental progress during connection establishmentAll steps needed to establish a connection succeededAny failure in any of the steps needed to establish connectionFatal failure encountered while attempting a connection.
READYIncremental successful communication on established channel.Any failure encountered while expecting successful communication on + established channel.
TRANSIENT_FAILUREWait time required to implement (exponential) backoff is over.
FATAL_FAILURE
+ + +Channel State API +----------------- + +All gRPC libraries will expose a channel-level API method to poll the current +state of a channel. In C++, this method is called GetCurrentState and returns +an enum for one of the four legal states. + +All libraries should also expose an API that enables the application (user of +the gRPC API) to be notified when the channel state changes. Since state +changes can be rapid and race with any such notification, the notification +should just inform the user that some state change has happened, leaving it to +the user to poll the channel for the current state. + +The synchronous version of this API is: + +```cpp +bool WaitForStateChange(gpr_timespec deadline, ChannelState source_state); +``` + +which returns true when the state changes to something other than the +source_state and false if the deadline expires. Asynchronous and futures based +APIs should have a corresponding method that allows the application to be +notified when the state of a channel changes. + +Note that a notification is delivered every time there is a transition from any +state to any *other* state. On the other hand the rules for legal state +transition, require a transition from CONNECTING to TRANSIENT_FAILURE and back +to CONNECTING for every recoverable failure, even if the corresponding +exponential backoff requires no wait before retry. The combined effect is that +the application may receive state change notifications that appear spurious. +e.g., an application waiting for state changes on a channel that is CONNECTING +may receive a state change notification but find the channel in the same +CONNECTING state on polling for current state because the channel may have +spent infinitesimally small amount of time in the TRANSIENT_FAILURE state. From eb741d1772101852e8b5ff8e3d764691e2b62ab8 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Thu, 11 Jun 2015 19:09:15 -0700 Subject: [PATCH 02/21] fix native callback signature to match with C# --- src/csharp/ext/grpc_csharp_ext.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/csharp/ext/grpc_csharp_ext.c b/src/csharp/ext/grpc_csharp_ext.c index 59b8993ad3d..83372182552 100644 --- a/src/csharp/ext/grpc_csharp_ext.c +++ b/src/csharp/ext/grpc_csharp_ext.c @@ -65,8 +65,6 @@ grpc_byte_buffer *string_to_byte_buffer(const char *buffer, size_t len) { return bb; } -typedef void(GPR_CALLTYPE *callback_funcptr)(gpr_int32 success, void *batch_context); - /* * Helper to maintain lifetime of batch op inputs and store batch op outputs. */ @@ -731,10 +729,12 @@ GPR_EXPORT void GPR_CALLTYPE grpcsharp_redirect_log(grpcsharp_log_func func) { gpr_set_log_function(grpcsharp_log_handler); } +typedef void(GPR_CALLTYPE *test_callback_funcptr)(gpr_int32 success); + /* For testing */ GPR_EXPORT void GPR_CALLTYPE -grpcsharp_test_callback(callback_funcptr callback) { - callback(1, NULL); +grpcsharp_test_callback(test_callback_funcptr callback) { + callback(1); } /* For testing */ From bc17b3f0aee4b071a8bcbba90ad77764d39e80f7 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Tue, 9 Jun 2015 08:22:51 -0700 Subject: [PATCH 03/21] fetch PR refspec for docker jenkins builds --- tools/jenkins/run_jenkins.sh | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/tools/jenkins/run_jenkins.sh b/tools/jenkins/run_jenkins.sh index a95819af601..3fa5914bc11 100755 --- a/tools/jenkins/run_jenkins.sh +++ b/tools/jenkins/run_jenkins.sh @@ -41,9 +41,17 @@ if [ "$platform" == "linux" ] then echo "building $language on Linux" + if [ "$ghprbPullId" != "" ] + then + # if we are building a pull request, grab corresponding refs. + FETCH_PULL_REQUEST_CMD="&& git fetch $GIT_URL +refs/pull/$ghprbPullId:refs/remotes/origin/pr/$ghprbPullId" + fi + # Run tests inside docker docker run grpc/grpc_jenkins_slave bash -c -l "git clone --recursive $GIT_URL /var/local/git/grpc \ - && cd /var/local/git/grpc && git checkout -f $GIT_COMMIT \ + && cd /var/local/git/grpc \ + $FETCH_PULL_REQUEST_CMD \ + && git checkout -f $GIT_COMMIT \ && git submodule update \ && nvm use 0.12 \ && rvm use ruby-2.1 \ From c3dd3bfb85c9456765814093a7dc9661950ba795 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 12 Jun 2015 09:18:49 -0700 Subject: [PATCH 04/21] fix fetch to actually work --- tools/jenkins/run_jenkins.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/jenkins/run_jenkins.sh b/tools/jenkins/run_jenkins.sh index 3fa5914bc11..a754015f400 100755 --- a/tools/jenkins/run_jenkins.sh +++ b/tools/jenkins/run_jenkins.sh @@ -44,7 +44,7 @@ then if [ "$ghprbPullId" != "" ] then # if we are building a pull request, grab corresponding refs. - FETCH_PULL_REQUEST_CMD="&& git fetch $GIT_URL +refs/pull/$ghprbPullId:refs/remotes/origin/pr/$ghprbPullId" + FETCH_PULL_REQUEST_CMD="&& git fetch $GIT_URL refs/pull/$ghprbPullId/merge refs/pull/$ghprbPullId/head" fi # Run tests inside docker From b66f19f1e093ded75f150bf1d4190ce338ece918 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 12 Jun 2015 11:02:16 -0700 Subject: [PATCH 05/21] Update connectivity-semantics-and-api.md --- doc/connectivity-semantics-and-api.md | 28 +++++++++++++++++++-------- 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/doc/connectivity-semantics-and-api.md b/doc/connectivity-semantics-and-api.md index 842ff8adc18..1e388466bfc 100644 --- a/doc/connectivity-semantics-and-api.md +++ b/doc/connectivity-semantics-and-api.md @@ -20,7 +20,7 @@ channel, we use a state machine with four states, defined below: CONNECTING: The channel is trying to establish a connection and is waiting to make progress on one of the steps involved in name resolution, TCP connection -establishment or TLS handshake. This is the initial state for all channels upon +establishment or TLS handshake. This may be used as the initial state for channels upon creation. READY: The channel has successfully established a connection all the way @@ -37,11 +37,10 @@ state. For many non-fatal failures (e.g., TCP connection attempts timing out because the server is not yet available), the channel may be stuck in this state for an indefinitely large amount of time. -FATAL_FAILURE: There has been a fatal failure and the channel will never -attempt to establish a connection again. (e.g., a server presenting an invalid -TLS certificate) +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 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 period, channels that are READY or CONNECTING switch to IDLE. Additionaly, channels that receive a GO_AWAY when there are no active or pending RPCs should also switch to IDLE to avoid connection overload at servers that are attempting to shed connections. We will use a default IDLE_TIMEOUT of 300 seconds (5 minutes). -Channels that enter this state never leave this state. +SHUTDOWN: This channel has started shutting down. Any new RPCs should fail immediately. Pending RPCs may continue running till the application cancels them. Channels may enter this state either because the application explicitly requested a shutdown or if a non-recoverable error has happened during attempts to connect communicate . (As of 6/12/2015, there are no known errors (while connecting or communicating) that are classified as non-recoverable) +Channels that enter this state never leave this state. The following table lists the legal transitions from one state to another and corresponding reasons. Empty cells denote disallowed transitions. @@ -52,14 +51,16 @@ corresponding reasons. Empty cells denote disallowed transitions. CONNECTING READY TRANSIENT_FAILURE - FATAL_FAILURE + IDLE + SHUTDOWN CONNECTING Incremental progress during connection establishment All steps needed to establish a connection succeeded Any failure in any of the steps needed to establish connection - Fatal failure encountered while attempting a connection. + No RPC activity on channel for IDLE_TIMEOUT + Shutdown triggered by application. READY @@ -67,7 +68,8 @@ corresponding reasons. Empty cells denote disallowed transitions. Incremental successful communication on established channel. Any failure encountered while expecting successful communication on established channel. - + No RPC activity on channel for IDLE_TIMEOUT
OR
upon receiving a GO_AWAY while there are no pending RPCs. + Shutdown triggered by application. TRANSIENT_FAILURE @@ -75,6 +77,15 @@ corresponding reasons. Empty cells denote disallowed transitions. + Shutdown triggered by application. + + + IDLE + Any new RPC activity on the channel + + + + Shutdown triggered by application. FATAL_FAILURE @@ -82,6 +93,7 @@ corresponding reasons. Empty cells denote disallowed transitions. + From 56514def9e9d00d82e5dd5855d41e9aef7d52a12 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Fri, 12 Jun 2015 11:22:41 -0700 Subject: [PATCH 06/21] Update connectivity-semantics-and-api.md --- doc/connectivity-semantics-and-api.md | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) diff --git a/doc/connectivity-semantics-and-api.md b/doc/connectivity-semantics-and-api.md index 1e388466bfc..ef4986d9405 100644 --- a/doc/connectivity-semantics-and-api.md +++ b/doc/connectivity-semantics-and-api.md @@ -34,12 +34,25 @@ retries are done with exponential backoff, channels that fail to connect will start out spending very little time in this state but as the attempts fail repeatedly, the channel will spend increasingly large amounts of time in this state. For many non-fatal failures (e.g., TCP connection attempts timing out -because the server is not yet available), the channel may be stuck in this -state for an indefinitely large amount of time. - -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 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 period, channels that are READY or CONNECTING switch to IDLE. Additionaly, channels that receive a GO_AWAY when there are no active or pending RPCs should also switch to IDLE to avoid connection overload at servers that are attempting to shed connections. We will use a default IDLE_TIMEOUT of 300 seconds (5 minutes). - -SHUTDOWN: This channel has started shutting down. Any new RPCs should fail immediately. Pending RPCs may continue running till the application cancels them. Channels may enter this state either because the application explicitly requested a shutdown or if a non-recoverable error has happened during attempts to connect communicate . (As of 6/12/2015, there are no known errors (while connecting or communicating) that are classified as non-recoverable) +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 +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 +period, channels that are READY or CONNECTING switch to IDLE. Additionaly, +channels that receive a GO_AWAY when there are no active or pending RPCs should +also switch to IDLE to avoid connection overload at servers that are attempting +to shed connections. We will use a default IDLE_TIMEOUT of 300 seconds (5 minutes). + +SHUTDOWN: This channel has started shutting down. Any new RPCs should fail +immediately. Pending RPCs may continue running till the application cancels them. +Channels may enter this state either because the application explicitly requested +a shutdown or if a non-recoverable error has happened during attempts to connect +communicate . (As of 6/12/2015, there are no known errors (while connecting or +communicating) that are classified as non-recoverable) Channels that enter this state never leave this state. The following table lists the legal transitions from one state to another and From 0ee84dc10feb0eeddc304d18638bbcf3faff8f04 Mon Sep 17 00:00:00 2001 From: Jan Tattermusch Date: Fri, 12 Jun 2015 11:22:49 -0700 Subject: [PATCH 07/21] only keep docker containers for tests that failed --- tools/jenkins/run_jenkins.sh | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tools/jenkins/run_jenkins.sh b/tools/jenkins/run_jenkins.sh index a754015f400..57cec774248 100755 --- a/tools/jenkins/run_jenkins.sh +++ b/tools/jenkins/run_jenkins.sh @@ -47,15 +47,29 @@ then FETCH_PULL_REQUEST_CMD="&& git fetch $GIT_URL refs/pull/$ghprbPullId/merge refs/pull/$ghprbPullId/head" fi + # Make sure the CID file is gone. + rm -f docker.cid + # Run tests inside docker - docker run grpc/grpc_jenkins_slave bash -c -l "git clone --recursive $GIT_URL /var/local/git/grpc \ + docker run --cidfile=docker.cid grpc/grpc_jenkins_slave bash -c -l "git clone --recursive $GIT_URL /var/local/git/grpc \ && cd /var/local/git/grpc \ $FETCH_PULL_REQUEST_CMD \ && git checkout -f $GIT_COMMIT \ && git submodule update \ && nvm use 0.12 \ && rvm use ruby-2.1 \ - && tools/run_tests/run_tests.py -t -l $language" + && tools/run_tests/run_tests.py -t -l $language" || DOCKER_FAILED="true" + + DOCKER_CID=`cat docker.cid` + if [ "$DOCKER_FAILED" == "" ] + then + echo "Docker finished successfully, deleting the container $DOCKER_CID" + docker rm $DOCKER_CID + else + echo "Docker exited with failure, keeping container $DOCKER_CID." + echo "You can SSH to the worker and use 'docker start CID' and 'docker exec -i -t CID bash' to debug the problem." + fi + elif [ "$platform" == "windows" ] then echo "building $language on Windows" From d666fd068bfcd37461c73354f5dc2fdc2b8f72b0 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 12 Jun 2015 18:59:11 -0700 Subject: [PATCH 08/21] Cleanup GRPCClientTests.m --- src/objective-c/tests/GRPCClientTests.m | 69 +++++++++++++------------ 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 713ea2848a1..4f4fe591e84 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -43,24 +43,38 @@ // These are a few tests similar to InteropTests, but which use the generic gRPC client (GRPCCall) // rather than a generated proto library on top of it. +static NSString * const kHostAddress = @"grpc-test.sandbox.google.com"; +static NSString * const kPackage = @"grpc.testing"; +static NSString * const kService = @"TestService"; + +static GRPCMethodName *kInexistentMethod; +static GRPCMethodName *kEmptyCallMethod; +static GRPCMethodName *kUnaryCallMethod; + @interface GRPCClientTests : XCTestCase @end @implementation GRPCClientTests +- (void)setUp { + // This method isn't implemented by the remote server. + kInexistentMethod = [[GRPCMethodName alloc] initWithPackage:kPackage + interface:kService + method:@"Inexistent"]; + kEmptyCallMethod = [[GRPCMethodName alloc] initWithPackage:kPackage + interface:kService + method:@"EmptyCall"]; + kUnaryCallMethod = [[GRPCMethodName alloc] initWithPackage:kPackage + interface:kService + method:@"UnaryCall"]; +} + - (void)testConnectionToRemoteServer { __weak XCTestExpectation *expectation = [self expectationWithDescription:@"Server reachable."]; - // This method isn't implemented by the remote server. - GRPCMethodName *method = [[GRPCMethodName alloc] initWithPackage:@"grpc.testing" - interface:@"TestService" - method:@"Nonexistent"]; - - id requestsWriter = [GRXWriter writerWithValue:[NSData data]]; - - GRPCCall *call = [[GRPCCall alloc] initWithHost:@"grpc-test.sandbox.google.com" - method:method - requestsWriter:requestsWriter]; + GRPCCall *call = [[GRPCCall alloc] initWithHost:kHostAddress + method:kInexistentMethod + requestsWriter:[GRXWriter writerWithValue:[NSData data]]]; id responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { XCTFail(@"Received unexpected response: %@", value); @@ -80,15 +94,9 @@ __weak XCTestExpectation *response = [self expectationWithDescription:@"Empty response received."]; __weak XCTestExpectation *completion = [self expectationWithDescription:@"Empty RPC completed."]; - GRPCMethodName *method = [[GRPCMethodName alloc] initWithPackage:@"grpc.testing" - interface:@"TestService" - method:@"EmptyCall"]; - - id requestsWriter = [GRXWriter writerWithValue:[NSData data]]; - - GRPCCall *call = [[GRPCCall alloc] initWithHost:@"grpc-test.sandbox.google.com" - method:method - requestsWriter:requestsWriter]; + GRPCCall *call = [[GRPCCall alloc] initWithHost:kHostAddress + method:kEmptyCallMethod + requestsWriter:[GRXWriter writerWithValue:[NSData data]]]; id responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { XCTAssertNotNil(value, @"nil value received as response."); @@ -105,34 +113,27 @@ } - (void)testSimpleProtoRPC { - __weak XCTestExpectation *response = [self expectationWithDescription:@"Response received."]; - __weak XCTestExpectation *expectedResponse = - [self expectationWithDescription:@"Expected response."]; + __weak XCTestExpectation *response = [self expectationWithDescription:@"Expected response."]; __weak XCTestExpectation *completion = [self expectationWithDescription:@"RPC completed."]; - GRPCMethodName *method = [[GRPCMethodName alloc] initWithPackage:@"grpc.testing" - interface:@"TestService" - method:@"UnaryCall"]; - - RMTSimpleRequest *request = [[RMTSimpleRequest alloc] init]; + RMTSimpleRequest *request = [RMTSimpleRequest message]; request.responseSize = 100; request.fillUsername = YES; request.fillOauthScope = YES; id requestsWriter = [GRXWriter writerWithValue:[request data]]; - GRPCCall *call = [[GRPCCall alloc] initWithHost:@"grpc-test.sandbox.google.com" - method:method + GRPCCall *call = [[GRPCCall alloc] initWithHost:kHostAddress + method:kUnaryCallMethod requestsWriter:requestsWriter]; id responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { XCTAssertNotNil(value, @"nil value received as response."); - [response fulfill]; XCTAssertGreaterThan(value.length, 0, @"Empty response received."); - RMTSimpleResponse *response = [RMTSimpleResponse parseFromData:value error:NULL]; + RMTSimpleResponse *responseProto = [RMTSimpleResponse parseFromData:value error:NULL]; // We expect empty strings, not nil: - XCTAssertNotNil(response.username, @"Response's username is nil."); - XCTAssertNotNil(response.oauthScope, @"Response's OAuth scope is nil."); - [expectedResponse fulfill]; + XCTAssertNotNil(responseProto.username, @"Response's username is nil."); + XCTAssertNotNil(responseProto.oauthScope, @"Response's OAuth scope is nil."); + [response fulfill]; } completionHandler:^(NSError *errorOrNil) { XCTAssertNil(errorOrNil, @"Finished with unexpected error: %@", errorOrNil); [completion fulfill]; From d7981253de93fcfd3666a3e3bd1714b2f66a448b Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 12 Jun 2015 19:15:18 -0700 Subject: [PATCH 09/21] Add test to set and read headers. Flaky!! --- src/objective-c/tests/GRPCClientTests.m | 31 +++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 4f4fe591e84..917e637d68b 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -144,4 +144,35 @@ static GRPCMethodName *kUnaryCallMethod; [self waitForExpectationsWithTimeout:2. handler:nil]; } +- (void)testMetadata { + __weak XCTestExpectation *expectation = [self expectationWithDescription:@"RPC unauthorized."]; + + RMTSimpleRequest *request = [RMTSimpleRequest message]; + request.fillUsername = YES; + request.fillOauthScope = YES; + id requestsWriter = [GRXWriter writerWithValue:[request data]]; + + GRPCCall *call = [[GRPCCall alloc] initWithHost:kHostAddress + method:kUnaryCallMethod + requestsWriter:requestsWriter]; + + call.requestMetadata = [NSMutableDictionary dictionaryWithDictionary: + @{@"Authorization": @"Bearer bogusToken"}]; + + id responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { + XCTFail(@"Received unexpected response: %@", value); + } completionHandler:^(NSError *errorOrNil) { + XCTAssertNotNil(errorOrNil, @"Finished without error!"); + XCTAssertEqual(errorOrNil.code, 16, @"Finished with unexpected error: %@", errorOrNil); + NSString *challengeHeader = call.responseMetadata[@"www-authenticate"][0]; + XCTAssertGreaterThan(challengeHeader.length, 0, + @"No challenge in response headers %@", call.responseMetadata); + [expectation fulfill]; + }]; + + [call startWithWriteable:responsesWriteable]; + + [self waitForExpectationsWithTimeout:2. handler:nil]; +} + @end From 544963e18a9a498fc668153c9deb63d2d22c21a2 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 12 Jun 2015 19:46:27 -0700 Subject: [PATCH 10/21] Let set NSDictionary as headers, and init the property (not nil) --- src/objective-c/GRPCClient/GRPCCall.h | 44 +++++++++++++++---------- src/objective-c/GRPCClient/GRPCCall.m | 14 ++++++++ src/objective-c/tests/GRPCClientTests.m | 3 +- 3 files changed, 41 insertions(+), 20 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 81b409b9ffe..133ac4b75a6 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -40,34 +40,42 @@ // The gRPC protocol is an RPC protocol on top of HTTP2. // -// While the most common type of RPC receives only one request message and -// returns only one response message, the protocol also supports RPCs that -// return multiple individual messages in a streaming fashion, RPCs that -// accept a stream of request messages, or RPCs with both streaming requests -// and responses. +// While the most common type of RPC receives only one request message and returns only one response +// message, the protocol also supports RPCs that return multiple individual messages in a streaming +// fashion, RPCs that accept a stream of request messages, or RPCs with both streaming requests and +// responses. // -// Conceptually, each gRPC call consists of a bidirectional stream of binary -// messages, with RPCs of the "non-streaming type" sending only one message in -// the corresponding direction (the protocol doesn't make any distinction). +// Conceptually, each gRPC call consists of a bidirectional stream of binary messages, with RPCs of +// the "non-streaming type" sending only one message in the corresponding direction (the protocol +// doesn't make any distinction). // -// Each RPC uses a different HTTP2 stream, and thus multiple simultaneous RPCs -// can be multiplexed transparently on the same TCP connection. +// Each RPC uses a different HTTP2 stream, and thus multiple simultaneous RPCs can be multiplexed +// transparently on the same TCP connection. @interface GRPCCall : NSObject -// These HTTP2 headers will be passed to the server as part of this call. Each -// HTTP2 header is a name-value pair with string names and either string or binary values. +// 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.: // -// call.requestMetadata = @{ -// @"Authorization": @"Bearer ...", -// @"SomeBinaryHeader": someData -// }; +// call.requestMetadata = @{@"Authorization": @"Bearer ..."}; +// +// call.requestMetadata[@"SomeBinaryHeader"] = someData; // // After the call is started, modifying this won't have any effect. -@property(nonatomic, readwrite) NSMutableDictionary *requestMetadata; +// +// 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 -// This isn't populated until the first event is delivered to the handler. +// 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. +// +// 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; // The request writer has to write NSData objects into the provided Writeable. The server will diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index a4a0ddb3246..92084d9d4ff 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -82,6 +82,8 @@ // correct ordering. GRPCDelegateWrapper *_responseWriteable; id _requestWriter; + + NSMutableDictionary *_requestMetadata; } @synthesize state = _state; @@ -116,10 +118,22 @@ _callQueue = dispatch_queue_create("org.grpc.call", NULL); _requestWriter = requestWriter; + + _requestMetadata = [NSMutableDictionary dictionary]; } return self; } +#pragma mark Metadata + +- (NSMutableDictionary *)requestMetadata { + return _requestMetadata; +} + +- (void)setRequestMetadata:(NSDictionary *)requestMetadata { + _requestMetadata = [NSMutableDictionary dictionaryWithDictionary:requestMetadata]; +} + #pragma mark Finish - (void)finishWithError:(NSError *)errorOrNil { diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index 917e637d68b..ae7aba811a6 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -156,8 +156,7 @@ static GRPCMethodName *kUnaryCallMethod; method:kUnaryCallMethod requestsWriter:requestsWriter]; - call.requestMetadata = [NSMutableDictionary dictionaryWithDictionary: - @{@"Authorization": @"Bearer bogusToken"}]; + call.requestMetadata[@"Authorization"] = @"Bearer bogusToken"; id responsesWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) { XCTFail(@"Received unexpected response: %@", value); From f3a4f2cee922d31a6eca9f2d9bc38312d5bffec7 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 12 Jun 2015 22:12:50 -0700 Subject: [PATCH 11/21] Merge trailers into responseMetadata. Add it to NSError. --- src/objective-c/GRPCClient/GRPCCall.h | 17 +++++----- src/objective-c/GRPCClient/GRPCCall.m | 31 +++++++++++++------ .../GRPCClient/private/GRPCWrappedCall.h | 2 +- .../GRPCClient/private/GRPCWrappedCall.m | 29 ++++++++--------- .../GRPCClient/private/NSDictionary+GRPC.h | 3 +- .../GRPCClient/private/NSDictionary+GRPC.m | 4 +++ .../GRPCClient/private/NSError+GRPC.h | 16 +++------- .../GRPCClient/private/NSError+GRPC.m | 11 +++---- 8 files changed, 63 insertions(+), 50 deletions(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index 133ac4b75a6..c01d1f235bc 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -31,13 +31,6 @@ * */ -#import -#import - -@class GRPCMethodName; - -@class GRPCCall; - // The gRPC protocol is an RPC protocol on top of HTTP2. // // While the most common type of RPC receives only one request message and returns only one response @@ -51,6 +44,16 @@ // // Each RPC uses a different HTTP2 stream, and thus multiple simultaneous RPCs can be multiplexed // transparently on the same TCP connection. + +#import +#import + +@class GRPCMethodName; + +// Key used in |NSError|'s |userInfo| dictionary to store the response metadata sent by the server. +extern id const kGRPCStatusMetadataKey; + +// Represents a single gRPC remote call. @interface GRPCCall : NSObject // These HTTP headers will be passed to the server as part of this call. Each HTTP header is a diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m index 92084d9d4ff..279ef935d50 100644 --- a/src/objective-c/GRPCClient/GRPCCall.m +++ b/src/objective-c/GRPCClient/GRPCCall.m @@ -46,9 +46,9 @@ #import "private/NSDictionary+GRPC.h" #import "private/NSError+GRPC.h" +NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey"; + @interface GRPCCall () -// Makes it readwrite. -@property(atomic, strong) NSDictionary *responseMetadata; @end // The following methods of a C gRPC call object aren't reentrant, and thus @@ -84,6 +84,7 @@ id _requestWriter; NSMutableDictionary *_requestMetadata; + NSMutableDictionary *_responseMetadata; } @synthesize state = _state; @@ -120,6 +121,7 @@ _requestWriter = requestWriter; _requestMetadata = [NSMutableDictionary dictionary]; + _responseMetadata = [NSMutableDictionary dictionary]; } return self; } @@ -134,6 +136,10 @@ _requestMetadata = [NSMutableDictionary dictionaryWithDictionary:requestMetadata]; } +- (NSDictionary *)responseMetadata { + return _responseMetadata; +} + #pragma mark Finish - (void)finishWithError:(NSError *)errorOrNil { @@ -291,7 +297,7 @@ // The first one (metadataHandler), when the response headers are received. // The second one (completionHandler), whenever the RPC finishes for any reason. - (void)invokeCallWithMetadataHandler:(void(^)(NSDictionary *))metadataHandler - completionHandler:(void(^)(NSError *))completionHandler { + completionHandler:(void(^)(NSError *, NSDictionary *))completionHandler { // TODO(jcanizales): Add error handlers for async failures [_wrappedCall startBatchWithOperations:@[[[GRPCOpRecvMetadata alloc] initWithHandler:metadataHandler]]]; @@ -301,16 +307,23 @@ - (void)invokeCall { __weak GRPCCall *weakSelf = self; - [self invokeCallWithMetadataHandler:^(NSDictionary *metadata) { - // Response metadata received. + [self invokeCallWithMetadataHandler:^(NSDictionary *headers) { + // Response headers received. GRPCCall *strongSelf = weakSelf; if (strongSelf) { - strongSelf.responseMetadata = metadata; + [strongSelf->_responseMetadata addEntriesFromDictionary:headers]; [strongSelf startNextRead]; } - } completionHandler:^(NSError *error) { - // TODO(jcanizales): Merge HTTP2 trailers into response metadata. - [weakSelf finishWithError:error]; + } completionHandler:^(NSError *error, NSDictionary *trailers) { + GRPCCall *strongSelf = weakSelf; + if (strongSelf) { + [strongSelf->_responseMetadata addEntriesFromDictionary:trailers]; + + NSMutableDictionary *userInfo = [NSMutableDictionary dictionaryWithDictionary:error.userInfo]; + userInfo[kGRPCStatusMetadataKey] = strongSelf->_responseMetadata; + error = [NSError errorWithDomain:error.domain code:error.code userInfo:userInfo]; + [strongSelf finishWithError:error]; + } }]; // Now that the RPC has been initiated, request writes can start. [_requestWriter startWithWriteable:self]; diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h index 91cd703faf0..4deeec04754 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h @@ -79,7 +79,7 @@ typedef void(^GRPCCompletionHandler)(NSDictionary *); @interface GRPCOpRecvStatus : NSObject -- (instancetype)initWithHandler:(void(^)(NSError *))handler NS_DESIGNATED_INITIALIZER; +- (instancetype)initWithHandler:(void(^)(NSError *, NSDictionary *))handler NS_DESIGNATED_INITIALIZER; @end diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m index 9bc46930b4d..ea482b29ef5 100644 --- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m +++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m @@ -165,9 +165,7 @@ } - (void)finish { - NSDictionary *metadata = [NSDictionary - grpc_dictionaryFromMetadata:_recvInitialMetadata.metadata - count:_recvInitialMetadata.count]; + NSDictionary *metadata = [NSDictionary grpc_dictionaryFromMetadataArray:_recvInitialMetadata]; if (_handler) { _handler(metadata); } @@ -209,41 +207,44 @@ @end @implementation GRPCOpRecvStatus{ - void(^_handler)(NSError *); + void(^_handler)(NSError *, NSDictionary *); + grpc_status_code _statusCode; + char *_details; size_t _detailsCapacity; - grpc_status _status; + grpc_metadata_array _metadata; } - (instancetype) init { return [self initWithHandler:nil]; } -- (instancetype) initWithHandler:(void (^)(NSError *))handler { +- (instancetype) initWithHandler:(void (^)(NSError *, NSDictionary *))handler { if (self = [super init]) { _handler = handler; - grpc_metadata_array_init(&_status.metadata); + grpc_metadata_array_init(&_metadata); } return self; } - (void)getOp:(grpc_op *)op { op->op = GRPC_OP_RECV_STATUS_ON_CLIENT; - op->data.recv_status_on_client.status = &_status.status; - op->data.recv_status_on_client.status_details = &_status.details; + op->data.recv_status_on_client.status = &_statusCode; + op->data.recv_status_on_client.status_details = &_details; op->data.recv_status_on_client.status_details_capacity = &_detailsCapacity; - op->data.recv_status_on_client.trailing_metadata = &_status.metadata; + op->data.recv_status_on_client.trailing_metadata = &_metadata; } - (void)finish { if (_handler) { - NSError *error = [NSError grpc_errorFromStatus:&_status]; - _handler(error); + NSError *error = [NSError grpc_errorFromStatusCode:_statusCode details:_details]; + NSDictionary *trailers = [NSDictionary grpc_dictionaryFromMetadataArray:_metadata]; + _handler(error, trailers); } } - (void)dealloc { - grpc_metadata_array_destroy(&_status.metadata); - gpr_free(_status.details); + grpc_metadata_array_destroy(&_metadata); + gpr_free(_details); } @end diff --git a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.h b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.h index 622fddcf8e9..7335681ac77 100644 --- a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.h +++ b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.h @@ -35,6 +35,7 @@ #include @interface NSDictionary (GRPC) -+ (instancetype)grpc_dictionaryFromMetadata:(struct grpc_metadata *)entries count:(size_t)count; ++ (instancetype)grpc_dictionaryFromMetadataArray:(grpc_metadata_array)array; ++ (instancetype)grpc_dictionaryFromMetadata:(grpc_metadata *)entries count:(size_t)count; - (grpc_metadata *)grpc_metadataArray; @end diff --git a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m index e14e503ae0a..af8f4a2510e 100644 --- a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m +++ b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m @@ -98,6 +98,10 @@ #pragma mark Category for metadata arrays @implementation NSDictionary (GRPC) ++ (instancetype)grpc_dictionaryFromMetadataArray:(grpc_metadata_array)array { + return [self grpc_dictionaryFromMetadata:array.metadata count:array.count]; +} + + (instancetype)grpc_dictionaryFromMetadata:(grpc_metadata *)entries count:(size_t)count { NSMutableDictionary *metadata = [NSMutableDictionary dictionaryWithCapacity:count]; for (grpc_metadata *entry = entries; entry < entries + count; entry++) { diff --git a/src/objective-c/GRPCClient/private/NSError+GRPC.h b/src/objective-c/GRPCClient/private/NSError+GRPC.h index 6577d34e807..e7127912713 100644 --- a/src/objective-c/GRPCClient/private/NSError+GRPC.h +++ b/src/objective-c/GRPCClient/private/NSError+GRPC.h @@ -32,6 +32,7 @@ */ #import +#include // TODO(jcanizales): Make the domain string public. extern NSString *const kGRPCErrorDomain; @@ -56,17 +57,8 @@ typedef NS_ENUM(NSInteger, GRPCErrorCode) { GRPCErrorCodeDataLoss = 15 }; -// TODO(jcanizales): This is conflating trailing metadata with Status details. Fix it once there's -// a decision on how to codify Status. -#include -typedef struct grpc_status { - grpc_status_code status; - char *details; - grpc_metadata_array metadata; -} grpc_status; - @interface NSError (GRPC) -// Returns nil if the status is OK. Otherwise, a NSError whose code is one of -// GRPCErrorCode and whose domain is kGRPCErrorDomain. -+ (instancetype)grpc_errorFromStatus:(struct grpc_status *)status; +// Returns nil if the status code is OK. Otherwise, a NSError whose code is one of |GRPCErrorCode| +// and whose domain is |kGRPCErrorDomain|. ++ (instancetype)grpc_errorFromStatusCode:(grpc_status_code)statusCode details:(char *)details; @end diff --git a/src/objective-c/GRPCClient/private/NSError+GRPC.m b/src/objective-c/GRPCClient/private/NSError+GRPC.m index 15c0208681f..f7390476d9a 100644 --- a/src/objective-c/GRPCClient/private/NSError+GRPC.m +++ b/src/objective-c/GRPCClient/private/NSError+GRPC.m @@ -35,17 +35,16 @@ #include -NSString *const kGRPCErrorDomain = @"org.grpc"; +NSString * const kGRPCErrorDomain = @"io.grpc"; @implementation NSError (GRPC) -+ (instancetype)grpc_errorFromStatus:(struct grpc_status *)status { - if (status->status == GRPC_STATUS_OK) { ++ (instancetype)grpc_errorFromStatusCode:(grpc_status_code)statusCode details:(char *)details { + if (statusCode == GRPC_STATUS_OK) { return nil; } - NSString *message = - [NSString stringWithFormat:@"Code=%i Message='%s'", status->status, status->details]; + NSString *message = [NSString stringWithCString:details encoding:NSASCIIStringEncoding]; return [NSError errorWithDomain:kGRPCErrorDomain - code:status->status + code:statusCode userInfo:@{NSLocalizedDescriptionKey: message}]; } @end From 8c6bc6e5aa7051b10d612fb9e2c7f9059b03c273 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 12 Jun 2015 22:19:23 -0700 Subject: [PATCH 12/21] Test call and error metadata are the same. --- src/objective-c/tests/GRPCClientTests.m | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index ae7aba811a6..be609b1a6a6 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -163,6 +163,8 @@ static GRPCMethodName *kUnaryCallMethod; } completionHandler:^(NSError *errorOrNil) { XCTAssertNotNil(errorOrNil, @"Finished without error!"); XCTAssertEqual(errorOrNil.code, 16, @"Finished with unexpected error: %@", errorOrNil); + XCTAssertEqualObjects(call.responseMetadata, errorOrNil.userInfo[kGRPCStatusMetadataKey], + @"Metadata in the NSError object and call object differ."); NSString *challengeHeader = call.responseMetadata[@"www-authenticate"][0]; XCTAssertGreaterThan(challengeHeader.length, 0, @"No challenge in response headers %@", call.responseMetadata); From 1a3847509308f2e79be13ab1965103397ccaf4c6 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 12 Jun 2015 22:40:37 -0700 Subject: [PATCH 13/21] Drop repeated response headers. --- src/objective-c/GRPCClient/private/NSDictionary+GRPC.m | 9 +++------ src/objective-c/tests/GRPCClientTests.m | 2 +- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m index af8f4a2510e..99c890e4ee7 100644 --- a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m +++ b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m @@ -108,8 +108,8 @@ // TODO(jcanizales): Verify in a C library test that it's converting header names to lower case // automatically. NSString *name = [NSString stringWithCString:entry->key encoding:NSASCIIStringEncoding]; - if (!name) { - // log? + if (!name || metadata[name]) { + // Log if name is nil? continue; } id value; @@ -119,10 +119,7 @@ } else { value = [NSString grpc_stringFromMetadataValue:entry]; } - if (!metadata[name]) { - metadata[name] = [NSMutableArray array]; - } - [metadata[name] addObject:value]; + metadata[name] = value; } return metadata; } diff --git a/src/objective-c/tests/GRPCClientTests.m b/src/objective-c/tests/GRPCClientTests.m index be609b1a6a6..268e67af2f0 100644 --- a/src/objective-c/tests/GRPCClientTests.m +++ b/src/objective-c/tests/GRPCClientTests.m @@ -165,7 +165,7 @@ static GRPCMethodName *kUnaryCallMethod; XCTAssertEqual(errorOrNil.code, 16, @"Finished with unexpected error: %@", errorOrNil); XCTAssertEqualObjects(call.responseMetadata, errorOrNil.userInfo[kGRPCStatusMetadataKey], @"Metadata in the NSError object and call object differ."); - NSString *challengeHeader = call.responseMetadata[@"www-authenticate"][0]; + NSString *challengeHeader = call.responseMetadata[@"www-authenticate"]; XCTAssertGreaterThan(challengeHeader.length, 0, @"No challenge in response headers %@", call.responseMetadata); [expectation fulfill]; From 2285b362cadf82e399921e937c45aa30fbb53ac7 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 12 Jun 2015 23:16:19 -0700 Subject: [PATCH 14/21] Specify responseMetadata structure --- src/objective-c/GRPCClient/GRPCCall.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h index c01d1f235bc..7b42498d42b 100644 --- a/src/objective-c/GRPCClient/GRPCCall.h +++ b/src/objective-c/GRPCClient/GRPCCall.h @@ -74,7 +74,8 @@ extern id const kGRPCStatusMetadataKey; - (void)setRequestMetadata:(NSDictionary *)requestMetadata; // 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. +// the HTTP trailers received are added to the dictionary too. It has the same structure as the +// request metadata dictionary. // // The first time this object calls |writeValue| on the writeable passed to |startWithWriteable|, // the |responseMetadata| dictionary already contains the response headers. When it calls From cbf50c8bf28f0b63c0f63f3af238c47f16629f75 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Sat, 13 Jun 2015 01:34:44 -0700 Subject: [PATCH 15/21] Advance version to 0.6, per the metadata breaking change --- gRPC.podspec | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gRPC.podspec b/gRPC.podspec index eaebb27423f..ad621cd2e8a 100644 --- a/gRPC.podspec +++ b/gRPC.podspec @@ -1,6 +1,6 @@ Pod::Spec.new do |s| s.name = 'gRPC' - s.version = '0.5.1' + s.version = '0.6.0' s.summary = 'gRPC client library for iOS/OSX' s.homepage = 'http://www.grpc.io' s.license = 'New BSD' From a1e32ba927a418fa6d0aeed41c9e22253e286fde Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Sat, 13 Jun 2015 23:30:38 -0700 Subject: [PATCH 16/21] Remove failing globs from examples and tests --- src/objective-c/README.md | 24 ++++++++++++++----- .../RemoteTestClient/RemoteTest.podspec | 6 ++--- .../RouteGuideClient/RouteGuide.podspec | 6 ++--- 3 files changed, 24 insertions(+), 12 deletions(-) diff --git a/src/objective-c/README.md b/src/objective-c/README.md index 728e2264805..0e5eb9a25f6 100644 --- a/src/objective-c/README.md +++ b/src/objective-c/README.md @@ -52,11 +52,11 @@ Pod::Spec.new do |s| # Run protoc with the Objective-C and gRPC plugins to generate protocol messages and gRPC clients. # You can run this command manually if you later change your protos and need to regenerate. - s.prepare_command = "protoc --objc_out=. --objcgrpc_out=. *.proto **/*.proto" + s.prepare_command = "protoc --objc_out=. --objcgrpc_out=. *.proto" # The --objc_out plugin generates a pair of .pbobjc.h/.pbobjc.m files for each .proto file. s.subspec "Messages" do |ms| - ms.source_files = "*.pbobjc.{h,m}", "**/*.pbobjc.{h,m}" + ms.source_files = "*.pbobjc.{h,m}" ms.header_mappings_dir = "." ms.requires_arc = false ms.dependency "Protobuf", "~> 3.0.0-alpha-3" @@ -65,7 +65,7 @@ Pod::Spec.new do |s| # The --objcgrpc_out plugin generates a pair of .pbrpc.h/.pbrpc.m files for each .proto file with # a service defined. s.subspec "Services" do |ss| - ss.source_files = "*.pbrpc.{h,m}", "**/*.pbrpc.{h,m}" + ss.source_files = "*.pbrpc.{h,m}" ss.header_mappings_dir = "." ss.requires_arc = true ss.dependency "gRPC", "~> 0.5" @@ -74,9 +74,21 @@ Pod::Spec.new do |s| end ``` -The file should be named `.podspec`. Once your library has a Podspec, Cocoapods -can install it into any XCode project. For that, go into your project's directory and create a -Podfile by running: +The file should be named `.podspec`. + +Note: If your proto files are in a directory hierarchy, you might want to adjust the _globs_ used in +the sample Podspec above. For example, you could use: + +```ruby + s.prepare_command = "protoc --objc_out=. --objcgrpc_out=. *.proto **/*.proto" + ... + ms.source_files = "*.pbobjc.{h,m}", "**/*.pbobjc.{h,m}" + ... + ss.source_files = "*.pbrpc.{h,m}", "**/*.pbrpc.{h,m}" +``` + +Once your library has a Podspec, Cocoapods can install it into any XCode project. For that, go into +your project's directory and create a Podfile by running: ```sh pod init diff --git a/src/objective-c/generated_libraries/RemoteTestClient/RemoteTest.podspec b/src/objective-c/generated_libraries/RemoteTestClient/RemoteTest.podspec index 0066313ff6c..dd0dab352d1 100644 --- a/src/objective-c/generated_libraries/RemoteTestClient/RemoteTest.podspec +++ b/src/objective-c/generated_libraries/RemoteTestClient/RemoteTest.podspec @@ -7,17 +7,17 @@ Pod::Spec.new do |s| s.osx.deployment_target = "10.8" # Run protoc with the Objective-C and gRPC plugins to generate protocol messages and gRPC clients. - s.prepare_command = "protoc --objc_out=. --objcgrpc_out=. *.proto **/*.proto" + s.prepare_command = "protoc --objc_out=. --objcgrpc_out=. *.proto" s.subspec "Messages" do |ms| - ms.source_files = "*.pbobjc.{h,m}", "**/*.pbobjc.{h,m}" + ms.source_files = "*.pbobjc.{h,m}" ms.header_mappings_dir = "." ms.requires_arc = false ms.dependency "Protobuf", "~> 3.0.0-alpha-3" end s.subspec "Services" do |ss| - ss.source_files = "*.pbrpc.{h,m}", "**/*.pbrpc.{h,m}" + ss.source_files = "*.pbrpc.{h,m}" ss.header_mappings_dir = "." ss.requires_arc = true ss.dependency "gRPC", "~> 0.5" diff --git a/src/objective-c/generated_libraries/RouteGuideClient/RouteGuide.podspec b/src/objective-c/generated_libraries/RouteGuideClient/RouteGuide.podspec index 58ccb4873ee..e26e62f5bbb 100644 --- a/src/objective-c/generated_libraries/RouteGuideClient/RouteGuide.podspec +++ b/src/objective-c/generated_libraries/RouteGuideClient/RouteGuide.podspec @@ -7,17 +7,17 @@ Pod::Spec.new do |s| s.osx.deployment_target = "10.8" # Run protoc with the Objective-C and gRPC plugins to generate protocol messages and gRPC clients. - s.prepare_command = "protoc --objc_out=. --objcgrpc_out=. *.proto **/*.proto" + s.prepare_command = "protoc --objc_out=. --objcgrpc_out=. *.proto" s.subspec "Messages" do |ms| - ms.source_files = "*.pbobjc.{h,m}", "**/*.pbobjc.{h,m}" + ms.source_files = "*.pbobjc.{h,m}" ms.header_mappings_dir = "." ms.requires_arc = false ms.dependency "Protobuf", "~> 3.0.0-alpha-3" end s.subspec "Services" do |ss| - ss.source_files = "*.pbrpc.{h,m}", "**/*.pbrpc.{h,m}" + ss.source_files = "*.pbrpc.{h,m}" ss.header_mappings_dir = "." ss.requires_arc = true ss.dependency "gRPC", "~> 0.5" From b37307cfd864c315ea1de068990709b55cfb91ec Mon Sep 17 00:00:00 2001 From: Eric Dobson Date: Sun, 14 Jun 2015 10:49:43 -0700 Subject: [PATCH 17/21] Fix docs on gpr_slice_unref. --- include/grpc/support/slice.h | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/include/grpc/support/slice.h b/include/grpc/support/slice.h index 9026602f15f..3ee103bfb9c 100644 --- a/include/grpc/support/slice.h +++ b/include/grpc/support/slice.h @@ -110,8 +110,9 @@ gpr_slice gpr_slice_ref(gpr_slice s); /* Decrement the ref count of s. If the ref count of s reaches zero, all slices sharing the ref count are destroyed, and considered no longer initialized. If s is ultimately derived from a call to gpr_slice_new(start, - len, dest) where dest!=NULL , then (*dest)(start, len) is called. Requires - s initialized. */ + len, dest) where dest!=NULL , then (*dest)(start) is called, else if s is + ultimately derived from a call to gpr_slice_new_with_len(start, len, dest) + where dest!=NULL , then (*dest)(start, len). Requires s initialized. */ void gpr_slice_unref(gpr_slice s); /* Create a slice pointing at some data. Calls malloc to allocate a refcount From 255971d99a5c4cd447f2d97dbfc19155d659249a Mon Sep 17 00:00:00 2001 From: Eric Dobson Date: Sun, 14 Jun 2015 16:04:15 -0700 Subject: [PATCH 18/21] Fix docs on grpc_op_type. --- include/grpc/grpc.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/include/grpc/grpc.h b/include/grpc/grpc.h index 62ee2a15051..73c30118a4a 100644 --- a/include/grpc/grpc.h +++ b/include/grpc/grpc.h @@ -221,7 +221,7 @@ typedef enum { GRPC_OP_SEND_INITIAL_METADATA = 0, /* Send a message: 0 or more of these operations can occur for each call */ GRPC_OP_SEND_MESSAGE, - /* Send a close from the server: one and only one instance MUST be sent from + /* Send a close from the client: one and only one instance MUST be sent from the client, unless the call was cancelled - in which case this can be skipped */ GRPC_OP_SEND_CLOSE_FROM_CLIENT, @@ -240,7 +240,7 @@ typedef enum { the status will indicate some failure. */ GRPC_OP_RECV_STATUS_ON_CLIENT, - /* Receive status on the server: one and only one must be made on the server + /* Receive close on the server: one and only one must be made on the server */ GRPC_OP_RECV_CLOSE_ON_SERVER } grpc_op_type; From be2a75c51e622a9ab3d913d95c8ae214b5530deb Mon Sep 17 00:00:00 2001 From: Oliver Smith Date: Sun, 14 Jun 2015 21:17:02 -0700 Subject: [PATCH 19/21] include string_util.h for the prototype of gpr_strdup and typo. Fixes two warnings in log_win32.c under msvc2015 log_win32.c(109): warning C4013: 'gpr_strdup' undefined; assuming extern returning int log_win32.c(109): warning C4047: 'return': 'char *' differs in levels of indirection from 'int' and corrects a typo in an error message --- src/core/support/log_win32.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/core/support/log_win32.c b/src/core/support/log_win32.c index 159c7e052c5..f878e6aaa17 100644 --- a/src/core/support/log_win32.c +++ b/src/core/support/log_win32.c @@ -42,6 +42,7 @@ #include #include #include +#include #include "src/core/support/string.h" #include "src/core/support/string_win32.h" @@ -106,7 +107,7 @@ char *gpr_format_message(DWORD messageid) { NULL, messageid, MAKELANGID(LANG_NEUTRAL, SUBLANG_DEFAULT), (LPTSTR)(&tmessage), 0, NULL); - if (status == 0) return gpr_strdup("Unable to retreive error string"); + if (status == 0) return gpr_strdup("Unable to retrieve error string"); message = gpr_tchar_to_char(tmessage); LocalFree(tmessage); return message; From 783ad1e9d1de8430847ce8063c8af21ebe78af47 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 15 Jun 2015 09:52:33 -0700 Subject: [PATCH 20/21] Fixed Go_AWAY to GOAWAY --- 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 ef4986d9405..0cd3c6dc5cf 100644 --- a/doc/connectivity-semantics-and-api.md +++ b/doc/connectivity-semantics-and-api.md @@ -43,7 +43,7 @@ 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 period, channels that are READY or CONNECTING switch to IDLE. Additionaly, -channels that receive a GO_AWAY when there are no active or pending RPCs should +channels that receive a GOAWAY when there are no active or pending RPCs should also switch to IDLE to avoid connection overload at servers that are attempting to shed connections. We will use a default IDLE_TIMEOUT of 300 seconds (5 minutes). From 33094a7c6214d1c21819c18343017e1215dbd104 Mon Sep 17 00:00:00 2001 From: Abhishek Kumar Date: Mon, 15 Jun 2015 09:53:41 -0700 Subject: [PATCH 21/21] More fixes to GO_AWAY spelling --- 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 0cd3c6dc5cf..930dff265fe 100644 --- a/doc/connectivity-semantics-and-api.md +++ b/doc/connectivity-semantics-and-api.md @@ -81,7 +81,7 @@ corresponding reasons. Empty cells denote disallowed transitions. Incremental successful communication on established channel. Any failure encountered while expecting successful communication on established channel. - No RPC activity on channel for IDLE_TIMEOUT
OR
upon receiving a GO_AWAY while there are no pending RPCs. + No RPC activity on channel for IDLE_TIMEOUT
OR
upon receiving a GOAWAY while there are no pending RPCs. Shutdown triggered by application.