From c8d9231b2b8c88af42d373a42ebc0cfdc57702e7 Mon Sep 17 00:00:00 2001 From: Michael Lehenbauer Date: Sat, 17 Feb 2018 15:30:03 -0800 Subject: [PATCH] Add stricter assertions regarding stream state. --- .../Tests/Integration/FSTStreamTests.mm | 8 +++++--- Firestore/Source/Remote/FSTStream.mm | 18 ++++++------------ 2 files changed, 11 insertions(+), 15 deletions(-) diff --git a/Firestore/Example/Tests/Integration/FSTStreamTests.mm b/Firestore/Example/Tests/Integration/FSTStreamTests.mm index 3faf67859a4..cec35c10e8f 100644 --- a/Firestore/Example/Tests/Integration/FSTStreamTests.mm +++ b/Firestore/Example/Tests/Integration/FSTStreamTests.mm @@ -16,6 +16,8 @@ #import +#import + #import #import "Firestore/Example/Tests/Util/FSTHelpers.h" @@ -36,7 +38,7 @@ /** Exposes otherwise private methods for testing. */ @interface FSTStream (Testing) -- (void)writesFinishedWithError:(NSError *_Nullable)error; +@property(nonatomic, strong, readwrite) id callbackFilter; @end /** @@ -204,7 +206,7 @@ - (void)testWatchStreamStopBeforeHandshake { // Simulate a final callback from GRPC [_workerDispatchQueue dispatchAsync:^{ - [watchStream writesFinishedWithError:nil]; + [watchStream.callbackFilter writesFinishedWithError:nil]; }]; [self verifyDelegateObservedStates:@[ @"watchStreamDidOpen" ]]; @@ -228,7 +230,7 @@ - (void)testWriteStreamStopBeforeHandshake { // Simulate a final callback from GRPC [_workerDispatchQueue dispatchAsync:^{ - [writeStream writesFinishedWithError:nil]; + [writeStream.callbackFilter writesFinishedWithError:nil]; }]; [self verifyDelegateObservedStates:@[ @"writeStreamDidOpen" ]]; diff --git a/Firestore/Source/Remote/FSTStream.mm b/Firestore/Source/Remote/FSTStream.mm index e07718236ae..4f96d5407df 100644 --- a/Firestore/Source/Remote/FSTStream.mm +++ b/Firestore/Source/Remote/FSTStream.mm @@ -263,12 +263,12 @@ - (void)startWithDelegate:(id)delegate { /** Add an access token to our RPC, after obtaining one from the credentials provider. */ - (void)resumeStartWithToken:(FSTGetTokenResult *)token error:(NSError *)error { + [self.workerDispatchQueue verifyIsCurrentQueue]; + if (self.state == FSTStreamStateStopped) { // Streams can be stopped while waiting for authorization. return; } - - [self.workerDispatchQueue verifyIsCurrentQueue]; FSTAssert(self.state == FSTStreamStateAuth, @"State should still be auth (was %ld)", (long)self.state); @@ -525,11 +525,7 @@ - (void)handleStreamMessage:(id)value { */ - (void)handleStreamClose:(nullable NSError *)error { FSTLog(@"%@ %p close: %@", NSStringFromClass([self class]), (__bridge void *)self, error); - - if (![self isStarted]) { // The stream could have already been closed by the idle close timer. - FSTLog(@"%@ Ignoring server close for already closed stream.", NSStringFromClass([self class])); - return; - } + FSTAssert([self isStarted], @"handleStreamClose: called for non-started stream."); // In theory the stream could close cleanly, however, in our current model we never expect this // to happen because if we stop a stream ourselves, this callback will never be called. To @@ -550,11 +546,7 @@ - (void)handleStreamClose:(nullable NSError *)error { */ - (void)writeValue:(id)value __used { [self.workerDispatchQueue verifyIsCurrentQueue]; - - if (![self isStarted]) { - FSTLog(@"%@ Ignoring stream message from inactive stream.", NSStringFromClass([self class])); - return; - } + FSTAssert([self isStarted], @"writeValue: called for stopped stream."); if (!self.messageReceived) { self.messageReceived = YES; @@ -587,6 +579,8 @@ - (void)writeValue:(id)value __used { - (void)writesFinishedWithError:(nullable NSError *)error __used { error = [FSTDatastore firestoreErrorForError:error]; [self.workerDispatchQueue verifyIsCurrentQueue]; + FSTAssert([self isStarted], @"writesFinishedWithError: called for stopped stream."); + [self handleStreamClose:error]; }