fix(database): prevent FUIArray crash on desynced child events

FirebaseDatabaseUI/FirebaseDatabaseUITests/FUIArrayTest.m

@@ -485,4 +485,76 @@ - (void)testRemovesAllElementsWhenInvalidated {
             self.firebaseArray.count);
 }
 
+#pragma mark - Invariant violations (see GitHub issue #517)
+
+// The local model can desync from the database (e.g. queryLimited(toLast:) plus
+// concurrent server-side writes), causing a child event to reference a key that
+// isn't in the array. These must not crash; each event reconciles toward the
+// correct end-state instead.
+
+- (void)testRemovingUnknownKeyDoesNotCrash {
+  [self.observable populateWithCount:10]; // keys "0".."9"
+  self.snap.key = @"this-key-was-never-added";
+
+  XCTAssertNoThrow(
+    [self.observable sendEvent:FIRDataEventTypeChildRemoved
+                    withObject:self.snap
+                   previousKey:@"9"
+                         error:nil]);
+
+  XCTAssert(self.firebaseArray.count == 10,
+            @"expected count to stay 10 when removing an unknown key, got %ld",
+            self.firebaseArray.count);
+}
+
+- (void)testInsertingWithUnknownPreviousKeyDoesNotCrash {
+  [self.observable populateWithCount:10]; // keys "0".."9"
+  self.snap.key = @"new";
+
+  XCTAssertNoThrow(
+    [self.observable sendEvent:FIRDataEventTypeChildAdded
+                    withObject:self.snap
+                   previousKey:@"this-key-was-never-added"
+                         error:nil]);
+
+  // Best-effort: the row is kept (appended) rather than dropped or crashing.
+  XCTAssert(self.firebaseArray.count == 11,
+            @"expected count to become 11 after insert, got %ld",
+            self.firebaseArray.count);
+  XCTAssert([[self.firebaseArray snapshotAtIndex:10].key isEqualToString:@"new"],
+            @"expected the new snapshot to be appended at the end");
+}
+
+- (void)testChangingUnknownKeyDoesNotCrash {
+  [self.observable populateWithCount:10]; // keys "0".."9"
+  self.snap.key = @"this-key-was-never-added";
+
+  XCTAssertNoThrow(
+    [self.observable sendEvent:FIRDataEventTypeChildChanged
+                    withObject:self.snap
+                   previousKey:@"9"
+                         error:nil]);
+
+  // Recovered as an insert rather than dropping the update.
+  XCTAssert(self.firebaseArray.count == 11,
+            @"expected count to become 11 after recovering change as insert, got %ld",
+            self.firebaseArray.count);
+}
+
+- (void)testMovingUnknownKeyDoesNotCrash {
+  [self.observable populateWithCount:10]; // keys "0".."9"
+  self.snap.key = @"this-key-was-never-added";
+
+  XCTAssertNoThrow(
+    [self.observable sendEvent:FIRDataEventTypeChildMoved
+                    withObject:self.snap
+                   previousKey:@"3"
+                         error:nil]);
+
+  // Recovered as an insert rather than dropping the item.
+  XCTAssert(self.firebaseArray.count == 11,
+            @"expected count to become 11 after recovering move as insert, got %ld",
+            self.firebaseArray.count);
+}
+
 @end

FirebaseDatabaseUI/Sources/FUIArray.m

@@ -189,20 +189,18 @@ - (NSUInteger)indexForKey:(NSString *)key {
 }
 
 - (void)insertSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *)previous {
+  // The local model can desync from the database (e.g. a query limited via
+  // -queryLimitedToLast: combined with concurrent server-side writes), so a
+  // child event may reference a key that isn't in the array. Reconcile toward
+  // the correct end-state instead of throwing. See GitHub issue #517.
   NSUInteger index = 0;
   if (previous != nil) {
-    NSInteger previousChildIndex = (NSInteger)[self indexForKey:previous];
-
-    if (previousChildIndex == NSNotFound) {
-      NSString *reason = [NSString stringWithFormat:@"Attempted to insert snapshot with unknown"
-                          @" previousChildKey %@ into array: %@", previous, self.snapshots];
-      NSException *exception = [NSException exceptionWithName:NSInternalInconsistencyException
-                                                       reason:reason
-                                                     userInfo:nil];
-      @throw exception;
-    }
+    NSUInteger previousChildIndex = [self indexForKey:previous];
 
-    index = previousChildIndex + 1;
+    // The previous sibling was never delivered locally. Append rather than
+    // crash or drop the row.
+    index = (previousChildIndex == NSNotFound) ? self.snapshots.count
+                                               : previousChildIndex + 1;
   }
 
   [self.snapshots insertObject:snap atIndex:index];
@@ -217,12 +215,9 @@ - (void)removeSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *)
   NSUInteger index = [self indexForKey:snap.key];
 
   if (index == NSNotFound) {
-    NSString *reason = [NSString stringWithFormat:@"Attempted to remove snapshot with unknown"
-                        @" key %@ from array: %@", snap.key, self.snapshots];
-    NSException *exception = [NSException exceptionWithName:NSInternalInconsistencyException
-                                                     reason:reason
-                                                   userInfo:nil];
-    @throw exception;
+    // Already absent from the local model; the desired end-state (item gone)
+    // already holds, so there is nothing to remove. See GitHub issue #517.
+    return;
   }
 
   [self.snapshots removeObjectAtIndex:index];
@@ -237,12 +232,10 @@ - (void)changeSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *)
   NSUInteger index = [self indexForKey:snap.key];
 
   if (index == NSNotFound) {
-    NSString *reason = [NSString stringWithFormat:@"Attempted to replace snapshot with unknown"
-                        @" key %@ in array: %@", snap.key, self.snapshots];
-    NSException *exception = [NSException exceptionWithName:NSInternalInconsistencyException
-                                                     reason:reason
-                                                   userInfo:nil];
-    @throw exception;
+    // We never had this item; recover it as an insert rather than dropping the
+    // update. See GitHub issue #517.
+    [self insertSnapshot:snap withPreviousChildKey:previous];
+    return;
   }
 
   [self.snapshots replaceObjectAtIndex:index withObject:snap];
@@ -257,12 +250,10 @@ - (void)moveSnapshot:(FIRDataSnapshot *)snap withPreviousChildKey:(NSString *)pr
   NSUInteger fromIndex = [self indexForKey:snap.key];
 
   if (fromIndex == NSNotFound) {
-    NSString *reason = [NSString stringWithFormat:@"Attempted to remove snapshot with unknown"
-                        @" key %@ from array: %@", snap.key, self.snapshots];
-    NSException *exception = [NSException exceptionWithName:NSInternalInconsistencyException
-                                                     reason:reason
-                                                   userInfo:nil];
-    @throw exception;
+    // The item we were asked to move isn't in the local model; recover it as an
+    // insert at the destination rather than dropping it. See GitHub issue #517.
+    [self insertSnapshot:snap withPreviousChildKey:previous];
+    return;
   }
 
   [self.snapshots removeObjectAtIndex:fromIndex];