Replace `QueryListenOptions` with simple booleans (#1106)

* Replace `QueryListenOptions` with simple booleans

Instead of calling

  addSnapshotListener(
      options:QueryListenOptions.includeQueryMetadataChanges()
                                .includeDocumentMetadataChanges())

call

  addSnapshotListener(includeMetadataChanges:true)

Also change `QuerySnapshot.documentChanges()` into a method which
optionally takes `includeMetadataChanges:true`. By default even when
listening to a query with `inlcudeMetadataChanges:true` metadata-only
document changes are suppressed because they're confusing.

* Revert QuerySnapshot.documentChanges back to a property

Add usage.

4 files changed, 120 insertions, 25 deletions

diff --git a/Firestore/Example/SwiftBuildTest/main.swift b/Firestore/Example/SwiftBuildTest/main.swift
index ad6c0f6..8de5ed4 100644
--- a/Firestore/Example/SwiftBuildTest/main.swift
+++ b/Firestore/Example/SwiftBuildTest/main.swift
@@ -321,6 +321,26 @@ func listenToQueryDiffs(onQuery query: Query) {
   listener.remove()
 }
 
+func listenToQueryDiffsWithMetadata(onQuery query: Query) {
+  let listener = query.addSnapshotListener(includeMetadataChanges: true) { snap, error in
+    if let snap = snap {
+      for change in snap.documentChanges(includeMetadataChanges: true) {
+        switch change.type {
+        case .added:
+          print("New document: \(change.document.data())")
+        case .modified:
+          print("Modified document: \(change.document.data())")
+        case .removed:
+          print("Removed document: \(change.document.data())")
+        }
+      }
+    }
+  }
+
+  // Unsubscribe
+  listener.remove()
+}
+
 func transactions() {
   let db = Firestore.firestore()
 
@@ -361,7 +381,6 @@ func types() {
   let _: GeoPoint
   let _: Timestamp
   let _: ListenerRegistration
-  let _: QueryListenOptions
   let _: Query
   let _: QuerySnapshot
   let _: SnapshotMetadata
diff --git a/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm b/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm
index f8c7d60..746c107 100644
--- a/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm
+++ b/Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm
@@ -19,9 +19,31 @@
 #import <XCTest/XCTest.h>
 
 #import "Firestore/Example/Tests/API/FSTAPIHelpers.h"
+#import "Firestore/Example/Tests/Util/FSTHelpers.h"
+#import "Firestore/Source/API/FIRDocumentChange+Internal.h"
+#import "Firestore/Source/API/FIRDocumentSnapshot+Internal.h"
+#import "Firestore/Source/API/FIRQuerySnapshot+Internal.h"
+#import "Firestore/Source/API/FIRSnapshotMetadata+Internal.h"
+#import "Firestore/Source/Core/FSTViewSnapshot.h"
+#import "Firestore/Source/Model/FSTDocument.h"
+#import "Firestore/Source/Model/FSTDocumentSet.h"
+
+#include "Firestore/core/src/firebase/firestore/util/string_apple.h"
+
+namespace util = firebase::firestore::util;
 
 NS_ASSUME_NONNULL_BEGIN
 
+@interface FIRDocumentChange ()
+
+// Expose initializer for testing.
+- (instancetype)initWithType:(FIRDocumentChangeType)type
+                    document:(FIRQueryDocumentSnapshot *)document
+                    oldIndex:(NSUInteger)oldIndex
+                    newIndex:(NSUInteger)newIndex;
+
+@end
+
 @interface FIRQuerySnapshotTests : XCTestCase
 @end
 
@@ -51,6 +73,67 @@ NS_ASSUME_NONNULL_BEGIN
   XCTAssertNotEqual([foo hash], [fromCache hash]);
 }
 
+- (void)testIncludeMetadataChanges {
+  FSTDocument *doc1Old = FSTTestDoc("foo/bar", 1, @{@"a" : @"b"}, YES);
+  FSTDocument *doc1New = FSTTestDoc("foo/bar", 1, @{@"a" : @"b"}, NO);
+
+  FSTDocument *doc2Old = FSTTestDoc("foo/baz", 1, @{@"a" : @"b"}, NO);
+  FSTDocument *doc2New = FSTTestDoc("foo/baz", 1, @{@"a" : @"c"}, NO);
+
+  FSTDocumentSet *oldDocuments = FSTTestDocSet(FSTDocumentComparatorByKey, @[ doc1Old, doc2Old ]);
+  FSTDocumentSet *newDocuments = FSTTestDocSet(FSTDocumentComparatorByKey, @[ doc2New, doc2New ]);
+  NSArray<FSTDocumentViewChange *> *documentChanges = @[
+    [FSTDocumentViewChange changeWithDocument:doc1New type:FSTDocumentViewChangeTypeMetadata],
+    [FSTDocumentViewChange changeWithDocument:doc2New type:FSTDocumentViewChangeTypeModified],
+  ];
+
+  FIRFirestore *firestore = FSTTestFirestore();
+  FSTQuery *query = FSTTestQuery("foo");
+  FSTViewSnapshot *viewSnapshot = [[FSTViewSnapshot alloc] initWithQuery:query
+                                                               documents:newDocuments
+                                                            oldDocuments:oldDocuments
+                                                         documentChanges:documentChanges
+                                                               fromCache:NO
+                                                        hasPendingWrites:NO
+                                                        syncStateChanged:YES];
+  FIRSnapshotMetadata *metadata =
+      [FIRSnapshotMetadata snapshotMetadataWithPendingWrites:NO fromCache:NO];
+  FIRQuerySnapshot *snapshot = [FIRQuerySnapshot snapshotWithFirestore:firestore
+                                                         originalQuery:query
+                                                              snapshot:viewSnapshot
+                                                              metadata:metadata];
+
+  FIRQueryDocumentSnapshot *doc1Snap = [FIRQueryDocumentSnapshot snapshotWithFirestore:firestore
+                                                                           documentKey:doc1New.key
+                                                                              document:doc1New
+                                                                             fromCache:NO];
+  FIRQueryDocumentSnapshot *doc2Snap = [FIRQueryDocumentSnapshot snapshotWithFirestore:firestore
+                                                                           documentKey:doc2New.key
+                                                                              document:doc2New
+                                                                             fromCache:NO];
+
+  NSArray<FIRDocumentChange *> *changesWithoutMetadata = @[
+    [[FIRDocumentChange alloc] initWithType:FIRDocumentChangeTypeModified
+                                   document:doc2Snap
+                                   oldIndex:1
+                                   newIndex:1],
+  ];
+  XCTAssertEqualObjects(snapshot.documentChanges, changesWithoutMetadata);
+
+  NSArray<FIRDocumentChange *> *changesWithMetadata = @[
+    [[FIRDocumentChange alloc] initWithType:FIRDocumentChangeTypeModified
+                                   document:doc1Snap
+                                   oldIndex:0
+                                   newIndex:0],
+    [[FIRDocumentChange alloc] initWithType:FIRDocumentChangeTypeModified
+                                   document:doc2Snap
+                                   oldIndex:1
+                                   newIndex:1],
+  ];
+  XCTAssertEqualObjects([snapshot documentChangesWithIncludeMetadataChanges:YES],
+                        changesWithMetadata);
+}
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm
index 32d746e..d1c0d75 100644
--- a/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm
+++ b/Firestore/Example/Tests/Integration/API/FIRQueryTests.mm
@@ -221,16 +221,14 @@
 
   FIRFirestore *firestore = collectionRef.firestore;
 
-  FIRQueryListenOptions *options = [[[FIRQueryListenOptions options]
-      includeDocumentMetadataChanges:YES] includeQueryMetadataChanges:YES];
-
-  [collectionRef addSnapshotListenerWithOptions:options
-                                       listener:^(FIRQuerySnapshot *snapshot, NSError *error) {
-                                         XCTAssertNil(error);
-                                         if (!snapshot.empty && !snapshot.metadata.fromCache) {
-                                           [testExpectiation fulfill];
-                                         }
-                                       }];
+  [collectionRef
+      addSnapshotListenerWithIncludeMetadataChanges:YES
+                                           listener:^(FIRQuerySnapshot *snapshot, NSError *error) {
+                                             XCTAssertNil(error);
+                                             if (!snapshot.empty && !snapshot.metadata.fromCache) {
+                                               [testExpectiation fulfill];
+                                             }
+                                           }];
 
   [firestore disableNetworkWithCompletion:^(NSError *error) {
     XCTAssertNil(error);
@@ -249,11 +247,9 @@
   };
   FIRCollectionReference *collection = [self collectionRefWithDocuments:testDocs];
 
-  FIRQueryListenOptions *options = [[[FIRQueryListenOptions options]
-      includeDocumentMetadataChanges:YES] includeQueryMetadataChanges:YES];
-  id<FIRListenerRegistration> registration =
-      [collection addSnapshotListenerWithOptions:options
-                                        listener:self.eventAccumulator.valueEventHandler];
+  id<FIRListenerRegistration> registration = [collection
+      addSnapshotListenerWithIncludeMetadataChanges:YES
+                                           listener:self.eventAccumulator.valueEventHandler];
 
   FIRQuerySnapshot *querySnap = [self.eventAccumulator awaitEventWithName:@"initial event"];
   XCTAssertEqualObjects(FIRQuerySnapshotGetData(querySnap), @[ @{ @"foo" : @1 } ]);
diff --git a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm
index 3f2d64b..5340873 100644
--- a/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm
+++ b/Firestore/Example/Tests/Integration/API/FIRWriteBatchTests.mm
@@ -147,9 +147,8 @@
   FIRDocumentReference *docA = [collection documentWithPath:@"a"];
   FIRDocumentReference *docB = [collection documentWithPath:@"b"];
   FSTEventAccumulator *accumulator = [FSTEventAccumulator accumulatorForTest:self];
-  [collection addSnapshotListenerWithOptions:[[FIRQueryListenOptions options]
-                                                 includeQueryMetadataChanges:YES]
-                                    listener:accumulator.valueEventHandler];
+  [collection addSnapshotListenerWithIncludeMetadataChanges:YES
+                                                   listener:accumulator.valueEventHandler];
   FIRQuerySnapshot *initialSnap = [accumulator awaitEventWithName:@"initial event"];
   XCTAssertEqual(initialSnap.count, 0);
 
@@ -177,9 +176,8 @@
   FIRDocumentReference *docA = [collection documentWithPath:@"a"];
   FIRDocumentReference *docB = [collection documentWithPath:@"b"];
   FSTEventAccumulator *accumulator = [FSTEventAccumulator accumulatorForTest:self];
-  [collection addSnapshotListenerWithOptions:[[FIRQueryListenOptions options]
-                                                 includeQueryMetadataChanges:YES]
-                                    listener:accumulator.valueEventHandler];
+  [collection addSnapshotListenerWithIncludeMetadataChanges:YES
+                                                   listener:accumulator.valueEventHandler];
   FIRQuerySnapshot *initialSnap = [accumulator awaitEventWithName:@"initial event"];
   XCTAssertEqual(initialSnap.count, 0);
 
@@ -211,9 +209,8 @@
   FIRDocumentReference *docA = [collection documentWithPath:@"a"];
   FIRDocumentReference *docB = [collection documentWithPath:@"b"];
   FSTEventAccumulator *accumulator = [FSTEventAccumulator accumulatorForTest:self];
-  [collection addSnapshotListenerWithOptions:[[FIRQueryListenOptions options]
-                                                 includeQueryMetadataChanges:YES]
-                                    listener:accumulator.valueEventHandler];
+  [collection addSnapshotListenerWithIncludeMetadataChanges:YES
+                                                   listener:accumulator.valueEventHandler];
   FIRQuerySnapshot *initialSnap = [accumulator awaitEventWithName:@"initial event"];
   XCTAssertEqual(initialSnap.count, 0);