Merge pull request #5722 from jcanizales/fix-writeable

Make GRXWriteable with single handler robust against streaming Writers
pull/5756/head
Jan Tattermusch 9 years ago
commit f21b0d1436
  1. 43
      src/objective-c/RxLibrary/GRXWriteable.m
  2. 51
      src/objective-c/tests/RxLibraryUnitTests.m

@ -1,6 +1,6 @@
/*
*
* Copyright 2015, Google Inc.
* Copyright 2015-2016, Google Inc.
* All rights reserved.
*
* Redistribution and use in source and binary forms, with or without
@ -42,11 +42,42 @@
if (!handler) {
return [[self alloc] init];
}
return [[self alloc] initWithValueHandler:^(id value) {
handler(value, nil);
} completionHandler:^(NSError *errorOrNil) {
if (errorOrNil) {
handler(nil, errorOrNil);
// We nilify this variable when the block is invoked, so that handler is only invoked once even if
// the writer tries to write multiple values.
__block GRXEventHandler eventHandler = ^(BOOL done, id value, NSError *error) {
// Nillify eventHandler before invoking handler, in case the latter causes the former to be
// executed recursively. Because blocks can be deallocated even during execution, we have to
// first retain handler locally to guarantee it's valid.
// TODO(jcanizales): Just turn this craziness into a simple subclass of GRXWriteable.
GRXSingleHandler singleHandler = handler;
eventHandler = nil;
if (value) {
singleHandler(value, nil);
} else if (error) {
singleHandler(nil, error);
} else {
NSDictionary *userInfo = @{
NSLocalizedDescriptionKey: @"The writer finished without producing any value."
};
// Even though RxLibrary is independent of gRPC, the domain and code here are, for the moment,
// set to the values of kGRPCErrorDomain and GRPCErrorCodeInternal. This way, the error formed
// is the one user of gRPC would expect if the server failed to produce a response.
//
// TODO(jcanizales): Figure out a way to keep errors of RxLibrary generic without making users
// of gRPC take care of two different error domains and error code enums. A possibility is to
// add error handling to GRXWriters or GRXWriteables, and use them to translate errors between
// the two domains.
static NSString *kGRPCErrorDomain = @"io.grpc";
static NSUInteger kGRPCErrorCodeInternal = 13;
singleHandler(nil, [NSError errorWithDomain:kGRPCErrorDomain
code:kGRPCErrorCodeInternal
userInfo:userInfo]);
}
};
return [self writeableWithEventHandler:^(BOOL done, id value, NSError *error) {
if (eventHandler) {
eventHandler(done, value, error);
}
}];
}

@ -64,6 +64,8 @@
}
@end
// TODO(jcanizales): Split into one file per tested class.
@interface RxLibraryUnitTests : XCTestCase
@end
@ -79,6 +81,7 @@
// If:
id<GRXWriteable> writeable = [GRXWriteable writeableWithSingleHandler:handler.block];
[writeable writeValue:anyValue];
[writeable writesFinishedWithError:nil];
// Then:
XCTAssertEqual(handler.timesCalled, 1);
@ -101,6 +104,54 @@
XCTAssertEqualObjects(handler.errorOrNil, anyError);
}
- (void)testWriteableSingleHandlerIsCalledOnlyOnce_ValueThenError {
// Given:
CapturingSingleValueHandler *handler = [CapturingSingleValueHandler handler];
id anyValue = @7;
NSError *anyError = [NSError errorWithDomain:@"domain" code:7 userInfo:nil];
// If:
id<GRXWriteable> writeable = [GRXWriteable writeableWithSingleHandler:handler.block];
[writeable writeValue:anyValue];
[writeable writesFinishedWithError:anyError];
// Then:
XCTAssertEqual(handler.timesCalled, 1);
XCTAssertEqualObjects(handler.value, anyValue);
XCTAssertEqualObjects(handler.errorOrNil, nil);
}
- (void)testWriteableSingleHandlerIsCalledOnlyOnce_ValueThenValue {
// Given:
CapturingSingleValueHandler *handler = [CapturingSingleValueHandler handler];
id anyValue = @7;
// If:
id<GRXWriteable> writeable = [GRXWriteable writeableWithSingleHandler:handler.block];
[writeable writeValue:anyValue];
[writeable writeValue:anyValue];
[writeable writesFinishedWithError:nil];
// Then:
XCTAssertEqual(handler.timesCalled, 1);
XCTAssertEqualObjects(handler.value, anyValue);
XCTAssertEqualObjects(handler.errorOrNil, nil);
}
- (void)testWriteableSingleHandlerFailsOnEmptyWriter {
// Given:
CapturingSingleValueHandler *handler = [CapturingSingleValueHandler handler];
// If:
id<GRXWriteable> writeable = [GRXWriteable writeableWithSingleHandler:handler.block];
[writeable writesFinishedWithError:nil];
// Then:
XCTAssertEqual(handler.timesCalled, 1);
XCTAssertEqualObjects(handler.value, nil);
XCTAssertNotNil(handler.errorOrNil);
}
#pragma mark BufferedPipe
- (void)testBufferedPipePropagatesValue {

Loading…
Cancel
Save