Merge pull request grpc#2051 from jcanizales/simplify-metadata

Simplify metadata usage
erlangzhang · Jun 15, 2015 · e8d55cf · e8d55cf
2 parents 62c5b12 + cbf50c8
commit e8d55cf
Show file tree

Hide file tree

Showing 10 changed files with 173 additions and 107 deletions.
diff --git 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'

diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h
@@ -31,43 +31,55 @@
  *
  */
 
+// 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.
+//
+// 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.
+
 #import <Foundation/Foundation.h>
 #import <gRPC/GRXWriter.h>
 
 @class GRPCMethodName;
 
-@class GRPCCall;
+// Key used in |NSError|'s |userInfo| dictionary to store the response metadata sent by the server.
+extern id const kGRPCStatusMetadataKey;
 
-// 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.
-//
-// 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.
+// Represents a single gRPC remote call.
 @interface GRPCCall : NSObject<GRXWriter>
 
-// 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. 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
+// |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
@@ -46,9 +46,9 @@
 #import "private/NSDictionary+GRPC.h"
 #import "private/NSError+GRPC.h"
 
+NSString * const kGRPCStatusMetadataKey = @"io.grpc.StatusMetadataKey";
+
 @interface GRPCCall () <GRXWriteable>
-// Makes it readwrite.
-@property(atomic, strong) NSDictionary *responseMetadata;
 @end
 
 // The following methods of a C gRPC call object aren't reentrant, and thus
@@ -82,6 +82,9 @@ @implementation GRPCCall {
   // correct ordering.
   GRPCDelegateWrapper *_responseWriteable;
   id<GRXWriter> _requestWriter;
+
+  NSMutableDictionary *_requestMetadata;
+  NSMutableDictionary *_responseMetadata;
 }
 
 @synthesize state = _state;
@@ -116,10 +119,27 @@ - (instancetype)initWithHost:(NSString *)host
     _callQueue = dispatch_queue_create("org.grpc.call", NULL);
 
     _requestWriter = requestWriter;
+
+    _requestMetadata = [NSMutableDictionary dictionary];
+    _responseMetadata = [NSMutableDictionary dictionary];
   }
   return self;
 }
 
+#pragma mark Metadata
+
+- (NSMutableDictionary *)requestMetadata {
+  return _requestMetadata;
+}
+
+- (void)setRequestMetadata:(NSDictionary *)requestMetadata {
+  _requestMetadata = [NSMutableDictionary dictionaryWithDictionary:requestMetadata];
+}
+
+- (NSDictionary *)responseMetadata {
+  return _responseMetadata;
+}
+
 #pragma mark Finish
 
 - (void)finishWithError:(NSError *)errorOrNil {
@@ -277,7 +297,7 @@ - (void)writesFinishedWithError:(NSError *)errorOrNil {
 // 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]]];
@@ -287,16 +307,23 @@ - (void)invokeCallWithMetadataHandler:(void(^)(NSDictionary *))metadataHandler
 
 - (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
@@ -79,7 +79,7 @@ typedef void(^GRPCCompletionHandler)(NSDictionary *);
 
 @interface GRPCOpRecvStatus : NSObject <GRPCOp>
 
-- (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
@@ -165,9 +165,7 @@ - (void)getOp:(grpc_op *)op {
 }
 
 - (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 @@ - (void)finish {
 @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
@@ -35,6 +35,7 @@
 #include <grpc/grpc.h>
 
 @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
@@ -98,14 +98,18 @@ - (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key {
 #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++) {
     // 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;
@@ -115,10 +119,7 @@ + (instancetype)grpc_dictionaryFromMetadata:(grpc_metadata *)entries count:(size
     } 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/GRPCClient/private/NSError+GRPC.h b/src/objective-c/GRPCClient/private/NSError+GRPC.h
@@ -32,6 +32,7 @@
  */
 
 #import <Foundation/Foundation.h>
+#include <grpc/grpc.h>
 
 // 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 <grpc/grpc.h>
-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
@@ -35,17 +35,16 @@
 
 #include <grpc.h>
 
-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
-Original file line number
+Diff line change
@@ Expand Up / @@ -79,7 +79,7 @@ typedef void(^GRPCCompletionHandler)(NSDictionary *); @@
     @interface GRPCOpRecvStatus : NSObject <GRPCOp>
-    - (instancetype)initWithHandler:(void(^)(NSError *))handler NS_DESIGNATED_INITIALIZER;
+    - (instancetype)initWithHandler:(void(^)(NSError *, NSDictionary *))handler NS_DESIGNATED_INITIALIZER;
     @end
@@ Expand Down @@