firebase / firebase/firebase-ios-sdk

Multiple FieldValue.delete() in same setData call causes listener to be called with wrong keys the first time

Open
#10,153 3 comments 1 reaction 1 assignee Claimed by @MarkDuckworth View on GitHub
api: firestore
Dominant language
C++
Stars
6.7k
Forks
1.8k
Avg merge
2d 14h
Merged PRs (30d)
72

Description

### Description

Please see source code below to understand the context and screenshot from demo app reproducing the issue.

## Actual result:
When adding two (or more) FieldValue.delete() fields where only one would have an effect, the listener is called with invalid/missing keys the first time. Illustrated with the demo app:

After pressing **set Data with born date** and then **Fail: setData w/multiple FieldValue.delete()**, addSnapshotListener is called twice:
![Simulator Screen Shot - iPhone 13 Pro - 2022-08-30 at 19 05 06](https://user-images.githubusercontent.com/215435/187497316-b45946c4-d909-47a7-b9a4-8b79d813b99a.png)

* diff 2 in screenshot above: The first time it is called with `born` and `first` as keys, resulting in being unable to instantiate the User model (from demo app below) at this point, since `last` is missing for some reason. I would have expected `first` and `last` being set, since it is `born` that is being removed, not `last`.

* diff 3 in screenshot above: The listener is called a second time, almost immediately. With `first` and `last`, as I would expect the first time.

## Expected result:
addSnapshotListener is called **once with existing keys** (`first` and `last` in this example), or if that's impossible, at least with the **existing keys** so mapping doesn't fail if required fields are missing.

One might argue that it isn't a big issue since it "auto-correct" itself by calling the listener a second time with correct data, but there is inconsistent behaviour here and IMO it doesn't make sense to call the listener with incomplete data that may trigger weird bugs that are hard to debug.

## Additional info:
- The bug only seem to occur when 2 or more FieldValue.delete() are in place, and only if at least one of them ends up modifying the underlying data.
- The bug does not occur if multiple FieldValue.delete() does not end up modifying the underlying data. So pressing **Fail: setData w/multiple fields** multiple times in the demo app only triggers the bug if "born" is set.
- It seems that it is the last key (sorted alphabetically) is the one that is removed. In this case that is `last`, but if you had a key `zzz` that would be the one that is removed.

### Reproducing the issue

```
import SwiftUI
import FirebaseCore
import FirebaseFirestore

@main
struct MappingBugApp: App {
init() {
FirebaseApp.configure()
}
var body: some Scene {
WindowGroup {
ContentView()
}
}
}
```

**ContentView.swift**
```
import SwiftUI
import FirebaseCore
import FirebaseFirestore

struct DebugEntry: Identifiable {
var id: String
var keys: String
var user: User?
}

struct User {
var first: String
var last: String
var alwaysDeleted: String?
var born: Int?
}

struct ContentView: View {
var db = Firestore.firestore()
var documentId = "B50009BC-0200-4E78-A922-D01F76FA3040"

@State private var debugEntryCounter = 0
@State private var debugEntries: [DebugEntry] = []

private func setupListener() {
db.collection("users")
.addSnapshotListener { querySnapshot, error in
guard let querySnapshot = querySnapshot else {
print("Error \(String(describing: error))")
return
}

querySnapshot.documentChanges.forEach { diff in
let data = diff.document.data()

var mappedUser: User?
if let first = data["first"] as? String,
let last = data["last"] as? String {

let born = data["born"] as? Int
let alwaysDeleted = data["alwaysDeleted"] as? String

mappedUser = User(first: first, last: last, alwaysDeleted: alwaysDeleted, born: born)
}

self.debugEntryCounter += 1
let debugEntry = DebugEntry(id: String(debugEntryCounter),
keys: data.keys.sorted().joined(separator: ", "),
user: mappedUser)
self.debugEntries.insert(debugEntry, at: 0)
}
}
}

func setData(_ data: [String: Any]) {
let userRef = db.collection("users").document(documentId)
userRef.setData(data, merge: true)
}

var body: some View {
VStack {
Button(action: {
setData([
"first": "Ada",
"last": "Lovelace",
"born": 1815,
])
}) {
Text("setData with born date")
}

// Triggers the bug if "born" is set (multiple FieldValue.delete() and one is affecting underlying data while the other one is not)
Button(action: {
setData([
"first": "Ada",
"last": "Lovelace",
"alwaysDeleted": FieldValue.delete(),
"born": FieldValue.delete(),
])
}) {
Text("Fail: setData w/multiple FieldValue.delete()")
}

// This always works as intended (only one FieldValue.delete())
Button(action: {
setData([
"first": "Ada",
"last": "Lovelace",
"born": FieldValue.delete(),
])
}) {
Text("Works: setData w/single FieldValue.delete()")
}

List {
ForEach(self.debugEntries) { entries in
VStack(alignment: .leading) {
if let user = entries.user {
Text("\(user.first) \(user.last) \(user.born != nil ? String(user.born!) : "")")
} else {
Text("Failed to map user!").foregroundColor(.red)
}
Text("keys: \(entries.keys)").foregroundColor(.secondary)
Text("diff #\(entries.id)").font(.footnote).foregroundColor(.secondary)
}
}
}
}.onAppear {
setupListener()
}
}
}
```

### Firebase SDK Version

9.5.0

### Xcode Version

13.4.1

### Installation Method

Swift Package Manager

### Firebase Product(s)

Firestore

### Targeted Platforms

iOS

### Relevant Log Output

_No response_

### If using Swift Package Manager, the project's Package.resolved

Expand Package.resolved snippet

```json

Replace this line with the contents of your Package.resolved.

```

### If using CocoaPods, the project's Podfile.lock

Expand Podfile.lock snippet

```yml

Replace this line with the contents of your Podfile.lock!

```

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.