Addressed review comments

pull/1363/head
murgatroid99 10 years ago
parent 463a7a86ba
commit 69927d65c3
  1. 38
      src/objective-c/GRPCClient/GRPCCall.m
  2. 4
      src/objective-c/GRPCClient/private/GRPCCompletionQueue.m
  3. 13
      src/objective-c/GRPCClient/private/GRPCWrappedCall.h
  4. 51
      src/objective-c/GRPCClient/private/GRPCWrappedCall.m
  5. 6
      src/objective-c/GRPCClient/private/NSData+GRPC.m

@ -41,7 +41,7 @@
#import "private/GRPCCompletionQueue.h" #import "private/GRPCCompletionQueue.h"
#import "private/GRPCDelegateWrapper.h" #import "private/GRPCDelegateWrapper.h"
#import "private/GRPCMethodName+HTTP2Encoding.h" #import "private/GRPCMethodName+HTTP2Encoding.h"
#import "GRPCWrappedCall.h" #import "private/GRPCWrappedCall.h"
#import "private/NSData+GRPC.h" #import "private/NSData+GRPC.h"
#import "private/NSDictionary+GRPC.h" #import "private/NSDictionary+GRPC.h"
#import "private/NSError+GRPC.h" #import "private/NSError+GRPC.h"
@ -176,7 +176,7 @@ static void AssertNoErrorInCall(grpc_call_error error) {
// Only called from the call queue. // Only called from the call queue.
// The handler will be called from the network queue. // The handler will be called from the network queue.
- (void)startReadWithHandler:(GRPCCompletionHandler)handler { - (void)startReadWithHandler:(GRPCCompletionHandler)handler {
[_wrappedCall startBatch:@{@(GRPC_OP_RECV_MESSAGE): @YES} handleCompletion:handler]; [_wrappedCall startBatchWithOperations:@{@(GRPC_OP_RECV_MESSAGE): @YES} handleCompletion:handler];
} }
// Called initially from the network queue once response headers are received, // Called initially from the network queue once response headers are received,
@ -191,15 +191,11 @@ static void AssertNoErrorInCall(grpc_call_error error) {
} }
__weak GRPCCall *weakSelf = self; __weak GRPCCall *weakSelf = self;
__weak GRPCDelegateWrapper *weakWriteable = _responseWriteable; __weak GRPCDelegateWrapper *weakWriteable = _responseWriteable;
dispatch_async(_callQueue, ^{ dispatch_async(_callQueue, ^{
[weakSelf startReadWithHandler:^(NSDictionary *event) { [weakSelf startReadWithHandler:^(NSDictionary *result) {
NSData *data = event[[NSNumber numberWithInt:GRPC_OP_RECV_MESSAGE]]; NSData *data = result[@(GRPC_OP_RECV_MESSAGE)];
if (data == [NSNull null]) { if (data == [NSNull null]) {
data = nil;
}
if (data == nil) {
// No more responses from the server
return; return;
} }
if (!data) { if (!data) {
@ -226,11 +222,9 @@ static void AssertNoErrorInCall(grpc_call_error error) {
#pragma mark Send headers #pragma mark Send headers
// TODO(jcanizales): Rename to commitHeaders.
- (void)sendHeaders:(NSDictionary *)metadata { - (void)sendHeaders:(NSDictionary *)metadata {
if (metadata == nil) { [_wrappedCall startBatchWithOperations:@{@(GRPC_OP_SEND_INITIAL_METADATA): metadata?:@{}} handleCompletion:nil];
metadata = @{};
}
[_wrappedCall startBatch:@{@(GRPC_OP_SEND_INITIAL_METADATA): metadata} handleCompletion:^(NSDictionary * dict){}];
} }
#pragma mark GRXWriteable implementation #pragma mark GRXWriteable implementation
@ -240,7 +234,7 @@ static void AssertNoErrorInCall(grpc_call_error error) {
- (void)writeMessage:(NSData *)message withErrorHandler:(void (^)())errorHandler { - (void)writeMessage:(NSData *)message withErrorHandler:(void (^)())errorHandler {
__weak GRPCCall *weakSelf = self; __weak GRPCCall *weakSelf = self;
GRPCCompletionHandler resumingHandler = ^(NSDictionary *event) { GRPCCompletionHandler resumingHandler = ^(NSDictionary *result) {
// Resume the request writer (even in the case of error). // Resume the request writer (even in the case of error).
// TODO(jcanizales): No need to do it in the case of errors anymore? // TODO(jcanizales): No need to do it in the case of errors anymore?
GRPCCall *strongSelf = weakSelf; GRPCCall *strongSelf = weakSelf;
@ -248,7 +242,7 @@ static void AssertNoErrorInCall(grpc_call_error error) {
strongSelf->_requestWriter.state = GRXWriterStateStarted; strongSelf->_requestWriter.state = GRXWriterStateStarted;
} }
}; };
[_wrappedCall startBatch:@{@(GRPC_OP_SEND_MESSAGE): message} handleCompletion:resumingHandler]; [_wrappedCall startBatchWithOperations:@{@(GRPC_OP_SEND_MESSAGE): message} handleCompletion:resumingHandler errorHandler:errorHandler];
} }
- (void)didReceiveValue:(id)value { - (void)didReceiveValue:(id)value {
@ -271,7 +265,7 @@ static void AssertNoErrorInCall(grpc_call_error error) {
// Only called from the call queue. The error handler will be called from the // Only called from the call queue. The error handler will be called from the
// network queue if the requests stream couldn't be closed successfully. // network queue if the requests stream couldn't be closed successfully.
- (void)finishRequestWithErrorHandler:(void (^)())errorHandler { - (void)finishRequestWithErrorHandler:(void (^)())errorHandler {
[_wrappedCall startBatch:@{@(GRPC_OP_SEND_CLOSE_FROM_CLIENT): @YES} handleCompletion:^(NSDictionary *event){}]; [_wrappedCall startBatchWithOperations:@{@(GRPC_OP_SEND_CLOSE_FROM_CLIENT): @YES} handleCompletion:nil errorHandler:errorHandler];
} }
- (void)didFinishWithError:(NSError *)errorOrNil { - (void)didFinishWithError:(NSError *)errorOrNil {
@ -297,22 +291,22 @@ static void AssertNoErrorInCall(grpc_call_error error) {
// The second one (completionHandler), whenever the RPC finishes for any reason. // The second one (completionHandler), whenever the RPC finishes for any reason.
- (void)invokeCallWithMetadataHandler:(GRPCCompletionHandler)metadataHandler - (void)invokeCallWithMetadataHandler:(GRPCCompletionHandler)metadataHandler
completionHandler:(GRPCCompletionHandler)completionHandler { completionHandler:(GRPCCompletionHandler)completionHandler {
[_wrappedCall startBatch:@{@(GRPC_OP_RECV_INITIAL_METADATA): @YES} handleCompletion:metadataHandler]; [_wrappedCall startBatchWithOperations:@{@(GRPC_OP_RECV_INITIAL_METADATA): @YES} handleCompletion:metadataHandler];
[_wrappedCall startBatch:@{@(GRPC_OP_RECV_STATUS_ON_CLIENT): @YES} handleCompletion:completionHandler]; [_wrappedCall startBatchWithOperations:@{@(GRPC_OP_RECV_STATUS_ON_CLIENT): @YES} handleCompletion:completionHandler];
} }
- (void)invokeCall { - (void)invokeCall {
__weak GRPCCall *weakSelf = self; __weak GRPCCall *weakSelf = self;
[self invokeCallWithMetadataHandler:^(NSDictionary *event) { [self invokeCallWithMetadataHandler:^(NSDictionary *result) {
// Response metadata received. // Response metadata received.
GRPCCall *strongSelf = weakSelf; GRPCCall *strongSelf = weakSelf;
if (strongSelf) { if (strongSelf) {
strongSelf.responseMetadata = event[@(GRPC_OP_RECV_INITIAL_METADATA)]; strongSelf.responseMetadata = result[@(GRPC_OP_RECV_INITIAL_METADATA)];
[strongSelf startNextRead]; [strongSelf startNextRead];
} }
} completionHandler:^(NSDictionary *event) { } completionHandler:^(NSDictionary *result) {
// TODO(jcanizales): Merge HTTP2 trailers into response metadata. // TODO(jcanizales): Merge HTTP2 trailers into response metadata.
id error = event[@(GRPC_OP_RECV_STATUS_ON_CLIENT)]; id error = result[@(GRPC_OP_RECV_STATUS_ON_CLIENT)];
if (error == [NSNull null]) { if (error == [NSNull null]) {
error = nil; error = nil;
} }

@ -46,7 +46,7 @@
- (instancetype)init { - (instancetype)init {
if ((self = [super init])) { if ((self = [super init])) {
_unmanagedQueue = grpc_completion_queue_create(); _unmanagedQueue = grpc_completion_queue_create();
// This is for the following block to capture the pointer by value (instead // This is for the following block to capture the pointer by value (instead
// of retaining self and doing self->_unmanagedQueue). This is essential // of retaining self and doing self->_unmanagedQueue). This is essential
// because the block doesn't end until after grpc_completion_queue_shutdown // because the block doesn't end until after grpc_completion_queue_shutdown
@ -54,7 +54,7 @@
// anymore (i.e. on self dealloc). So the block would never end if it // anymore (i.e. on self dealloc). So the block would never end if it
// retained self. // retained self.
grpc_completion_queue *unmanagedQueue = _unmanagedQueue; grpc_completion_queue *unmanagedQueue = _unmanagedQueue;
// Start a loop on a concurrent queue to read events from the completion // Start a loop on a concurrent queue to read events from the completion
// queue and dispatch each. // queue and dispatch each.
static dispatch_once_t initialization; static dispatch_once_t initialization;

@ -31,21 +31,18 @@
* *
*/ */
#ifndef Pods_GRPCWrappedCall_h
#define Pods_GRPCWrappedCall_h
#import <Foundation/Foundation.h> #import <Foundation/Foundation.h>
#import "GRPCChannel.h" #import "GRPCChannel.h"
typedef void(^GRPCCompletionHandler)(NSDictionary *); typedef void(^GRPCCompletionHandler)(NSDictionary *);
@interface GRPCWrappedCall:NSObject; @interface GRPCWrappedCall : NSObject
- (instancetype)initWithChannel:(GRPCChannel *)channel method:(NSString *)method host:(NSString *)host NS_DESIGNATED_INITIALIZER;
- (instancetype)initWithChannel:(GRPCChannel *)channel method:(NSString *)method host:(NSString *)host; - (void)startBatchWithOperations:(NSDictionary *)ops handleCompletion:(GRPCCompletionHandler)handleCompletion errorHandler:(void(^)())errorHandler;
- (void)startBatch:(NSDictionary *)ops handleCompletion:(GRPCCompletionHandler)handleCompletion; - (void)startBatchWithOperations:(NSDictionary *)ops handleCompletion:(GRPCCompletionHandler)handleCompletion;
- (void)cancel; - (void)cancel;
@end @end
#endif

@ -31,19 +31,19 @@
* *
*/ */
#import <Foundation/Foundation.h>
#import "GRPCWrappedCall.h" #import "GRPCWrappedCall.h"
#import <Foundation/Foundation.h>
#include <grpc/grpc.h>
#include <grpc/byte_buffer.h>
#include <grpc/support/alloc.h>
#import "GRPCCompletionQueue.h" #import "GRPCCompletionQueue.h"
#import "NSDictionary+GRPC.h" #import "NSDictionary+GRPC.h"
#import "NSData+GRPC.h" #import "NSData+GRPC.h"
#import "NSError+GRPC.h" #import "NSError+GRPC.h"
#include <grpc/grpc.h>
#include <grpc/byte_buffer.h>
#include <grpc/support/alloc.h>
@implementation GRPCWrappedCall{ @implementation GRPCWrappedCall{
grpc_call *call; grpc_call *_call;
GRPCCompletionQueue *queue; GRPCCompletionQueue *_queue;
} }
- (instancetype)init { - (instancetype)init {
@ -61,19 +61,21 @@
grpc_init(); grpc_init();
}); });
const char *method_str = [method UTF8String]; _queue = [GRPCCompletionQueue completionQueue];
const char *host_str = [host UTF8String]; _call = grpc_channel_create_call(channel.unmanagedChannel, _queue.unmanagedQueue, method.UTF8String, host.UTF8String, gpr_inf_future);
queue = [GRPCCompletionQueue completionQueue]; if (_call == NULL) {
call = grpc_channel_create_call(channel.unmanagedChannel, queue.unmanagedQueue, method_str, host_str, gpr_inf_future);
if (call == NULL) {
return nil; return nil;
} }
} }
return self; return self;
} }
- (void)startBatch:(NSDictionary *)ops handleCompletion:(GRPCCompletionHandler)handleCompletion { - (void)startBatchWithOperations:(NSDictionary *)operations handleCompletion:(GRPCCompletionHandler)handleCompletion {
size_t nops = ops.count; [self startBatchWithOperations:operations handleCompletion:handleCompletion errorHandler:nil];
}
- (void)startBatchWithOperations:(NSDictionary *)operations handleCompletion:(GRPCCompletionHandler)handleCompletion errorHandler:(void (^)())errorHandler {
size_t nops = operations.count;
grpc_op *ops_array = gpr_malloc(nops * sizeof(grpc_op)); grpc_op *ops_array = gpr_malloc(nops * sizeof(grpc_op));
size_t index = 0; size_t index = 0;
NSMutableDictionary * __block opProcessors = [NSMutableDictionary new]; NSMutableDictionary * __block opProcessors = [NSMutableDictionary new];
@ -86,12 +88,13 @@
grpc_status_code *status_code; grpc_status_code *status_code;
char **status_details; char **status_details;
size_t *status_details_capacity; size_t *status_details_capacity;
for (id key in ops) { for (id key in operations) {
id (^opBlock)(void); id (^opBlock)(void);
grpc_op *current = &ops_array[index]; grpc_op *current = &ops_array[index];
switch ([key intValue]) { switch ([key intValue]) {
case GRPC_OP_SEND_INITIAL_METADATA: case GRPC_OP_SEND_INITIAL_METADATA:
current->data.send_initial_metadata.count = [ops[key] grpc_toMetadataArray:&send_metadata]; // TODO(jcanizales): Name the type of current->data.send_initial_metadata in the C library so a pointer to it can be returned from methods.
current->data.send_initial_metadata.count = [operations[key] grpc_toMetadataArray:&send_metadata];
current->data.send_initial_metadata.metadata = send_metadata; current->data.send_initial_metadata.metadata = send_metadata;
opBlock = ^{ opBlock = ^{
gpr_free(send_metadata); gpr_free(send_metadata);
@ -99,7 +102,7 @@
}; };
break; break;
case GRPC_OP_SEND_MESSAGE: case GRPC_OP_SEND_MESSAGE:
send_message = [ops[key] grpc_byteBuffer]; send_message = [operations[key] grpc_byteBuffer];
current->data.send_message = send_message; current->data.send_message = send_message;
opBlock = ^{ opBlock = ^{
grpc_byte_buffer_destroy(send_message); grpc_byte_buffer_destroy(send_message);
@ -162,9 +165,13 @@
current->op = [key intValue]; current->op = [key intValue];
[opProcessors setObject:opBlock forKey:key]; [opProcessors setObject:opBlock forKey:key];
} }
grpc_call_error error = grpc_call_start_batch(call, ops_array, nops, (__bridge_retained void *)(^(grpc_op_error error){ grpc_call_error error = grpc_call_start_batch(_call, ops_array, nops, (__bridge_retained void *)(^(grpc_op_error error){
if (error != GRPC_OP_OK) { if (error != GRPC_OP_OK) {
[NSException raise:@"Operation Exception" format:@"The batch failed with an unknown error"]; if (errorHandler) {
errorHandler();
} else {
[NSException raise:@"Operation Exception" format:@"The batch failed with an unknown error"];
}
} }
NSMutableDictionary *result = [NSMutableDictionary new]; NSMutableDictionary *result = [NSMutableDictionary new];
for (id key in opProcessors) { for (id key in opProcessors) {
@ -175,7 +182,9 @@
} }
[result setObject:value forKey:key]; [result setObject:value forKey:key];
} }
handleCompletion(result); if (handleCompletion) {
handleCompletion(result);
}
})); }));
if (error != GRPC_CALL_OK) { if (error != GRPC_CALL_OK) {
[NSException raise:NSInvalidArgumentException format:@"The batch did not start successfully"]; [NSException raise:NSInvalidArgumentException format:@"The batch did not start successfully"];
@ -183,11 +192,11 @@
} }
- (void)cancel { - (void)cancel {
grpc_call_cancel(call); grpc_call_cancel(_call);
} }
- (void)dealloc { - (void)dealloc {
grpc_call_destroy(call); grpc_call_destroy(_call);
} }
@end @end

@ -59,9 +59,9 @@ static grpc_byte_buffer *CopyCharArrayToNewByteBuffer(const char *array, size_t
@implementation NSData (GRPC) @implementation NSData (GRPC)
+ (instancetype)grpc_dataWithByteBuffer:(grpc_byte_buffer *)buffer { + (instancetype)grpc_dataWithByteBuffer:(grpc_byte_buffer *)buffer {
if (buffer == NULL) { if (buffer == NULL) {
return nil; return nil;
} }
NSUInteger length = grpc_byte_buffer_length(buffer); NSUInteger length = grpc_byte_buffer_length(buffer);
char *array = malloc(length * sizeof(*array)); char *array = malloc(length * sizeof(*array));
if (!array) { if (!array) {

Loading…
Cancel
Save