Fix FIRDocumentChange returning incorrect values for not found (#3361)

Fixes #3298.

Author: wilhuff

Firestore/CHANGELOG.md

@@ -4,6 +4,9 @@
 - [fixed] Fixed an internal assertion that was triggered when an update
    with a `FieldValue.serverTimestamp()` and an update with a
   `FieldValue.increment()` were pending for the same document.
+- [fixed] Fixed the `oldIndex` and `newIndex` values in `DocumentChange` to
+  actually be `NSNotFound` when documents are added or removed, respectively
+  (#3298).
 - [changed] Failed transactions now return the failure from the last attempt,
   instead of `ABORTED.`
 

Firestore/Example/Tests/Integration/API/FIRQueryTests.mm

@@ -252,6 +252,35 @@ - (void)testQueriesFireFromCacheWhenOffline {
   [registration remove];
 }
 
+- (void)testDocumentChangesUseNSNotFound {
+  NSDictionary *testDocs = @{
+    @"a" : @{@"foo" : @1},
+  };
+  FIRCollectionReference *collection = [self collectionRefWithDocuments:testDocs];
+
+  id<FIRListenerRegistration> registration =
+      [collection addSnapshotListener:self.eventAccumulator.valueEventHandler];
+
+  FIRQuerySnapshot *querySnap = [self.eventAccumulator awaitEventWithName:@"initial event"];
+  XCTAssertEqual(querySnap.documentChanges.count, 1);
+
+  FIRDocumentChange *change = querySnap.documentChanges[0];
+  XCTAssertEqual(change.oldIndex, NSNotFound);
+  XCTAssertEqual(change.newIndex, 0);
+
+  FIRDocumentReference *doc = change.document.reference;
+  [self deleteDocumentRef:doc];
+
+  querySnap = [self.eventAccumulator awaitEventWithName:@"delete"];
+  XCTAssertEqual(querySnap.documentChanges.count, 1);
+
+  change = querySnap.documentChanges[0];
+  XCTAssertEqual(change.oldIndex, 0);
+  XCTAssertEqual(change.newIndex, NSNotFound);
+
+  [registration remove];
+}
+
 - (void)testCanHaveMultipleMutationsWhileOffline {
   FIRCollectionReference *col = [self collectionRef];
 

Firestore/Source/API/FIRDocumentChange.mm

@@ -25,6 +25,18 @@
 
 NS_ASSUME_NONNULL_BEGIN
 
+namespace {
+
+/**
+ * Converts from C++ document change indexes to Objective-C document change
+ * indexes. Objective-C's NSNotFound is signed NSIntegerMax, not unsigned -1.
+ */
+constexpr NSUInteger MakeIndex(size_t index) {
+  return index == DocumentChange::npos ? NSNotFound : index;
+}
+
+}  // namespace
+
 @implementation FIRDocumentChange {
   DocumentChange _documentChange;
 }
@@ -66,11 +78,11 @@ - (FIRQueryDocumentSnapshot *)document {
 }
 
 - (NSUInteger)oldIndex {
-  return _documentChange.old_index();
+  return MakeIndex(_documentChange.old_index());
 }
 
 - (NSUInteger)newIndex {
-  return _documentChange.new_index();
+  return MakeIndex(_documentChange.new_index());
 }
 
 @end