Manually merge release-0_11 into make-errors-public-beta to solve conflict

8 files changed, 253 insertions, 45 deletions

diff --git a/src/objective-c/GRPCClient/GRPCCall.h b/src/objective-c/GRPCClient/GRPCCall.h
index b2968b8e98..c098d4b701 100644
--- a/src/objective-c/GRPCClient/GRPCCall.h
+++ b/src/objective-c/GRPCClient/GRPCCall.h
@@ -48,6 +48,8 @@
 #import <Foundation/Foundation.h>
 #import <RxLibrary/GRXWriter.h>
 
+#include <grpc/grpc.h>
+
 #pragma mark gRPC errors
 
 // Domain of NSError objects produced by gRPC.
@@ -135,6 +137,25 @@ extern id const kGRPCTrailersKey;
 
 #pragma mark GRPCCall
 
+// The container of the request headers of an RPC conforms to this protocol, which is a subset of
+// NSMutableDictionary's interface. It will become a NSMutableDictionary later on.
+// The keys of this container are the header names, which per the HTTP standard are case-
+// insensitive. They are stored in lowercase (which is how HTTP/2 mandates them on the wire), and
+// can only consist of ASCII characters.
+// A header value is a NSString object (with only ASCII characters), unless the header name has the
+// suffix "-bin", in which case the value has to be a NSData object.
+@protocol GRPCRequestHeaders <NSObject>
+
+@property(nonatomic, readonly) NSUInteger count;
+
+- (id)objectForKeyedSubscript:(NSString *)key;
+- (void)setObject:(id)obj forKeyedSubscript:(NSString *)key;
+
+- (void)removeAllObjects;
+- (void)removeObjectForKey:(NSString *)key;
+
+@end
+
 // Represents a single gRPC remote call.
 @interface GRPCCall : GRXWriter
 
@@ -152,7 +173,7 @@ extern id const kGRPCTrailersKey;
 //
 // For convenience, the property is initialized to an empty NSMutableDictionary, and the setter
 // accepts (and copies) both mutable and immutable dictionaries.
-- (NSMutableDictionary *)requestHeaders; // nonatomic
+- (id<GRPCRequestHeaders>)requestHeaders; // nonatomic
 - (void)setRequestHeaders:(NSDictionary *)requestHeaders; // nonatomic, copy
 
 // This dictionary is populated with the HTTP headers received from the server. This happens before
diff --git a/src/objective-c/GRPCClient/GRPCCall.m b/src/objective-c/GRPCClient/GRPCCall.m
index ff5d1c5aaf..1be753e688 100644
--- a/src/objective-c/GRPCClient/GRPCCall.m
+++ b/src/objective-c/GRPCClient/GRPCCall.m
@@ -37,6 +37,7 @@
 #include <grpc/support/time.h>
 #import <RxLibrary/GRXConcurrentWriteable.h>
 
+#import "private/GRPCRequestHeaders.h"
 #import "private/GRPCWrappedCall.h"
 #import "private/NSData+GRPC.h"
 #import "private/NSDictionary+GRPC.h"
@@ -93,7 +94,7 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey";
   // the response arrives.
   GRPCCall *_retainSelf;
 
-  NSMutableDictionary *_requestHeaders;
+  GRPCRequestHeaders *_requestHeaders;
 }
 
 @synthesize state = _state;
@@ -124,19 +125,23 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey";
 
     _requestWriter = requestWriter;
 
-    _requestHeaders = [NSMutableDictionary dictionary];
+    _requestHeaders = [[GRPCRequestHeaders alloc] initWithCall:self];
   }
   return self;
 }
 
 #pragma mark Metadata
 
-- (NSMutableDictionary *)requestHeaders {
+- (id<GRPCRequestHeaders>)requestHeaders {
   return _requestHeaders;
 }
 
 - (void)setRequestHeaders:(NSDictionary *)requestHeaders {
-  _requestHeaders = [NSMutableDictionary dictionaryWithDictionary:requestHeaders];
+  GRPCRequestHeaders *newHeaders = [[GRPCRequestHeaders alloc] initWithCall:self];
+  for (id key in requestHeaders) {
+    newHeaders[key] = requestHeaders[key];
+  }
+  _requestHeaders = newHeaders;
 }
 
 #pragma mark Finish
@@ -230,10 +235,10 @@ NSString * const kGRPCTrailersKey = @"io.grpc.TrailersKey";
 
 #pragma mark Send headers
 
-- (void)sendHeaders:(NSDictionary *)headers {
+- (void)sendHeaders:(id<GRPCRequestHeaders>)headers {
   // TODO(jcanizales): Add error handlers for async failures
-  [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc]
-                                            initWithMetadata:headers ?: @{} handler:nil]]];
+  [_wrappedCall startBatchWithOperations:@[[[GRPCOpSendMetadata alloc] initWithMetadata:headers
+                                                                                handler:nil]]];
 }
 
 #pragma mark GRXWriteable implementation
diff --git a/src/objective-c/GRPCClient/private/GRPCRequestHeaders.h b/src/objective-c/GRPCClient/private/GRPCRequestHeaders.h
new file mode 100644
index 0000000000..1391b5725f
--- /dev/null
+++ b/src/objective-c/GRPCClient/private/GRPCRequestHeaders.h
@@ -0,0 +1,52 @@
+/*
+ *
+ * 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.
+ *
+ */
+
+#import <Foundation/Foundation.h>
+#include <grpc/grpc.h>
+
+#import "GRPCCall.h"
+
+@interface GRPCRequestHeaders : NSObject<GRPCRequestHeaders>
+
+@property(nonatomic, readonly) NSUInteger count;
+@property(nonatomic, readonly) grpc_metadata *grpc_metadataArray;
+
+- (instancetype)initWithCall:(GRPCCall *)call;
+
+- (id)objectForKeyedSubscript:(NSString *)key;
+- (void)setObject:(id)obj forKeyedSubscript:(NSString *)key;
+
+- (void)removeAllObjects;
+- (void)removeObjectForKey:(NSString *)key;
+
+@end
diff --git a/src/objective-c/GRPCClient/private/GRPCRequestHeaders.m b/src/objective-c/GRPCClient/private/GRPCRequestHeaders.m
new file mode 100644
index 0000000000..dfec2a7e7e
--- /dev/null
+++ b/src/objective-c/GRPCClient/private/GRPCRequestHeaders.m
@@ -0,0 +1,119 @@
+/*
+ *
+ * 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.
+ *
+ */
+
+#import "GRPCRequestHeaders.h"
+
+#import <Foundation/Foundation.h>
+
+#import "GRPCCall.h"
+#import "NSDictionary+GRPC.h"
+
+// Used by the setter.
+static void CheckIsNonNilASCII(NSString *name, NSString* value) {
+  if (!value) {
+    [NSException raise:NSInvalidArgumentException format:@"%@ cannot be nil", name];
+  }
+  if (![value canBeConvertedToEncoding:NSASCIIStringEncoding]) {
+    [NSException raise:NSInvalidArgumentException
+                format:@"%@ %@ contains non-ASCII characters", name, value];
+  }
+}
+
+// Precondition: key isn't nil.
+static void CheckKeyValuePairIsValid(NSString *key, id value) {
+  if ([key hasSuffix:@"-bin"]) {
+    if (![value isKindOfClass:NSData.class]) {
+      [NSException raise:NSInvalidArgumentException
+                  format:@"Expected NSData value for header %@ ending in \"-bin\", "
+       @"instead got %@", key, value];
+    }
+  } else {
+    if (![value isKindOfClass:NSString.class]) {
+      [NSException raise:NSInvalidArgumentException
+                  format:@"Expected NSString value for header %@ not ending in \"-bin\", "
+       @"instead got %@", key, value];
+    }
+    CheckIsNonNilASCII(@"Text header value", (NSString *)value);
+  }
+}
+
+@implementation GRPCRequestHeaders {
+  __weak GRPCCall *_call;
+  NSMutableDictionary *_delegate;
+}
+
+- (instancetype)initWithCall:(GRPCCall *)call {
+  if ((self = [super init])) {
+    _call = call;
+    _delegate = [NSMutableDictionary dictionary];
+  }
+  return self;
+}
+
+- (void)checkCallIsNotStarted {
+  if (_call.state != GRXWriterStateNotStarted) {
+    [NSException raise:@"Invalid modification"
+                format:@"Cannot modify request headers after call is started"];
+  }
+}
+
+- (id)objectForKeyedSubscript:(NSString *)key {
+  return _delegate[key.lowercaseString];
+}
+
+- (void)setObject:(id)obj forKeyedSubscript:(NSString *)key {
+  [self checkCallIsNotStarted];
+  CheckIsNonNilASCII(@"Header name", key);
+  key = key.lowercaseString;
+  CheckKeyValuePairIsValid(key, obj);
+  _delegate[key] = obj;
+}
+
+- (void)removeObjectForKey:(NSString *)key {
+  [self checkCallIsNotStarted];
+  [_delegate removeObjectForKey:key.lowercaseString];
+}
+
+- (void)removeAllObjects {
+  [self checkCallIsNotStarted];
+  [_delegate removeAllObjects];
+}
+
+- (NSUInteger)count {
+  return _delegate.count;
+}
+
+- (grpc_metadata *)grpc_metadataArray {
+  return _delegate.grpc_metadataArray;
+}
+@end
diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h
index da11cbb761..4ca2766147 100644
--- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.h
+++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.h
@@ -35,6 +35,7 @@
 #include <grpc/grpc.h>
 
 #import "GRPCChannel.h"
+#import "GRPCRequestHeaders.h"
 
 @interface GRPCOperation : NSObject
 @property(nonatomic, readonly) grpc_op op;
@@ -44,7 +45,7 @@
 
 @interface GRPCOpSendMetadata : GRPCOperation
 
-- (instancetype)initWithMetadata:(NSDictionary *)metadata
+- (instancetype)initWithMetadata:(GRPCRequestHeaders *)metadata
                          handler:(void(^)())handler NS_DESIGNATED_INITIALIZER;
 
 @end
diff --git a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m
index fe3d51da53..cea7c479e0 100644
--- a/src/objective-c/GRPCClient/private/GRPCWrappedCall.m
+++ b/src/objective-c/GRPCClient/private/GRPCWrappedCall.m
@@ -65,7 +65,7 @@
   return [self initWithMetadata:nil handler:nil];
 }
 
-- (instancetype)initWithMetadata:(NSDictionary *)metadata handler:(void (^)())handler {
+- (instancetype)initWithMetadata:(GRPCRequestHeaders *)metadata handler:(void (^)())handler {
   if (self = [super init]) {
     _op.op = GRPC_OP_SEND_INITIAL_METADATA;
     _op.data.send_initial_metadata.count = metadata.count;
diff --git a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m
index 99c890e4ee..7477da7619 100644
--- a/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m
+++ b/src/objective-c/GRPCClient/private/NSDictionary+GRPC.m
@@ -40,8 +40,8 @@
 @interface NSData (GRPCMetadata)
 + (instancetype)grpc_dataFromMetadataValue:(grpc_metadata *)metadata;
 
-// Fill a metadata object with the binary value in this NSData and the given key.
-- (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key;
+// Fill a metadata object with the binary value in this NSData.
+- (void)grpc_initMetadata:(grpc_metadata *)metadata;
 @end
 
 @implementation NSData (GRPCMetadata)
@@ -50,9 +50,7 @@
   return [self dataWithBytes:metadata->value length:metadata->value_length];
 }
 
-- (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key {
-  // TODO(jcanizales): Encode Unicode chars as ASCII.
-  metadata->key = [key stringByAppendingString:@"-bin"].UTF8String;
+- (void)grpc_initMetadata:(grpc_metadata *)metadata {
   metadata->value = self.bytes;
   metadata->value_length = self.length;
 }
@@ -63,8 +61,8 @@
 @interface NSString (GRPCMetadata)
 + (instancetype)grpc_stringFromMetadataValue:(grpc_metadata *)metadata;
 
-// Fill a metadata object with the textual value in this NSString and the given key.
-- (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key;
+// Fill a metadata object with the textual value in this NSString.
+- (void)grpc_initMetadata:(grpc_metadata *)metadata;
 @end
 
 @implementation NSString (GRPCMetadata)
@@ -74,22 +72,8 @@
                             encoding:NSASCIIStringEncoding];
 }
 
-- (void)grpc_initMetadata:(grpc_metadata *)metadata withKey:(NSString *)key {
-  if ([key hasSuffix:@"-bin"]) {
-    // Disallow this, as at best it will confuse the server. If the app really needs to send a
-    // textual header with a name ending in "-bin", it can be done by removing the suffix and
-    // encoding the NSString as a NSData object.
-    //
-    // Why raise an exception: In the most common case, the developer knows this won't happen in
-    // their code, so the exception isn't triggered. In the rare cases when the developer can't
-    // tell, it's easy enough to add a sanitizing filter before the header is set. There, the
-    // developer can choose whether to drop such a header, or trim its name. Doing either ourselves,
-    // silently, would be very unintuitive for the user.
-    [NSException raise:NSInvalidArgumentException
-                format:@"Metadata keys ending in '-bin' are reserved for NSData values."];
-  }
-  // TODO(jcanizales): Encode Unicode chars as ASCII.
-  metadata->key = key.UTF8String;
+// Precondition: This object contains only ASCII characters.
+- (void)grpc_initMetadata:(grpc_metadata *)metadata {
   metadata->value = self.UTF8String;
   metadata->value_length = self.length;
 }
@@ -105,8 +89,6 @@
 + (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 || metadata[name]) {
       // Log if name is nil?
@@ -114,7 +96,6 @@
     }
     id value;
     if ([name hasSuffix:@"-bin"]) {
-      name = [name substringToIndex:name.length - 4];
       value = [NSData grpc_dataFromMetadataValue:entry];
     } else {
       value = [NSString grpc_stringFromMetadataValue:entry];
@@ -124,19 +105,21 @@
   return metadata;
 }
 
+// Preconditions: All keys are ASCII strings. Keys ending in -bin have NSData values; the others
+// have NSString values.
 - (grpc_metadata *)grpc_metadataArray {
   grpc_metadata *metadata = gpr_malloc([self count] * sizeof(grpc_metadata));
-  int i = 0;
-  for (id key in self) {
+  grpc_metadata *current = metadata;
+  for (NSString* key in self) {
     id value = self[key];
-    grpc_metadata *current = &metadata[i];
-    if ([value respondsToSelector:@selector(grpc_initMetadata:withKey:)]) {
-      [value grpc_initMetadata:current withKey:key];
+    current->key = key.UTF8String;
+    if ([value respondsToSelector:@selector(grpc_initMetadata:)]) {
+      [value grpc_initMetadata:current];
     } else {
       [NSException raise:NSInvalidArgumentException
                   format:@"Metadata values must be NSString or NSData."];
     }
-    i += 1;
+    ++current;
   }
   return metadata;
 }
diff --git a/src/objective-c/ProtoRPC/ProtoRPC.m b/src/objective-c/ProtoRPC/ProtoRPC.m
index 889d71a308..9bf66f347a 100644
--- a/src/objective-c/ProtoRPC/ProtoRPC.m
+++ b/src/objective-c/ProtoRPC/ProtoRPC.m
@@ -37,6 +37,22 @@
 #import <RxLibrary/GRXWriteable.h>
 #import <RxLibrary/GRXWriter+Transformations.h>
 
+static NSError *ErrorForBadProto(id proto, Class expectedClass, NSError *parsingError) {
+  NSDictionary *info = @{
+                         NSLocalizedDescriptionKey: @"Unable to parse response from the server",
+                         NSLocalizedRecoverySuggestionErrorKey: @"If this RPC is idempotent, retry "
+                         @"with exponential backoff. Otherwise, query the server status before "
+                         @"retrying.",
+                         NSUnderlyingErrorKey: parsingError,
+                         @"Expected class": expectedClass,
+                         @"Received value": proto,
+                         };
+  // TODO(jcanizales): Use kGRPCErrorDomain and GRPCErrorCodeInternal when they're public.
+  return [NSError errorWithDomain:@"io.grpc"
+                             code:13
+                         userInfo:info];
+}
+
 @implementation ProtoRPC {
   id<GRXWriteable> _responseWriteable;
 }
@@ -65,14 +81,25 @@
   }
   // A writer that serializes the proto messages to send.
   GRXWriter *bytesWriter = [requestsWriter map:^id(GPBMessage *proto) {
-    // TODO(jcanizales): Fail with an understandable error message if the requestsWriter isn't
-    // sending GPBMessages.
+    if (![proto isKindOfClass:GPBMessage.class]) {
+      [NSException raise:NSInvalidArgumentException
+                  format:@"Request must be a proto message: %@", proto];
+    }
     return [proto data];
   }];
   if ((self = [super initWithHost:host path:method.HTTPPath requestsWriter:bytesWriter])) {
+    __weak ProtoRPC *weakSelf = self;
+
     // A writeable that parses the proto messages received.
     _responseWriteable = [[GRXWriteable alloc] initWithValueHandler:^(NSData *value) {
-      [responsesWriteable writeValue:[responseClass parseFromData:value error:NULL]];
+      // TODO(jcanizales): This is done in the main thread, and needs to happen in another thread.
+      NSError *error = nil;
+      id parsed = [responseClass parseFromData:value error:&error];
+      if (parsed) {
+        [responsesWriteable writeValue:parsed];
+      } else {
+        [weakSelf finishWithError:ErrorForBadProto(value, responseClass, error)];
+      }
     } completionHandler:^(NSError *errorOrNil) {
       [responsesWriteable writesFinishedWithError:errorOrNil];
     }];