Merge pull request #2914 from jcanizales/separate-headers-and-trailers

Separate headers and trailers

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,14 +50,14 @@ 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];
   }
 }
 
 - (NSString *)oauth2ChallengeHeader {
-  return self.responseMetadata[kChallengeHeader];
+  return self.responseHeaders[kChallengeHeader];
 }
 
 @end

src/objective-c/GRPCClient/GRPCCall.h

@@ -48,8 +48,10 @@
 #import <Foundation/Foundation.h>
 #import <RxLibrary/GRXWriter.h>
 
-// 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
@@ -57,43 +59,49 @@ 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.
+// 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.
-- (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
-// 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 *responseMetadata;
+// 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.
-// 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?

src/objective-c/GRPCClient/GRPCCall.m

@@ -42,9 +42,13 @@
 #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 () <GRXWriteable>
+// 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
@@ -89,8 +93,7 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey";
   // the response arrives.
   GRPCCall *_retainSelf;
 
-  NSMutableDictionary *_requestMetadata;
-  NSMutableDictionary *_responseMetadata;
+  NSMutableDictionary *_requestHeaders;
 }
 
 @synthesize state = _state;
@@ -121,24 +124,19 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey";
 
     _requestWriter = requestWriter;
 
-    _requestMetadata = [NSMutableDictionary dictionary];
-    _responseMetadata = [NSMutableDictionary dictionary];
+    _requestHeaders = [NSMutableDictionary dictionary];
   }
   return self;
 }
 
 #pragma mark Metadata
 
-- (NSMutableDictionary *)requestMetadata {
-  return _requestMetadata;
+- (NSMutableDictionary *)requestHeaders {
+  return _requestHeaders;
 }
 
-- (void)setRequestMetadata:(NSDictionary *)requestMetadata {
-  _requestMetadata = [NSMutableDictionary dictionaryWithDictionary:requestMetadata];
-}
-
-- (NSDictionary *)responseMetadata {
-  return _responseMetadata;
+- (void)setRequestHeaders:(NSDictionary *)requestHeaders {
+  _requestHeaders = [NSMutableDictionary dictionaryWithDictionary:requestHeaders];
 }
 
 #pragma mark Finish
@@ -232,11 +230,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
@@ -305,35 +302,45 @@ 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->_responseMetadata addEntriesFromDictionary:headers];
+      strongSelf.responseHeaders = headers;
       [strongSelf startNextRead];
     }
   } completionHandler:^(NSError *error, NSDictionary *trailers) {
     GRPCCall *strongSelf = weakSelf;
     if (strongSelf) {
-      [strongSelf->_responseMetadata addEntriesFromDictionary:trailers];
+      strongSelf.responseTrailers = trailers;
 
       if (error) {
-        NSMutableDictionary *userInfo =
-            [NSMutableDictionary dictionaryWithDictionary:error.userInfo];
-        userInfo[kGRPCStatusMetadataKey] = strongSelf->_responseMetadata;
+        NSMutableDictionary *userInfo = [NSMutableDictionary dictionary];
+        if (error.userInfo) {
+          [userInfo addEntriesFromDictionary:error.userInfo];
+        }
+        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 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];
@@ -356,7 +363,7 @@ NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey";
   _retainSelf = self;
 
   _responseWriteable = [[GRXConcurrentWriteable alloc] initWithWriteable:writeable];
-  [self sendHeaders:_requestMetadata];
+  [self sendHeaders:_requestHeaders];
   [self invokeCall];
 }
 

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.responseMetadata, 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.responseMetadata);
+                         @"No challenge in response headers %@", call.responseHeaders);
     [expectation fulfill];
   }];