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, 47 insertions, 56 deletions

diff --git a/Firestore/Source/API/FIRDocumentChange+Internal.h b/Firestore/Source/API/FIRDocumentChange+Internal.h
index 7c9723c..2aaced1 100644
--- a/Firestore/Source/API/FIRDocumentChange+Internal.h
+++ b/Firestore/Source/API/FIRDocumentChange+Internal.h
@@ -26,6 +26,7 @@ NS_ASSUME_NONNULL_BEGIN
 
 /** Calculates the array of FIRDocumentChange's based on the given FSTViewSnapshot. */
 + (NSArray<FIRDocumentChange *> *)documentChangesForSnapshot:(FSTViewSnapshot *)snapshot
+                                      includeMetadataChanges:(BOOL)includeMetadataChanges
                                                    firestore:(FIRFirestore *)firestore;
 
 @end
diff --git a/Firestore/Source/API/FIRDocumentChange.mm b/Firestore/Source/API/FIRDocumentChange.mm
index d1d9999..7bb24d2 100644
--- a/Firestore/Source/API/FIRDocumentChange.mm
+++ b/Firestore/Source/API/FIRDocumentChange.mm
@@ -50,9 +50,11 @@ NS_ASSUME_NONNULL_BEGIN
 }
 
 + (NSArray<FIRDocumentChange *> *)documentChangesForSnapshot:(FSTViewSnapshot *)snapshot
+                                      includeMetadataChanges:(BOOL)includeMetadataChanges
                                                    firestore:(FIRFirestore *)firestore {
   if (snapshot.oldDocuments.isEmpty) {
-    // Special case the first snapshot because index calculation is easy and fast
+    // Special case the first snapshot because index calculation is easy and fast. Also all changes
+    // on the first snapshot are adds so there are also no metadata-only changes to filter out.
     FSTDocument *_Nullable lastDocument = nil;
     NSUInteger index = 0;
     NSMutableArray<FIRDocumentChange *> *changes = [NSMutableArray array];
@@ -79,6 +81,10 @@ NS_ASSUME_NONNULL_BEGIN
     FSTDocumentSet *indexTracker = snapshot.oldDocuments;
     NSMutableArray<FIRDocumentChange *> *changes = [NSMutableArray array];
     for (FSTDocumentViewChange *change in snapshot.documentChanges) {
+      if (!includeMetadataChanges && change.type == FSTDocumentViewChangeTypeMetadata) {
+        continue;
+      }
+
       FIRQueryDocumentSnapshot *document =
           [FIRQueryDocumentSnapshot snapshotWithFirestore:firestore
                                               documentKey:change.document.key
@@ -124,6 +130,23 @@ NS_ASSUME_NONNULL_BEGIN
   return self;
 }
 
+- (BOOL)isEqual:(nullable id)other {
+  if (other == self) return YES;
+  if (![other isKindOfClass:[FIRDocumentChange class]]) return NO;
+
+  FIRDocumentChange *change = (FIRDocumentChange *)other;
+  return self.type == change.type && [self.document isEqual:change.document] &&
+         self.oldIndex == change.oldIndex && self.newIndex == change.newIndex;
+}
+
+- (NSUInteger)hash {
+  NSUInteger result = (NSUInteger)self.type;
+  result = result * 31u + [self.document hash];
+  result = result * 31u + (NSUInteger)self.oldIndex;
+  result = result * 31u + (NSUInteger)self.newIndex;
+  return result;
+}
+
 @end
 
 NS_ASSUME_NONNULL_END
diff --git a/Firestore/Source/API/FIRQuery.mm b/Firestore/Source/API/FIRQuery.mm
index 9cdc572..14dcaef 100644
--- a/Firestore/Source/API/FIRQuery.mm
+++ b/Firestore/Source/API/FIRQuery.mm
@@ -48,47 +48,6 @@ using firebase::firestore::model::ResourcePath;
 
 NS_ASSUME_NONNULL_BEGIN
 
-@interface FIRQueryListenOptions ()
-
-- (instancetype)initWithIncludeQueryMetadataChanges:(BOOL)includeQueryMetadataChanges
-                     includeDocumentMetadataChanges:(BOOL)includeDocumentMetadataChanges
-    NS_DESIGNATED_INITIALIZER;
-
-@end
-
-@implementation FIRQueryListenOptions
-
-+ (instancetype)options {
-  return [[FIRQueryListenOptions alloc] init];
-}
-
-- (instancetype)initWithIncludeQueryMetadataChanges:(BOOL)includeQueryMetadataChanges
-                     includeDocumentMetadataChanges:(BOOL)includeDocumentMetadataChanges {
-  if (self = [super init]) {
-    _includeQueryMetadataChanges = includeQueryMetadataChanges;
-    _includeDocumentMetadataChanges = includeDocumentMetadataChanges;
-  }
-  return self;
-}
-
-- (instancetype)init {
-  return [self initWithIncludeQueryMetadataChanges:NO includeDocumentMetadataChanges:NO];
-}
-
-- (instancetype)includeQueryMetadataChanges:(BOOL)includeQueryMetadataChanges {
-  return [[FIRQueryListenOptions alloc]
-      initWithIncludeQueryMetadataChanges:includeQueryMetadataChanges
-           includeDocumentMetadataChanges:_includeDocumentMetadataChanges];
-}
-
-- (instancetype)includeDocumentMetadataChanges:(BOOL)includeDocumentMetadataChanges {
-  return [[FIRQueryListenOptions alloc]
-      initWithIncludeQueryMetadataChanges:_includeQueryMetadataChanges
-           includeDocumentMetadataChanges:includeDocumentMetadataChanges];
-}
-
-@end
-
 @interface FIRQuery ()
 @property(nonatomic, strong, readonly) FSTQuery *query;
 @end
@@ -162,14 +121,14 @@ NS_ASSUME_NONNULL_BEGIN
 }
 
 - (id<FIRListenerRegistration>)addSnapshotListener:(FIRQuerySnapshotBlock)listener {
-  return [self addSnapshotListenerWithOptions:nil listener:listener];
+  return [self addSnapshotListenerWithIncludeMetadataChanges:NO listener:listener];
 }
 
-- (id<FIRListenerRegistration>)addSnapshotListenerWithOptions:
-                                   (nullable FIRQueryListenOptions *)options
-                                                     listener:(FIRQuerySnapshotBlock)listener {
-  return [self addSnapshotListenerInternalWithOptions:[self internalOptions:options]
-                                             listener:listener];
+- (id<FIRListenerRegistration>)
+addSnapshotListenerWithIncludeMetadataChanges:(BOOL)includeMetadataChanges
+                                     listener:(FIRQuerySnapshotBlock)listener {
+  auto options = [self internalOptionsForIncludeMetadataChanges:includeMetadataChanges];
+  return [self addSnapshotListenerInternalWithOptions:options listener:listener];
 }
 
 - (id<FIRListenerRegistration>)
@@ -629,11 +588,10 @@ addSnapshotListenerInternalWithOptions:(FSTListenOptions *)internalOptions
 }
 
 /** Converts the public API options object to the internal options object. */
-- (FSTListenOptions *)internalOptions:(nullable FIRQueryListenOptions *)options {
-  return [[FSTListenOptions alloc]
-      initWithIncludeQueryMetadataChanges:options.includeQueryMetadataChanges
-           includeDocumentMetadataChanges:options.includeDocumentMetadataChanges
-                    waitForSyncWhenOnline:NO];
+- (FSTListenOptions *)internalOptionsForIncludeMetadataChanges:(BOOL)includeMetadataChanges {
+  return [[FSTListenOptions alloc] initWithIncludeQueryMetadataChanges:includeMetadataChanges
+                                        includeDocumentMetadataChanges:includeMetadataChanges
+                                                 waitForSyncWhenOnline:NO];
 }
 
 @end
diff --git a/Firestore/Source/API/FIRQuerySnapshot.mm b/Firestore/Source/API/FIRQuerySnapshot.mm
index abee84c..fb7a849 100644
--- a/Firestore/Source/API/FIRQuerySnapshot.mm
+++ b/Firestore/Source/API/FIRQuerySnapshot.mm
@@ -62,6 +62,7 @@ NS_ASSUME_NONNULL_BEGIN
 
   // Cached value of the documentChanges property.
   NSArray<FIRDocumentChange *> *_documentChanges;
+  BOOL _documentChangesIncludeMetadataChanges;
 }
 
 - (instancetype)initWithFirestore:(FIRFirestore *)firestore
@@ -73,6 +74,7 @@ NS_ASSUME_NONNULL_BEGIN
     _originalQuery = query;
     _snapshot = snapshot;
     _metadata = metadata;
+    _documentChangesIncludeMetadataChanges = NO;
   }
   return self;
 }
@@ -139,9 +141,16 @@ NS_ASSUME_NONNULL_BEGIN
 }
 
 - (NSArray<FIRDocumentChange *> *)documentChanges {
-  if (!_documentChanges) {
-    _documentChanges =
-        [FIRDocumentChange documentChangesForSnapshot:self.snapshot firestore:self.firestore];
+  return [self documentChangesWithIncludeMetadataChanges:NO];
+}
+
+- (NSArray<FIRDocumentChange *> *)documentChangesWithIncludeMetadataChanges:
+    (BOOL)includeMetadataChanges {
+  if (!_documentChanges || _documentChangesIncludeMetadataChanges != includeMetadataChanges) {
+    _documentChanges = [FIRDocumentChange documentChangesForSnapshot:self.snapshot
+                                              includeMetadataChanges:includeMetadataChanges
+                                                           firestore:self.firestore];
+    _documentChangesIncludeMetadataChanges = includeMetadataChanges;
   }
   return _documentChanges;
 }