Addressing review comments

Author: schmidt-sebastian

Firestore/Example/Tests/Integration/FSTStreamTests.m

@@ -16,7 +16,6 @@
 
 #import <XCTest/XCTest.h>
 
-#import <FirebaseCommunity/FIRLogger.h>
 #import <Firestore/FIRFirestoreSettings.h>
 
 #import "Auth/FSTEmptyCredentialsProvider.h"
@@ -42,61 +41,61 @@ @interface FSTStreamStatusDelegate : NSObject <FSTWatchStreamDelegate, FSTWriteS
 @property(nonatomic, readonly) NSMutableArray<NSString *> *states;
 @property(atomic, readwrite) BOOL invokeCallbacks;
 @property(nonatomic, weak) XCTestExpectation *expectation;
-@property(nonatomic, weak, readonly) FSTStream *stream;
 
-- (instancetype)initWithStream:(FSTStream *)stream NS_DESIGNATED_INITIALIZER;
-- (instancetype)init NS_UNAVAILABLE;
 @end
 
 @implementation FSTStreamStatusDelegate
 
-- (instancetype)initWithStream:(FSTStream *)stream {
+- (instancetype)init {
   if (self = [super init]) {
     _states = [NSMutableArray new];
-    _stream = stream;
   }
 
   return self;
 }
 
-- (void)streamDidReceiveChange:(FSTWatchChange *)change
-               snapshotVersion:(FSTSnapshotVersion *)snapshotVersion {
-  [_states addObject:@"didReceiveChange"];
+- (void)watchStreamDidOpen {
+  [_states addObject:@"watchStreamDidOpen"];
   [_expectation fulfill];
   _expectation = nil;
 }
 
-- (void)streamDidOpen {
-  [_states addObject:@"didOpen"];
+- (void)writeStreamDidOpen {
+  [_states addObject:@"writeStreamDidOpen"];
   [_expectation fulfill];
   _expectation = nil;
 }
 
-- (void)streamDidClose:(NSError *_Nullable)error {
-  [_states addObject:@"didClose"];
+- (void)writeStreamDidCompleteHandshake {
+  [_states addObject:@"writeStreamDidCompleteHandshake"];
   [_expectation fulfill];
   _expectation = nil;
 }
 
-- (void)streamDidCompleteHandshake {
-  [_states addObject:@"didCompleteHandshake"];
+- (void)writeStreamWasInterrupted:(NSError *_Nullable)error {
+  [_states addObject:@"writeStreamWasInterrupted"];
   [_expectation fulfill];
   _expectation = nil;
 }
 
-- (void)streamDidReceiveResponseWithVersion:(FSTSnapshotVersion *)commitVersion
-                            mutationResults:(NSArray<FSTMutationResult *> *)results {
-  [_states addObject:@"didReceiveResponse"];
+- (void)watchStreamWasInterrupted:(NSError *_Nullable)error {
+  [_states addObject:@"watchStreamWasInterrupted"];
   [_expectation fulfill];
   _expectation = nil;
 }
 
-- (void)writeValue:(id)value {
-  [self.stream writeValue:value];
+- (void)watchStreamDidChange:(FSTWatchChange *)change
+             snapshotVersion:(FSTSnapshotVersion *)snapshotVersion {
+  [_states addObject:@"watchStreamDidChange"];
+  [_expectation fulfill];
+  _expectation = nil;
 }
 
-- (void)writesFinishedWithError:(NSError *)errorOrNil {
-  [self.stream writesFinishedWithError:errorOrNil];
+- (void)writeStreamDidReceiveResponseWithVersion:(FSTSnapshotVersion *)commitVersion
+                                 mutationResults:(NSArray<FSTMutationResult *> *)results {
+  [_states addObject:@"writeStreamDidReceiveResponseWithVersion"];
+  [_expectation fulfill];
+  _expectation = nil;
 }
 
 - (void)fulfillOnCallback:(XCTestExpectation *)expectation {
@@ -113,9 +112,9 @@ @interface FSTStreamTests : XCTestCase
 @implementation FSTStreamTests {
   dispatch_queue_t _testQueue;
   FSTDatabaseInfo *_databaseInfo;
-  FSTTestDispatchQueue *_workerDispatchQueue;
   FSTEmptyCredentialsProvider *_credentials;
   FSTStreamStatusDelegate *_delegate;
+  FSTTestDispatchQueue *_workerDispatchQueue;
 
   /** Single mutation to send to the write stream. */
   NSArray<FSTMutation *> *_mutations;
@@ -149,20 +148,16 @@ - (FSTWriteStream *)setUpWriteStream {
                                                    workerDispatchQueue:_workerDispatchQueue
                                                            credentials:_credentials];
 
-  FSTWriteStream *stream = [datastore createWriteStream];
-  _delegate = [[FSTStreamStatusDelegate alloc] initWithStream:stream];
-
-  return stream;
+  _delegate = [FSTStreamStatusDelegate new];
+  return [datastore createWriteStream];
 }
 
 - (FSTWatchStream *)setUpWatchStream {
   FSTDatastore *datastore = [[FSTDatastore alloc] initWithDatabaseInfo:_databaseInfo
                                                    workerDispatchQueue:_workerDispatchQueue
                                                            credentials:_credentials];
-  FSTWatchStream *stream = [datastore createWatchStream];
-  _delegate = [[FSTStreamStatusDelegate alloc] initWithStream:stream];
-
-  return stream;
+  _delegate = [FSTStreamStatusDelegate new];
+  return [datastore createWatchStream];
 }
 
 - (void)verifyDelegate:(NSArray<NSString *> *)expectedStates {
@@ -193,7 +188,7 @@ - (void)testWatchStreamStopBeforeHandshake {
   // Simulate a final callback from GRPC
   [watchStream writesFinishedWithError:nil];
 
-  [self verifyDelegate:@[ @"didOpen" ]];
+  [self verifyDelegate:@[ @"watchStreamDidOpen" ]];
 }
 
 /** Verifies that the write stream does not issue an onClose callback after a call to stop(). */
@@ -218,7 +213,7 @@ - (void)testWriteStreamStopBeforeHandshake {
   // Simulate a final callback from GRPC
   [writeStream writesFinishedWithError:nil];
 
-  [self verifyDelegate:@[ @"didOpen" ]];
+  [self verifyDelegate:@[ @"writeStreamDidOpen" ]];
 }
 
 - (void)testWriteStreamStopAfterHandshake {
@@ -255,7 +250,7 @@ - (void)testWriteStreamStopAfterHandshake {
     [writeStream stop];
   }];
 
-  [self verifyDelegate:@[ @"didOpen", @"didCompleteHandshake", @"didReceiveResponse" ]];
+  [self verifyDelegate:@[ @"writeStreamDidOpen", @"writeStreamDidCompleteHandshake", @"writeStreamDidReceiveResponseWithVersion" ]];
 }
 
 - (void)testStreamClosesWhenIdle {
@@ -286,7 +281,7 @@ - (void)testStreamClosesWhenIdle {
     XCTAssertFalse([writeStream isOpen]);
   });
 
-  [self verifyDelegate:@[ @"didOpen", @"didCompleteHandshake", @"didClose" ]];
+  [self verifyDelegate:@[ @"writeStreamDidOpen", @"writeStreamDidCompleteHandshake", @"writeStreamWasInterrupted" ]];
 }
 
 - (void)testStreamCancelsIdleOnWrite {
@@ -327,7 +322,7 @@ - (void)testStreamCancelsIdleOnWrite {
     [writeStream stop];
   }];
 
-  [self verifyDelegate:@[ @"didOpen", @"didCompleteHandshake", @"didReceiveResponse" ]];
+  [self verifyDelegate:@[ @"writeStreamDidOpen", @"writeStreamDidCompleteHandshake", @"writeStreamDidReceiveResponseWithVersion" ]];
 }
 
 @end

Firestore/Example/Tests/SpecTests/FSTMockDatastore.m

@@ -51,14 +51,10 @@ - (instancetype)initWithDatabase:(FSTDatabaseInfo *)database
 @property(nonatomic, strong, readonly)
     NSMutableDictionary<FSTBoxedTargetID *, FSTQueryData *> *activeTargets;
 
-@property(nonatomic, strong, readonly) id<FSTWatchStreamDelegate> delegate;
-
 @end
 
 @implementation FSTMockWatchStream
 
-@synthesize delegate = _delegate;
-
 - (instancetype)initWithDatabase:(FSTDatabaseInfo *)database
              workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue
                      credentials:(id<FSTCredentialsProvider>)credentials
@@ -79,12 +75,12 @@ - (instancetype)initWithDatabase:(FSTDatabaseInfo *)database
 - (void)start:(id<FSTWatchStreamDelegate>)delegate {
   FSTAssert(!self.open, @"Trying to start already started watch stream");
   self.open = YES;
-  _delegate = delegate;
-  [self.delegate streamDidOpen];
+  self.delegate = delegate;
+  [self.delegate watchStreamDidOpen];
 }
 
 - (void)stop {
-  _delegate = nil;
+  self.delegate = nil;
 }
 
 - (BOOL)isOpen {
@@ -95,8 +91,8 @@ - (BOOL)isStarted {
   return self.open;
 }
 
-- (void)streamDidOpen {
-  [self.delegate streamDidOpen];
+- (void)notifyWatchStreamDidOpen {
+  [self.delegate watchStreamDidOpen];
 }
 
 - (void)watchQuery:(FSTQueryData *)query {
@@ -115,7 +111,7 @@ - (void)unwatchTargetID:(FSTTargetID)targetID {
 
 - (void)failStreamWithError:(NSError *)error {
   self.open = NO;
-  [self.delegate streamDidClose:error];
+  [self.delegate watchStreamWasInterrupted:error];
 }
 
 #pragma mark - Helper methods.
@@ -135,7 +131,7 @@ - (void)writeWatchChange:(FSTWatchChange *)change snapshotVersion:(FSTSnapshotVe
       }
     }
   }
-  [self.delegate streamDidReceiveChange:change snapshotVersion:snap];
+  [self.delegate watchStreamDidChange:change snapshotVersion:snap];
 }
 
 @end
@@ -156,14 +152,10 @@ - (instancetype)initWithDatabase:(FSTDatabaseInfo *)database
 
 @property(nonatomic, assign) BOOL open;
 @property(nonatomic, strong, readonly) NSMutableArray<NSArray<FSTMutation *> *> *sentMutations;
-@property(nonatomic, strong, readonly) id<FSTWriteStreamDelegate> delegate;
-
 @end
 
 @implementation FSTMockWriteStream
 
-@synthesize delegate = _delegate;
-
 - (instancetype)initWithDatabase:(FSTDatabaseInfo *)database
              workerDispatchQueue:(FSTDispatchQueue *)workerDispatchQueue
                      credentials:(id<FSTCredentialsProvider>)credentials
@@ -184,12 +176,12 @@ - (void)start:(id<FSTWriteStreamDelegate>)delegate {
   FSTAssert(!self.open, @"Trying to start already started write stream");
   self.open = YES;
   [self.sentMutations removeAllObjects];
-  _delegate = delegate;
-  [self.delegate streamDidOpen];
+  self.delegate = delegate;
+  [self.delegate writeStreamDidOpen];
 }
 
 - (void)stop {
-  _delegate = nil;
+  self.delegate = nil;
 }
 
 - (BOOL)isOpen {
@@ -202,29 +194,29 @@ - (BOOL)isStarted {
 
 - (void)writeHandshake {
   self.handshakeComplete = YES;
-  [self.delegate streamDidCompleteHandshake];
+  [self.delegate writeStreamDidCompleteHandshake];
 }
 
 - (void)writeMutations:(NSArray<FSTMutation *> *)mutations {
   [self.sentMutations addObject:mutations];
 }
 
-- (void)streamDidOpen {
-  [self.delegate streamDidOpen];
+- (void)notifyStreamDidOpen {
+  [self.delegate writeStreamDidOpen];
 }
 
 #pragma mark - Helper methods.
 
 /** Injects a write ack as though it had come from the backend in response to a write. */
 - (void)ackWriteWithVersion:(FSTSnapshotVersion *)commitVersion
             mutationResults:(NSArray<FSTMutationResult *> *)results {
-  [self.delegate streamDidReceiveResponseWithVersion:commitVersion mutationResults:results];
+  [self.delegate writeStreamDidReceiveResponseWithVersion:commitVersion mutationResults:results];
 }
 
 /** Injects a failed write response as though it had come from the backend. */
 - (void)failStreamWithError:(NSError *)error {
   self.open = NO;
-  [self.delegate streamDidClose:error];
+  [self.delegate writeStreamWasInterrupted:error];
 }
 
 /**