From 232b6a8f191d9cbb905680dd2a95f3b1815d1eb7 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 11 Mar 2016 12:26:27 -0800 Subject: [PATCH 1/4] Test robustness of WriteableSingleHandler against non-single Writers --- src/objective-c/tests/RxLibraryUnitTests.m | 51 ++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/objective-c/tests/RxLibraryUnitTests.m b/src/objective-c/tests/RxLibraryUnitTests.m index d3426628141..ae9465f58cc 100644 --- a/src/objective-c/tests/RxLibraryUnitTests.m +++ b/src/objective-c/tests/RxLibraryUnitTests.m @@ -64,6 +64,8 @@ } @end +// TODO(jcanizales): Split into one file per tested class. + @interface RxLibraryUnitTests : XCTestCase @end @@ -79,6 +81,7 @@ // If: id 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 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 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 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 { From 5e6723203013b365c2a60158578f73abf480b8bf Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Wed, 9 Mar 2016 13:34:28 -0800 Subject: [PATCH 2/4] Make Writeable with single handler robust against stream Writers --- src/objective-c/RxLibrary/GRXWriteable.m | 37 ++++++++++++++++++++---- 1 file changed, 32 insertions(+), 5 deletions(-) diff --git a/src/objective-c/RxLibrary/GRXWriteable.m b/src/objective-c/RxLibrary/GRXWriteable.m index 2729d62b72f..b122c04ce0c 100644 --- a/src/objective-c/RxLibrary/GRXWriteable.m +++ b/src/objective-c/RxLibrary/GRXWriteable.m @@ -42,11 +42,38 @@ 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. + singleHandler(nil, [NSError errorWithDomain:@"io.grpc" code:13 userInfo:userInfo]); + } + }; + return [self writeableWithEventHandler:^(BOOL done, id value, NSError *error) { + if (eventHandler) { + eventHandler(done, value, error); } }]; } From 5a8ec75012862815bc28a2145c6d294b1993cbd4 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Fri, 11 Mar 2016 18:41:54 -0800 Subject: [PATCH 3/4] Use a named constant for error domain and code --- src/objective-c/RxLibrary/GRXWriteable.m | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/objective-c/RxLibrary/GRXWriteable.m b/src/objective-c/RxLibrary/GRXWriteable.m index b122c04ce0c..c15ccb17fb9 100644 --- a/src/objective-c/RxLibrary/GRXWriteable.m +++ b/src/objective-c/RxLibrary/GRXWriteable.m @@ -68,7 +68,11 @@ // 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. - singleHandler(nil, [NSError errorWithDomain:@"io.grpc" code:13 userInfo:userInfo]); + 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) { From 7cfb396bf6c2194ca9c1bb6514cc57ce1403f3d8 Mon Sep 17 00:00:00 2001 From: Jorge Canizales Date: Mon, 14 Mar 2016 19:03:54 -0700 Subject: [PATCH 4/4] Make copyright not expire one year too soon. --- src/objective-c/RxLibrary/GRXWriteable.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objective-c/RxLibrary/GRXWriteable.m b/src/objective-c/RxLibrary/GRXWriteable.m index c15ccb17fb9..028ba9b551a 100644 --- a/src/objective-c/RxLibrary/GRXWriteable.m +++ b/src/objective-c/RxLibrary/GRXWriteable.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